feat(broker): enforce X-User-Login and project membership on internal routes - #29
Merged
jpricardo merged 12 commits intoApr 19, 2026
Merged
Conversation
… routes (#12) - Add RPCCheckMembershipArgs to toolbox and CheckMembership RPC to logger, backed by the existing IsMember query - Add GetAPIKeyByID and ListAPIKeysByProject helpers to toolbox so handlers can scope key reads/writes to a project without cross-project leakage - Add requireUserLogin middleware (401 on missing X-User-Login header) and checkProjectMembership helper (RPC to logger) to broker - Wire requireUserLogin into the internal route group alongside requireInternalSecret - Update all 6 internal handlers to require project_id, validate membership (403 on non-member), and scope data to the given project - Update frontend Api class: add X-User-Login header via per-request createApi(userLogin) factory; add project_id to all six method signatures - Update dashboard, keys, and settings loaders/actions to use createApi with the authenticated user login and projectId from URL search params Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…a call GetRetention, UpdateRetention, and GetMetrics each opened two sequential TCP connections to the logger: one inside checkProjectMembership and a second for the actual data RPC. This doubled latency on those handlers. Change checkProjectMembership to accept a pre-dialed *rpc.Client instead of dialing internally. All six internal handlers now dial once, pass the client to checkProjectMembership, and — for the three handlers that also need logger RPC for data — reuse that same connection for the data call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GetAPIKeyByID previously propagated mongo.ErrNoDocuments unchanged, causing the broker to respond with 500. Add a sentinel ErrKeyNotFound to the toolbox and have GetAPIKeyByID return it when FindOne finds no document. The RevokeAPIKey handler now checks errors.Is against that sentinel and replies with 404 Not Found. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
requireUserLogin silently rejected missing X-User-Login headers, making it impossible to detect misconfigured clients or probing in production logs. Add a structured JSON log line on deny, matching the format already used by requireAPIKey. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… absent When no ?projectId= URL param is present, the loaders previously forwarded an empty string to the broker, which responded 400 and threw an unhandled error. Each page now short-circuits in the loader when projectId is missing and returns noProject:true; the component renders a brief instructional message instead of hitting an error boundary. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ListAPIKeys returned all API keys across all projects and is no longer called by any handler — the broker now uses ListAPIKeysByProject for every listing operation. Remove the dead method to avoid confusion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lpers - broker middleware_test: add three requireUserLogin cases — missing header (401), valid header propagated to context, empty header value treated as missing (401) - toolbox apikey_test: verify ErrKeyNotFound sentinel wraps with errors.Is, APIKey struct carries ProjectID, RPCCheckMembershipArgs fields, and GenerateAPIKey propagates ProjectID to the generated key - logger rpc_test: verify CheckMembership returns an error (not silent false) for both an invalid ObjectID hex and an empty project ID, exercising the validation path without requiring a live DB connection Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
checkProjectMembership has no dependency on Config state — it only uses the *rpc.Client passed to it. Remove the unnecessary method receiver so the function signature clearly communicates that invariant and callers do not need an app reference to invoke it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All error messages in the auth middleware follow the terse lowercase
pattern established by requireAPIKey ("missing or malformed …").
Change "X-User-Login header is required" to "missing X-User-Login header"
to match that convention.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…aders Both headers are required by the dashboard's SSR server to reach the internal broker routes, but neither was listed in the CORS AllowedHeaders config. Any browser-initiated preflight (e.g. direct JS fetch, integration test harness) would silently drop them. Add both to keep the CORS declaration consistent with actual usage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Document that requireUserLogin must run after requireInternalSecret, as X-User-Login is caller-supplied and trusted without verification. Replace noProject discriminant check with a direct null check on metrics so TypeScript narrows Promise<Metrics>|null correctly — React Router typegen flattens loader unions, preventing narrowing via the noProject field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The unscoped ListAPIKeys was removed from toolbox in favour of the project-scoped variant; update the integration test accordingly and simplify the assertion to check that no keys remain for the project. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Closes #12
Summary
CheckMembershipRPC in logger backed by the existingIsMemberquery;RPCCheckMembershipArgsadded to toolboxGetAPIKeyByIDandListAPIKeysByProjectso handlers can scope key operations per-project without cross-project leakagerequireUserLoginmiddleware in broker — rejects requests missingX-User-Loginwith401checkProjectMembershiphelper — dials logger RPC and returnsfalse(→403) if the user is not a member of the target projectproject_id, validates membership, and scopes its data accordinglyApiclass refactored: module-level singleton replaced bycreateApi(userLogin)factory; every method receivesprojectId;X-User-Loginheader sent on all callsrequireAuthfor the user login and readprojectIdfrom the?projectId=URL search paramAcceptance criteria
X-User-Loginare rejected with401403project_id?projectId=URL param)Test plan
go test ./...passes inbrokerandtoolboxnpm run typecheckpasses infrontendGET /keys?project_id=<id>withoutX-User-Login→401GET /keys?project_id=<id>with a user not in the project →403GET /keys?project_id=<id>with a valid member →200with scoped keys onlyPOST /keys,DELETE /keys/:id,GET /settings/retention,PATCH /settings/retention,GET /metrics🤖 Generated with Claude Code