Revamp platform api API keys to API Level#643
Conversation
WalkthroughAdds a platform-level API key management feature: server services and controllers, routes and CSRF middleware, templates/partials and client script, styles, and sidebar wiring to list, generate, regenerate, and revoke API-scoped keys via the control plane for Platform Gateway APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Browser as Browser
participant DevPortal as DevPortal
participant DAO as DAO
participant ControlPlane as Control Plane
participant Renderer as Renderer
User->>Browser: Navigate to /api/{apiHandle}/api-keys
Browser->>DevPortal: GET /api/{apiHandle}/api-keys
DevPortal->>DevPortal: Authenticate & authorize
DevPortal->>DAO: Resolve API ID & metadata
DAO-->>DevPortal: API metadata (gatewayType, apiReferenceID)
DevPortal->>DevPortal: Determine showPlatformApiKeysNav (may call Control Plane)
DevPortal->>ControlPlane: GET /platform-api-keys?apiId={refId}
ControlPlane-->>DevPortal: List of platform API keys
DevPortal->>Renderer: Render API keys page with keys + config
Renderer-->>DevPortal: HTML
DevPortal-->>Browser: 200 OK + HTML
Note over Browser,User: User triggers Generate/Regenerate/Revoke
User->>Browser: Submit action (name/expires / confirm)
Browser->>DevPortal: POST /devportal/organizations/{orgId}/platform-api-keys/* (with CSRF)
DevPortal->>DevPortal: Validate & resolve refId
DevPortal->>ControlPlane: POST /platform-api-keys/(generate|{id}/regenerate|{id}/revoke)
ControlPlane-->>DevPortal: Response (key/secret or status)
DevPortal-->>Browser: 200 + payload
Browser->>User: Show secret modal / success feedback
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
src/controllers/applicationsContentController.js (1)
665-669: Skip the platform fetch path on the manage-keys route if these lists stay empty.
loadApplicationData()still buildsplatformSubscriptionsandnoSubPlatformAPIswith CP lookups above, but this branch only forwards the*ForApplicationKeysvariants, which are hard-coded to[]in the shared loader. On tenants with many platform APIs, that adds page-load latency for data this template can never render.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/controllers/applicationsContentController.js` around lines 665 - 669, The manage-keys route is still triggering platform fetch work because loadApplicationData builds platformSubscriptions and noSubPlatformAPIs but the branch only forwards platformSubscriptionsForApplicationKeys and noSubPlatformAPIsForApplicationKeys (which are always []); update the manage-keys branch in loadApplicationData (or the caller that assembles the returned object) to detect the manage-keys route and skip computing/platform-fetching platformSubscriptions and noSubPlatformAPIs entirely, or only include platformSubscriptionsForApplicationKeys/noSubPlatformAPIsForApplicationKeys when they are non-empty; reference loadApplicationData, platformSubscriptions, noSubPlatformAPIs, platformSubscriptionsForApplicationKeys and noSubPlatformAPIsForApplicationKeys to locate and short-circuit the unnecessary CP lookups so the manage-keys template doesn't incur extra latency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/controllers/apiContentController.js`:
- Line 443: The DEV branch in loadAPIContent() fails to set
templateContent.showPlatformApiKeysNav, causing the API Keys nav to be hidden in
DEV; update loadAPIContent() to call shouldShowPlatformApiKeysNav(req, metaData,
apiDetail) and assign its result to templateContent.showPlatformApiKeysNav in
the DEV path just like the non-DEV branch (consistent with loadDocsPage() and
loadDocument()), so the flag is populated for mock platform APIs used by the DEV
landing path.
In `@src/controllers/applicationsContentController.js`:
- Around line 123-124: The legacy empty-state is being shown because platform
gateway APIs are explicitly excluded when computing apiKeyEnabledAPICount and
isApiKey; update the conditional(s) that reference apiDTO.security and
apiDTO.gatewayType (where we currently check gatewayType !==
PLATFORM_GATEWAY_TYPE) so platform APIs are treated as API-key-enabled for the
purpose of deciding the legacy subscription flow (i.e., remove or adjust the
gatewayType exclusion in the checks that update apiKeyEnabledAPICount and
isApiKey), and make the same change for the second occurrence noted in the diff
so platform-only applications do not fall back to the old manage-keys empty
state.
In `@src/controllers/platformApiKeysContentController.js`:
- Around line 94-101: The catch block for CP failures currently logs via
logger.warn and falls through to render an empty list; instead surface the
failure by setting a platformApiKeysLoadError flag (or throwing/propagating the
error) so the caller/controller can return a 5xx or render an error state and
disable mutating actions; update the catch that references cpError and
logger.warn to either set platformApiKeysLoadError = true (and include
cpError.message/details in the log) or call
next(cpError)/res.status(502).json(...) so the controller does not map CP
failures to a normal “no keys” state.
In
`@src/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbs`:
- Around line 1-140: The new Handlebars partial folder
pages/api-platform-keys/partials isn't included in the list of registered
partial folders, causing "partial not found" at runtime; update the partial
registration in registerPartials.js by adding "pages/api-platform-keys/partials"
to the partialFolders array (or equivalent collection) used by the
registerPartials (or registerPartialsMiddleware) function so the template in
platform-api-key-list.hbs is discovered and registered. Ensure the added entry
follows the same path joining logic used for other folders in that file.
In `@src/routes/devportalRoute.js`:
- Around line 132-133: The POST routes for platform API key management
(router.post('/organizations/:orgId/platform-api-keys/generate',
enforceSecuirty(...), platformApiKeyService.generatePlatformApiKey) and the
similar regenerate/revoke routes) lack CSRF protection; update the route
declarations to validate a CSRF token (e.g., insert an existing csrfProtection
middleware or a new validateCsrfToken middleware before
platformApiKeyService.generatePlatformApiKey / regenerate / revoke handlers) or
require a non-cookie auth method (add a requireNonCookieAuth middleware that
enforces an Authorization header and reject cookie sessions for these
endpoints); reference the enforceSecuirty middleware and ensureAuthenticated.js
to avoid relying on cookie sessions without CSRF checks.
In `@src/scripts/common.js`:
- Around line 126-129: The sidebar is missing the DOM node targeted by the
script (element id "api-platform-keys"), so the code in common.js that sets
apiKeysLink.href (and the similar logic around lines 134-135) never activates;
add a corresponding sidebar entry in the sidebar partial (create a link element
with id "api-platform-keys" and the correct href pattern for the API keys
submenu, and any additional elements referenced by the other selectors at
134-135) so that document.getElementById('api-platform-keys') returns the
element and the script can set its href and active state.
In `@src/scripts/platform-api-keys-page.js`:
- Around line 123-249: The click handlers for generate
(btn-submit-generate-platform-api-key), regenerate
(btn-submit-regenerate-platform-api-key) and revoke (.btn-revoke-key) need
protection against double-submits: disable the UI control(s) as soon as the
action starts and re-enable them in a finally block so concurrent clicks don't
send duplicate postGenerate/postRegenerate/postRevoke requests; for
.btn-regenerate-key and .btn-revoke-key also set a simple in-flight flag or data
attribute on the specific button (e.g. data-loading) and ignore clicks while
set; ensure you clear the flag and re-enable buttons after the try/catch (and
before showing showSecretModal or reloading) to restore normal behaviour.
In `@src/services/platformApiKeyService.js`:
- Around line 111-131: The handler currently accepts any string for name and a
free-form expiresAt then forwards to CP; trim and validate both server-side
before building cpBody and calling resolvePlatformGatewayApi. Specifically, in
the function handling this route validate name = name && typeof name ===
'string' ? name.trim() : '' and reject with 400 if empty or doesn't match your
canonical pattern/length (e.g., allowed chars and min/max length), and validate
expiresAt is a proper ISO-8601 datetime or numeric timestamp within allowed
range (reject with 400 and clear description on invalid format/range) before
assigning cpBody.expiresAt; apply the same checks in the other handler block
referenced around the 164-184 area so you never proxy invalid values to
resolvePlatformGatewayApi/cpBody.
- Around line 100-241: These write handlers (generatePlatformApiKey,
regeneratePlatformApiKey, revokePlatformApiKey) do not enforce server-side
read-only mode; add a check for config.readOnlyMode at the start of each handler
(or create a checkReadOnly middleware and apply to the three routes) and return
a 403 JSON response when read-only is enabled. Specifically, before calling
resolvePlatformGatewayApi in generatePlatformApiKey, regeneratePlatformApiKey,
and revokePlatformApiKey, verify if config.readOnlyMode is truthy and if so
respond with res.status(403).json({ code: '403', message: 'Forbidden',
description: 'Write operations are disabled in read-only mode' }); do not
proceed to invokeApiRequest when read-only is set.
In `@src/styles/platform-api-keys.css`:
- Line 80: Update the CSS blocks in platform-api-keys.css so the font-family
value quotes the multi-word family and remove the redundant border-width that is
immediately overridden by border:none: change Liberation Mono to "Liberation
Mono" in the font-family declarations and delete the border-width: 1px
declarations (or consolidate into a single non-conflicting border rule) in the
same selector blocks where border: none is present (also apply the same fixes to
the similar blocks around lines 95-100).
---
Nitpick comments:
In `@src/controllers/applicationsContentController.js`:
- Around line 665-669: The manage-keys route is still triggering platform fetch
work because loadApplicationData builds platformSubscriptions and
noSubPlatformAPIs but the branch only forwards
platformSubscriptionsForApplicationKeys and noSubPlatformAPIsForApplicationKeys
(which are always []); update the manage-keys branch in loadApplicationData (or
the caller that assembles the returned object) to detect the manage-keys route
and skip computing/platform-fetching platformSubscriptions and noSubPlatformAPIs
entirely, or only include
platformSubscriptionsForApplicationKeys/noSubPlatformAPIsForApplicationKeys when
they are non-empty; reference loadApplicationData, platformSubscriptions,
noSubPlatformAPIs, platformSubscriptionsForApplicationKeys and
noSubPlatformAPIsForApplicationKeys to locate and short-circuit the unnecessary
CP lookups so the manage-keys template doesn't incur extra latency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1334b3c8-1f25-4c03-a0b7-ce5a7923c80f
📒 Files selected for processing (22)
src/app.jssrc/controllers/apiContentController.jssrc/controllers/applicationsContentController.jssrc/controllers/platformApiKeysContentController.jssrc/controllers/subscriptionsContentController.jssrc/defaultContent/pages/api-landing/page.hbssrc/defaultContent/pages/api-landing/partials/api-detail-banner.hbssrc/defaultContent/pages/api-platform-keys/page.hbssrc/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbssrc/defaultContent/pages/api-subscriptions/page.hbssrc/defaultContent/pages/docs/page.hbssrc/defaultContent/pages/mcp-landing/page.hbssrc/defaultContent/partials/sidebar.hbssrc/middlewares/registerPartials.jssrc/pages/application/partials/manage-keys.hbssrc/routes/apiContentRoute.jssrc/routes/devportalRoute.jssrc/scripts/common.jssrc/scripts/platform-api-keys-page.jssrc/services/platformApiKeyService.jssrc/services/platformApiKeysNavService.jssrc/styles/platform-api-keys.css
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/services/platformApiKeyService.js (1)
177-207: Consider extracting shared generate/regenerate payload preparation.
name/expiresAtvalidation andcpBodyassembly are duplicated and can drift over time.♻️ Refactor sketch
+function buildKeyMutationPayload({ name, expiresAt }, resolvedRefId) { + const normalizedName = parseAndValidateName(name); + if (!normalizedName) { + return { error: 'name must match ^[a-z0-9][a-z0-9_-]{0,127}$' }; + } + const expiresParsed = parseExpiresAtForCp(expiresAt); + if (!expiresParsed.ok) { + return { error: expiresParsed.description }; + } + const cpBody = { apiId: resolvedRefId, name: normalizedName }; + if (expiresParsed.iso) cpBody.expiresAt = expiresParsed.iso; + return { cpBody }; +}Also applies to: 242-271
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/platformApiKeyService.js` around lines 177 - 207, Extract the duplicated name/expiresAt validation and cpBody assembly into a shared helper and call it from both generate and regenerate flows: create a function (e.g., buildCpPayload or preparePlatformKeyPayload) that accepts (name, expiresAt, orgID, apiId) or at least (name, expiresAt, resolvedRefId) and internally calls parseAndValidateName and parseExpiresAtForCp, returns { errorResponse? , cpBody } or throws; replace the duplicated blocks around resolvePlatformGatewayApi usage in the functions that currently call parseAndValidateName/parseExpiresAtForCp and build cpBody so both use this helper (update callers where they previously checked expiresParsed.ok and normalizedName). Ensure the helper populates apiId with resolved.refId and sets expiresAt only when expiresParsed.iso exists so behavior stays identical.src/middlewares/csrfProtection.js (1)
18-18: Consider narrowing the ESLint disable scope.A blanket
no-undefdisable on a security-critical module reduces linting coverage. If this is needed forprocess, consider using/* global process */or disabling only on specific lines.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middlewares/csrfProtection.js` at line 18, The file-level "/* eslint-disable no-undef */" is too broad; narrow it by replacing that blanket disable with a targeted declaration such as "/* global process */" (or remove the disable and use inline disables only on the specific lines referencing process) in src/middlewares/csrfProtection.js so ESLint still checks other undefined variables while allowing use of the global process identifier in functions that reference process (e.g., any middleware function that reads process.env).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/controllers/apiContentController.js`:
- Around line 234-235: The schemaUrl is being built with a hardcoded '.xml'
extension which breaks GraphQL APIs in DEV; update the code that sets schemaUrl
(where schemaUrl: orgName + '/mock/' + apiHandle + '/apiDefinition.xml') to
choose the correct file based on the API type (use the same logic/constant used
in the non-DEV branch, e.g. API_DEFINITION_GRAPHQL vs XML) by checking
metaData/apiType or the same predicate used there, so GraphQL APIs use the
GraphQL definition filename instead of forcing '.xml'.
In `@src/middlewares/csrfProtection.js`:
- Around line 89-94: Replace the direct equality check between CSRF tokens (the
current if using sent !== expected) with a timing-safe comparison:
import/require Node's crypto, ensure both token values (expected and sent) are
present and converted to Buffers of equal length, and use crypto.timingSafeEqual
to compare them; if lengths differ or either token is missing treat as invalid
and return the same 403 response (keep the existing res.status(403).json payload
and the surrounding logic in the middleware in
src/middlewares/csrfProtection.js).
- Around line 60-67: The hasMTLSClient function uses the deprecated
req.connection API; update it to use req.socket instead: replace any usage of
req.connection?.getPeerCertificate?.(true) with
req.socket?.getPeerCertificate?.(true) and keep the existing checks (cert
presence via Object.keys(cert).length and req.client?.authorized) and the
try/catch behavior so functionality and error handling remain unchanged.
In `@src/services/platformApiKeyService.js`:
- Around line 131-138: The current check for req.query.apiId only tests
truthiness and allows non-string or whitespace values; update the validation
where apiId is read (const apiId = req.query.apiId) to ensure it is a string and
not empty/whitespace (e.g., typeof apiId !== 'string' || apiId.trim() === '')
and return the same 400 JSON response when invalid. Apply the same stricter
validation pattern to the other similar checks flagged in this file (the blocks
around the other apiId checks at the locations corresponding to lines 168-176,
233-241, and 298-306) so downstream DAO/CP code always receives a valid
non-empty string.
---
Nitpick comments:
In `@src/middlewares/csrfProtection.js`:
- Line 18: The file-level "/* eslint-disable no-undef */" is too broad; narrow
it by replacing that blanket disable with a targeted declaration such as "/*
global process */" (or remove the disable and use inline disables only on the
specific lines referencing process) in src/middlewares/csrfProtection.js so
ESLint still checks other undefined variables while allowing use of the global
process identifier in functions that reference process (e.g., any middleware
function that reads process.env).
In `@src/services/platformApiKeyService.js`:
- Around line 177-207: Extract the duplicated name/expiresAt validation and
cpBody assembly into a shared helper and call it from both generate and
regenerate flows: create a function (e.g., buildCpPayload or
preparePlatformKeyPayload) that accepts (name, expiresAt, orgID, apiId) or at
least (name, expiresAt, resolvedRefId) and internally calls parseAndValidateName
and parseExpiresAtForCp, returns { errorResponse? , cpBody } or throws; replace
the duplicated blocks around resolvePlatformGatewayApi usage in the functions
that currently call parseAndValidateName/parseExpiresAtForCp and build cpBody so
both use this helper (update callers where they previously checked
expiresParsed.ok and normalizedName). Ensure the helper populates apiId with
resolved.refId and sets expiresAt only when expiresParsed.iso exists so behavior
stays identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2478d14b-64da-40f2-91a1-030866565419
📒 Files selected for processing (23)
src/app.jssrc/controllers/apiContentController.jssrc/controllers/applicationsContentController.jssrc/controllers/platformApiKeysContentController.jssrc/controllers/subscriptionsContentController.jssrc/defaultContent/pages/api-landing/page.hbssrc/defaultContent/pages/api-landing/partials/api-detail-banner.hbssrc/defaultContent/pages/api-platform-keys/page.hbssrc/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbssrc/defaultContent/pages/api-subscriptions/page.hbssrc/defaultContent/pages/docs/page.hbssrc/defaultContent/pages/mcp-landing/page.hbssrc/defaultContent/partials/sidebar.hbssrc/middlewares/csrfProtection.jssrc/middlewares/registerPartials.jssrc/pages/application/partials/manage-keys.hbssrc/routes/apiContentRoute.jssrc/routes/devportalRoute.jssrc/scripts/common.jssrc/scripts/platform-api-keys-page.jssrc/services/platformApiKeyService.jssrc/services/platformApiKeysNavService.jssrc/styles/platform-api-keys.css
✅ Files skipped from review due to trivial changes (6)
- src/defaultContent/pages/api-landing/partials/api-detail-banner.hbs
- src/defaultContent/pages/docs/page.hbs
- src/controllers/subscriptionsContentController.js
- src/routes/devportalRoute.js
- src/defaultContent/pages/api-platform-keys/page.hbs
- src/scripts/platform-api-keys-page.js
🚧 Files skipped from review as they are similar to previous changes (13)
- src/middlewares/registerPartials.js
- src/routes/apiContentRoute.js
- src/defaultContent/pages/mcp-landing/page.hbs
- src/scripts/common.js
- src/defaultContent/pages/api-subscriptions/page.hbs
- src/defaultContent/partials/sidebar.hbs
- src/pages/application/partials/manage-keys.hbs
- src/app.js
- src/defaultContent/pages/api-landing/page.hbs
- src/styles/platform-api-keys.css
- src/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbs
- src/services/platformApiKeysNavService.js
- src/controllers/platformApiKeysContentController.js
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app.js (1)
180-186: UnreachableNumber.isNaN(n)check after regex validation.The condition on lines 182-184 can never be true. Since
shas already passed the/^\d+$/regex test,parseInt(s, 10)will always return a valid number (neverNaN). This is dead code that adds unnecessary complexity.🧹 Proposed simplification
if (/^\d+$/.test(s)) { const n = parseInt(s, 10); - if (Number.isNaN(n)) { - return s; - } const digitLen = String(n).length; d = digitLen <= 10 ? new Date(n * 1000) : new Date(n); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app.js` around lines 180 - 186, The Number.isNaN(n) check after /^\d+$/.test(s) is unreachable; remove that conditional and its return path and simplify the numeric branch: parse the integer (const n = parseInt(s, 10)), compute digitLen = String(n).length, then set d = digitLen <= 10 ? new Date(n * 1000) : new Date(n); (update the code around variables s, n, digitLen, and d in src/app.js to eliminate the dead check).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/platformApiKeyService.js`:
- Around line 70-97: In parseExpiresAtForCp(), add a defensive check for
timezone-qualified strings before the Date.parse() call: when handling the
string branch (after trimming and failing numeric parsing) verify the string
contains either a 'Z' or an explicit timezone offset (e.g., matches a regex like
/(?:Z|[+-]\d{2}:\d{2})$/); if the timezone is missing, return { ok: false,
description: 'expiresAt must include timezone (Z or +HH:MM)' } instead of
calling Date.parse(), otherwise proceed to parse and continue existing
validation logic (preserving the existing numeric branch and the final range
checks).
In `@src/services/platformApiKeysNavService.js`:
- Around line 41-54: The early return when config.controlPlane?.enabled ===
false incorrectly shows the API Keys nav; change the logic so
controlPlane.enabled does not short-circuit to true — either remove that early
return or move the controlPlane check after validating metaData.apiReferenceID
and existingApiDetail; only return true from the function when
metaData.apiReferenceID exists and existingApiDetail is present and
securitySchemeHasApiKey(existingApiDetail.securityScheme) is true (and
controlPlane is allowed), otherwise return false. Ensure you modify the block
referencing config.controlPlane, metaData.apiReferenceID, existingApiDetail, and
securitySchemeHasApiKey accordingly so the API Keys link is shown only when the
API is actually manageable.
---
Nitpick comments:
In `@src/app.js`:
- Around line 180-186: The Number.isNaN(n) check after /^\d+$/.test(s) is
unreachable; remove that conditional and its return path and simplify the
numeric branch: parse the integer (const n = parseInt(s, 10)), compute digitLen
= String(n).length, then set d = digitLen <= 10 ? new Date(n * 1000) : new
Date(n); (update the code around variables s, n, digitLen, and d in src/app.js
to eliminate the dead check).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcdb85f9-9407-4450-b4e7-e492191d29ce
📒 Files selected for processing (23)
src/app.jssrc/controllers/apiContentController.jssrc/controllers/applicationsContentController.jssrc/controllers/platformApiKeysContentController.jssrc/controllers/subscriptionsContentController.jssrc/defaultContent/pages/api-landing/page.hbssrc/defaultContent/pages/api-landing/partials/api-detail-banner.hbssrc/defaultContent/pages/api-platform-keys/page.hbssrc/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbssrc/defaultContent/pages/api-subscriptions/page.hbssrc/defaultContent/pages/docs/page.hbssrc/defaultContent/pages/mcp-landing/page.hbssrc/defaultContent/partials/sidebar.hbssrc/middlewares/csrfProtection.jssrc/middlewares/registerPartials.jssrc/pages/application/partials/manage-keys.hbssrc/routes/apiContentRoute.jssrc/routes/devportalRoute.jssrc/scripts/common.jssrc/scripts/platform-api-keys-page.jssrc/services/platformApiKeyService.jssrc/services/platformApiKeysNavService.jssrc/styles/platform-api-keys.css
✅ Files skipped from review due to trivial changes (8)
- src/defaultContent/pages/api-landing/partials/api-detail-banner.hbs
- src/defaultContent/pages/mcp-landing/page.hbs
- src/defaultContent/pages/api-subscriptions/page.hbs
- src/defaultContent/partials/sidebar.hbs
- src/scripts/platform-api-keys-page.js
- src/styles/platform-api-keys.css
- src/controllers/platformApiKeysContentController.js
- src/defaultContent/pages/docs/page.hbs
🚧 Files skipped from review as they are similar to previous changes (10)
- src/controllers/subscriptionsContentController.js
- src/middlewares/registerPartials.js
- src/scripts/common.js
- src/routes/apiContentRoute.js
- src/pages/application/partials/manage-keys.hbs
- src/routes/devportalRoute.js
- src/defaultContent/pages/api-platform-keys/page.hbs
- src/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbs
- src/controllers/applicationsContentController.js
- src/controllers/apiContentController.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/services/platformApiKeyService.js (1)
341-341: Consider returning 204 No Content or consistent JSON for revoke response.The revoke handler returns an empty 200 response, while other handlers return
res.status(200).json(cpResponse). For RESTful consistency, consider returning either204 No Content(semantically correct for successful deletion without response body) or an empty JSON object.♻️ Optional: Use 204 No Content
- return res.status(200).send(); + return res.status(204).send();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/platformApiKeyService.js` at line 341, The revoke endpoint currently returns an empty 200 response (return res.status(200).send()); change this to return a 204 No Content response to be RESTful by replacing that line with res.status(204).send(), and update any related tests or callers that expect a 200 body; locate the revoke handler in platformApiKeyService.js (the function that contains the return res.status(200).send() statement) and make the replacement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/services/platformApiKeyService.js`:
- Line 341: The revoke endpoint currently returns an empty 200 response (return
res.status(200).send()); change this to return a 204 No Content response to be
RESTful by replacing that line with res.status(204).send(), and update any
related tests or callers that expect a 200 body; locate the revoke handler in
platformApiKeyService.js (the function that contains the return
res.status(200).send() statement) and make the replacement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d8ecc7b8-1bd6-451a-ad5a-29a386ecf832
📒 Files selected for processing (2)
src/services/platformApiKeyService.jssrc/services/platformApiKeysNavService.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/platformApiKeysNavService.js


Purpose
Revamp platform api API keys to API Level
Issue
Part Fix: wso2/api-platform#1501
Summary by CodeRabbit
New Features
UX
Formatting
Security