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 2.0 's security scheme validated against OAS 3.0 instead of swagger 2.0 #1711

Closed
shockey opened this issue Mar 24, 2018 · 12 comments · Fixed by #1985
Closed

Swagger 2.0 's security scheme validated against OAS 3.0 instead of swagger 2.0 #1711

shockey opened this issue Mar 24, 2018 · 12 comments · Fixed by #1985

Comments

@shockey
Copy link
Contributor

shockey commented Mar 24, 2018

From @pbarbier on March 23, 2018 18:15

When editing a swagger 2.0 yaml content featuring oauth2 security schemes like below:

swagger: "2.0"

info:
  title: wow
  version: 1.0.0
  
paths:
  /:
    get:
      description: wow
      responses:
        200:
          description: ok

securityDefinitions:
  oauth2_Password:
    type: oauth2
    tokenUrl: /authorizationserver/oauth/token
    flow: password
    scopes:
      basic: ''
  oauth2_client_credentials:
    type: oauth2
    tokenUrl: /authorizationserver/oauth/token
    flow: application
    scopes:
      extended: ''
  apikey:
    type: apiKey
    name: myAuth
  basic:
    type: basic
    in: header

Several nonsensical errors are reported:

Structural error at securityDefinitions.oauth2_Password
should NOT have additional properties
additionalProperty: tokenUrl, flow, scopes

Structural error at securityDefinitions.oauth2_Password.type
should be equal to one of the allowed values
allowedValues: basic, apiKey

Structural error at securityDefinitions.oauth2_Password.tokenUrl
should match format "uri"
format: uri

Structural error at securityDefinitions.oauth2_Password.flow
should be equal to one of the allowed values
allowedValues: implicit, application, accessCode

Structural error at securityDefinitions.oauth2_client_credentials
should NOT have additional properties
additionalProperty: tokenUrl, flow, scopes

Structural error at securityDefinitions.oauth2_client_credentials.type
should be equal to one of the allowed values
allowedValues: basic, apiKey

Structural error at securityDefinitions.oauth2_client_credentials.tokenUrl
should match format "uri"
format: uri

Structural error at securityDefinitions.oauth2_client_credentials.flow
should be equal to one of the allowed values
allowedValues: implicit, password, accessCode

Structural error at securityDefinitions.apikey
should NOT have additional properties
additionalProperty: name

Structural error at securityDefinitions.apikey.type
should be equal to one of the allowed values
allowedValues: basic, oauth2

Structural error at securityDefinitions.basic
should NOT have additional properties
additionalProperty: in

Structural error at securityDefinitions.basic.type
should be equal to one of the allowed values
allowedValues: apiKey, oauth2

Q A
Bug or feature request? Bug
Which Swagger/OpenAPI version? Swagger 2.0
Which Swagger-UI version? 3.13.0
How did you install Swagger-UI? Use through editor.swagger.io
Which browser & version? Chrome latest
Which operating system? WIN10

Demonstration API definition

See above

Configuration (browser query string, constructor, config.yaml)

regular use of editor.swagger.io

Expected Behavior

No error should be shown for this part of the yaml

Current Behavior

See above

Possible Solution

Context

The error messages are misleading. They claim the content is not valid when it is.

Copied from original issue: swagger-api/swagger-ui#4364

@shockey
Copy link
Contributor Author

shockey commented Mar 24, 2018

@pbarbier, not quite sure what's going on here - the validator is in Swagger 2.0 mode, but the output is quite strange.

I'll look into this in the coming week. Sorry for the trouble!

@shockey shockey added this to the March 30, 2018 milestone Mar 24, 2018
@hkosova
Copy link
Contributor

hkosova commented Mar 24, 2018

@pbarbier I guess the relative tokenUrl is the issue. OpenAPI 2.0 Specification does not state whether OAuth endpoints can be relative. This was clarified in OpenAPI 3.0, which explicitly supports relative URLs in tokenUrl.

