Skip to content

Commit

Permalink
Fix VA cookie to be only set if basePaths match (#2300)
Browse files Browse the repository at this point in the history
  • Loading branch information
taranvohra committed May 6, 2024
1 parent c8d5f82 commit 9a86965
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 29 deletions.
25 changes: 21 additions & 4 deletions src/lib/visitor-auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { it, describe, expect } from 'bun:test';
import { NextRequest } from 'next/server';

import {
VisitorAuthCookieValue,
getVisitorAuthCookieName,
getVisitorAuthCookieValue,
getVisitorAuthToken,
Expand All @@ -17,14 +18,18 @@ describe('getVisitorAuthToken', () => {
const request = nextRequest('https://example.com', {
[getVisitorAuthCookieName('/')]: { value: getVisitorAuthCookieValue('/', '123') },
});
expect(getVisitorAuthToken(request, request.nextUrl)).toEqual('123');
const visitorAuth = getVisitorAuthToken(request, request.nextUrl);
assertVisitorAuthCookieValue(visitorAuth);
expect(visitorAuth.token).toEqual('123');
});

it('should return the token from the cookie root basepath for a sub-path', () => {
const request = nextRequest('https://example.com/hello/world', {
[getVisitorAuthCookieName('/')]: { value: getVisitorAuthCookieValue('/', '123') },
});
expect(getVisitorAuthToken(request, request.nextUrl)).toEqual('123');
const visitorAuth = getVisitorAuthToken(request, request.nextUrl);
assertVisitorAuthCookieValue(visitorAuth);
expect(visitorAuth.token).toEqual('123');
});

it('should return the closest token from the path', () => {
Expand All @@ -34,7 +39,9 @@ describe('getVisitorAuthToken', () => {
value: getVisitorAuthCookieValue('/hello/', '123'),
},
});
expect(getVisitorAuthToken(request, request.nextUrl)).toEqual('123');
const visitorAuth = getVisitorAuthToken(request, request.nextUrl);
assertVisitorAuthCookieValue(visitorAuth);
expect(visitorAuth.token).toEqual('123');
});

it('should return the token from the cookie in a collection type url', () => {
Expand All @@ -43,7 +50,9 @@ describe('getVisitorAuthToken', () => {
value: getVisitorAuthCookieValue('/hello/v/space1/', '123'),
},
});
expect(getVisitorAuthToken(request, request.nextUrl)).toEqual('123');
const visitorAuth = getVisitorAuthToken(request, request.nextUrl);
assertVisitorAuthCookieValue(visitorAuth);
expect(visitorAuth.token).toEqual('123');
});

it('should return undefined if no cookie and no query param', () => {
Expand All @@ -52,6 +61,14 @@ describe('getVisitorAuthToken', () => {
});
});

function assertVisitorAuthCookieValue(value: unknown): asserts value is VisitorAuthCookieValue {
if (value && typeof value === 'object' && 'token' in value) {
return;
}

throw new Error('Expected a VisitorAuthCookieValue');
}

