Skip to content

Commit

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

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

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

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

it('should return undefined if no cookie and no query param', () => {
Expand All @@ -61,14 +52,6 @@ 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: 11 additions & 20 deletions src/lib/visitor-auth.ts
Expand Up @@ -16,10 +16,7 @@ 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 | VisitorAuthCookieValue | undefined {
export function getVisitorAuthToken(request: NextRequest, url: URL): string | undefined {
return url.searchParams.get(VISITOR_AUTH_PARAM) ?? getVisitorAuthTokenFromCookies(request, url);
}

Expand Down Expand Up @@ -71,10 +68,7 @@ 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,
): VisitorAuthCookieValue | undefined {
function getVisitorAuthTokenFromCookies(request: NextRequest, url: URL): string | 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 @@ -96,17 +90,14 @@ function getVisitorAuthTokenFromCookies(
function findVisitorAuthCookieForBasePath(
request: NextRequest,
basePath: string,
): 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;
}
): 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;
}
return acc;
},
undefined,
);
}
return acc;
}, undefined);
}
51 changes: 14 additions & 37 deletions src/middleware.ts
Expand Up @@ -22,7 +22,6 @@ 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 @@ -589,7 +588,7 @@ async function lookupSpaceInMultiPathMode(request: NextRequest, url: URL): Promi
*/
async function lookupSpaceByAPI(
lookupURL: URL,
visitorAuthToken: ReturnType<typeof getVisitorAuthToken>,
visitorAuthToken: string | undefined,
): Promise<LookupResult> {
const url = stripURLSearch(lookupURL);
const lookup = getURLLookupAlternatives(url);
Expand All @@ -599,17 +598,9 @@ async function lookupSpaceByAPI(
);

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

if ('error' in data) {
if (alternative.primary) {
Expand Down Expand Up @@ -681,36 +672,22 @@ async function lookupSpaceByAPI(
*/
function getLookupResultForVisitorAuth(
basePath: string,
visitorAuthToken: string | VisitorAuthCookieValue,
visitorAuthToken: string,
): Partial<LookupResult> {
return {
// No caching for content served with visitor auth
cacheMaxAge: undefined,
cacheTags: [],
cookies: {
/**
* 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,
},
},
}
: {}),
[getVisitorAuthCookieName(basePath)]: {
value: getVisitorAuthCookieValue(basePath, visitorAuthToken),
options: {
httpOnly: true,
sameSite: 'none',
secure: process.env.NODE_ENV === 'production',
maxAge: 7 * 24 * 60 * 60,
},
},
},
};
}
Expand Down

0 comments on commit 74ba476

Please sign in to comment.