Skip to content

Commit

Permalink
Merge pull request #664 from auth0/session-lifecycle
Browse files Browse the repository at this point in the history
[SDK-3332] Constrain session lifecycle to `withPageAuthrequired` to avoid Next warning
  • Loading branch information
adamjmcgrath authored May 20, 2022
2 parents 01b158b + e523099 commit 4589269
Show file tree
Hide file tree
Showing 16 changed files with 241 additions and 39 deletions.
9 changes: 9 additions & 0 deletions FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down Expand Up @@ -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.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
49 changes: 49 additions & 0 deletions src/helpers/get-server-side-props-wrapper.ts
Original file line number Diff line number Diff line change
@@ -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 <div>Protected content</div>;
* }
*
* 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;
};
};
}
4 changes: 4 additions & 0 deletions src/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ export {
WithPageAuthRequiredOptions,
PageRoute
} from './with-page-auth-required';
export {
default as getServerSidePropsWrapperFactory,
GetServerSidePropsWrapper
} from './get-server-side-props-wrapper';
53 changes: 32 additions & 21 deletions src/helpers/with-page-auth-required.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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<WithPageAuthRequiredProps & UserProps> = {},
csrOpts?: WithPageAuthRequiredCSROptions
Expand All @@ -108,25 +112,32 @@ export default function withPageAuthRequiredFactory(loginUrl: string, getSession
return withPageAuthRequiredCSR(optsOrComponent, csrOpts);
}
const { getServerSideProps, returnTo } = optsOrComponent;
return async (ctx: GetServerSidePropsContext): Promise<GetServerSidePropsResultWithSession> => {
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<GetServerSidePropsResultWithSession> => {
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 } };
};
);
};
}
3 changes: 3 additions & 0 deletions src/index.browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ const instance: SignInWithAuth0 = {
},
withPageAuthRequired() {
throw new Error(serverSideOnly('withPageAuthRequired'));
},
getServerSidePropsWrapper() {
throw new Error(serverSideOnly('getServerSidePropsWrapper'));
}
};

Expand Down
27 changes: 20 additions & 7 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -76,18 +78,21 @@ 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);
const handleProfile = profileHandler(getClient, getAccessToken, sessionCache);
const handleAuth = handlerFactory({ handleLogin, handleLogout, handleCallback, handleProfile });

return {
sessionCache,
getSession,
getAccessToken,
withApiAuthRequired,
withPageAuthRequired,
getServerSidePropsWrapper,
handleLogin,
handleLogout,
handleCallback,
Expand All @@ -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);
Expand Down Expand Up @@ -132,6 +144,7 @@ export {
PageRoute,
WithApiAuthRequired,
WithPageAuthRequired,
GetServerSidePropsWrapper,
SessionCache,
GetSession,
GetAccessToken,
Expand Down
8 changes: 7 additions & 1 deletion src/instance.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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
*/
Expand Down
15 changes: 12 additions & 3 deletions src/session/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,31 @@ type NextApiOrPageResponse = ServerResponse | NextApiResponse;

export default class SessionCache implements ISessionCache {
private cache: WeakMap<NextApiOrPageRequest, Session | null>;
private iatCache: WeakMap<NextApiOrPageRequest, number | undefined>;

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 {
Expand Down
11 changes: 9 additions & 2 deletions tests/fixtures/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export type SetupOptions = {
discoveryOptions?: Record<string, string>;
userInfoPayload?: Record<string, string>;
userInfoToken?: string;
asyncProps?: boolean;
};

export const setup = async (
Expand All @@ -44,7 +45,8 @@ export const setup = async (
getAccessTokenOptions,
discoveryOptions,
userInfoPayload = {},
userInfoToken = 'eyJz93a...k4laUWw'
userInfoToken = 'eyJz93a...k4laUWw',
asyncProps
}: SetupOptions = {}
): Promise<string> => {
discovery(config, discoveryOptions);
Expand All @@ -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) {
Expand Down Expand Up @@ -102,6 +105,8 @@ export const setup = async (
(global as any).withPageAuthRequiredCSR = withPageAuthRequired;
(global as any).getAccessToken = (req: NextApiRequest, res: NextApiResponse): Promise<GetAccessTokenResult> =>
getAccessToken(req, res, getAccessTokenOptions);
(global as any).getServerSidePropsWrapper = getServerSidePropsWrapper;
(global as any).asyncProps = asyncProps;
return start();
};

Expand All @@ -114,6 +119,8 @@ export const teardown = async (): Promise<void> => {
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<CookieJar> => {
Expand Down
1 change: 0 additions & 1 deletion tests/fixtures/test-app/next-env.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/// <reference types="next" />
/// <reference types="next/types/global" />
/// <reference types="next/image-types/global" />

// NOTE: This file should not be edited
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/test-app/pages/protected.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import React from 'react';
import { NextPageContext } from 'next';

export default function protectedPage(): React.ReactElement {
return <div>Protected Page</div>;
export default function protectedPage({ user }: { user?: { sub: string } }): React.ReactElement {
return <div>Protected Page {user ? user.sub : ''}</div>;
}

export const getServerSideProps = (ctx: NextPageContext): any => (global as any).withPageAuthRequired()(ctx);
18 changes: 18 additions & 0 deletions tests/fixtures/test-app/pages/wrapped-get-server-side-props.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from 'react';
import { NextPageContext } from 'next';

export default function wrappedGetServerSidePropsPage({
isAuthenticated
}: {
isAuthenticated?: boolean;
}): React.ReactElement {
return <div>isAuthenticated: {String(isAuthenticated)}</div>;
}

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);
Loading

0 comments on commit 4589269

Please sign in to comment.