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

swagger v2 - no security parsing #71

Closed
djcuvcuv opened this issue Oct 22, 2019 · 5 comments · Fixed by #72
Closed

swagger v2 - no security parsing #71

djcuvcuv opened this issue Oct 22, 2019 · 5 comments · Fixed by #72
Assignees
Labels
bug Something isn't working

Comments

@djcuvcuv
Copy link

djcuvcuv commented Oct 22, 2019

I noticed that the normalize security functionality seems to misinterpret security: [] as being an empty and therefore invalid security reference; however, it is documented in swagger.io as being the way in which to signify that a particular API has "no security." In other words, when using a globally-applied security definition, one can add security: [] to the APIs which should be exempt from the globally-applied security definition. swagger.io has documented it here:
https://swagger.io/docs/specification/2-0/authentication/
^see the "ping" API under the "Global security can be overridden..." section.

The error that is thrown on app startup time by swagger routes express is as follows:

/node_modules/swagger-routes-express/src/normalise/v2/normaliseSecurity.js:3
    ? Object.values(security[0])[0]
             ^

TypeError: Cannot convert undefined or null to object
    at Function.values (<anonymous>)
    at normaliseSecurity (/node_modules/swagger-routes-express/src/normalise/v2/normaliseSecurity.js:3:14)
    at METHODS.forEach.method (/node_modules/swagger-routes-express/src/extract/v2/extractPaths.js:37:21)
    at Array.forEach (<anonymous>)
    at reduceRoutes (/node_modules/swagger-routes-express/src/extract/v2/extractPaths.js:29:13)
    at Array.reduce (<anonymous>)
    at extractPaths (/node_modules/swagger-routes-express/src/extract/v2/extractPaths.js:45:29)
    at connector (/node_modules/swagger-routes-express/src/connector/index.js:33:17)
    at callSubscriberWithImmediateExceptions (/node_modules/pubsub-js/src/pubsub.js:69:9)
    at deliverMessage (/node_modules/pubsub-js/src/pubsub.js:83:17)
    at Timeout.deliverNamespaced [as _onTimeout] (/node_modules/pubsub-js/src/pubsub.js:94:13)
    at ontimeout (timers.js:424:11)
    at tryOnTimeout (timers.js:288:5)
    at listOnTimeout (timers.js:251:5)
    at Timer.processTimers (timers.js:211:10)
@davesag davesag self-assigned this Oct 23, 2019
@davesag
Copy link
Owner

davesag commented Oct 23, 2019

Hi @djcuvcuv that's interesting, and yes appears to be a bug in this module. FYI the code for normaliseSecurity was updated in the latest release (see PR #70) but yes, this assumes that the presence of a security: [] block under a path just meant a security block without a scope and attempts to map it accordingly.

I can see now, on closer reading of https://swagger.io/docs/specification/2-0/authentication/ that this is in fact incorrect and the logic should be

  1. check for a global security block (with keys but those can be with or without scope info)
  2. then if a path has a security: [] that implies the path has no security,
  3. else if the path has no security then use the global,
  4. else if the path defines a different security, such as
security:
  - somethingElse: []

then look for an auth middleware under the key somethingElse as passed in to connector configuration options, or if there are scopes

security:
  - somethingElse: [read, write]

then look for an auth middleware under the key read,write as passed in to connector configuration options, or if there are scopes

I'll double check how this works in Open API v3 as well and get back to you. In the meantime please ensure you are upgraded to version 3.1.0 of this module.

@davesag davesag added bug Something isn't working in progress labels Oct 23, 2019
@djcuvcuv
Copy link
Author

Hi @djcuvcuv that's interesting, and yes appears to be a bug in this module. FYI the code for normaliseSecurity was updated in the latest release (see PR #70) but yes, this assumes that the presence of a security: [] block under a path just meant a security block without a scope and attempts to map it accordingly.

I can see now, on closer reading of https://swagger.io/docs/specification/2-0/authentication/ that this is in fact incorrect and the logic should be

  1. check for a global security block (with keys but those can be with or without scope info)
  2. then if a path has a security: [] that implies the path has no security,
  3. else if the path has no security then use the global,
  4. else if the path defines a different security, such as
security:
  - somethingElse: []

then look for an auth middleware under the key somethingElse as passed in to connector configuration options, or if there are scopes

security:
  - somethingElse: [read, write]

then look for an auth middleware under the key read,write as passed in to connector configuration options, or if there are scopes

I'll double check how this works in Open API v3 as well and get back to you. In the meantime please ensure you are upgraded to version 3.1.0 of this module.

Thanks very much @davesag. It is an interesting and subtle thing but essential for those that want 99% of their APIs to use the global security, and only add the empty security ("no security") stanza to the exempt 1%.

Ah ok, I did not know this was updated in the latest version. I will go ahead and give that a try as well. And for what it is worth, I am using swagger v2.0, but do intend at some point to join the openAPI v3.0 party in the not so far future. Thanks again!

@davesag
Copy link
Owner

davesag commented Oct 24, 2019

Hi @djcuvcuv just letting you know I will have a proper fix for this ready soon. I am just cleaning up the unit tests now.

davesag added a commit that referenced this issue Oct 24, 2019
davesag added a commit that referenced this issue Oct 24, 2019
davesag added a commit that referenced this issue Oct 24, 2019
[bug fix, #71] global security vs path level security
@davesag
Copy link
Owner

davesag commented Oct 24, 2019

This is now resolved in version 3.1.1. Please upgrade.

@djcuvcuv
Copy link
Author

@davesag Thanks very much for your quick turnaround on this. Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants