Skip to content

Commit

Permalink
feat(backend): Limit handshake to document requests (#2352)
Browse files Browse the repository at this point in the history
* feat(backend): Support dev handshake where the redirect ends up tainted

* feat(backend): Fix tests, only check sec-fetch-dest for handshake elgibility

* chore(repo): Add changeset

* fix(backend): Update request unit tests

* fix(fastify): Update snapshot tests
  • Loading branch information
BRKalow authored Dec 15, 2023
1 parent 7e76b41 commit 1e98187
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/itchy-papayas-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/backend': patch
---

Update the handshake flow to only trigger for document requests.
35 changes: 35 additions & 0 deletions integration/tests/handshake.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie} __client_uat=${clientUat}; __session=${token}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -101,6 +102,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
Authorization: `Bearer ${token}`,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -118,6 +120,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `__client_uat=${clientUat}; __session=${token}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand Down Expand Up @@ -153,6 +156,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie} __client_uat=${clientUat}; __session=${token}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -175,6 +179,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `__client_uat=${clientUat}; __session=${token}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -196,6 +201,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
Authorization: `Bearer ${token}`,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -216,6 +222,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie} __client_uat=${clientUat}; __session=${token}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -239,6 +246,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
Authorization: `Bearer ${token}`,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -262,6 +270,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'X-Proxy-Url': 'https://example.com/clerk',
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -285,6 +294,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'X-Proxy-Url': 'https://example.com/clerk',
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -306,6 +316,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'X-Domain': 'localhost:3000',
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -329,6 +340,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'X-Domain': 'example.com',
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -347,6 +359,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie} __client_uat=1`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -367,6 +380,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `__client_uat=1`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -385,6 +399,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie} __client_uat=0`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -400,6 +415,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `__client_uat=0`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -415,6 +431,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -429,6 +446,7 @@ test.describe('Client handshake @generic', () => {
headers: new Headers({
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand Down Expand Up @@ -496,6 +514,7 @@ test.describe('Client handshake @generic', () => {
headers: new Headers({
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -513,6 +532,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie} __client_uat=${clientUat}; __session=${token}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -535,6 +555,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `__client_uat=${clientUat}; __session=${token}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -559,6 +580,7 @@ test.describe('Client handshake @generic', () => {
'X-Secret-Key': config.sk,
'X-Forwarded-Host': 'example.com',
'X-Forwarded-Proto': 'https',
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -581,6 +603,7 @@ test.describe('Client handshake @generic', () => {
'X-Secret-Key': config.sk,
'X-Forwarded-Host': 'example.com',
'X-Forwarded-Proto': 'https',
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -603,6 +626,7 @@ test.describe('Client handshake @generic', () => {
'X-Secret-Key': config.sk,
'X-Forwarded-Host': 'example.com:3213',
'X-Forwarded-Proto': 'https',
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -625,6 +649,7 @@ test.describe('Client handshake @generic', () => {
'X-Secret-Key': config.sk,
'X-Forwarded-Host': 'example.com:3213',
'X-Forwarded-Proto': 'https',
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -646,6 +671,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -669,6 +695,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -688,6 +715,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -707,6 +735,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -726,6 +755,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -746,6 +776,7 @@ test.describe('Client handshake @generic', () => {
Cookie: `${devBrowserCookie}`,
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -769,6 +800,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
Cookie: `__clerk_handshake=${handshake}`,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -791,6 +823,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
Cookie: `__clerk_handshake=${handshake}`,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -809,6 +842,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
Cookie: `__clerk_handshake=${handshake}`,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand All @@ -828,6 +862,7 @@ test.describe('Client handshake @generic', () => {
'X-Publishable-Key': config.pk,
'X-Secret-Key': config.sk,
Cookie: `__clerk_handshake=${handshake}`,
'Sec-Fetch-Dest': 'document',
}),
redirect: 'manual',
});
Expand Down
1 change: 1 addition & 0 deletions packages/backend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const Headers = {
ForwardedPort: 'x-forwarded-port',
ForwardedProto: 'x-forwarded-proto',
ForwardedHost: 'x-forwarded-host',
Accept: 'accept',
Referrer: 'referer',
UserAgent: 'user-agent',
Origin: 'origin',
Expand Down
9 changes: 5 additions & 4 deletions packages/backend/src/tokens/__tests__/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export default (QUnit: QUnit) => {
const defaultHeaders: Record<string, string> = {
host: 'example.com',
'user-agent': 'Mozilla/TestAgent',
'sec-fetch-dest': 'document',
};

const mockRequest = (headers = {}, requestUrl = 'http://clerk.com/path') => {
Expand Down Expand Up @@ -268,9 +269,7 @@ export default (QUnit: QUnit) => {
test('cookieToken: returns handshake when clientUat is missing or equals to 0 and is satellite and not is synced [11y]', async assert => {
const requestState = await authenticateRequest(
mockRequestWithCookies(
{
'Sec-Fetch-Dest': 'document',
},
{},
{
__client_uat: '0',
},
Expand Down Expand Up @@ -299,6 +298,7 @@ export default (QUnit: QUnit) => {
mockRequestWithCookies(
{
...defaultHeaders,
'sec-fetch-dest': 'empty',
'user-agent': '[some-agent]',
},
{ __client_uat: '0' },
Expand Down Expand Up @@ -388,12 +388,13 @@ export default (QUnit: QUnit) => {
assert.strictEqual(requestState.toAuth(), null);
});

test('cookieToken: returns undefined when crossOriginReferrer in development and is satellite [6n]', async assert => {
test('cookieToken: returns signedIn when satellite but valid token and clientUat', async assert => {
// Scenario: after auth action on Clerk-hosted UIs
const requestState = await authenticateRequest(
mockRequestWithCookies(
{
...defaultHeaders,
'sec-fetch-dest': 'empty',
// this is not a typo, it's intentional to be `referer` to match HTTP header key
referer: 'https://clerk.com',
},
Expand Down
Loading

0 comments on commit 1e98187

Please sign in to comment.