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

Add client secret to refreshToken and remove ID token validation #433

Merged
merged 2 commits into from
Dec 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions src/auth/OAuthAuthenticator.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,57 +206,62 @@ OAuthAuthenticator.prototype.passwordGrant = function(userData, options, cb) {
};

/**
* Sign in using a refresh token
* Exchange a refresh token
*
* @method refreshToken
* @memberOf module:auth.OAuthAuthenticator.prototype
*
* @example <caption>
* Given a refresh token from a previous authentication request
* it will return a JSON with the access_token and id_token.
* it will return a JSON with the access_token and id_token if
* the openid scope was originally included.
* More information in the
* <a href="https://auth0.com/docs/api/authentication#refresh-token">
* API Docs
* </a>.
* </caption>
*
* var data = {
* client_id: '{CLIENT_ID}', // Optional field.
* refresh_token: '{REFRESH_TOKEN}',
* };
*
* auth0.oauth.refreshToken(data, function (err, userData) {
* auth0.oauth.refreshToken(data, function (err, data) {
* if (err) {
* // Handle error.
* }
*
* console.log(userData);
* console.log(data);
lbalmaceda marked this conversation as resolved.
Show resolved Hide resolved
* });
*
* @param {Object} userData User credentials object.
* @param {String} userData.refresh_token Refresh token.
* @param {Object} data Data object.
* @param {String} data.refresh_token Refresh token.
*
* @return {Promise|undefined}
*/
OAuthAuthenticator.prototype.refreshToken = function(userData, cb) {
var params = {
type: 'token'
};
OAuthAuthenticator.prototype.refreshToken = function(data, cb) {
if (!data || typeof data !== 'object') {
lbalmaceda marked this conversation as resolved.
Show resolved Hide resolved
throw new ArgumentError('Missing data object');
}

var defaultFields = {
client_id: this.clientId,
client_secret: this.clientSecret,
joshcanhelp marked this conversation as resolved.
Show resolved Hide resolved
grant_type: 'refresh_token'
};
var data = extend(defaultFields, userData);
if (!userData || typeof userData !== 'object') {
throw new ArgumentError('Missing user data object');
}

var data = extend(defaultFields, data);
if (typeof data.refresh_token !== 'string' || data.refresh_token.split().length === 0) {
throw new ArgumentError('refresh_token is required');
}

var params = {
type: 'token'
};

if (cb && cb instanceof Function) {
return this.oauthWithIDTokenValidation.create(params, data, cb);
return this.oauth.create(params, data, cb);
joshcanhelp marked this conversation as resolved.
Show resolved Hide resolved
}
return this.oauthWithIDTokenValidation.create(params, data);
return this.oauth.create(params, data);
};

/**
Expand Down
17 changes: 16 additions & 1 deletion test/auth/oauth.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ describe('OAuthAuthenticator', function() {
.reply(200);
});
it('should require an object as first argument', function() {
expect(this.authenticator.refreshToken).to.throw(ArgumentError, 'Missing user data object');
expect(this.authenticator.refreshToken).to.throw(ArgumentError, 'Missing data object');
});
it('should require a refreshToken', function() {
var auth = this.authenticator;
Expand Down Expand Up @@ -531,6 +531,21 @@ describe('OAuthAuthenticator', function() {
})
.catch(done);
});
it('should include the Auth0 client secret in the request', function(done) {
nock.cleanAll();
var request = nock(API_URL)
.post(path, function(body) {
return body.client_secret === CLIENT_SECRET;
})
.reply(200);
this.authenticator
.refreshToken(userData)
.then(function() {
expect(request.isDone()).to.be.true;
done();
})
.catch(done);
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't calling done imply the test is over successfully? And if so, I don't think we want that for the case where something goes wrong like here that an exception is thrown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I'm not totally sure. This is only one of a few tests for refreshToken and we're just looking for something specific, that the secret is there.

});
it('should use refresh_token as default grant type', function(done) {
nock.cleanAll();
var request = nock(API_URL)
Expand Down