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

Fixed OAuth2 Password flow to match the RFC requirements #3226

Closed
wants to merge 2 commits into from

Conversation

frol
Copy link

@frol frol commented Jun 10, 2017

RFC 6749 says that the scope parameter should be named as scope (rather than scopes) and there is no need to encode the string as it is passed via form parameters, which are encoded automatically.

RFC 6749 says that the `scope` parameter should be named as `scope` (rather than `scopes`) and there is no need to encode the string as it is passed via form parameters, which are encoded automatically.
@frol frol changed the title Fixed OAuth2 Password flow to match the RFC requirements WIP: Fixed OAuth2 Password flow to match the RFC requirements Jun 10, 2017
@frol
Copy link
Author

frol commented Jun 10, 2017

While this PR is valid, I discovered another inconsistency with RFC, so I will report the issue shortly and then update the PR to fix it.

@frol frol changed the title WIP: Fixed OAuth2 Password flow to match the RFC requirements Fixed OAuth2 Password flow to match the RFC requirements Jun 10, 2017
@frol
Copy link
Author

frol commented Jun 10, 2017

Well, I think this PR can be merged as is because it can be considered as a typo (other OAuth2 Flows use scope parameter name just fine).

Here is my thorough summary of the state of OAuth2 Password Flow implementation in Swagger-UI: #3227.

@zyro23
Copy link

zyro23 commented Jun 24, 2017

please merge. hit this just now. issue confirmed - parameter needs to be named scope.

@MugeSo
Copy link
Contributor

MugeSo commented Jul 21, 2017

@frol
I send duplicated PR #3404 and it merged, and now I found this.
Please closed this.

@shockey
Copy link
Contributor

shockey commented Aug 11, 2017

Thanks @frol, as @MugeSo mentioned, there was another PR that was merged for this.

Sorry this didn't get onto our radar first - thanks for the contribution!

Closing.

@shockey shockey closed this Aug 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants