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

adding responseType as parameter #79

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

adding responseType as parameter #79

wants to merge 1 commit into from

Conversation

jeffersonpenna
Copy link

I've got cases where the response_type is different from 'code', so I made a modification that leaves 'code' as default, but allows the user to enter the desired type of response_type.

@coveralls
Copy link

coveralls commented Feb 16, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 783d656 on jeffersonpenna:master into a02839a on jaredhanson:master.

@rwky
Copy link

rwky commented Jul 7, 2018

rwky added a commit to passport-next/passport-oauth2 that referenced this pull request Jul 7, 2018
@jaredhanson
Copy link
Owner

What response_type type do you need to set other than code? The authorization code flow is the one specified by the OAuth 2.0 specification, and any other response_type is likely to have a far different protocol (and thus not be compatible anyway).

(Similar to #107)

@kara-ryli
Copy link

What response_type type do you need to set other than code?

OpenID Connect hybrid flows would use code id_token or code id_token token, and would (in theory) be compatible with OAuth2 for the most part.

@jaredhanson
Copy link
Owner

OpenID Connect also introduces a number of other required checks (nonce, etc), that would not be handled by a base OAuth 2 strategy, such as this. That might lead to subtle security concerns, if you set code id_token or code id_token token and the strategy then does not also do the additional nonce checking and the like.

Using an OpenID Connect specific strategy would be more appropriate than overriding the grant_type.

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.

7 participants