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

Ensure only 1 authentication method is used during /token access in o… #99

Merged
merged 5 commits into from
Jun 10, 2015

Conversation

ldesplat
Copy link
Contributor

@ldesplat ldesplat commented Jun 5, 2015

…auth2

Added authMethod in provider options schema which is required when oauth2 is selected.

Now, when retrieving the token bell will either use body parameters or basic header auth but not both at the same time. See Issue #98 .

I have updated all the oauth2 providers as per their specs:

arcgisonline
dropbox
facebook
foursquare
github
google
instagram
linkedin
live
nest
phabricator
reddit
vk

@geek geek added the feature New functionality or improvement label Jun 9, 2015
@geek geek self-assigned this Jun 9, 2015
@geek geek added this to the 3.0.1 milestone Jun 9, 2015
@geek geek added the breaking changes Change that can breaking existing code label Jun 9, 2015
@@ -38,6 +38,7 @@ internals.schema = Joi.object({
protocol: Joi.string().valid('oauth', 'oauth2'),
temporary: Joi.string().when('protocol', { is: 'oauth', then: Joi.required(), otherwise: Joi.forbidden() }),
auth: Joi.string().required(),
authMethod: Joi.string().valid('basic', 'param').when('protocol', { is: 'oauth2', then: Joi.required(), otherwise: Joi.forbidden() }),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would implement this as a boolean since there are only two ways to authenticate. I would make the default using the header because that's the proper way to do it and allow for a params override.

@ldesplat
Copy link
Contributor Author

@hueniverse Updated as per your comments. Thank you!

@@ -71,6 +71,7 @@ The `server.auth.strategy()` method requires the following strategy options:
- `'oauth'` - OAuth 1.0a
- `'oauth2'` - OAuth 2.0
- `temporary` - the temporary credentials (request token) endpoint (OAuth 1.0a only).
- `useParamsAuth` - boolean that determines if OAuth client id and client secret will be sent as parameters as opposed to an Authorization header (OAuth 2.0 only). Defaults to false.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

false

@ldesplat
Copy link
Contributor Author

Thanks, done.

geek added a commit that referenced this pull request Jun 10, 2015
Ensure only 1 authentication method is used during /token access in o…
@geek geek merged commit 98419f5 into hapijs:master Jun 10, 2015
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking changes Change that can breaking existing code feature New functionality or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants