Add error message when there is no access_token and id_token is HS256 #727
Conversation
luisrudge
changed the title
src/web-auth/index.js
Outdated
| var noAccessTokenError = { | ||
| error: 'invalid_token', | ||
| description: 'The id_token cannot be validated because it was signed with the HS256 algorithm and public clients (like a browser) can’t store secrets. Please read the associated doc for possible ways to fix this.', | ||
| error_uri: 'https://auth0.com/docs/errors/libraries/auth0-js/invalid-token#parsing-an-hs256-signed-id-token-without-an-access-token' |
There was a problem hiding this comment.
is this a new field on the "error object" or you have already been using it?
There was a problem hiding this comment.
Why are you hardcoding the URL? IMO this is not easy to maintain. Anyway, instead of adding a new property to the error object I'd append it to the description.
| @@ -1487,7 +1515,7 @@ describe('auth0.WebAuth', function() { | |||
| stub(SilentAuthenticationHandler.prototype, 'login', function(usePostMessage, cb) { | |||
There was a problem hiding this comment.
Please clarify in the test name why this token validation is going to fail and which token you're talking about (I'm guessing the AT)
There was a problem hiding this comment.
I renamed the test, but I only changed the hash value it doesn't try to validate the access_token
| @@ -95,7 +95,7 @@ describe('auth0.WebAuth', function() { | |||
| stub(SilentAuthenticationHandler.prototype, 'login', function(usePostMessage, cb) { | |||
| cb( | |||
| null, | |||
| '#state=foo&id_token=eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IlF6RTROMFpCTTBWRFF6RTJSVVUwTnpJMVF6WTFNelE0UVRrMU16QXdNRUk0UkRneE56RTRSZyJ9.eyJpc3MiOiJodHRwczovL3dwdGVzdC5hdXRoMC5jb20vIiwic3ViIjoiYXV0aDB8NTVkNDhjNTdkNWIwYWQwMjIzYzQwOGQ3IiwiYXVkIjoiZ1lTTmxVNFlDNFYxWVBkcXE4elBRY3VwNnJKdzFNYnQiLCJleHAiOjE0ODI5NjkwMzEsImlhdCI6MTQ4MjkzMzAzMSwibm9uY2UiOiJhc2ZkIn0.PPoh-pITcZ8qbF5l5rMZwXiwk5efbESuqZ0IfMUcamB6jdgLwTxq-HpOT_x5q6-sO1PBHchpSo1WHeDYMlRrOFd9bh741sUuBuXdPQZ3Zb0i2sNOAC2RFB1E11mZn7uNvVPGdPTg-Y5xppz30GSXoOJLbeBszfrVDCmPhpHKGGMPL1N6HV-3EEF77L34YNAi2JQ-b70nFK_dnYmmv0cYTGUxtGTHkl64UEDLi3u7bV-kbGky3iOOCzXKzDDY6BBKpCRTc2KlbrkO2A2PuDn27WVv1QCNEFHvJN7HxiDDzXOsaUmjrQ3sfrHhzD7S9BcCRkekRfD9g95SKD5J0Fj8NA' | |||
| '#state=foo&access_token=123&id_token=eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6IlF6RTROMFpCTTBWRFF6RTJSVVUwTnpJMVF6WTFNelE0UVRrMU16QXdNRUk0UkRneE56RTRSZyJ9.eyJpc3MiOiJodHRwczovL3dwdGVzdC5hdXRoMC5jb20vIiwic3ViIjoiYXV0aDB8NTVkNDhjNTdkNWIwYWQwMjIzYzQwOGQ3IiwiYXVkIjoiZ1lTTmxVNFlDNFYxWVBkcXE4elBRY3VwNnJKdzFNYnQiLCJleHAiOjE0ODI5NjkwMzEsImlhdCI6MTQ4MjkzMzAzMSwibm9uY2UiOiJhc2ZkIn0.PPoh-pITcZ8qbF5l5rMZwXiwk5efbESuqZ0IfMUcamB6jdgLwTxq-HpOT_x5q6-sO1PBHchpSo1WHeDYMlRrOFd9bh741sUuBuXdPQZ3Zb0i2sNOAC2RFB1E11mZn7uNvVPGdPTg-Y5xppz30GSXoOJLbeBszfrVDCmPhpHKGGMPL1N6HV-3EEF77L34YNAi2JQ-b70nFK_dnYmmv0cYTGUxtGTHkl64UEDLi3u7bV-kbGky3iOOCzXKzDDY6BBKpCRTc2KlbrkO2A2PuDn27WVv1QCNEFHvJN7HxiDDzXOsaUmjrQ3sfrHhzD7S9BcCRkekRfD9g95SKD5J0Fj8NA' | |||
There was a problem hiding this comment.
why have you changed this?
There was a problem hiding this comment.
added the AT so it doesn't try to validate it
src/web-auth/index.js
Outdated
| @@ -283,6 +283,14 @@ WebAuth.prototype.validateAuthenticationResponse = function(options, parsedHash, | |||
| if (validationError.error !== 'invalid_token') { | |||
| return callback(validationError); | |||
| } | |||
| if (!parsedHash.access_token) { | |||
There was a problem hiding this comment.
how can you know with just this !parsedHash.access_token that the token is HS256 signed?
There was a problem hiding this comment.
ops, moved it below the .alg check
| @@ -289,6 +289,14 @@ WebAuth.prototype.validateAuthenticationResponse = function(options, parsedHash, | |||
| if (decodedToken.header.alg !== 'HS256') { | |||
There was a problem hiding this comment.
Is there any chance that this get called with a HS384/512 alg? That also uses "raw secrets" and the warning message below would apply to them as well.
No description provided.