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

Add support for customizing returnTo in middleware #1342

Merged
merged 4 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
56 changes: 49 additions & 7 deletions src/helpers/with-middleware-auth-required.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import { NextMiddleware, NextResponse } from 'next/server';
import { NextMiddleware, NextRequest, NextResponse } from 'next/server';
import { SessionCache } from '../session';

/**
* Pass custom options to {@link WithMiddlewareAuthRequired}
adamjmcgrath marked this conversation as resolved.
Show resolved Hide resolved
*
* @category Server
*/
export type WithMiddlewareAuthRequiredOptions = {
middleware?: NextMiddleware;
returnTo?: string | ((req: NextRequest) => Promise<string> | string);
};

/**
* Protect your pages with Next.js Middleware. For example:
*
Expand Down Expand Up @@ -41,9 +51,35 @@ import { SessionCache } from '../session';
* });
* ```
*
* To provide a custom `returnTo` url to login:
*
* ```js
* // middleware.js
* import { withMiddlewareAuthRequired, getSession } from '@auth0/nextjs-auth0/edge';
*
* export default withMiddlewareAuthRequired({
* returnTo: '/foo',
* // Custom middleware is provided with the `middleware` config option
* async middleware(req) { return NextResponse.next(); }
* });
* ```
*
* You can also provide a method for `returnTo` that takes the req as an argument.
*
* ```js
* // middleware.js
* import { withMiddlewareAuthRequired, getSession } from '@auth0/nextjs-auth0/edge';
*
* export default withMiddlewareAuthRequired({
* returnTo(req) { return `${req.nextURL.basePath}${req.nextURL.pathname}`};
* });
* ```
*
* @category Server
*/
export type WithMiddlewareAuthRequired = (middleware?: NextMiddleware) => NextMiddleware;
export type WithMiddlewareAuthRequired = (
middlewareOrOpts?: NextMiddleware | WithMiddlewareAuthRequiredOptions
) => NextMiddleware;

/**
* @ignore
Expand All @@ -52,10 +88,18 @@ export default function withMiddlewareAuthRequiredFactory(
{ login, callback }: { login: string; callback: string },
getSessionCache: () => SessionCache
): WithMiddlewareAuthRequired {
return function withMiddlewareAuthRequired(middleware?): NextMiddleware {
return function withMiddlewareAuthRequired(opts?): NextMiddleware {
return async function wrappedMiddleware(...args) {
const [req] = args;
let middleware: NextMiddleware | undefined;
const { pathname, origin, search } = req.nextUrl;
let returnTo = `${pathname}${search}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include the basePath here? As requested in #1329

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe in another PR.

Copy link
Contributor Author

@adamjmcgrath adamjmcgrath Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I didn't add it was because I was worried it might be a breaking change.

If you were to work around this limitation currently, you might write something that prepended the base path to the returnTo in a custom login or callback handler and I didn't want to break that usage.

if (opts && typeof opts === 'function') {
adamjmcgrath marked this conversation as resolved.
Show resolved Hide resolved
middleware = opts;
} else if (opts) {
middleware = opts.middleware;
returnTo = (typeof opts.returnTo === 'function' ? await opts.returnTo(req) : opts.returnTo) || returnTo;
}
const ignorePaths = [login, callback, '/_next', '/favicon.ico'];
if (ignorePaths.some((p) => pathname.startsWith(p))) {
return;
Expand All @@ -75,15 +119,13 @@ export default function withMiddlewareAuthRequiredFactory(
{ status: 401 }
);
}
return NextResponse.redirect(
new URL(`${login}?returnTo=${encodeURIComponent(`${pathname}${search}`)}`, origin)
);
return NextResponse.redirect(new URL(`${login}?returnTo=${encodeURIComponent(returnTo)}`, origin));
}
const res = await (middleware && middleware(...args));

if (res) {
const nextRes = new NextResponse(res.body, res);
let cookies = authRes.cookies.getAll();
const cookies = authRes.cookies.getAll();
if ('cookies' in res) {
for (const cookie of res.cookies.getAll()) {
nextRes.cookies.set(cookie);
Expand Down
29 changes: 29 additions & 0 deletions tests/helpers/with-middleware-auth-required.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,29 @@ describe('with-middleware-auth-required', () => {
expect(redirect.searchParams.get('returnTo')).toEqual('/foo/bar?baz=hello');
});

test('return to provided url string', async () => {
const res = await setup({ url: 'http://example.com/foo/bar?baz=hello', middleware: { returnTo: '/qux' } });
const redirect = new URL(res.headers.get('location') as string);
expect(redirect).toMatchObject({
hostname: 'example.com',
pathname: '/api/auth/login'
});
expect(redirect.searchParams.get('returnTo')).toEqual('/qux');
});

test('return to provided url fn', async () => {
const res = await setup({
url: 'http://example.com/foo/bar?baz=hello',
middleware: { returnTo: (req: NextRequest) => Promise.resolve(`/qux${new URL(req.url).search}`) }
});
const redirect = new URL(res.headers.get('location') as string);
expect(redirect).toMatchObject({
hostname: 'example.com',
pathname: '/api/auth/login'
});
expect(redirect.searchParams.get('returnTo')).toEqual('/qux?baz=hello');
});

test('should ignore static urls', async () => {
const res = await setup({ url: 'http://example.com/_next/style.css' });
expect(res).toBeUndefined();
Expand Down Expand Up @@ -134,6 +157,12 @@ describe('with-middleware-auth-required', () => {
expect(middleware).toHaveBeenCalled();
});

test('should accept custom middleware in options obj', async () => {
const middleware = jest.fn();
await setup({ middleware: { middleware }, user: { name: 'dave' } });
expect(middleware).toHaveBeenCalled();
});

test('should honor redirects in custom middleware for authenticated users', async () => {
const middleware = jest.fn().mockImplementation(() => {
return NextResponse.redirect('https://example.com/redirect');
Expand Down