feat: add support to mark API keys as external#2507
Conversation
|
No actionable comments were generated in the recent review. 🎉 WalkthroughThis pull request introduces an "external" boolean field to API keys across the stack. The field indicates whether an API key is managed externally and is added to protobuf definitions, database schema, repository layer, service layer, types, and the UI component, with conditional logic to restrict modification of external keys. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Router image scan passed✅ No security vulnerabilities found in image: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2507 +/- ##
===========================================
- Coverage 62.17% 36.73% -25.45%
===========================================
Files 231 948 +717
Lines 24134 124096 +99962
Branches 0 5113 +5113
===========================================
+ Hits 15006 45589 +30583
- Misses 7863 76951 +69088
- Partials 1265 1556 +291
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
studio/src/pages/[organizationSlug]/apikeys.tsx (1)
908-941:⚠️ Potential issue | 🟡 MinorTable layout issue: missing cell for external keys when user can manage API keys.
When
canManageAPIKeysistrueandexternalistrue, the table header cell (line 851-853) will be rendered but the body cell will be skipped, causing a column mismatch and potential layout issues.Consider rendering an empty cell to maintain alignment:
📝 Proposed fix
- {canManageAPIKeys && !external && ( + {canManageAPIKeys && ( <TableCell> + {!external ? ( <DropdownMenu> <div className="flex justify-center"> <DropdownMenuTrigger asChild> <Button variant="ghost" size="icon"> <EllipsisVerticalIcon className="h-4 w-4" /> </Button> </DropdownMenuTrigger> </div> <DropdownMenuContent align="end"> <DropdownMenuItem onClick={() => { setOpenUpdateDialog(true); setSelectedGroup({ apiKeyName: name, groupId: group?.id, }); }} > Update group </DropdownMenuItem> <DropdownMenuItem onClick={() => { setDeleteApiKeyName(name); setOpenDeleteDialog(true); }} > Delete </DropdownMenuItem> </DropdownMenuContent> </DropdownMenu> + ) : null} </TableCell> )}
🤖 Fix all issues with AI agents
In `@controlplane/src/core/bufservices/api-key/deleteAPIKey.ts`:
- Around line 47-55: The error message returned when checking apiKey.external
uses incorrect grammar and plurality; update the details string in the
conditional that checks apiKey.external (where clientHdr and hubUserAgent are
used) to read "You cannot delete an external API key." so it uses "an" and
singular "key" (i.e., replace the existing details text in the return block).
In `@controlplane/src/core/bufservices/api-key/getAPIKeys.ts`:
- Around line 33-36: The pagination mismatch comes from calling
apiKeyRepo.getAPIKeysCount with includeExternal: true while getAPIKeys returns
all keys; fix by updating getAPIKeys to accept an includeExternal boolean (add
parameter to getAPIKeys signature, default to false or mirror callers), pass
that flag into both apiKeyRepo.getAPIKeys and apiKeyRepo.getAPIKeysCount so both
queries use the same external filter, and ensure any callers of getAPIKeys
propagate the desired includeExternal value so totals and returned rows align.
In `@controlplane/src/core/bufservices/api-key/updateAPIKey.ts`:
- Around line 47-55: The User-Agent substring check using ctx.requestHeader and
hubUserAgent is not a reliable authorization guard and should be removed;
instead gate updates to apiKey.external using a trusted identity/assertion from
the authenticated principal (e.g., a service account flag or internal claim on
ctx.auth/ctx.principal) that cannot be spoofed. Replace the clientHdr-based
early return with a check like "if (apiKey.external &&
!principalHasInternalServiceIdentity(ctx.principal)) { return error }" (where
principalHasInternalServiceIdentity is your code that inspects a verified claim
or service account flag), and ensure the existing isOrganizationApiKeyManager
RBAC logic remains intact. Use unique symbols apiKey.external,
ctx.requestHeader/clientHdr, hubUserAgent, and the RBAC check
(isOrganizationApiKeyManager) to locate where to remove the User-Agent logic and
add the new trusted-identity check.
In `@controlplane/src/core/repositories/ApiKeyRepository.ts`:
- Around line 163-169: Rename the misleading parameter includeExternal to
isExternal in the getAPIKeysCount method signature and update all uses inside
the method (the where clause: eq(apiKeys.external, input.includeExternal)) to
use input.isExternal instead; also update any callers or types that pass this
argument (e.g., places that currently pass true/false for external filtering) so
they reference the renamed property to preserve the existing exclusive filter
behavior. Ensure the method signature in ApiKeyRepository.getAPIKeysCount and
any related interface/type definitions reflect isExternal.
Summary by CodeRabbit
Checklist