Skip to content

Commit

Permalink
feat: added failure redirect override for each authorization request
Browse files Browse the repository at this point in the history
The failure urls now receive an error query string parameter describing the error.

This is part of #138. Thanks @njpearman for all the suggestions!
  • Loading branch information
meza committed Jul 8, 2023
1 parent 42b61fa commit d769c58
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 43 deletions.
25 changes: 25 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
1 change: 1 addition & 0 deletions src/Auth0RemixTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,5 @@ export type AuthorizeOptions =

export interface HandleCallbackOptions {
onSuccessRedirect?: string;
onFailureRedirect?: string;
}
125 changes: 84 additions & 41 deletions src/index.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -178,41 +178,113 @@ 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<LocalTestContext>('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<LocalTestContext>('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<LocalTestContext>(({ 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<LocalTestContext>('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"');
expect(fetchArgs[1]).toMatchSnapshot();
expect(consoleSpy).toHaveBeenCalledWith('Failed to get token from Auth0');
});

it<LocalTestContext>('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<LocalTestContext>('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<LocalTestContext>('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<LocalTestContext>('returns the user profile', async ({ authOptions }) => {
it('returns the user profile', async () => {
const auth0Response = {
access_token: 'test-access-token',
id_token: 'test-id-token',
Expand All @@ -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(`
Expand All @@ -246,6 +310,7 @@ describe('Auth0 Remix Server', () => {

it<LocalTestContext>('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',
Expand All @@ -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(`
Expand Down Expand Up @@ -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'
});
Expand Down Expand Up @@ -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'
});
Expand Down
29 changes: 27 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,13 @@ export class Auth0RemixServer {
public async handleCallback(request: Request, options: HandleCallbackOptions): Promise<UserCredentials> {
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();
Expand All @@ -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;
Expand Down Expand Up @@ -293,4 +297,25 @@ export class Auth0RemixServer {
const data = (await response.json()) as Auth0UserProfile;
return transformUserData(data);
}

private async getErrorReason(response: Response): Promise<string> {
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';
}
}

0 comments on commit d769c58

Please sign in to comment.