cc @webron

@pbarbier
Copy link

pbarbier commented Mar 24, 2018

@shockey While you're at it I have another oddity:

With:

securityDefinitions:
  oauth2:
    type: oauth2
    tokenUrl: /authorizationserver/oauth/token
    flow: password
    scopes:
      **basic: ''**

The following fires another error:

      security:
        - oauth2:
            **- basic**

The error: Security scope definition basic could not be resolved

@shockey
Copy link
Contributor Author

shockey commented Apr 6, 2018

@pbarbier, I've opened a PR that addresses the case you just mentioned 😄

As for the main part of this ticket, I'm going to backlog it for now, since we tend to batch schema validator fixes and this appears to be closely related to the other difficulties we're seeing at the moment.

@shockey shockey removed this from the March 30, 2018 milestone Apr 6, 2018
@webron
Copy link
Contributor

webron commented Apr 8, 2018

@hkosova is (as usual) right. Swagger/OAS2 does not allow for relative token URLs.

@vspiliopoulos
Copy link

vspiliopoulos commented Oct 8, 2018

@shockey Any plans for fixing this? I am facing the same issue as well

@anboo
Copy link

anboo commented Dec 17, 2018

I am facing the same issue too

@arpitgarg1
Copy link

arpitgarg1 commented Dec 19, 2018

I am facing same issue. Any solution please.

@shockey
Copy link
Contributor Author

shockey commented Jan 5, 2019

@vspiliopoulos @anboo @arpitgarg1 - please note @webron's comment above. If you're using Swagger 2.0 and a non-absolute token/authorization URL, you should fix that!

This ticket is still open so we can fix the quality of the errors reported. If you are seeing an error, there still most likely is something wrong with your definition!

@ksac1

This comment has been minimized.

@shockey
Copy link
Contributor Author

shockey commented Apr 11, 2019

I've changed the content of this ticket to cover other types of security schemes that spit out confusing errors as well.

shockey added a commit to shockey/swagger-editor that referenced this issue Apr 11, 2019
shockey added a commit to shockey/swagger-editor that referenced this issue Apr 11, 2019
shockey added a commit to shockey/swagger-editor that referenced this issue Apr 12, 2019
@shockey
Copy link
Contributor Author

shockey commented Apr 12, 2019

I've opened a pull request (#1985) that will close this issue.

Here's what Swagger Editor reports with my changes:

Structural error at securityDefinitions.oauth2_Password.tokenUrl
should be an absolute URI

Structural error at securityDefinitions.oauth2_client_credentials.tokenUrl
should be an absolute URI

Structural error at securityDefinitions.apikey
should have required property 'in'
missingProperty: in

Structural error at securityDefinitions.basic
should NOT have additional properties
additionalProperty: in

shockey added a commit to shockey/swagger-editor that referenced this issue Apr 17, 2019
shockey added a commit to shockey/swagger-editor that referenced this issue Apr 17, 2019
shockey added a commit to shockey/swagger-editor that referenced this issue Apr 17, 2019
shockey added a commit that referenced this issue Apr 18, 2019
* adopt @webron's OpenAPI 3.0 schema from OAI/OpenAPI-Specification#1270

permalink: https://github.com/OAI/OpenAPI-Specification/blob/92e15eba1d4591ebfe8c11898c48241e72854381/schemas/v3.0/schema.yaml

* add ajv-errors

* address error messages for #1808's Swagger 2.0 example

clarifies the schema and adds custom error messages for unclear error conditions

* address error messages for #1808's OpenAPI 3.0 example

