chore(repo): refactor machine auth tests for Next.js and Astro#8124
chore(repo): refactor machine auth tests for Next.js and Astro#8124wobsoriano merged 9 commits intomainfrom
Conversation
Restructure machine auth integration tests from shared test files into per-SDK test files for better isolation and maintainability. Add token type mismatch coverage to verify routes reject wrong token types (e.g., M2M token on API key route, API key on OAuth route). This is the first part of updating all machine auth tests across SDKs. Express will follow in a subsequent PR.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
wobsoriano
changed the title
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new test utility module Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integration/tests/next-machine.test.ts`:
- Around line 102-113: The test loop currently uses hard-coded placeholder
strings ('mt_test_mismatch' and 'oat_test_mismatch') as Bearer tokens, which can
yield false positives; replace these placeholders with real tokens minted by the
project's test auth helper (create/mint a real M2M token and a real OAuth token)
and pass those into the Authorization header in the same test (`test("rejects
${tokenType} token on API key route (token type mismatch)")` that calls
request.get('/api/me', ...) so the route receives a valid-but-wrong-type token;
apply the same replacement to the other occurrences mentioned (the similar cases
around the other ranges) so each mismatch case uses an actual token of the
opposite type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: d3276979-02af-45ff-b177-26ccfe44610f
📒 Files selected for processing (3)
integration/tests/astro/machine.test.tsintegration/tests/express/machine.test.tsintegration/tests/next-machine.test.ts
Express machine auth tests will be added in a follow-up PR.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integration/testUtils/machineAuthService.ts`:
- Around line 59-65: The cleanup block incorrectly awaits each call before
passing results into Promise.all, causing sequential execution; remove the inner
await keywords so Promise.all receives the promise objects directly (e.g., calls
to clerkClient.m2m.revokeToken({ m2mTokenId: scopedSenderToken.id }),
clerkClient.m2m.revokeToken({ m2mTokenId: unscopedSenderToken.id }),
clerkClient.machines.delete(scopedSender.id),
clerkClient.machines.delete(unscopedSender.id), and
clerkClient.machines.delete(primaryServer.id })) and await the Promise.all(...)
result to run them in parallel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: c3d92403-a1f2-436b-ab3c-d874bf28e29c
📒 Files selected for processing (1)
integration/testUtils/machineAuthService.ts
Co-authored-by: Tom Milewski <me@tm.codes>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integration/testUtils/machineAuthService.ts`:
- Around line 110-118: The obtainOAuthAccessToken function has three type-safety
gaps: ensure fakeUser.email is non-optional before calling
signInWithEmailAndInstantPassword (either make fakeUser.email required in
ObtainOAuthAccessTokenParams or check/throw if missing), read authCode via
URLSearchParams.get and assert/throw if it's null before using it in the token
request so the compiler knows it's a string, and validate that
tokenData.access_token is present (throw or handle the error) before returning
so the function can keep returning Promise<string>; reference the symbols
fakeUser.email, signInWithEmailAndInstantPassword, authCode (from
URLSearchParams.get), tokenData.access_token, and the obtainOAuthAccessToken
function when making these fixes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: d021c003-8fa2-4ffd-935f-c927b2127347
📒 Files selected for processing (1)
integration/testUtils/machineAuthService.ts
| export type ObtainOAuthAccessTokenParams = { | ||
| page: Page; | ||
| oAuthApp: OAuthApplication; | ||
| redirectUri: string; | ||
| fakeUser: { email?: string; password: string }; | ||
| signIn: { | ||
| waitForMounted: (...args: any[]) => Promise<any>; | ||
| signInWithEmailAndInstantPassword: (params: { email: string; password: string }) => Promise<any>; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tsconfig strictness flags =="
fd 'tsconfig*.json' | while read -r f; do
echo "--- $f ---"
python - "$f" <<'PY'
import json,sys
p=sys.argv[1]
try:
d=json.load(open(p))
c=d.get("compilerOptions",{})
print("strict:", c.get("strict"), "strictNullChecks:", c.get("strictNullChecks"))
except Exception as e:
print("unparseable:", e)
PY
done
echo
echo "== nullable flow in machineAuthService =="
rg -n -C2 "fakeUser: \{ email\?: string; password: string \}|searchParams\.get\('code'\)|code: authCode|return tokenData\.access_token" integration/testUtils/machineAuthService.tsRepository: clerk/javascript
Length of output: 5798
🏁 Script executed:
cat -n integration/testUtils/machineAuthService.ts | sed -n '135,190p'Repository: clerk/javascript
Length of output: 2229
🏁 Script executed:
cat -n integration/testUtils/machineAuthService.ts | sed -n '110,120p'Repository: clerk/javascript
Length of output: 475
🏁 Script executed:
fd machineAuthService -type fRepository: clerk/javascript
Length of output: 230
🏁 Script executed:
grep -r "machineAuthService" --include="*.ts" --include="*.js" --include="*.json" | head -20Repository: clerk/javascript
Length of output: 230
🏁 Script executed:
cd integration/testUtils && grep -r "obtainOAuthAccessToken" . && cd - || trueRepository: clerk/javascript
Length of output: 186
🏁 Script executed:
cat -n integration/testUtils/machineAuthService.ts | sed -n '1,20p'Repository: clerk/javascript
Length of output: 816
Type contract violations in OAuth token helper create potential runtime failures.
The function obtainOAuthAccessToken has three type mismatches that could cause test failures:
fakeUser.emailis optional (email?: string, line 114) but passed tosignInWithEmailAndInstantPasswordwhich requires a non-null string (line 150-151)authCodefromURLSearchParams.get('code')returnsstring | null(line 163) but is used directly in the token request payload (line 171) instead of being validated first- Return type declares
Promise<string>(line 137) but returnstokenData.access_tokenwhich is typed as optional (line 182, 185)
While runtime assertions exist at lines 164 and 183, they do not satisfy the type contracts and will only catch issues if tests actually hit them. Ensure email is required in the parameter type or validate it before use, check that authCode is not null before inclusion in the request, and verify access_token exists before returning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@integration/testUtils/machineAuthService.ts` around lines 110 - 118, The
obtainOAuthAccessToken function has three type-safety gaps: ensure
fakeUser.email is non-optional before calling signInWithEmailAndInstantPassword
(either make fakeUser.email required in ObtainOAuthAccessTokenParams or
check/throw if missing), read authCode via URLSearchParams.get and assert/throw
if it's null before using it in the token request so the compiler knows it's a
string, and validate that tokenData.access_token is present (throw or handle the
error) before returning so the function can keep returning Promise<string>;
reference the symbols fakeUser.email, signInWithEmailAndInstantPassword,
authCode (from URLSearchParams.get), tokenData.access_token, and the
obtainOAuthAccessToken function when making these fixes.
3 participants
Description
Restructures machine auth integration tests from shared
machine-auth/test files into per-SDK test files (next-machine.test.ts,astro/machine.test.ts) for better isolation and maintainability. Adds token type mismatch test coverage to verify that routes correctly reject tokens of the wrong type (e.g., sending an M2M token to an API key route).This is the start of updating all machine auth e2e tests across SDKs. Other SDKs will follow in a subsequent PR.
Changes
machine-auth/{api-keys,m2m,oauth}.test.tsto per-SDK files for Next.js and Astromachine-auth/component.test.tstoapi-keys-component.test.tsmachineAuthService.tstest utilityTest coverage
Next.js (
next-machine.test.ts)Astro (
astro/machine.test.ts)Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit