Skip to content

fix(auth): reject oauth on project key route#7030

Open
GautamBytes wants to merge 2 commits into
Infisical:mainfrom
GautamBytes:fix/project-key-auth-hook
Open

fix(auth): reject oauth on project key route#7030
GautamBytes wants to merge 2 commits into
Infisical:mainfrom
GautamBytes:fix/project-key-auth-hook

Conversation

@GautamBytes

@GautamBytes GautamBytes commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Context

This fixes a broken-access-control bug on the deprecated v2 project-key endpoint.

GET /api/v2/workspace/:workspaceId/encrypted-key intended to allow only first-party JWT and API key auth, but it registered verifyAuth([AuthMode.JWT, AuthMode.API_KEY]) as an onResponse hook. Fastify runs onResponse after the handler has executed, so delegated OAuth tokens could reach the handler before the auth-mode rejection ran.

This PR moves that guard to onRequest, matching sibling route patterns and ensuring delegated OAuth tokens are rejected before the encrypted project key handler executes. The accepted auth modes remain unchanged.

Screenshots

N/A

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Updated CLAUDE.md files (if needed)
  • Read the contributing guide

@GautamBytes GautamBytes marked this pull request as ready for review June 25, 2026 11:03
@greptile-apps

greptile-apps Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a broken-access-control bug on the deprecated GET /api/v2/workspace/:workspaceId/encrypted-key route by moving verifyAuth from the onResponse lifecycle hook (which fires after the response is already sent) to onRequest (which fires before the handler runs), ensuring OAuth delegated tokens are rejected before the encrypted project key is returned.

  • Core fix (deprecated-project-router.ts, line 62): a single-word change from onResponse to onRequest aligns this route with every other route in the same file, and actually enforces the auth-mode restriction instead of running it too late.
  • Regression tests (oauth-scope.spec.ts): two new cases verify the delegated-token 403 and the first-party-JWT 200 paths for the fixed route.

Confidence Score: 4/5

Safe to merge — the one-line change is correct and consistent with every other route in the same file, and the new tests directly cover both the rejection and the happy path.

The router change is minimal and targeted, and the rest of the file has used onRequest throughout; no logic was altered beyond the lifecycle hook placement. The only note is that the new v2-route regression tests were added to a file named for v1 routes, which is a minor organizational concern rather than a functional one.

No files require special attention beyond the minor test-file placement noted in the review comment.

Important Files Changed

Filename Overview
backend/src/server/routes/v2/deprecated-project-router.ts Moves verifyAuth for the encrypted-key route from onResponse (post-handler) to onRequest (pre-handler), closing a real bypass where OAuth tokens could reach the handler before auth was checked.
backend/e2e-test/routes/v1/oauth-scope.spec.ts Adds two regression tests for the fixed route: one verifying OAuth delegated tokens receive 403, and one verifying first-party JWTs receive 200. Tests live in a v1-named file despite covering a v2 route.

Reviews (1): Last reviewed commit: "fix(auth): reject oauth on project key r..." | Re-trigger Greptile

Comment thread backend/e2e-test/routes/v1/oauth-scope.spec.ts Outdated
@GautamBytes

Copy link
Copy Markdown
Contributor Author

@akhilmhdh can you review it once?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant