Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misleading validation errors for Security Scheme Objects #428

Closed
dedoussis opened this issue Dec 15, 2021 · 10 comments
Closed

Misleading validation errors for Security Scheme Objects #428

dedoussis opened this issue Dec 15, 2021 · 10 comments
Labels
bug Something isn't working stale

Comments

@dedoussis
Copy link

Describe the bug

Not exactly a bug but something that could take some improvement. components/securitySchemes objects (at least of type oauth2) when invalid, throw misleading validation errors.

How to Reproduce

Example YAML:

asyncapi: 2.2.0

info:
  title: My App
  version: 1.0.0

servers:
  demo:
    security:
      - foo: [bar]

channels:
  /:
    publish:
      message:
        oneOf:
          - $ref: "#/components/messages/NewMessage"

components:
  messages:
    NewMessage:
      name: new message
      payload:
        type: string

  securitySchemes:
    foo:
      type: oauth2
      flows:
        implicit:
          authorizationUrl: /oauth/basic  # Invalid URI format
          scopes:
            bar: Some scope

throws:

ParserError: There were errors validating the AsyncAPI document.
    at Object.parse (/Users/dimitrios.dedoussis/workspace/asyncapi-parser-tests/node_modules/@asyncapi/parser/lib/parser.js:101:23)
    at async main (/Users/dimitrios.dedoussis/workspace/asyncapi-parser-tests/index.js:6:17) {
  type: 'https://github.com/asyncapi/parser-js/validation-errors',
  title: 'There were errors validating the AsyncAPI document.',
  validationErrors: [
    {
      title: "/servers/demo should have required property 'url'",
      location: {
        jsonPointer: '/servers/demo',
        startLine: 8,
        startColumn: 3,
        startOffset: 68,
        endLine: 12,
        endColumn: 1,
        endOffset: 108
      }
    },
    {
      title: "/servers/demo should have required property 'protocol'",
      location: {
        jsonPointer: '/servers/demo',
        startLine: 8,
        startColumn: 3,
        startOffset: 68,
        endLine: 12,
        endColumn: 1,
        endOffset: 108
      }
    },
    {
      title: "/components/securitySchemes/foo should have required property '$ref'",
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: "/components/securitySchemes/foo should have required property 'in'",
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: "/components/securitySchemes/foo should have required property 'scheme'",
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT be valid',
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: "/components/securitySchemes/foo should have required property 'scheme'",
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: "/components/securitySchemes/foo should have required property 'name'",
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: "/components/securitySchemes/foo should have required property 'in'",
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo should match exactly one schema in oneOf',
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/flows/implicit/authorizationUrl should match format "uri"',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows/implicit/authorizationUrl',
        startLine: 31,
        startColumn: 11,
        startOffset: 409,
        endLine: 31,
        endColumn: 41,
        endOffset: 439
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: "/components/securitySchemes/foo should have required property 'openIdConnectUrl'",
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: '/components/securitySchemes/foo should NOT have additional properties',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows',
        startLine: 29,
        startColumn: 7,
        startOffset: 374,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo/type should be equal to one of the allowed values',
      location: {
        jsonPointer: '/components/securitySchemes/foo/type',
        startLine: 28,
        startColumn: 7,
        startOffset: 355,
        endLine: 28,
        endColumn: 19,
        endOffset: 367
      }
    },
    {
      title: '/components/securitySchemes/foo should match exactly one schema in oneOf',
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo should match exactly one schema in oneOf',
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    },
    {
      title: '/components/securitySchemes/foo should match exactly one schema in oneOf',
      location: {
        jsonPointer: '/components/securitySchemes/foo',
        startLine: 27,
        startColumn: 5,
        startOffset: 344,
        endLine: 33,
        endColumn: 28,
        endOffset: 485
      }
    }
  ],
  parsedJSON: {
    asyncapi: '2.2.0',
    info: { title: 'My App', version: '1.0.0' },
    servers: {
      demo: { security: [ { foo: [Array] } ] }
    },
    channels: {
      '/': { publish: { message: { oneOf: [Array] } } }
    },
    components: {
      messages: {
        NewMessage: { name: 'new message', payload: { type: 'string' } }
      },
      securitySchemes: { foo: { type: 'oauth2', flows: { implicit: [Object] } } }
    }
  }
}

Node.js v17.0.1

As you can see from the snippet above, there are multiple misleading validationErrors returned, when in practice there is just the single URI formatting error.

Once I change the invalid /oauth/basic to https://company.com/oauth/basic, parsing is successful.

For a more compact visualisation of the errors, you can see the original asynction issue: dedoussis/asynction#146

Expected behavior

Rather than multiple misleading error messages, there should be a single message indicating that the securitySchemes/{{ name }}/flows/implicit/authorizationUrl is not of a valid URI format.

@dedoussis dedoussis added the bug Something isn't working label Dec 15, 2021
@github-actions
Copy link

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@derberg
Copy link
Member

derberg commented Dec 20, 2021

Hey. In your example, you are also missing url and protocol under demo server. So some errors are coming from there.

Nevertheless, even if you add them you are still getting 37 errors while I agree you should only get:

    {
      title: '/components/securitySchemes/foo/flows/implicit/authorizationUrl should match format "uri"',
      location: {
        jsonPointer: '/components/securitySchemes/foo/flows/implicit/authorizationUrl',
        startLine: 31,
        startColumn: 11,
        startOffset: 409,
        endLine: 31,
        endColumn: 41,
        endOffset: 439
      }
    }

Validation errors come from validating the document against the JSON Schema using https://github.com/ajv-validator/ajv. We would have to investigate why it is happening. Unless @magicmatatjahu or @jonaslagoni already have some ideas?

@jonaslagoni
Copy link
Member

jonaslagoni commented Dec 20, 2021

Yea I completely agree it is something we should fix...

My guess is that it throws all the validation errors it finds per security schema and bundles all of them together. So even though we are only interested in the errors found in oauth2Flow we still get the errors for being a valid apiKey for example.

I don't know any way of "suppressing" it or workaround, unfortunately. Cause at runtime, the parser would have a hard time guessing which schema, you the user, are trying to achieve.

@magicmatatjahu any ideas?

@magicmatatjahu
Copy link
Member

It seems to me that like @jonaslagoni wrote AJV checks all the schemas and throws an error for every oneOf, anyOf etc if something is wrong. Maybe we should try to merge similar errors into one? Maybe there are some libraries for this?

@github-actions
Copy link

github-actions bot commented May 4, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label May 4, 2022
@derberg derberg removed the stale label May 10, 2022
@github-actions
Copy link

github-actions bot commented Sep 8, 2022

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Sep 8, 2022
@magicmatatjahu
Copy link
Member

Still valid.

@github-actions github-actions bot removed the stale label Sep 9, 2022
@github-actions
Copy link

github-actions bot commented Jan 8, 2023

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jan 8, 2023
@magicmatatjahu
Copy link
Member

magicmatatjahu commented Jan 30, 2023

In new ParserJS we have such an errors:

image

Can we close this one?

cc @jonaslagoni

@github-actions github-actions bot removed the stale label Jan 31, 2023
@github-actions
Copy link

github-actions bot commented Jun 1, 2023

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jun 1, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale
Projects
None yet
Development

No branches or pull requests

4 participants