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

Added custom OAuth2 client authentication #286

Merged
merged 9 commits into from Nov 19, 2017
Merged

Conversation

@cvillerm
Copy link
Contributor

cvillerm commented Dec 22, 2016

In relation with issue #285 to allow custom OAuth2 client authentication, this PR allows option clientSecret to be passed as an object, that will be used to be merged with Wreck request options when calling the token endpoint.

This can allow passing a client certificate (e.g. clientSecret: { agent: new Https.Agent({ cert: myClientCert, key: myClientKey }) } ) to be used for client-certificate-based authentication when calling the OAuth2 Authorization Server token endpoint.

If clientSecret is formatted as a string, this is behaving as before (backward compatibility).

Unit tests and updated API.md included.
Test results:

122 tests complete
Test duration: 2905 ms
No global variable leaks detected
Coverage: 100.00%
Linting results: No issues
@ldesplat ldesplat added the feature label Jan 26, 2017
@feenst

This comment has been minimized.

Copy link

feenst commented Sep 22, 2017

Any word on this?

Copy link
Contributor

ldesplat left a comment

If we specify an object with an oauth v1 provider, then this breaks. Does it make sense to support it for oauth v1? If not, then we should not allow it in the Joi schema when protocol = oauth

@ldesplat ldesplat added this to the 8.9.0 milestone Sep 24, 2017
@cvillerm

This comment has been minimized.

Copy link
Contributor Author

cvillerm commented Sep 25, 2017

@ldesplat, thanks for this review.
I updated the validation schema accordingly (and refreshed from the latest master branch).

I ran local tests of the updated validation schema:

const Joi = require('joi');

clientSecretSchema = {
    protocol: Joi.string(),
    secret: Joi.alternatives().when('protocol', {
        is: 'oauth',
        then: Joi.string().required().allow(''),
        otherwise: Joi.alternatives().try(Joi.string().allow(''), Joi.object())
    }).required()
};

Joi.assert({ protocol: 'oauth2', secret: { a: 12 } }, clientSecretSchema); // Pass

Joi.assert({ protocol: 'oauth', secret: 'a' }, clientSecretSchema); // Pass
Joi.assert({ protocol: 'oauth', secret: '' } , clientSecretSchema); // Pass

Joi.assert({ protocol: 'oauth2', secret: 'a' }, clientSecretSchema); // Pass
Joi.assert({ protocol: 'oauth2', secret: '' }, clientSecretSchema); // Pass

Joi.assert({ protocol: 'oauth2' }, clientSecretSchema); // Fail

Joi.assert({ protocol: 'oauth', secret: 12 }, clientSecretSchema); // Fail
Joi.assert({ protocol: 'oauth2', secret: 12 }, clientSecretSchema); // Fail
Joi.assert({ protocol: 'oauth', secret: { a: '12' } }, clientSecretSchema); // Fail

I didn't add formal unit tests as I am not sure that current tests are going down to this level of details, but if you think that I should do, let me know.

@feenst

This comment has been minimized.

Copy link

feenst commented Oct 16, 2017

Is this good to merge?

@hueniverse hueniverse removed this from the 8.9.0 milestone Nov 4, 2017
@ldesplat ldesplat added this to the 8.9.0 milestone Nov 19, 2017
@ldesplat

This comment has been minimized.

Copy link
Contributor

ldesplat commented Nov 19, 2017

Thanks!

@ldesplat ldesplat merged commit c8781d9 into hapijs:master Nov 19, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.