Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SDK-3332] Constrain session lifecycle to withPageAuthrequired to avoid Next warning #664

Merged
merged 5 commits into from
May 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 };
Widcket marked this conversation as resolved.
Show resolved Hide resolved

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