diff --git a/FAQ.md b/FAQ.md index 9b4c08e63..fd931db20 100644 --- a/FAQ.md +++ b/FAQ.md @@ -2,6 +2,7 @@ 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) +3. [I'm getting the warning/error `You should not access 'res' after getServerSideProps resolves.`](#3-i-m-getting-the-warning-error--you-should-not-access--res--after-getserversideprops-resolves.) ## 1. Why do I get a `checks.state argument is missing` error if I try to log in from different tabs? @@ -63,3 +64,11 @@ export default async function MyHandler(req, res) { ``` > Note: support for custom session stores [is in our roadmap](https://github.com/auth0/nextjs-auth0/issues/279). + +## 3. I'm getting the warning/error `You should not access 'res' after getServerSideProps resolves.` + +Because this SDK provides a rolling session by default, it writes to the header at the end of every request. This can cause the above warning when you use `getSession` or `getAccessToken` in >=Next.js 12, and an error if your `props` are defined as a `Promise`. + +Wrapping your `getServerSideProps` in `getServerSidePropsWrapper` will fix this because it will constrain the lifecycle of the session to the life of `getServerSideProps`. + +> Note: you should not use this if you are already using `withPageAuthenticationRequired` since this should already constrain the lifecycle of the session. diff --git a/README.md b/README.md index c7494add4..cb954d9d0 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,7 @@ For other comprehensive examples, see the [EXAMPLES.md](./EXAMPLES.md) document. - [handleProfile](https://auth0.github.io/nextjs-auth0/modules/handlers_profile.html) - [withApiAuthRequired](https://auth0.github.io/nextjs-auth0/modules/helpers_with_api_auth_required.html) - [withPageAuthRequired](https://auth0.github.io/nextjs-auth0/modules/helpers_with_page_auth_required.html#withpageauthrequired) +- [getServerSidePropsWrapper](https://auth0.github.io/nextjs-auth0/modules/helpers_get_server_side_props_wrapper.html) - [getSession](https://auth0.github.io/nextjs-auth0/modules/session_get_session.html) - [getAccessToken](https://auth0.github.io/nextjs-auth0/modules/session_get_access_token.html) - [initAuth0](https://auth0.github.io/nextjs-auth0/modules/instance.html) diff --git a/src/helpers/get-server-side-props-wrapper.ts b/src/helpers/get-server-side-props-wrapper.ts new file mode 100644 index 000000000..71a953261 --- /dev/null +++ b/src/helpers/get-server-side-props-wrapper.ts @@ -0,0 +1,49 @@ +import { GetServerSideProps } from 'next'; +import SessionCache from '../session/cache'; + +/** + * If you're using >=Next 12 and {@link getSession} or {@link getAccessToken} without `withPageAuthRequired`, because + * you don't want to require authentication on your route, you might get a warning/error: "You should not access 'res' + * after getServerSideProps resolves". You can work around this by wrapping your `getServerSideProps` in + * `getServerSidePropsWrapper`, this ensures that the code that accesses `res` will run within + * the lifecycle of `getServerSideProps`, avoiding the warning/error eg: + * + * **NOTE: you do not need to do this if you're already using {@link WithPageAuthRequired}** + * + * ```js + * // pages/protected-page.js + * import { withPageAuthRequired } from '@auth0/nextjs-auth0'; + * + * export default function ProtectedPage() { + * return
Protected content
; + * } + * + * export const getServerSideProps = getServerSidePropsWrapper(async (ctx) => { + * const session = getSession(ctx.req, ctx.res); + * if (session) { + * // Use is authenticated + * } else { + * // User is not authenticated + * } + * }); + * ``` + * + * @category Server + */ +export type GetServerSidePropsWrapper = (getServerSideProps: GetServerSideProps) => GetServerSideProps; + +/** + * @ignore + */ +export default function getServerSidePropsWrapperFactory(getSessionCache: () => SessionCache) { + return function getServerSidePropsWrapper(getServerSideProps: GetServerSideProps): GetServerSideProps { + return function wrappedGetServerSideProps(...args) { + const sessionCache = getSessionCache(); + const [ctx] = args; + sessionCache.init(ctx.req, ctx.res, false); + const ret = getServerSideProps(...args); + sessionCache.save(ctx.req, ctx.res); + return ret; + }; + }; +} diff --git a/src/helpers/index.ts b/src/helpers/index.ts index b2024e2e5..d57ed453d 100644 --- a/src/helpers/index.ts +++ b/src/helpers/index.ts @@ -6,3 +6,7 @@ export { WithPageAuthRequiredOptions, PageRoute } from './with-page-auth-required'; +export { + default as getServerSidePropsWrapperFactory, + GetServerSidePropsWrapper +} from './get-server-side-props-wrapper'; diff --git a/src/helpers/with-page-auth-required.ts b/src/helpers/with-page-auth-required.ts index a5008937a..2ca7b9291 100644 --- a/src/helpers/with-page-auth-required.ts +++ b/src/helpers/with-page-auth-required.ts @@ -1,5 +1,5 @@ import { GetServerSideProps, GetServerSidePropsContext, GetServerSidePropsResult } from 'next'; -import { Claims, GetSession } from '../session'; +import { Claims, SessionCache } from '../session'; import { assertCtx } from '../utils/assert'; import React, { ComponentType } from 'react'; import { @@ -9,6 +9,7 @@ import { } from '../frontend/with-page-auth-required'; import { withPageAuthRequired as withPageAuthRequiredCSR } from '../frontend'; import { ParsedUrlQuery } from 'querystring'; +import getServerSidePropsWrapperFactory from './get-server-side-props-wrapper'; /** * If you wrap your `getServerSideProps` with {@link WithPageAuthRequired} your props object will be augmented with @@ -99,7 +100,10 @@ export type WithPageAuthRequired = { /** * @ignore */ -export default function withPageAuthRequiredFactory(loginUrl: string, getSession: GetSession): WithPageAuthRequired { +export default function withPageAuthRequiredFactory( + loginUrl: string, + getSessionCache: () => SessionCache +): WithPageAuthRequired { return ( optsOrComponent: WithPageAuthRequiredOptions | ComponentType = {}, csrOpts?: WithPageAuthRequiredCSROptions @@ -108,25 +112,32 @@ export default function withPageAuthRequiredFactory(loginUrl: string, getSession return withPageAuthRequiredCSR(optsOrComponent, csrOpts); } const { getServerSideProps, returnTo } = optsOrComponent; - return async (ctx: GetServerSidePropsContext): Promise => { - assertCtx(ctx); - const session = getSession(ctx.req, ctx.res); - if (!session?.user) { - // 10 - redirect - // 9.5.4 - unstable_redirect - // 9.4 - res.setHeaders - return { - redirect: { - destination: `${loginUrl}?returnTo=${encodeURIComponent(returnTo || ctx.resolvedUrl)}`, - permanent: false - } - }; + const getServerSidePropsWrapper = getServerSidePropsWrapperFactory(getSessionCache); + return getServerSidePropsWrapper( + async (ctx: GetServerSidePropsContext): Promise => { + assertCtx(ctx); + const sessionCache = getSessionCache(); + const session = sessionCache.get(ctx.req, ctx.res); + if (!session?.user) { + // 10 - redirect + // 9.5.4 - unstable_redirect + // 9.4 - res.setHeaders + return { + redirect: { + destination: `${loginUrl}?returnTo=${encodeURIComponent(returnTo || ctx.resolvedUrl)}`, + permanent: false + } + }; + } + let ret: any = { props: {} }; + if (getServerSideProps) { + ret = await getServerSideProps(ctx); + } + if (ret.props instanceof Promise) { + return { ...ret, props: ret.props.then((props: any) => ({ ...props, user: session.user })) }; + } + return { ...ret, props: { ...ret.props, user: session.user } }; } - let ret: any = { props: {} }; - if (getServerSideProps) { - ret = await getServerSideProps(ctx); - } - return { ...ret, props: { ...ret.props, user: session.user } }; - }; + ); }; } diff --git a/src/index.browser.ts b/src/index.browser.ts index b5be28679..1d0732da9 100644 --- a/src/index.browser.ts +++ b/src/index.browser.ts @@ -42,6 +42,9 @@ const instance: SignInWithAuth0 = { }, withPageAuthRequired() { throw new Error(serverSideOnly('withPageAuthRequired')); + }, + getServerSidePropsWrapper() { + throw new Error(serverSideOnly('getServerSidePropsWrapper')); } }; diff --git a/src/index.ts b/src/index.ts index c5ec78091..8c7e559e0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -44,23 +44,25 @@ import { WithPageAuthRequired, GetServerSidePropsResultWithSession, WithPageAuthRequiredOptions, - PageRoute + PageRoute, + getServerSidePropsWrapperFactory, + GetServerSidePropsWrapper } from './helpers'; import { InitAuth0, SignInWithAuth0 } from './instance'; import version from './version'; import { getConfig, getLoginUrl, ConfigParameters } from './config'; -let instance: SignInWithAuth0; +let instance: SignInWithAuth0 & { sessionCache: SessionCache }; -function getInstance(): SignInWithAuth0 { +function getInstance(): SignInWithAuth0 & { sessionCache: SessionCache } { if (instance) { return instance; } - instance = initAuth0(); + instance = _initAuth(); return instance; } -export const initAuth0: InitAuth0 = (params) => { +export const _initAuth = (params?: ConfigParameters): SignInWithAuth0 & { sessionCache: SessionCache } => { const { baseConfig, nextConfig } = getConfig(params); // Init base layer (with base config) @@ -76,7 +78,8 @@ export const initAuth0: InitAuth0 = (params) => { const getSession = sessionFactory(sessionCache); const getAccessToken = accessTokenFactory(nextConfig, getClient, sessionCache); const withApiAuthRequired = withApiAuthRequiredFactory(sessionCache); - const withPageAuthRequired = withPageAuthRequiredFactory(nextConfig.routes.login, getSession); + const withPageAuthRequired = withPageAuthRequiredFactory(nextConfig.routes.login, () => sessionCache); + const getServerSidePropsWrapper = getServerSidePropsWrapperFactory(() => sessionCache); const handleLogin = loginHandler(baseHandleLogin, nextConfig, baseConfig); const handleLogout = logoutHandler(baseHandleLogout); const handleCallback = callbackHandler(baseHandleCallback, nextConfig); @@ -84,10 +87,12 @@ export const initAuth0: InitAuth0 = (params) => { const handleAuth = handlerFactory({ handleLogin, handleLogout, handleCallback, handleProfile }); return { + sessionCache, getSession, getAccessToken, withApiAuthRequired, withPageAuthRequired, + getServerSidePropsWrapper, handleLogin, handleLogout, handleCallback, @@ -96,10 +101,17 @@ export const initAuth0: InitAuth0 = (params) => { }; }; +export const initAuth0: InitAuth0 = (params) => { + const { sessionCache, ...publicApi } = _initAuth(params); + return publicApi; +}; + +const getSessionCache = () => getInstance().sessionCache; export const getSession: GetSession = (...args) => getInstance().getSession(...args); export const getAccessToken: GetAccessToken = (...args) => getInstance().getAccessToken(...args); export const withApiAuthRequired: WithApiAuthRequired = (...args) => getInstance().withApiAuthRequired(...args); -export const withPageAuthRequired: WithPageAuthRequired = withPageAuthRequiredFactory(getLoginUrl(), getSession); +export const withPageAuthRequired: WithPageAuthRequired = withPageAuthRequiredFactory(getLoginUrl(), getSessionCache); +export const getServerSidePropsWrapper: GetServerSidePropsWrapper = getServerSidePropsWrapperFactory(getSessionCache); export const handleLogin: HandleLogin = (...args) => getInstance().handleLogin(...args); export const handleLogout: HandleLogout = (...args) => getInstance().handleLogout(...args); export const handleCallback: HandleCallback = (...args) => getInstance().handleCallback(...args); @@ -132,6 +144,7 @@ export { PageRoute, WithApiAuthRequired, WithPageAuthRequired, + GetServerSidePropsWrapper, SessionCache, GetSession, GetAccessToken, diff --git a/src/instance.ts b/src/instance.ts index a52632812..462768bfd 100644 --- a/src/instance.ts +++ b/src/instance.ts @@ -1,5 +1,5 @@ import { GetSession, GetAccessToken } from './session'; -import { WithApiAuthRequired, WithPageAuthRequired } from './helpers'; +import { GetServerSidePropsWrapper, WithApiAuthRequired, WithPageAuthRequired } from './helpers'; import { HandleAuth, HandleCallback, HandleLogin, HandleLogout, HandleProfile } from './handlers'; import { ConfigParameters } from './auth0-session'; @@ -53,6 +53,12 @@ export interface SignInWithAuth0 { */ withPageAuthRequired: WithPageAuthRequired; + /** + * Wrap `getServerSideProps` to avoid accessing `res` after getServerSideProps resolves, + * see {@link GetServerSidePropsWrapper} + */ + getServerSidePropsWrapper: GetServerSidePropsWrapper; + /** * Create the main handlers for your api routes */ diff --git a/src/session/cache.ts b/src/session/cache.ts index 92acfd557..b8217987c 100644 --- a/src/session/cache.ts +++ b/src/session/cache.ts @@ -10,22 +10,31 @@ type NextApiOrPageResponse = ServerResponse | NextApiResponse; export default class SessionCache implements ISessionCache { private cache: WeakMap; + private iatCache: WeakMap; constructor(private config: Config, private cookieStore: CookieStore) { this.cache = new WeakMap(); + this.iatCache = new WeakMap(); } - init(req: NextApiOrPageRequest, res: NextApiOrPageResponse): void { + init(req: NextApiOrPageRequest, res: NextApiOrPageResponse, autoSave = true): void { if (!this.cache.has(req)) { const [json, iat] = this.cookieStore.read(req); + this.iatCache.set(req, iat); this.cache.set(req, fromJson(json)); - onHeaders(res, () => this.cookieStore.save(req, res, this.cache.get(req), iat)); + if (autoSave) { + onHeaders(res, () => this.save(req, res)); + } } } + save(req: NextApiOrPageRequest, res: NextApiOrPageResponse): void { + this.cookieStore.save(req, res, this.cache.get(req), this.iatCache.get(req)); + } + create(req: NextApiOrPageRequest, res: NextApiOrPageResponse, session: Session): void { this.cache.set(req, session); - onHeaders(res, () => this.cookieStore.save(req, res, this.cache.get(req))); + onHeaders(res, () => this.save(req, res)); } delete(req: NextApiOrPageRequest, res: NextApiOrPageResponse): void { diff --git a/tests/fixtures/setup.ts b/tests/fixtures/setup.ts index d0abb22ea..5cc912038 100644 --- a/tests/fixtures/setup.ts +++ b/tests/fixtures/setup.ts @@ -30,6 +30,7 @@ export type SetupOptions = { discoveryOptions?: Record; userInfoPayload?: Record; userInfoToken?: string; + asyncProps?: boolean; }; export const setup = async ( @@ -44,7 +45,8 @@ export const setup = async ( getAccessTokenOptions, discoveryOptions, userInfoPayload = {}, - userInfoToken = 'eyJz93a...k4laUWw' + userInfoToken = 'eyJz93a...k4laUWw', + asyncProps }: SetupOptions = {} ): Promise => { discovery(config, discoveryOptions); @@ -60,7 +62,8 @@ export const setup = async ( getSession, getAccessToken, withApiAuthRequired, - withPageAuthRequired + withPageAuthRequired, + getServerSidePropsWrapper } = await initAuth0(config); (global as any).handleAuth = handleAuth.bind(null, { async callback(req, res) { @@ -102,6 +105,8 @@ export const setup = async ( (global as any).withPageAuthRequiredCSR = withPageAuthRequired; (global as any).getAccessToken = (req: NextApiRequest, res: NextApiResponse): Promise => getAccessToken(req, res, getAccessTokenOptions); + (global as any).getServerSidePropsWrapper = getServerSidePropsWrapper; + (global as any).asyncProps = asyncProps; return start(); }; @@ -114,6 +119,8 @@ export const teardown = async (): Promise => { delete (global as any).withPageAuthRequired; delete (global as any).withPageAuthRequiredCSR; delete (global as any).getAccessToken; + delete (global as any).getServerSidePropsWrapper; + delete (global as any).asyncProps; }; export const login = async (baseUrl: string): Promise => { diff --git a/tests/fixtures/test-app/next-env.d.ts b/tests/fixtures/test-app/next-env.d.ts index 9bc3dd46b..4f11a03dc 100644 --- a/tests/fixtures/test-app/next-env.d.ts +++ b/tests/fixtures/test-app/next-env.d.ts @@ -1,5 +1,4 @@ /// -/// /// // NOTE: This file should not be edited diff --git a/tests/fixtures/test-app/pages/protected.tsx b/tests/fixtures/test-app/pages/protected.tsx index 3f70a72b4..c2867745f 100644 --- a/tests/fixtures/test-app/pages/protected.tsx +++ b/tests/fixtures/test-app/pages/protected.tsx @@ -1,8 +1,8 @@ import React from 'react'; import { NextPageContext } from 'next'; -export default function protectedPage(): React.ReactElement { - return
Protected Page
; +export default function protectedPage({ user }: { user?: { sub: string } }): React.ReactElement { + return
Protected Page {user ? user.sub : ''}
; } export const getServerSideProps = (ctx: NextPageContext): any => (global as any).withPageAuthRequired()(ctx); diff --git a/tests/fixtures/test-app/pages/wrapped-get-server-side-props.tsx b/tests/fixtures/test-app/pages/wrapped-get-server-side-props.tsx new file mode 100644 index 000000000..338682498 --- /dev/null +++ b/tests/fixtures/test-app/pages/wrapped-get-server-side-props.tsx @@ -0,0 +1,18 @@ +import React from 'react'; +import { NextPageContext } from 'next'; + +export default function wrappedGetServerSidePropsPage({ + isAuthenticated +}: { + isAuthenticated?: boolean; +}): React.ReactElement { + return
isAuthenticated: {String(isAuthenticated)}
; +} + +export const getServerSideProps = (_ctx: NextPageContext): any => + (global as any).getServerSidePropsWrapper(async (ctx: NextPageContext) => { + const session = (global as any).getSession(ctx.req, ctx.res); + const asyncProps = (global as any).asyncProps; + const props = { isAuthenticated: !!session }; + return { props: asyncProps ? Promise.resolve(props) : props }; + })(_ctx); diff --git a/tests/helpers/get-server-side-props-wrapper.test.ts b/tests/helpers/get-server-side-props-wrapper.test.ts new file mode 100644 index 000000000..82a0ca13b --- /dev/null +++ b/tests/helpers/get-server-side-props-wrapper.test.ts @@ -0,0 +1,53 @@ +import { login, setup, teardown } from '../fixtures/setup'; +import { withoutApi } from '../fixtures/default-settings'; +import { get } from '../auth0-session/fixtures/helpers'; + +describe('get-server-side-props-wrapper', () => { + afterEach(teardown); + + test('wrap getServerSideProps', async () => { + const baseUrl = await setup(withoutApi); + + const { + res: { statusCode }, + data + } = await get(baseUrl, '/wrapped-get-server-side-props', { fullResponse: true }); + expect(statusCode).toBe(200); + expect(data).toMatch(/isAuthenticated: .*false/); + }); + + test('wrap getServerSideProps with session', async () => { + const baseUrl = await setup(withoutApi); + const cookieJar = await login(baseUrl); + + const { + res: { statusCode }, + data + } = await get(baseUrl, '/wrapped-get-server-side-props', { fullResponse: true, cookieJar }); + expect(statusCode).toBe(200); + expect(data).toMatch(/isAuthenticated: .*true/); + }); + + test('wrap getServerSideProps with async props', async () => { + const baseUrl = await setup(withoutApi, { asyncProps: true }); + + const { + res: { statusCode }, + data + } = await get(baseUrl, '/wrapped-get-server-side-props', { fullResponse: true }); + expect(statusCode).toBe(200); + expect(data).toMatch(/isAuthenticated: .*false/); + }); + + test('wrap getServerSideProps with async props and session', async () => { + const baseUrl = await setup(withoutApi, { asyncProps: true }); + const cookieJar = await login(baseUrl); + + const { + res: { statusCode }, + data + } = await get(baseUrl, '/wrapped-get-server-side-props', { fullResponse: true, cookieJar }); + expect(statusCode).toBe(200); + expect(data).toMatch(/isAuthenticated: .*true/); + }); +}); diff --git a/tests/helpers/with-page-auth-required.test.ts b/tests/helpers/with-page-auth-required.test.ts index 4fbb403b3..4dee56cfd 100644 --- a/tests/helpers/with-page-auth-required.test.ts +++ b/tests/helpers/with-page-auth-required.test.ts @@ -24,7 +24,7 @@ describe('with-page-auth-required ssr', () => { data } = await get(baseUrl, '/protected', { cookieJar, fullResponse: true }); expect(statusCode).toBe(200); - expect(data).toMatch(/
Protected Page<\/div>/); + expect(data).toMatch(/Protected Page.*__test_sub__/); }); test('accept a custom returnTo url', async () => { @@ -80,4 +80,24 @@ describe('with-page-auth-required ssr', () => { const url = new URL(headers.location, baseUrl); expect(url.searchParams.get('returnTo')).toEqual('/foo?bar=baz&qux=quux'); }); + + test('allow access to a page with a valid session and async props', async () => { + const baseUrl = await setup(withoutApi, { + withPageAuthRequiredOptions: { + getServerSideProps() { + return Promise.resolve({ props: Promise.resolve({}) }); + } + } + }); + const cookieJar = await login(baseUrl); + + const { + res: { statusCode, headers }, + data + } = await get(baseUrl, '/protected', { cookieJar, fullResponse: true }); + expect(statusCode).toBe(200); + expect(data).toMatch(/Protected Page.*__test_sub__/); + const [cookie] = headers['set-cookie']; + expect(cookie).toMatch(/^appSession=/); + }); }); diff --git a/tests/session/cache.test.ts b/tests/session/cache.test.ts index 0aa3cfc25..903923529 100644 --- a/tests/session/cache.test.ts +++ b/tests/session/cache.test.ts @@ -32,7 +32,7 @@ describe('SessionCache', () => { test('should create the session entry', () => { cache.create(req, res, session); expect(cache.get(req, res)).toEqual(session); - expect(cookieStore.save).toHaveBeenCalledWith(req, res, session); + expect(cookieStore.save).toHaveBeenCalledWith(req, res, session, undefined); }); test('should delete the session entry', () => {