function nextRequest(url: string, cookies: Record<string, { value: string }> = {}) {
const nextUrl = new URL(url);
// @ts-ignore
Expand Down
31 changes: 20 additions & 11 deletions src/lib/visitor-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ export type VisitorAuthCookieValue = {
* Get the visitor authentication token for the request. This token can either be in the
* query parameters or stored as a cookie.
*/
export function getVisitorAuthToken(request: NextRequest, url: URL): string | undefined {
export function getVisitorAuthToken(
request: NextRequest,
url: URL,
): string | VisitorAuthCookieValue | undefined {
return url.searchParams.get(VISITOR_AUTH_PARAM) ?? getVisitorAuthTokenFromCookies(request, url);
}

Expand Down Expand Up @@ -68,7 +71,10 @@ function getUrlBasePathCombinations(url: URL): string[] {
* checking all cookies for a matching "visitor authentication cookie" and returning the
* best possible match for the current URL.
*/
function getVisitorAuthTokenFromCookies(request: NextRequest, url: URL): string | undefined {
function getVisitorAuthTokenFromCookies(
request: NextRequest,
url: URL,
): VisitorAuthCookieValue | undefined {
const urlBasePaths = getUrlBasePathCombinations(url);
// Try to find a visitor authentication token for the current URL. The request
// for the content could be hosted on a base path like `/foo/v/bar` or `/foo` or just `/`
Expand All @@ -90,14 +96,17 @@ function getVisitorAuthTokenFromCookies(request: NextRequest, url: URL): string
function findVisitorAuthCookieForBasePath(
request: NextRequest,
basePath: string,
): string | undefined {
return Array.from(request.cookies).reduce<string | undefined>((acc, [name, cookie]) => {
if (name === getVisitorAuthCookieName(basePath)) {
const value = JSON.parse(cookie.value) as VisitorAuthCookieValue;
if (value.basePath === basePath) {
acc = value.token;
): VisitorAuthCookieValue | undefined {
return Array.from(request.cookies).reduce<VisitorAuthCookieValue | undefined>(
(acc, [name, cookie]) => {
if (name === getVisitorAuthCookieName(basePath)) {
const value = JSON.parse(cookie.value) as VisitorAuthCookieValue;
if (value.basePath === basePath) {
acc = value;
}
}
}
return acc;
}, undefined);
return acc;
},
undefined,
);
}
51 changes: 37 additions & 14 deletions src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { buildVersion } from '@/lib/build';
import { createContentSecurityPolicyNonce, getContentSecurityPolicy } from '@/lib/csp';
import { getURLLookupAlternatives, normalizeURL } from '@/lib/middleware';
import {
VisitorAuthCookieValue,
getVisitorAuthCookieName,
getVisitorAuthCookieValue,
getVisitorAuthToken,
Expand Down Expand Up @@ -588,7 +589,7 @@ async function lookupSpaceInMultiPathMode(request: NextRequest, url: URL): Promi
*/
async function lookupSpaceByAPI(
lookupURL: URL,
visitorAuthToken: string | undefined,
visitorAuthToken: ReturnType<typeof getVisitorAuthToken>,
): Promise<LookupResult> {
const url = stripURLSearch(lookupURL);
const lookup = getURLLookupAlternatives(url);
Expand All @@ -598,9 +599,17 @@ async function lookupSpaceByAPI(
);

const result = await race(lookup.urls, async (alternative, { signal }) => {
const data = await getPublishedContentByUrl(alternative.url, visitorAuthToken, {
signal,
});
const data = await getPublishedContentByUrl(
alternative.url,
typeof visitorAuthToken === 'undefined'
? undefined
: typeof visitorAuthToken === 'string'
? visitorAuthToken
: visitorAuthToken.token,
{
signal,
},
);

if ('error' in data) {
if (alternative.primary) {
Expand Down Expand Up @@ -672,22 +681,36 @@ async function lookupSpaceByAPI(
*/
function getLookupResultForVisitorAuth(
basePath: string,
visitorAuthToken: string,
visitorAuthToken: string | VisitorAuthCookieValue,
): Partial<LookupResult> {
return {
// No caching for content served with visitor auth
cacheMaxAge: undefined,
cacheTags: [],
cookies: {
[getVisitorAuthCookieName(basePath)]: {
value: getVisitorAuthCookieValue(basePath, visitorAuthToken),
options: {
httpOnly: true,
sameSite: 'none',
secure: process.env.NODE_ENV === 'production',
maxAge: 7 * 24 * 60 * 60,
},
},
/**
* If the visitorAuthToken has been retrieved from a cookie, we set it back only
* if the basePath matches the current one. This is to avoid setting cookie for
* different base paths.
*/
...(typeof visitorAuthToken === 'string' || visitorAuthToken.basePath === basePath
? {
[getVisitorAuthCookieName(basePath)]: {
value: getVisitorAuthCookieValue(
basePath,
typeof visitorAuthToken === 'string'
? visitorAuthToken
: visitorAuthToken.token,
),
options: {
httpOnly: true,
sameSite: 'none',
secure: process.env.NODE_ENV === 'production',
maxAge: 7 * 24 * 60 * 60,
},
},
}
: {}),
},
};
}
Expand Down

0 comments on commit 9a86965

Please sign in to comment.