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 access_token support to getWithoutPrompt #28

Merged
merged 4 commits into from
Jul 20, 2016
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
65 changes: 58 additions & 7 deletions lib/token.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,10 @@ function getIdToken(sdk, oauthOptions, options) {
throw new AuthSdkError('A clientId must be specified in the OktaAuth constructor to get an idToken');
}

if (util.isString(oauthParams.responseType) && oauthParams.responseType.indexOf(' ') !== -1) {

Choose a reason for hiding this comment

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

oauthParams.responseType.indexOf(' ') Is checking just space good enough?

Copy link
Contributor Author

@lboyette-okta lboyette-okta Jul 20, 2016

Choose a reason for hiding this comment

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

It should be. The way the authorize endpoint works, it accepts a response_type that's space separated ie. id_token token. We don't want devs to pass in these response types as a string and space-separation seems like the most common slipup.

Also, space-separation is the only scenario where the api works, but the sdk doesn't (it'd simply return the first token in the list). If a user passed a comma, semicolon, or anything else, the api would throw an error about an incorrect response_type and the sdk would pass that error to the dev.

throw new AuthSdkError('Multiple OAuth responseTypes must be defined as an array');
}

// Convert our params to their actual OAuth equivalents
var oauthQueryHash = util.removeNils({
'client_id': oauthParams.clientId,
Expand All @@ -357,6 +361,10 @@ function getIdToken(sdk, oauthOptions, options) {
'idp': oauthParams.idp
});

if (Array.isArray(oauthQueryHash['response_type'])) {
oauthQueryHash['response_type'] = oauthQueryHash['response_type'].join(' ');
}

if (oauthParams.scope.indexOf('openid') !== -1) {
oauthQueryHash.scope = oauthParams.scope.join(' ');
} else {
Expand All @@ -379,26 +387,69 @@ function getIdToken(sdk, oauthOptions, options) {
function handleOAuthResponse(res) {
if (res['error'] || res['error_description']) {
throw new OAuthError(res['error'], res['error_description']);
}

} else if (res['id_token']) {
if (res.state !== oauthParams.state) {
throw new AuthSdkError('OAuth flow response state doesn\'t match request state');
}
if (res.state !== oauthParams.state) {
throw new AuthSdkError('OAuth flow response state doesn\'t match request state');
}

// If we're passed an array of tokenTypes,
// we return an array in the order specified.
// Otherwise, we return only the token requested
var tokenTypes = oauthParams.responseType;
var scopes = oauthQueryHash.scope.split(' ');
var tokenDict = {};

if (res['id_token']) {
Copy link
Contributor

@rchild-okta rchild-okta Jul 20, 2016

Choose a reason for hiding this comment

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

Can we add some comment at the top of this function or somewhere in the code where we document the response types we're returning? I.e. something along the lines of "if we get an array of tokenTypes, we return an array in the order specified, else we just return the one token that was asked for"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do

var jwt = sdk.idToken.decode(res['id_token']);
if (jwt.payload.nonce !== oauthParams.nonce) {
throw new AuthSdkError('OAuth flow response nonce doesn\'t match request nonce');
}

validateClaims(jwt.payload, sdk.options.url, oauthParams.clientId);
return {

var idToken = {
idToken: res['id_token'],
claims: jwt.payload
claims: jwt.payload,
expiresAt: jwt.payload.exp,
scopes: scopes
};

} else {
if (Array.isArray(tokenTypes)) {
tokenDict['id_token'] = idToken;
} else {
return idToken;
}
}

if (res['access_token']) {
var accessToken = {
accessToken: res['access_token'],
expiresAt: Number(res['expires_in']) + Math.floor(Date.now()/1000),
Copy link
Contributor

Choose a reason for hiding this comment

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

Am curious about the behavior here when asking for id_token and token - do we expect to have a payload.exp for the idToken? And if so, are we overwriting it here with the expiresIn calculation?

Copy link
Contributor Author

@lboyette-okta lboyette-okta Jul 15, 2016

Choose a reason for hiding this comment

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

We will always have a payload.exp for the idToken, but we can't parse the access_token. We have to get expires_in from the oauth response (in the postMessage or hash fragment) and calculate expiresAt.

This will overwrite any expiresAt copied from the idToken. It's possible to remove expiresAt entirely for the idToken, because it does duplicate the information and it'd avoid confusion.

edit: removed expiresAt for idTokens

tokenType: res['token_type'],
scopes: scopes
};

if (Array.isArray(tokenTypes)) {
tokenDict['token'] = accessToken;
} else {
return accessToken;
}
}

if (!tokenDict['token'] && !tokenDict['id_token']) {
throw new AuthSdkError('Unable to parse OAuth flow response');
}

var tokens = [];

// Create token array in the order of the responseType array
for (var t = 0, tl = tokenTypes.length; t < tl; t++) {
var tokenType = tokenTypes[t];
tokens.push(tokenDict[tokenType]);
}

return tokens;
}

function getOrigin(url) {
Expand Down
Loading