fix(api): allow bearer API keys for POST /api/secrets#514
Open
viprapp wants to merge 2 commits intoHemmeligOrg:v7from
Open
fix(api): allow bearer API keys for POST /api/secrets#514viprapp wants to merge 2 commits intoHemmeligOrg:v7from
viprapp wants to merge 2 commits intoHemmeligOrg:v7from
Conversation
Add optional API key auth for POST /api/secrets so bearer tokens can satisfy requireRegisteredUser without breaking anonymous creation when it is disabled. - extract API key authentication into shared helpers - add optionalApiKeyOrAuthMiddleware for auth-optional routes - apply optional auth to secret creation - document Uint8Array payload format - add Hurl coverage for bearer, anonymous, and invalid auth cases
Contributor
📝 WalkthroughWalkthroughThis PR introduces bearer token API-key authentication for secret creation and listing endpoints. The auth middleware is refactored to extract shared API-key validation logic and adds optional authentication support. OpenAPI documentation is updated to reflect bearer token support and request body schema changes to Uint8Array format. Comprehensive tests and documentation examples demonstrate the new authentication flow. Changes
Sequence DiagramsequenceDiagram
participant Client
participant PostRoute as POST /secrets Route
participant OptAuthMW as optionalApiKeyOrAuthMiddleware
participant AuthHelper as Auth Helper Functions
participant Prisma as Prisma (DB)
participant SecretHandler as Secret Creation Handler
Client->>PostRoute: POST with Authorization: Bearer {key}
PostRoute->>OptAuthMW: Check authentication
OptAuthMW->>OptAuthMW: Check for session (absent)
OptAuthMW->>OptAuthMW: Check Authorization header (present)
OptAuthMW->>AuthHelper: authenticateApiKeyHeader()
AuthHelper->>AuthHelper: Extract & validate key prefix (hemmelig_)
AuthHelper->>AuthHelper: SHA-256 hash the key
AuthHelper->>Prisma: Lookup by keyHash (include user)
Prisma-->>AuthHelper: Return apiKey record with user
AuthHelper->>Prisma: Update lastUsedAt asynchronously
AuthHelper-->>OptAuthMW: Return { user }
OptAuthMW->>OptAuthMW: Set c.set('user', user)
OptAuthMW->>SecretHandler: Proceed to next() with user context
SecretHandler->>SecretHandler: Validate body, check permissions
SecretHandler->>Prisma: Create secret record
Prisma-->>SecretHandler: Confirm creation
SecretHandler-->>Client: 200 { id, ... }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/routes/secrets.ts (1)
281-295: Consider whether exposing constraint details is intentional.The P2002 error handler returns
details: prismaError.meta?.target, which exposes internal database field names to API consumers. While useful for debugging, this could leak schema information.Given that the
Secretsmodel only has the auto-generated UUIDidas a unique constraint (perprisma/schema.prisma), this code path is effectively unreachable under normal operation. If kept for defensive purposes, consider removing thedetailsfield or logging it server-side only.🔧 Optional: Remove internal details from error response
if ( error && typeof error === 'object' && 'code' in error && error.code === 'P2002' ) { - const prismaError = error as { meta?: { target?: string } }; return c.json( { error: 'Could not create secrets', - details: prismaError.meta?.target, }, 409 ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/routes/secrets.ts` around lines 281 - 295, Remove the internal DB field detail from the P2002 response in the P2002 error handler in api/routes/secrets.ts: instead of returning prismaError.meta?.target to the client, omit the "details" field from the JSON response (keep the 409 and the public message) and log the prismaError.meta?.target server-side using your existing logger (refer to the prismaError variable in that P2002 branch) so schema info is not exposed to API consumers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/routes/secrets.ts`:
- Around line 281-295: Remove the internal DB field detail from the P2002
response in the P2002 error handler in api/routes/secrets.ts: instead of
returning prismaError.meta?.target to the client, omit the "details" field from
the JSON response (keep the 409 and the public message) and log the
prismaError.meta?.target server-side using your existing logger (refer to the
prismaError variable in that P2002 branch) so schema info is not exposed to API
consumers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fbca057e-cf6c-4430-90dd-f5fb133c8820
📒 Files selected for processing (5)
api/middlewares/auth.tsapi/openapi.tsapi/routes/secrets.tsapi/tests/secrets-create-auth.hurldocs/api.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This fixes an inconsistency in the secrets API where bearer API keys worked for listing secrets, but not for creating them when
requireRegisteredUser=true.Before this change,
POST /api/secretsonly relied on the existing session user in the request context. That meant browser-based requests could succeed thanks to a session cookie, while bearer-only clients receivedOnly registered users can create secretseven with a valid API key.What changed
api/middlewares/auth.tsoptionalApiKeyOrAuthMiddlewarePOST /api/secretsUint8Arraypayload format forsecretandtitlerequireRegisteredUser=truerequireRegisteredUser=falseGET /api/secretsbearer behaviorWhy this approach
Using the existing strict middleware directly on
POST /api/secretswould have broken anonymous creation entirely. The new optional middleware preserves current behavior:requireRegisteredUser=true, a valid session or bearer API key satisfies the requirement401Result
POST /api/secretsnow behaves consistently with the rest of the API and works properly for bearer-based automation clients, while keeping anonymous creation intact when enabled.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Documentation