From 56489f9ba1c98e3ec20214862fc35dfb06417ae0 Mon Sep 17 00:00:00 2001 From: Aaron Granick Date: Fri, 8 Jan 2021 00:04:17 +0000 Subject: [PATCH] detect error and error_description in redirect uri OKTA-348505 <<>> Artifact: okta-auth-js --- CHANGELOG.md | 6 ++ lib/oauthUtil.ts | 33 ++++++--- test/spec/oauthUtil.js | 150 ++++++++++++++++++++++++++++------------- test/spec/token.ts | 32 +++++++++ 4 files changed, 166 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58ba8b88c..b421256a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 4.6.0 + +### Other + +- [#583](https://github.com/okta/okta-auth-js/pull/583) Better error handling for redirect flows: if redirect URI contains `error` or `error_description` then `isLoginRedirect` will return true and `parseFromUrl` will throw `OAuthError` + ## 4.5.0 ### Features diff --git a/lib/oauthUtil.ts b/lib/oauthUtil.ts index aea4fd52b..d70078990 100644 --- a/lib/oauthUtil.ts +++ b/lib/oauthUtil.ts @@ -231,25 +231,40 @@ function hasCodeInUrl(hashOrSearch: string): boolean { return /(code=)/i.test(hashOrSearch); } +function hasErrorInUrl(hashOrSearch: string): boolean { + return /(error=)/i.test(hashOrSearch) || /(error_description)/i.test(hashOrSearch); +} + function isRedirectUri(uri: string, sdk: OktaAuth): boolean { var authParams = sdk.options; - return uri && uri.indexOf(authParams.redirectUri) == 0; + return uri && uri.indexOf(authParams.redirectUri) === 0; } /** * Check if tokens or a code have been passed back into the url, which happens in - * the social auth IDP redirect flow. + * the OIDC (including social auth IDP) redirect flow. */ function isLoginRedirect (sdk: OktaAuth) { + // First check, is this a redirect URI? + if (!isRedirectUri(window.location.href, sdk)){ + return false; + } + + // The location contains either a code, token, or an error&error_description var authParams = sdk.options; - if (authParams.pkce || authParams.responseType === 'code' || authParams.responseMode === 'query') { - // Look for code - var hasCode = authParams.responseMode === 'fragment' ? - hasCodeInUrl(window.location.hash) : - hasCodeInUrl(window.location.search); - return isRedirectUri(window.location.href, sdk) && hasCode; + var codeFlow = authParams.pkce || authParams.responseType === 'code' || authParams.responseMode === 'query'; + var useQuery = codeFlow && authParams.responseMode !== 'fragment'; + + if (hasErrorInUrl(useQuery ? window.location.search : window.location.hash)) { + return true; + } + + if (codeFlow) { + var hasCode = useQuery ? hasCodeInUrl(window.location.search) : hasCodeInUrl(window.location.hash); + return hasCode; } - // Look for tokens (Implicit OIDC flow) + + // implicit flow, will always be hash fragment return hasTokensInHash(window.location.hash); } diff --git a/test/spec/oauthUtil.js b/test/spec/oauthUtil.js index 52f578bbe..192e03fb3 100644 --- a/test/spec/oauthUtil.js +++ b/test/spec/oauthUtil.js @@ -913,6 +913,10 @@ describe('validateClaims', function () { }); describe('isLoginRedirect', function() { + const redirectUri = 'http://fake/login/callback'; + const issuer = 'https://auth-js-test.okta.com'; + const clientId = 'foo'; + let sdk; let originalLocation; beforeEach(() => { @@ -922,38 +926,54 @@ describe('isLoginRedirect', function() { window.location = originalLocation; }); + + function mockHash(hash) { + delete window.location; + window.location = { + hash: `#${hash}`, + href: `${redirectUri}#${hash}` + }; + } + + function mockSearch(search) { + delete window.location; + window.location = { + search: `?${search}`, + href: `${redirectUri}?${search}` + }; + } + describe('Implicit OIDC flow', () => { beforeEach(() => { sdk = new OktaAuth({ pkce: false, - issuer: 'https://auth-js-test.okta.com', - clientId: 'foo' + issuer, + clientId, + redirectUri }); }); it('should return true if there is id_token in hash', () => { - delete window.location; - window.location = { - hash: '#id_token=fakeidtoken' - }; + mockHash('id_token=fakeidtoken'); const result = oauthUtil.isLoginRedirect(sdk); expect(result).toBe(true); }); it('should return true if there is access_token in hash', () => { - delete window.location; - window.location = { - hash: '#access_token=fakeaccesstoken' - }; + mockHash('access_token=fakeaccesstoken'); const result = oauthUtil.isLoginRedirect(sdk); expect(result).toBe(true); }); it('should return false if there is no id or access token in hash', () => { - delete window.location; - window.location = { - hash: '#random_token=fakerandomtoken' - }; + mockHash('random_token=fakerandomtoken'); + const result = oauthUtil.isLoginRedirect(sdk); + expect(result).toBe(false); + }); + + it('should return false if current URI is not redirect URI', () => { + mockHash('id_token=fakeidtoken'); + window.location.href = 'http://fake/product/search' + window.location.hash; const result = oauthUtil.isLoginRedirect(sdk); expect(result).toBe(false); }); @@ -961,69 +981,107 @@ describe('isLoginRedirect', function() { describe('PKCE', () => { it('there should be code in hash when responseMode is fragment', () => { - delete window.location; - window.location = { - hash: '#code=fakecode', - href: 'https://exmple.com/implicit/callback#code=fakecode' - }; sdk = new OktaAuth({ pkce: true, responseMode: 'fragment', - issuer: 'https://auth-js-test.okta.com', - clientId: 'foo', - redirectUri: 'https://exmple.com/implicit/callback' + issuer, + clientId, + redirectUri }); + mockHash('code=fakecode'); const result = oauthUtil.isLoginRedirect(sdk); expect(result).toBe(true); }); it('there should be code in query when use default responseMode', () => { - delete window.location; - window.location = { - search: '?code=fakecode', - href: 'https://exmple.com/implicit/callback?code=fakecode' - }; sdk = new OktaAuth({ pkce: true, - issuer: 'https://auth-js-test.okta.com', - clientId: 'foo', - redirectUri: 'https://exmple.com/implicit/callback' + issuer, + clientId, + redirectUri }); + mockSearch('code=fakecode'); const result = oauthUtil.isLoginRedirect(sdk); expect(result).toBe(true); }); it('there should be code in query when responseMode is query', () => { - delete window.location; - window.location = { - search: '?code=fakecode', - href: 'https://exmple.com/implicit/callback?code=fakecode' - }; sdk = new OktaAuth({ pkce: true, responseMode: 'query', - issuer: 'https://auth-js-test.okta.com', - clientId: 'foo', - redirectUri: 'https://exmple.com/implicit/callback' + issuer, + clientId, + redirectUri }); + mockSearch('code=fakecode'); const result = oauthUtil.isLoginRedirect(sdk); expect(result).toBe(true); }); it('should return false if current URI is not redirect URI', () => { - delete window.location; - window.location = { - search: '?code=somecode', - href: 'https://exmple.com/products/search?code=somecode' - }; sdk = new OktaAuth({ pkce: true, - issuer: 'https://auth-js-test.okta.com', - clientId: 'foo', - redirectUri: 'https://exmple.com/implicit/callback' + issuer, + clientId, + redirectUri + }); + mockSearch('code=fakecode'); + window.location.href = 'https://exmple.com/products/search?code=somecode'; + const result = oauthUtil.isLoginRedirect(sdk); + expect(result).toBe(false); + }); + }); + + describe('OAuth errors', () => { + it('PKCE: should recognize error', () => { + sdk = new OktaAuth({ + pkce: true, + issuer, + clientId, + redirectUri }); + mockSearch('error=fakeerror&error_description=fakedescription'); + const result = oauthUtil.isLoginRedirect(sdk); + expect(result).toBe(true); + }); + + it('PKCE (fragment): should recognize error', () => { + sdk = new OktaAuth({ + pkce: true, + responseMode: 'fragment', + issuer, + clientId, + redirectUri + }); + mockHash('error=fakeerror&error_description=fakedescription'); + const result = oauthUtil.isLoginRedirect(sdk); + expect(result).toBe(true); + }); + + it('implicit flow: should recognize error', () => { + sdk = new OktaAuth({ + pkce: false, + issuer, + clientId, + redirectUri + }); + mockHash('error=fakeerror&error_description=fakedescription'); + const result = oauthUtil.isLoginRedirect(sdk); + expect(result).toBe(true); + }); + + it('should return false if current URI is not redirect URI', () => { + sdk = new OktaAuth({ + pkce: true, + issuer, + clientId, + redirectUri + }); + mockSearch('error=fakeerror&error_description=fakedescription'); + window.location.href = 'http://fake/product/search?error=fakeerror&error_description=fakedescription'; const result = oauthUtil.isLoginRedirect(sdk); expect(result).toBe(false); }); + }); }); diff --git a/test/spec/token.ts b/test/spec/token.ts index 278fc96b2..6838ccd2f 100644 --- a/test/spec/token.ts +++ b/test/spec/token.ts @@ -2847,6 +2847,38 @@ describe('token.parseFromUrl', function() { util.expectErrorToEqual(e, error); }); }); + + it('throws an OAuthError error if "error" and "error_description" in the url', () => { + const error = { + name: 'OAuthError', + message: 'fake_description', + errorCode: 'fake_error', + errorSummary: 'fake_description', + errorId: 'INTERNAL', + errorCauses: [] + }; + return oauthUtil.setupParseUrl({ + willFail: true, + hashMock: '#error=fake_error&error_description=fake_description', + oauthParams: JSON.stringify({ + responseType: ['id_token', 'token'], + state: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + nonce: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + scopes: ['openid', 'email'], + urls: { + issuer: 'https://auth-js-test.okta.com', + tokenUrl: 'https://auth-js-test.okta.com/oauth2/v1/token', + authorizeUrl: 'https://auth-js-test.okta.com/oauth2/v1/authorize', + userinfoUrl: 'https://auth-js-test.okta.com/oauth2/v1/userinfo' + } + }) + }) + .catch(function(e) { + util.expectErrorToEqual(e, error); + }); + + }); + }); describe('token.renew', function() {