-
Notifications
You must be signed in to change notification settings - Fork 358
chore(backend,nextjs): Introduce API keys methods and integration tests #6169
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
Conversation
🦋 Changeset detectedLatest commit: 429802b The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
📝 Walkthrough""" WalkthroughThe changes introduce support for API key management and authentication in the integration and backend layers. New methods for creating, revoking, and retrieving API key secrets are added to the backend SDK. Integration tests for Next.js validate API key authentication using Clerk's middleware and route handlers. Test utilities are updated to create and manage fake API keys. Environment and app configuration files are modified to support scenarios involving API keys. The Next.js authentication and middleware logic is refactored to handle explicit token types, enabling support for API key tokens in addition to session tokens. Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
@@ -465,7 +449,7 @@ const handleControlFlowErrors = ( | |||
requestState: RequestState, | |||
): Response => { | |||
if (isNextjsUnauthorizedError(e)) { | |||
const response = NextResponse.next({ status: 401 }); | |||
const response = new NextResponse(null, { status: 401 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to terminate requests for invalid machine tokens when used inside middleware. The previous one continues processing the middleware chain instead of ending the request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.changeset/weak-adults-clean.md (1)
6-7
: Inconsistent bullet formatting
Ensure all bullet items follow a consistent punctuation style (either all sentences ending with a period or none).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/weak-adults-clean.md
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
.changeset/weak-adults-clean.md
Outdated
- Introduce API keys Backend SDK methods | ||
- Fix `auth.protect()` unauthorized error propagation within middleware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we split into 2 changesets ?
@@ -187,7 +187,8 @@ auth.protect = async (...args: any[]) => { | |||
require('server-only'); | |||
|
|||
const request = await buildRequestLike(); | |||
const authObject = await auth(); | |||
const requestedToken = args?.[0]?.token || args?.[1]?.token || TokenType.SessionToken; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind for the default, but I assume acceptToken
is not required here, right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct.
acceptsToken
forauth()
token
forauth.protect()
Description
auth.protect()
calls are not correctly terminating requests for invalid tokensauth.protect()
in route handlers failed to respect thetoken
optionauth()
andauth.protect()
calls insideclerkMiddleware
auth()
andauth.protect()
The underlying tests also validate the general token acceptance mechanism, which is applicable to other token types like
oauth_token
andmachine_token
simply by changing theacceptsToken
option.Resolves USER-2233
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores