From d769c58e46c7dbd6edac565c8595ede80724b897 Mon Sep 17 00:00:00 2001 From: meza Date: Sat, 8 Jul 2023 12:33:50 +0100 Subject: [PATCH] feat: added failure redirect override for each authorization request The failure urls now receive an error query string parameter describing the error. This is part of #138. Thanks @njpearman for all the suggestions! --- README.md | 25 +++++++++ src/Auth0RemixTypes.ts | 1 + src/index.test.ts | 125 +++++++++++++++++++++++++++-------------- src/index.ts | 29 +++++++++- 4 files changed, 137 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 22094725..0c0706d2 100644 --- a/README.md +++ b/README.md @@ -316,8 +316,33 @@ export const action: ActionFunction = () => { }; ``` +### Adding a redirect url override for each authorization request + +You can also specify a redirect url to be used for each authorization request. +This will override the default redirect url that you specified when you created the authenticator. + +```tsx +// src/routes/auth/callback.tsx +import { authenticator } from '../../auth.server'; +import type { ActionFunction } from '@remix-run/node'; + +export const action: ActionFunction = async ({ request }) => { + await authenticator.handleCallback(request, { + onSuccessRedirect: '/dashboard', // change this to be wherever you want to redirect to after a successful login + onFailureRedirect: '/login' // change this to be wherever you want to redirect to after a failed login + }); +}; +``` + ## Errors +### Authorization errors + +When the authorization process fails, the failure redirect url will be called with an `error` query parameter that +contains the error code auth0 has given us. + +### Verification errors + The verification errors each have a `code` property that you can use to determine what went wrong. | Code | Description | diff --git a/src/Auth0RemixTypes.ts b/src/Auth0RemixTypes.ts index 8d72a503..93ba8b5d 100644 --- a/src/Auth0RemixTypes.ts +++ b/src/Auth0RemixTypes.ts @@ -92,4 +92,5 @@ export type AuthorizeOptions = export interface HandleCallbackOptions { onSuccessRedirect?: string; + onFailureRedirect?: string; } diff --git a/src/index.test.ts b/src/index.test.ts index 5928d824..5e653807 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -1,7 +1,7 @@ /* eslint-disable max-nested-callbacks */ import { redirect } from '@remix-run/server-runtime'; import * as jose from 'jose'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, SpyInstance, vi } from 'vitest'; import { getCredentials, saveUserToSession } from './lib/session.js'; import { Auth0RemixServer, Token } from './index.js'; import type { Auth0RemixOptions } from './Auth0RemixTypes.js'; @@ -178,32 +178,61 @@ describe('Auth0 Remix Server', () => { await expect(authorizer.handleCallback(request, {})).rejects.toThrowError(redirectError); // a redirect happened const redirectUrl = vi.mocked(redirect).mock.calls[0][0]; - expect(redirectUrl).toEqual(authOptions.failedLoginRedirect); + expect(redirectUrl).toEqual(authOptions.failedLoginRedirect + '?error=no_code'); + + expect(consoleSpy).toHaveBeenCalledWith('No code found in callback'); + }); + + it('redirects to the overriden failed login url', async ({ authOptions }) => { + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + const authorizer = new Auth0RemixServer(authOptions); + const request = new Request('https://it-doesnt-matter.com', { + method: 'POST', + body: new FormData() + }); + + await expect(authorizer.handleCallback(request, { + onFailureRedirect: '/redirected' + })).rejects.toThrowError(redirectError); // a redirect happened + + const redirectUrl = vi.mocked(redirect).mock.calls[0][0]; + expect(redirectUrl).toEqual('/redirected?error=no_code'); expect(consoleSpy).toHaveBeenCalledWith('No code found in callback'); }); }); describe('when there is a code in the exchange', () => { - it('redirects to the failed login url if the token exchange fails', async ({ authOptions }) => { - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); - vi.mocked(fetch).mockResolvedValue({ - ok: false // return a non-ok response - } as never); + let consoleSpy: SpyInstance; + let authorizer: Auth0RemixServer; + let request: Request; - const authorizer = new Auth0RemixServer(authOptions); + beforeEach(({ authOptions }) => { + consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + authorizer = new Auth0RemixServer(authOptions); const formData = new FormData(); formData.append('code', 'test-code'); - const request = new Request('https://it-doesnt-matter.com', { + request = new Request('https://it-doesnt-matter.com', { method: 'POST', body: formData }); + }); + + it('redirects to the failed login url if the token exchange fails', async ({ authOptions }) => { + vi.mocked(fetch).mockResolvedValue({ + ok: false, // return a non-ok response + status: 400, + json: async () => ({ + error: 'invalid_grant', + error_description: 'Invalid authorization code' + }) + } as never); await expect(authorizer.handleCallback(request, {})).rejects.toThrowError(redirectError); // a redirect happened const redirectUrl = vi.mocked(redirect).mock.calls[0][0]; - expect(redirectUrl).toEqual(authOptions.failedLoginRedirect); + expect(redirectUrl).toEqual(authOptions.failedLoginRedirect + '?error=invalid_grant'); const fetchArgs = vi.mocked(fetch).mock.calls[0]; expect(fetchArgs[0]).toMatchInlineSnapshot('"https://test.domain.com/oauth/token"'); @@ -211,8 +240,51 @@ describe('Auth0 Remix Server', () => { expect(consoleSpy).toHaveBeenCalledWith('Failed to get token from Auth0'); }); + it('redirects to the overridden failed login url if the token exchange fails', async () => { + vi.mocked(fetch).mockResolvedValue({ + ok: false, // return a non-ok response + status: 401, + json: async () => ({ + error: 'unauthorized', + error_description: 'Invalid authorization code again' + }) + } as never); + + await expect(authorizer.handleCallback(request, { + onFailureRedirect: '/redirected' + })).rejects.toThrowError(redirectError); // a redirect happened + + const redirectUrl = vi.mocked(redirect).mock.calls[0][0]; + expect(redirectUrl).toEqual('/redirected?error=unauthorized'); + }); + + it('handles auth0 failures', async () => { + vi.mocked(fetch).mockResolvedValue({ + ok: false, // return a non-ok response + status: 500 + } as never); + + await expect(authorizer.handleCallback(request, { + onFailureRedirect: '/redirected' + })).rejects.toThrowError(redirectError); // a redirect happened + + const redirectUrl = vi.mocked(redirect).mock.calls[0][0]; + expect(redirectUrl).toEqual('/redirected?error=auth0_down'); + }); + + it('handles unknown failures', async ({ authOptions }) => { + vi.mocked(fetch).mockResolvedValue({ + ok: false // return a non-ok response + } as never); + + await expect(authorizer.handleCallback(request, {})).rejects.toThrowError(redirectError); // a redirect happened + + const redirectUrl = vi.mocked(redirect).mock.calls[0][0]; + expect(redirectUrl).toEqual(authOptions.failedLoginRedirect + '?error=unknown'); + }); + describe('and there is no success url', () => { - it('returns the user profile', async ({ authOptions }) => { + it('returns the user profile', async () => { const auth0Response = { access_token: 'test-access-token', id_token: 'test-id-token', @@ -224,14 +296,6 @@ describe('Auth0 Remix Server', () => { json: () => Promise.resolve(auth0Response) } as never); - const formData = new FormData(); - formData.append('code', 'test-code'); - const request = new Request('https://it-doesnt-matter.com', { - method: 'POST', - body: formData - }); - - const authorizer = new Auth0RemixServer(authOptions); const actual = await authorizer.handleCallback(request, {}); expect(actual).toMatchInlineSnapshot(` @@ -246,6 +310,7 @@ describe('Auth0 Remix Server', () => { it('includes the refresh token if the rotation is set', async ({ authOptions }) => { authOptions.refreshTokenRotationEnabled = true; + authorizer = new Auth0RemixServer(authOptions); const auth0Response = { access_token: 'test-access-token2', id_token: 'test-id-token2', @@ -258,14 +323,6 @@ describe('Auth0 Remix Server', () => { json: () => Promise.resolve(auth0Response) } as never); - const formData = new FormData(); - formData.append('code', 'test-code'); - const request = new Request('https://it-doesnt-matter.com', { - method: 'POST', - body: formData - }); - - const authorizer = new Auth0RemixServer(authOptions); const actual = await authorizer.handleCallback(request, {}); expect(actual).toMatchInlineSnapshot(` @@ -297,13 +354,6 @@ describe('Auth0 Remix Server', () => { json: () => Promise.resolve(auth0Response) } as never); - const formData = new FormData(); - formData.append('code', 'test-code'); - const request = new Request('https://it-doesnt-matter.com', { - method: 'POST', - body: formData - }); - vi.mocked(saveUserToSession).mockResolvedValue({ 'some-cookie': 'data' }); @@ -359,13 +409,6 @@ describe('Auth0 Remix Server', () => { json: () => Promise.resolve(auth0Response) } as never); - const formData = new FormData(); - formData.append('code', 'test-code'); - const request = new Request('https://it-doesnt-matter.com', { - method: 'POST', - body: formData - }); - vi.mocked(saveUserToSession).mockResolvedValue({ 'some-cookie': 'data' }); diff --git a/src/index.ts b/src/index.ts index 4688df6b..2a2e5613 100644 --- a/src/index.ts +++ b/src/index.ts @@ -148,10 +148,13 @@ export class Auth0RemixServer { public async handleCallback(request: Request, options: HandleCallbackOptions): Promise { const formData = await request.formData(); const code = formData.get('code'); + const redirectUrl = options.onFailureRedirect || this.failedLoginRedirect; + const searchParams = new URLSearchParams(); if (!code) { console.error('No code found in callback'); - throw redirect(this.failedLoginRedirect); + searchParams.set('error', 'no_code'); + throw redirect(redirectUrl.concat('?', searchParams.toString())); } const body = new URLSearchParams(); @@ -169,7 +172,8 @@ export class Auth0RemixServer { if (!response.ok) { console.error('Failed to get token from Auth0'); - throw redirect(this.failedLoginRedirect); + searchParams.set('error', await this.getErrorReason(response)); + throw redirect(redirectUrl.concat('?', searchParams.toString())); } const data = (await response.json()) as Auth0Credentials; @@ -293,4 +297,25 @@ export class Auth0RemixServer { const data = (await response.json()) as Auth0UserProfile; return transformUserData(data); } + + private async getErrorReason(response: Response): Promise { + if (String(response.status).startsWith('5')) { + console.error('Auth0 is having a moment'); + return 'auth0_down'; + } + + if (String(response.status).startsWith('4')) { + // The camelcase comes from Auth0 + // eslint-disable-next-line camelcase + const responseBody = (await response.json()) as {error: string, error_description: string}; + console.error('Auth0 rejected our request'); + console.error({ + error: responseBody.error, + description: responseBody.error_description + }); + return responseBody.error; + } + + return 'unknown'; + } }