From 2ff56278bd19c8bd565155b5f1009d30d2df2315 Mon Sep 17 00:00:00 2001 From: Rita Zerrizuela Date: Tue, 6 Sep 2022 23:17:40 -0300 Subject: [PATCH] Do not store the ID token by default --- FAQ.md | 58 ++-------------------- V2_MIGRATION_GUIDE.md | 9 ++++ src/auth0-session/config.ts | 9 +++- src/auth0-session/get-config.ts | 1 + src/config.ts | 11 ++++- src/session/session.ts | 5 +- tests/auth0-session/config.test.ts | 38 ++++++++++----- tests/config.test.ts | 7 ++- tests/handlers/callback.test.ts | 4 +- tests/session/get-access-token.test.ts | 23 ++++----- tests/session/session.test.ts | 66 +++++++++++++++++++------- 11 files changed, 124 insertions(+), 107 deletions(-) diff --git a/FAQ.md b/FAQ.md index 9b4c08e63..2b07d546a 100644 --- a/FAQ.md +++ b/FAQ.md @@ -1,65 +1,15 @@ # Frequently Asked Questions 1. [Why do I get a `checks.state argument is missing` error when logging in from different tabs?](#1-why-do-i-get-a-checks.state-argument-is-missing-error-if-i-try-to-log-in-from-different-tabs) -2. [How can I reduce the cookie size?](#2-how-can-i-reduce-the-cookie-size) ## 1. Why do I get a `checks.state argument is missing` error if I try to log in from different tabs? Every time you initiate login, the SDK stores in cookies some transient state (`nonce`, `state`, `code_verifier`) necessary to verify the callback request from Auth0. Initiating login concurrently from different tabs will result in that state being overwritten in each subsequent tab. Once the login is completed in some tab, the SDK will compare the state in the callback with the state stored in the cookies. As the cookies were overwritten, the values will not match (except for the tab that initiated login the last) and the SDK will return the `checks.state argument is missing` error. -Eg: +For example: -1. Open Tab 1 to login: stores some state in cookies. -2. Open Tab 2 to login: stores its own state overwritting Tab 1 state. +1. Open Tab 1 to log in: stores some state in cookies. +2. Open Tab 2 to log in: stores its own state overwritting Tab 1 state. 3. Complete login on Tab 1: SDK finds Tab 2 state on the cookies and returns error. -**You should handle the error and prompt the user to login again.** As they will have an active SSO session, they will not be asked to enter their credentials again and will be redirected back to your application. - -## 2. How can I reduce the cookie size? - -The SDK stores the session data in cookies. Since browsers reject cookies larger than 4 KB, the SDK breaks up lengthier sessions into multiple cookies. However, by default Node.js [limits the header size](https://nodejs.org/en/blog/vulnerability/november-2018-security-releases/#denial-of-service-with-large-http-headers-cve-2018-12121) to 8 KB. - -If the session cookies are pushing the header size over the limit, **you have two options**: - -- Use `-max-http-header-size` to increase Node's header size. -- Remove unused data from the session cookies. - -For the latter, you can add an [afterCallback](https://auth0.github.io/nextjs-auth0/modules/handlers_callback.html#aftercallback) hook to remove the ID Token and/or unused claims from the user profile: - -```js -// pages/api/auth/[...auth0].js -import { handleAuth, handleCallback } from '@auth0/nextjs-auth0'; - -const afterCallback = (req, res, session, state) => { - delete session.idToken; - return session; -}; - -export default handleAuth({ - async callback(req, res) { - try { - await handleCallback(req, res, { afterCallback }); - } catch (error) { - res.status(error.status || 500).end(error.message); - } - } -}); -``` - -> Note: if you are using refresh tokens you must also remove the item from the Session after it is refreshed using the [afterRefresh](https://auth0.github.io/nextjs-auth0/interfaces/session_get_access_token.accesstokenrequest.html#afterrefresh) hook (see also the [afterRefetch](https://auth0.github.io/nextjs-auth0/modules/handlers_profile.html#profileoptions) hook if you're removing claims from the user object). - -```js -// pages/api/my-handler.js -import { getAccessToken } from '@auth0/nextjs-auth0'; - -const afterRefresh = (req, res, session) => { - delete session.idToken; - return session; -}; - -export default async function MyHandler(req, res) { - const accessToken = await getAccessToken(req, res, { afterRefresh }); -} -``` - -> Note: support for custom session stores [is in our roadmap](https://github.com/auth0/nextjs-auth0/issues/279). +**You should handle the error and prompt the user to log in again.** As they will have an active SSO session, they will not be asked to enter their credentials again and will be redirected back to your application. diff --git a/V2_MIGRATION_GUIDE.md b/V2_MIGRATION_GUIDE.md index d23ee0eba..591e68adc 100644 --- a/V2_MIGRATION_GUIDE.md +++ b/V2_MIGRATION_GUIDE.md @@ -5,6 +5,7 @@ Guide to migrating from `1.x` to `2.x` - [`updateUser` has been added](#updateuser-has-been-added) - [`getServerSidePropsWrapper` has been removed](#getserversidepropswrapper-has-been-removed) - [Profile API route no longer returns a 401](#profile-api-route-no-longer-returns-a-401) +- [The ID token is no longer stored by default](#the-id-token-is-no-longer-stored-by-default) ## `updateUser` has been added @@ -77,3 +78,11 @@ export const getServerSideProps = async (ctx) => { ## Profile API route no longer returns a 401 Previously the profile API route, by default at `/api/auth/me`, would return a 401 error when the user was not authenticated. While it was technically the right status code for the situation, it showed up in the browser console as an error. This API route will now return a 204 instead. Since 204 is a successful status code, it will not produce a console error. + +## The ID token is no longer stored by default + +Previously the ID token would be stored in the session cookie, making the cookie unnecessarily large. Removing it required adding an `afterCallback` hook to the callback API route, and an `afterRefresh` hook to `getAccessToken()` –when using refresh tokens. + +Now the SDK will not store it by default. If you had been using hooks to strip it away, you can safely remove those. + +You can choose to store it by setting either the `session.storeIDToken` config property or the `AUTH0_SESSION_STORE_ID_TOKEN` environment variable to `true`. diff --git a/src/auth0-session/config.ts b/src/auth0-session/config.ts index d0dba3e95..196492201 100644 --- a/src/auth0-session/config.ts +++ b/src/auth0-session/config.ts @@ -98,7 +98,7 @@ export interface Config { httpTimeout: number; /** - * To opt-out of sending the library and Node.js version to your authorization server + * Boolean value to opt-out of sending the library and Node.js version to your authorization server * via the `Auth0-Client` header. Defaults to `true`. */ enableTelemetry: boolean; @@ -199,6 +199,13 @@ export interface SessionConfig { */ absoluteDuration: boolean | number; + /** + * Boolean value to store the ID token in the session. Storing it can make the session cookie too + * large. + * Defaults to `false`. + */ + storeIDToken: boolean; + cookie: CookieConfig; } diff --git a/src/auth0-session/get-config.ts b/src/auth0-session/get-config.ts index 691f030be..02b762159 100644 --- a/src/auth0-session/get-config.ts +++ b/src/auth0-session/get-config.ts @@ -33,6 +33,7 @@ const paramsSchema = Joi.object({ .optional() .default(7 * 24 * 60 * 60), // 7 days, name: Joi.string().token().optional().default('appSession'), + storeIDToken: Joi.boolean().optional().default(false), cookie: Joi.object({ domain: Joi.string().optional(), transient: Joi.boolean().optional().default(false), diff --git a/src/config.ts b/src/config.ts index e83cc5a6f..da92a9507 100644 --- a/src/config.ts +++ b/src/config.ts @@ -91,7 +91,7 @@ export interface BaseConfig { httpTimeout: number; /** - * To opt-out of sending the library and node version to your authorization server + * Boolean value to opt-out of sending the library and node version to your authorization server * via the `Auth0-Client` header. Defaults to `true`. * You can also use the `AUTH0_ENABLE_TELEMETRY` environment variable. */ @@ -212,6 +212,13 @@ export interface SessionConfig { */ absoluteDuration: boolean | number; + /** + * Boolean value to store the ID token in the session. Storing it can make the session cookie too + * large. + * Defaults to `false`. + */ + storeIDToken: boolean; + cookie: CookieConfig; } @@ -444,6 +451,7 @@ export const getConfig = (params: ConfigParameters = {}): { baseConfig: BaseConf const AUTH0_SESSION_ROLLING = process.env.AUTH0_SESSION_ROLLING; const AUTH0_SESSION_ROLLING_DURATION = process.env.AUTH0_SESSION_ROLLING_DURATION; const AUTH0_SESSION_ABSOLUTE_DURATION = process.env.AUTH0_SESSION_ABSOLUTE_DURATION; + const AUTH0_SESSION_STORE_ID_TOKEN = process.env.AUTH0_SESSION_STORE_ID_TOKEN; const AUTH0_COOKIE_DOMAIN = process.env.AUTH0_COOKIE_DOMAIN; const AUTH0_COOKIE_PATH = process.env.AUTH0_COOKIE_PATH; const AUTH0_COOKIE_TRANSIENT = process.env.AUTH0_COOKIE_TRANSIENT; @@ -488,6 +496,7 @@ export const getConfig = (params: ConfigParameters = {}): { baseConfig: BaseConf AUTH0_SESSION_ABSOLUTE_DURATION && isNaN(Number(AUTH0_SESSION_ABSOLUTE_DURATION)) ? bool(AUTH0_SESSION_ABSOLUTE_DURATION) : num(AUTH0_SESSION_ABSOLUTE_DURATION), + storeIDToken: bool(AUTH0_SESSION_STORE_ID_TOKEN), ...baseParams.session, cookie: { domain: AUTH0_COOKIE_DOMAIN, diff --git a/src/session/session.ts b/src/session/session.ts index 6be5e9ada..c23f1fd65 100644 --- a/src/session/session.ts +++ b/src/session/session.ts @@ -68,15 +68,16 @@ export function fromTokenSet(tokenSet: TokenSet, config: Config | NextConfig): S }); const { id_token, access_token, scope, expires_at, refresh_token, ...remainder } = tokenSet; + const storeIDToken = 'session' in config ? config.session.storeIDToken : false; return Object.assign( new Session({ ...claims }), { - idToken: id_token, accessToken: access_token, accessTokenScope: scope, accessTokenExpiresAt: expires_at, - refreshToken: refresh_token + refreshToken: refresh_token, + ...(storeIDToken && { idToken: id_token }) }, remainder ); diff --git a/tests/auth0-session/config.test.ts b/tests/auth0-session/config.test.ts index 710dc57f0..415704797 100644 --- a/tests/auth0-session/config.test.ts +++ b/tests/auth0-session/config.test.ts @@ -109,6 +109,7 @@ describe('Config', () => { expect(config.session).toMatchObject({ rollingDuration: 86400, name: 'appSession', + storeIDToken: false, cookie: { sameSite: 'lax', httpOnly: true, @@ -124,6 +125,7 @@ describe('Config', () => { session: { name: '__test_custom_session_name__', rollingDuration: 1234567890, + storeIDToken: true, cookie: { domain: '__test_custom_domain__', transient: true, @@ -140,6 +142,7 @@ describe('Config', () => { rollingDuration: 1234567890, absoluteDuration: 604800, rolling: true, + storeIDToken: true, cookie: { domain: '__test_custom_domain__', transient: true, @@ -229,7 +232,7 @@ describe('Config', () => { ...defaultConfig, session: { rolling: true, - rollingDuration: (false as unknown) as undefined // testing invalid configuration + rollingDuration: false as unknown as undefined // testing invalid configuration } }) ).toThrow('"session.rollingDuration" must be provided an integer value when "session.rolling" is true'); @@ -247,11 +250,22 @@ describe('Config', () => { ).toThrowError('"session.absoluteDuration" must be provided an integer value when "session.rolling" is false'); }); + it('should fail when app session storeIDToken is not a boolean', function () { + expect(() => + getConfig({ + ...defaultConfig, + session: { + storeIDToken: '__invalid_store_id_token__' as unknown as boolean // testing invalid configuration + } + }) + ).toThrowError('"session.storeIDToken" must be a boolean'); + }); + it('should fail when app session secret is invalid', function () { expect(() => getConfig({ ...defaultConfig, - secret: ({ key: '__test_session_secret__' } as unknown) as string // testing invalid configuration + secret: { key: '__test_session_secret__' } as unknown as string // testing invalid configuration }) ).toThrow('"secret" must be one of [string, binary, array]'); }); @@ -262,7 +276,7 @@ describe('Config', () => { ...defaultConfig, session: { cookie: { - httpOnly: ('__invalid_httponly__' as unknown) as boolean // testing invalid configuration + httpOnly: '__invalid_httponly__' as unknown as boolean // testing invalid configuration } } }) @@ -276,7 +290,7 @@ describe('Config', () => { secret: '__test_session_secret__', session: { cookie: { - secure: ('__invalid_secure__' as unknown) as boolean // testing invalid configuration + secure: '__invalid_secure__' as unknown as boolean // testing invalid configuration } } }) @@ -290,7 +304,7 @@ describe('Config', () => { secret: '__test_session_secret__', session: { cookie: { - sameSite: ('__invalid_samesite__' as unknown) as any // testing invalid configuration + sameSite: '__invalid_samesite__' as unknown as any // testing invalid configuration } } }) @@ -304,7 +318,7 @@ describe('Config', () => { secret: '__test_session_secret__', session: { cookie: { - domain: (false as unknown) as string // testing invalid configuration + domain: false as unknown as string // testing invalid configuration } } }) @@ -399,7 +413,7 @@ describe('Config', () => { }); it('should not allow empty scope', () => { - expect(() => validateAuthorizationParams({ scope: (null as unknown) as undefined })).toThrowError( + expect(() => validateAuthorizationParams({ scope: null as unknown as undefined })).toThrowError( new TypeError('"authorizationParams.scope" must be a string') ); expect(() => validateAuthorizationParams({ scope: '' })).toThrowError( @@ -420,10 +434,10 @@ describe('Config', () => { }); it('should not allow empty response_type', () => { - expect(() => validateAuthorizationParams({ response_type: (null as unknown) as undefined })).toThrowError( + expect(() => validateAuthorizationParams({ response_type: null as unknown as undefined })).toThrowError( new TypeError('"authorizationParams.response_type" must be one of [id_token, code id_token, code]') ); - expect(() => validateAuthorizationParams({ response_type: ('' as unknown) as undefined })).toThrowError( + expect(() => validateAuthorizationParams({ response_type: '' as unknown as undefined })).toThrowError( new TypeError('"authorizationParams.response_type" must be one of [id_token, code id_token, code]') ); }); @@ -452,16 +466,16 @@ describe('Config', () => { }); it('should not allow empty response_mode', () => { - expect(() => validateAuthorizationParams({ response_mode: (null as unknown) as undefined })).toThrowError( + expect(() => validateAuthorizationParams({ response_mode: null as unknown as undefined })).toThrowError( new TypeError('"authorizationParams.response_mode" must be [form_post]') ); - expect(() => validateAuthorizationParams({ response_mode: ('' as unknown) as undefined })).toThrowError( + expect(() => validateAuthorizationParams({ response_mode: '' as unknown as undefined })).toThrowError( new TypeError('"authorizationParams.response_mode" must be [form_post]') ); expect(() => validateAuthorizationParams({ response_type: 'code', - response_mode: ('' as unknown) as undefined + response_mode: '' as unknown as undefined }) ).toThrowError(new TypeError('"authorizationParams.response_mode" must be one of [query, form_post]')); }); diff --git a/tests/config.test.ts b/tests/config.test.ts index 70c199312..4e2ecf072 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -48,6 +48,7 @@ describe('config params', () => { rolling: true, rollingDuration: 86400, absoluteDuration: 604800, + storeIDToken: false, cookie: { domain: undefined, path: '/', @@ -107,7 +108,8 @@ describe('config params', () => { AUTH0_COOKIE_HTTP_ONLY: 'on', AUTH0_COOKIE_SAME_SITE: 'lax', AUTH0_COOKIE_SECURE: 'ok', - AUTH0_SESSION_ABSOLUTE_DURATION: 'no' + AUTH0_SESSION_ABSOLUTE_DURATION: 'no', + AUTH0_SESSION_STORE_ID_TOKEN: '1' }).baseConfig ).toMatchObject({ auth0Logout: false, @@ -116,6 +118,7 @@ describe('config params', () => { legacySameSiteCookie: false, session: { absoluteDuration: false, + storeIDToken: true, cookie: { httpOnly: true, sameSite: 'lax', @@ -182,6 +185,7 @@ describe('config params', () => { }, session: { absoluteDuration: 100, + storeIDToken: true, cookie: { transient: false }, @@ -201,6 +205,7 @@ describe('config params', () => { }, session: { absoluteDuration: 100, + storeIDToken: true, cookie: { transient: false }, diff --git a/tests/handlers/callback.test.ts b/tests/handlers/callback.test.ts index 5b2a5bba8..2d2df3926 100644 --- a/tests/handlers/callback.test.ts +++ b/tests/handlers/callback.test.ts @@ -177,7 +177,6 @@ describe('callback handler', () => { accessToken: 'eyJz93a...k4laUWw', accessTokenExpiresAt: 750, accessTokenScope: 'read:foo delete:foo', - idToken: makeIdToken({ iss: 'https://acme.auth0.local/' }), token_type: 'Bearer', refreshToken: 'GEbRxBN...edjnXbL', user: { @@ -188,7 +187,7 @@ describe('callback handler', () => { timekeeper.reset(); }); - test('remove tokens with afterCallback hook', async () => { + test('remove properties from session with afterCallback hook', async () => { timekeeper.freeze(0); const afterCallback: AfterCallback = (_req, _res, session: Session): Session => { delete session.accessToken; @@ -218,7 +217,6 @@ describe('callback handler', () => { expect(session).toStrictEqual({ accessTokenExpiresAt: 750, accessTokenScope: 'read:foo delete:foo', - idToken: makeIdToken({ iss: 'https://acme.auth0.local/' }), token_type: 'Bearer', user: { nickname: '__test_nickname__', diff --git a/tests/session/get-access-token.test.ts b/tests/session/get-access-token.test.ts index 05df43e71..d91d6bea9 100644 --- a/tests/session/get-access-token.test.ts +++ b/tests/session/get-access-token.test.ts @@ -233,32 +233,25 @@ describe('get access token', () => { }); test('should retrieve a new access token and update the session based on afterRefresh', async () => { - refreshTokenExchange( - withApi, - 'GEbRxBN...edjnXbL', - { - email: 'john@test.com', - name: 'john doe', - sub: '123' - }, - 'new-token' - ); + refreshTokenExchange(withApi, 'GEbRxBN...edjnXbL', {}, 'new-token'); const baseUrl = await setup(withApi, { getAccessTokenOptions: { refresh: true, afterRefresh(_req, _res, session) { - delete session.idToken; + delete session.accessTokenScope; return session; } } }); const cookieJar = await login(baseUrl); - const { idToken } = await get(baseUrl, '/api/session', { cookieJar }); - expect(idToken).not.toBeUndefined(); + const { accessTokenScope } = await get(baseUrl, '/api/session', { cookieJar }); + expect(accessTokenScope).not.toBeUndefined(); const { accessToken } = await get(baseUrl, '/api/access-token', { cookieJar }); expect(accessToken).toEqual('new-token'); - const { idToken: newIdToken } = await get(baseUrl, '/api/session', { cookieJar }); - expect(newIdToken).toBeUndefined(); + const { accessTokenScope: newAccessTokenScope } = await get(baseUrl, '/api/session', { + cookieJar + }); + expect(newAccessTokenScope).toBeUndefined(); }); test('should pass custom auth params in refresh grant request body', async () => { diff --git a/tests/session/session.test.ts b/tests/session/session.test.ts index faed75bbd..4396c7e2f 100644 --- a/tests/session/session.test.ts +++ b/tests/session/session.test.ts @@ -8,26 +8,56 @@ describe('session', () => { expect(new Session({ foo: 'bar' }).user).toEqual({ foo: 'bar' }); }); - test('should construct a session from a tokenSet', () => { - expect( - fromTokenSet(new TokenSet({ id_token: makeIdToken({ foo: 'bar', bax: 'qux' }) }), { - identityClaimFilter: ['baz'], - routes: { login: '', callback: '', postLogoutRedirect: '' } - }).user - ).toEqual({ - aud: '__test_client_id__', - bax: 'qux', - exp: expect.any(Number), - foo: 'bar', - iat: expect.any(Number), - iss: 'https://op.example.com/', - nickname: '__test_nickname__', - nonce: '__test_nonce__', - sub: '__test_sub__' + describe('from tokenSet', () => { + test('should construct a session from a tokenSet', () => { + expect( + fromTokenSet(new TokenSet({ id_token: makeIdToken({ foo: 'bar', bax: 'qux' }) }), { + identityClaimFilter: ['baz'], + routes: { login: '', callback: '', postLogoutRedirect: '' } + }).user + ).toEqual({ + aud: '__test_client_id__', + bax: 'qux', + exp: expect.any(Number), + foo: 'bar', + iat: expect.any(Number), + iss: 'https://op.example.com/', + nickname: '__test_nickname__', + nonce: '__test_nonce__', + sub: '__test_sub__' + }); + }); + + test('should not store the ID Token by default', () => { + expect( + fromTokenSet(new TokenSet({ id_token: makeIdToken({ foo: 'bar' }) }), { + identityClaimFilter: ['baz'], + routes: { login: '', callback: '', postLogoutRedirect: '' } + }).idToken + ).toBeUndefined(); + }); + + test('should store the ID Token', () => { + expect( + fromTokenSet(new TokenSet({ id_token: makeIdToken({ foo: 'bar' }) }), { + session: { + storeIDToken: true, + name: '', + rolling: false, + rollingDuration: 0, + absoluteDuration: 0, + cookie: { transient: false, httpOnly: false, sameSite: 'lax' } + }, + identityClaimFilter: ['baz'], + routes: { login: '', callback: '', postLogoutRedirect: '' } + }).idToken + ).not.toBeUndefined(); }); }); - test('should construct a session from json', () => { - expect(fromJson({ user: { foo: 'bar' } })?.user).toEqual({ foo: 'bar' }); + describe('from json', () => { + test('should construct a session from json', () => { + expect(fromJson({ user: { foo: 'bar' } })?.user).toEqual({ foo: 'bar' }); + }); }); });