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.

Sign up for GitHub

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 token.verify and deprecated idToken.verify #49

Merged
merged 6 commits into from
Oct 27, 2016
Merged

Added token.verify and deprecated idToken.verify #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f6aafb9
Add token.verify and deprecate idToken.verify
lboyette-okta Oct 17, 2016
067eba7
Added more tests
lboyette-okta Oct 21, 2016
e7fc3fd
Fixed Edge crypto bug
lboyette-okta Oct 25, 2016
1fc314a
Remove idToken validation for getWithRedirect, getWithoutPrompt and g…
lboyette-okta Oct 25, 2016
1672f66
Addressed tokenDict feedback
lboyette-okta Oct 26, 2016
0454de8
Removed the filter
lboyette-okta Oct 27, 2016
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
5 changes: 3 additions & 2 deletions lib/clientBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function OktaAuthBuilder(args) {
sdk.idToken = {
authorize: util.deprecateWrap('Use token.getWithoutPrompt, token.getWithPopup, or token.getWithRedirect ' +
'instead of idToken.authorize.', util.bind(token.getToken, null, sdk)),
verify: util.bind(token.verifyIdToken, null, sdk),
verify: util.deprecateWrap('Use token.verify instead of idToken.verify', util.bind(token.verifyIdToken, null, sdk)),
refresh: util.deprecateWrap('Use token.refresh instead of idToken.refresh',
util.bind(token.refreshIdToken, null, sdk)),
decode: util.deprecateWrap('Use token.decode instead of idToken.decode', token.decodeToken)
Expand All @@ -99,7 +99,8 @@ function OktaAuthBuilder(args) {
parseFromUrl: util.bind(token.parseFromUrl, null, sdk),
decode: token.decodeToken,
refresh: util.bind(token.refreshToken, null, sdk),
getUserInfo: util.bind(token.getUserInfo, null, sdk)
getUserInfo: util.bind(token.getUserInfo, null, sdk),
verify: util.bind(token.verifyToken, null, sdk)
};

// This is exposed so we can set window.location in our tests
Expand Down
7 changes: 7 additions & 0 deletions lib/crypto.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
var util = require('./util');

function verifyToken(idToken, key) {
key = util.clone(key);

var format = 'jwk';
var algo = {
name: 'RSASSA-PKCS1-v1_5',
Expand All @@ -21,6 +23,11 @@ function verifyToken(idToken, key) {
var extractable = true;
var usages = ['verify'];

// https://connect.microsoft.com/IE/feedback/details/2242108/webcryptoapi-importing-jwk-with-use-field-fails
// This is a metadata tag that specifies the intent of how the key should be used.
// It's not necessary to properly verify the jwt's signature.
delete key.use;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a metadata tag that specifies the intent of how the key should be used. It's not necessary to properly verify the key's signature, per @yuliu-okta.


return crypto.subtle.importKey(
format,
key,
Expand Down
55 changes: 51 additions & 4 deletions lib/oauthUtil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
var http = require('./http');
var util = require('./util');
var AuthSdkError = require('./errors/AuthSdkError');
var config = require('./config');
var storageBuilder = require('./storageBuilder');

var httpCache = storageBuilder(localStorage, config.CACHE_STORAGE_NAME);

function isToken(obj) {
if (obj &&
Expand Down Expand Up @@ -43,18 +47,60 @@ function loadPopup(src, options) {
return window.open(src, title, appearance);
}

function getWellKnown(sdk) {
// TODO: Use the issuer when known (usually from the id_token)
return http.get(sdk, sdk.options.url + '/.well-known/openid-configuration', {
function getWellKnown(sdk, issuer) {
return http.get(sdk, (issuer || sdk.options.url) + '/.well-known/openid-configuration', {
cacheResponse: true
});
}

function validateClaims(sdk, claims, aud, iss) {
function getKey(sdk, issuer, kid) {
return getWellKnown(sdk, issuer)
.then(function(wellKnown) {
var jwksUri = wellKnown['jwks_uri'];

// Check our kid against the cached version (if it exists and isn't expired)
var cacheContents = httpCache.getStorage();
var cachedResponse = cacheContents[jwksUri];
if (cachedResponse && Date.now()/1000 < cachedResponse.expiresAt) {
var cachedKey = util.find(cachedResponse.response.keys, {
kid: kid
});

if (cachedKey) {
return cachedKey;
}
}

// Remove cache for the key
httpCache.clearStorage(jwksUri);

// Pull the latest keys if the key wasn't in the cache
return http.get(sdk, jwksUri, {
cacheResponse: true
})
.then(function(res) {
var key = util.find(res.keys, {
kid: kid
});

if (key) {
return key;
}

throw new AuthSdkError('The key id, ' + kid + ', was not found in the server\'s keys');
});
});
}

function validateClaims(sdk, claims, aud, iss, nonce) {
if (!claims || !iss || !aud) {
throw new AuthSdkError('The jwt, iss, and aud arguments are all required');
}

if (nonce && claims.nonce !== nonce) {
throw new AuthSdkError('OAuth flow response nonce doesn\'t match request nonce');
}

var now = Math.floor(new Date().getTime()/1000);

if (claims.iss !== iss) {
Expand Down Expand Up @@ -180,6 +226,7 @@ function hashToObject(hash) {

module.exports = {
getWellKnown: getWellKnown,
getKey: getKey,
validateClaims: validateClaims,
getOAuthUrls: getOAuthUrls,
loadFrame: loadFrame,
Expand Down
9 changes: 7 additions & 2 deletions lib/storageBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@ function storageBuilder(webstorage, storageName) {
}
}

function clearStorage() {
setStorage({});
function clearStorage(key) {
if (!key) {
setStorage({});
}
var storage = getStorage();
delete storage[key];
setStorage(storage);
}

function updateStorage(key, value) {
Expand Down
152 changes: 86 additions & 66 deletions lib/token.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,37 @@ function verifyIdToken(sdk, idToken, options) {
});
}

function verifyToken(sdk, token, nonce, ignoreSignature) {
return new Q()
.then(function() {
if (!token || !token.idToken) {
throw new AuthSdkError('Only idTokens may be verified');
}

var jwt = decodeToken(token.idToken);

// Standard claim validation
oauthUtil.validateClaims(sdk, jwt.payload, token.clientId, token.issuer, nonce);
Copy link
Contributor

Choose a reason for hiding this comment

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

We dont expect validateClaims return any info before we proceed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

validateClaims is a synchronous method that throws errors. validateClaims could be renamed to assertClaims, but it does additional validation that makes the term "assert" feel inaccurate.


// If the browser doesn't support native crypto or we choose not
// to verify the signature, bail early
if (ignoreSignature || !sdk.features.isTokenVerifySupported()) {
return token;
}

return oauthUtil.getKey(sdk, token.issuer, jwt.header.kid)
.then(function(key) {
return sdkCrypto.verifyToken(token.idToken, key);
})
.then(function(valid) {
if (!valid) {
throw new AuthSdkError('The token signature is not valid');
}
return token;
});
});
}

function refreshIdToken(sdk, options) {
options = options || {};
options.display = null;
Expand Down Expand Up @@ -151,87 +182,75 @@ function addFragmentListener(sdk, windowEl, timeout) {
function handleOAuthResponse(sdk, oauthParams, res, urls) {
urls = urls || {};

if (res['error'] || res['error_description']) {
throw new OAuthError(res['error'], res['error_description']);
}

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

var tokenTypes = oauthParams.responseType;
var scopes = util.clone(oauthParams.scopes);
var tokenDict = {};
var clientId = oauthParams.clientId || sdk.options.clientId;

if (res['id_token']) {
var jwt = sdk.token.decode(res['id_token']);
if (jwt.payload.nonce !== oauthParams.nonce) {
throw new AuthSdkError('OAuth flow response nonce doesn\'t match request nonce');
return new Q()
.then(function() {
if (res['error'] || res['error_description']) {
throw new OAuthError(res['error'], res['error_description']);
}

var clientId = oauthParams.clientId || sdk.options.clientId;
oauthUtil.validateClaims(sdk, jwt.payload, clientId, urls.issuer);

var idToken = {
idToken: res['id_token'],
claims: jwt.payload,
expiresAt: jwt.payload.exp,
scopes: scopes,
authorizeUrl: urls.authorizeUrl,
issuer: urls.issuer
};
if (res.state !== oauthParams.state) {
throw new AuthSdkError('OAuth flow response state doesn\'t match request state');
}

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

if (res['access_token']) {
tokenDict['token'] = {
accessToken: res['access_token'],
expiresAt: Number(res['expires_in']) + Math.floor(Date.now()/1000),
tokenType: res['token_type'],
scopes: scopes,
authorizeUrl: urls.authorizeUrl,
userinfoUrl: urls.userinfoUrl
};
}
}

if (res['access_token']) {
var accessToken = {
accessToken: res['access_token'],
expiresAt: Number(res['expires_in']) + Math.floor(Date.now()/1000),
tokenType: res['token_type'],
scopes: scopes,
authorizeUrl: urls.authorizeUrl,
userinfoUrl: urls.userinfoUrl
};

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

if (res['code']) {
var authorizationCode = {
authorizationCode: res['code']
};
if (res['id_token']) {
var jwt = sdk.token.decode(res['id_token']);

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

if (Array.isArray(tokenTypes)) {
tokenDict['code'] = authorizationCode;
} else {
return authorizationCode;
return verifyToken(sdk, idToken, oauthParams.nonce, true)
.then(function(token) {
tokenDict['id_token'] = idToken;
return tokenDict;
});
}
}

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

var tokens = [];
return tokenDict;
})
.then(function(tokenDict) {
if (!Array.isArray(tokenTypes)) {
return tokenDict[tokenTypes];
}

// Create token array in the order of the responseType array
for (var t = 0, tl = tokenTypes.length; t < tl; t++) {
var tokenType = tokenTypes[t];
if (tokenDict[tokenType]) {
tokens.push(tokenDict[tokenType]);
if (!tokenDict['token'] && !tokenDict['id_token']) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big fan of tracking the tokenDict state outside of the promise chain approach. Couple alternatives:

  1. (preferred) Don't worry about response format in the preceding block. Always do the checks, and return an array of the tokens that you've extracted. In this block, just do an array sort on the tokens (by the tokenTypes), or return the token itself if the tokenType is not an array.
  2. Create a new tokenDict in the preceding promise block, and then return that, and accept it as an argument here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why 1 is preferred over 2. To do the sorting of the tokens (and checking for the presence of an idToken or accessToken), it seems more reasonable to return a tokenDict and build the array of tokens in this block.

throw new AuthSdkError('Unable to parse OAuth flow response');
}
}

return tokens;
// Create token array in the order of the responseType array
return tokenTypes.map(function(item) {
return tokenDict[item];
});
});
}

function getDefaultOAuthParams(sdk, oauthOptions) {
Expand Down Expand Up @@ -625,5 +644,6 @@ module.exports = {
decodeToken: decodeToken,
verifyIdToken: verifyIdToken,
refreshToken: refreshToken,
getUserInfo: getUserInfo
getUserInfo: getUserInfo,
verifyToken: verifyToken
};
Loading