* restrict underlying JSON Schema `type` field to simple types only (for #1832)

* fix limitation in JSON Pointer conversion helper

* add clear `not` error message (for #1489)

* add additionalProperties message (for #1394)

* add ajv-keywords

* use `switch` to intelligently identify inline vs referenced content (for #1853)

* use `switch` to XOR `schema` and `content` (for #1853)

* use `switch` to pivot security scheme based on type

(for #1672)

* use switch to fall-through to inline security scheme validation (for #1672)

* rewrite more Reference oneOfs (for #1519)

* add custom message for `Schema.required` type error (for #1519)

* rewrite Response/Reference oneOf (for #1489)

* use switch in ParameterLocation validation (for #1797)

* define pivot key switches for SecurityDefinitions (for #1711)

* give helpful `format: uri` messages for SecurityDefinitions (for #1711)

* eliminate NonBodyParameter; pivot on `Parameter.in` with a switch (for #1511)

* oneOf -> switch for Parameters.items reference

* (for #1711)

* remove redundant semantic validator (for #1511)

* adjust wording of custom error message (for #1853)

* add regression tests for all related issues

* revert to expect@^1.20.2

* linter fixes

* fix messaging flaw for #1832

* improve messaging for #1394

* use literal key for `$ref` in Reference Object

* remove commented legacy data from OAS3 schema

* remove superfluous quotation marks

* normalize test case paths to `/`

* normalize openapi fields to 3.0.0

* drop unused `paths` information

* ensure clear errors for 3.0 Parameter style/content exclusivity

* add `required` assertions to switch statements that pivot on a key's value

this prevents false positives when the pivot key is missing entirely

* remove stray space
shockey added a commit to shockey/swagger-editor that referenced this issue May 23, 2019
…i#1985)

* adopt @webron's OpenAPI 3.0 schema from OAI/OpenAPI-Specification#1270

permalink: https://github.com/OAI/OpenAPI-Specification/blob/92e15eba1d4591ebfe8c11898c48241e72854381/schemas/v3.0/schema.yaml

* add ajv-errors

* address error messages for swagger-api#1808's Swagger 2.0 example

clarifies the schema and adds custom error messages for unclear error conditions

* address error messages for swagger-api#1808's OpenAPI 3.0 example

* restrict underlying JSON Schema `type` field to simple types only (for swagger-api#1832)

* fix limitation in JSON Pointer conversion helper

* add clear `not` error message (for swagger-api#1489)

* add additionalProperties message (for swagger-api#1394)

* add ajv-keywords

* use `switch` to intelligently identify inline vs referenced content (for swagger-api#1853)

* use `switch` to XOR `schema` and `content` (for swagger-api#1853)

* use `switch` to pivot security scheme based on type

(for swagger-api#1672)

* use switch to fall-through to inline security scheme validation (for swagger-api#1672)

* rewrite more Reference oneOfs (for swagger-api#1519)

* add custom message for `Schema.required` type error (for swagger-api#1519)

* rewrite Response/Reference oneOf (for swagger-api#1489)

* use switch in ParameterLocation validation (for swagger-api#1797)

* define pivot key switches for SecurityDefinitions (for swagger-api#1711)

* give helpful `format: uri` messages for SecurityDefinitions (for swagger-api#1711)

* eliminate NonBodyParameter; pivot on `Parameter.in` with a switch (for swagger-api#1511)

* oneOf -> switch for Parameters.items reference

* (for swagger-api#1711)

* remove redundant semantic validator (for swagger-api#1511)

* adjust wording of custom error message (for swagger-api#1853)

* add regression tests for all related issues

* revert to expect@^1.20.2

* linter fixes

* fix messaging flaw for swagger-api#1832

* improve messaging for swagger-api#1394

* use literal key for `$ref` in Reference Object

* remove commented legacy data from OAS3 schema

* remove superfluous quotation marks

* normalize test case paths to `/`

* normalize openapi fields to 3.0.0

* drop unused `paths` information

* ensure clear errors for 3.0 Parameter style/content exclusivity

* add `required` assertions to switch statements that pivot on a key's value

this prevents false positives when the pivot key is missing entirely

* remove stray space
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants