Skip to content

Revamp platform api API keys to API Level#643

Merged
CrowleyRajapakse merged 2 commits into
wso2:mainfrom
CrowleyRajapakse:main
Mar 29, 2026
Merged

Revamp platform api API keys to API Level#643
CrowleyRajapakse merged 2 commits into
wso2:mainfrom
CrowleyRajapakse:main

Conversation

@CrowleyRajapakse

@CrowleyRajapakse CrowleyRajapakse commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

Purpose

Revamp platform api API keys to API Level

Issue

Part Fix: wso2/api-platform#1501

Summary by CodeRabbit

  • New Features

    • Adds an "API Keys" page plus sidebar/banner links to view and manage platform-bound API keys (generate, regenerate, revoke) with full client-side workflows.
  • UX

    • Read-only and load-error states are surfaced; actions disabled when not permitted. Sidebar highlights API Keys when active. Empty-state and modals guide key operations.
  • Formatting

    • Expiration dates shown as localized, human-readable strings.
  • Security

    • Mutating key actions require CSRF protection.

@coderabbitai

coderabbitai Bot commented Mar 29, 2026

Copy link
Copy Markdown

Walkthrough

Adds 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

Cohort / File(s) Summary
Service Layer
src/services/platformApiKeyService.js, src/services/platformApiKeysNavService.js
New services to proxy platform API key CRUD to the control plane with gateway-type and read-only enforcement; nav visibility logic to decide when to show API Keys link.
Controllers
src/controllers/platformApiKeysContentController.js, src/controllers/apiContentController.js, src/controllers/subscriptionsContentController.js, src/controllers/applicationsContentController.js
New API keys page controller; existing controllers now compute/pass showPlatformApiKeysNav into templates and filter/exclude platform-gateway APIs from app-level api_key counts.
Routing & Middleware
src/routes/apiContentRoute.js, src/routes/devportalRoute.js, src/middlewares/csrfProtection.js
New GET route for API keys page; new authenticated control-plane proxy endpoints under /organizations/:orgId/platform-api-keys; double-submit CSRF protection added and applied to mutating platform API key routes.
Templates & Partials
src/defaultContent/pages/api-platform-keys/page.hbs, src/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbs, src/defaultContent/partials/sidebar.hbs, src/defaultContent/pages/api-landing/..., src/defaultContent/pages/api-subscriptions/page.hbs, src/defaultContent/pages/docs/page.hbs, src/defaultContent/pages/mcp-landing/page.hbs, src/pages/application/partials/manage-keys.hbs
New API Keys page and partials (list, empty/read-only/error states, modals, secret display); sidebar and multiple pages now accept showPlatformApiKeysNav; manage-keys templates switched to gateway-filtered collections.
Client-side Scripts
src/scripts/platform-api-keys-page.js, src/scripts/common.js, src/app.js
New client script implements generate/regenerate/revoke flows with validation, fetch calls, modals and secret display; common.js updates sidebar link/active state; added Handlebars helper formatExpiresAt in src/app.js.
Partial Registration
src/middlewares/registerPartials.js
Conditional registration of new platform API keys partials when present.
Styling
src/styles/platform-api-keys.css
New stylesheet for platform API keys page (table layout, masked key UI, action buttons).
Asset & Template Wiring
src/controllers/platformApiKeysContentController.js, src/defaultContent/...
Controller rewrites API image URLs for assets; templates defer script and embed config metadata (orgID, apiID, csrf token, read-only flag) consumed by client script.
Minor Helpers
src/app.js
Added formatExpiresAt Handlebars helper for consistent expiry formatting in templates.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Poem

🐇 I nibble keys beneath the moonlit sky,
No apps, no subs—just APIs nearby.
Modals bloom, secrets softly peep,
Regenerate, revoke — safe dreams to keep.
Hop, dev friends, the portal sings, so spry!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description is minimal, providing only Purpose and Issue reference. The description template requires extensive sections (Goals, Approach, User Stories, Release Notes, Documentation, Testing, Security, etc.) that are either missing or incomplete. Expand the PR description to include Goals, Approach, User Stories, Release Notes, Documentation, Testing, Security checks, and other relevant sections from the template to provide comprehensive context for reviewers.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Revamp platform api API keys to API Level' clearly describes the main change: moving API key management from application/subscription level to API level for Platform Gateway APIs.
Linked Issues check ✅ Passed The code changes fully implement the objectives from issue #1501: moving API key management to API level, eliminating subscription dependency for API keys, supporting subscription-less APIs, and moving key management UI from Application to API-level views.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the linked issue #1501 objectives. No unrelated or extraneous modifications detected; changes focus exclusively on implementing API-level key management for Platform Gateway APIs.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@CrowleyRajapakse

Copy link
Copy Markdown
Contributor Author

API Keys Listing Page

Screenshot 2026-03-29 at 08 51 02

API Keys Generate

Screenshot 2026-03-29 at 08 51 22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 builds platformSubscriptions and noSubPlatformAPIs with CP lookups above, but this branch only forwards the *ForApplicationKeys variants, 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

📥 Commits

Reviewing files that changed from the base of the PR and between e9dfb6e and c95d485.

📒 Files selected for processing (22)
  • src/app.js
  • src/controllers/apiContentController.js
  • src/controllers/applicationsContentController.js
  • src/controllers/platformApiKeysContentController.js
  • src/controllers/subscriptionsContentController.js
  • src/defaultContent/pages/api-landing/page.hbs
  • src/defaultContent/pages/api-landing/partials/api-detail-banner.hbs
  • src/defaultContent/pages/api-platform-keys/page.hbs
  • src/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbs
  • src/defaultContent/pages/api-subscriptions/page.hbs
  • src/defaultContent/pages/docs/page.hbs
  • src/defaultContent/pages/mcp-landing/page.hbs
  • src/defaultContent/partials/sidebar.hbs
  • src/middlewares/registerPartials.js
  • src/pages/application/partials/manage-keys.hbs
  • src/routes/apiContentRoute.js
  • src/routes/devportalRoute.js
  • src/scripts/common.js
  • src/scripts/platform-api-keys-page.js
  • src/services/platformApiKeyService.js
  • src/services/platformApiKeysNavService.js
  • src/styles/platform-api-keys.css

Comment thread src/controllers/apiContentController.js
Comment thread src/controllers/applicationsContentController.js
Comment thread src/controllers/platformApiKeysContentController.js
Comment thread src/routes/devportalRoute.js Outdated
Comment thread src/scripts/common.js
Comment thread src/scripts/platform-api-keys-page.js
Comment thread src/services/platformApiKeyService.js
Comment thread src/services/platformApiKeyService.js Outdated
Comment thread src/styles/platform-api-keys.css Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
src/services/platformApiKeyService.js (1)

177-207: Consider extracting shared generate/regenerate payload preparation.

name/expiresAt validation and cpBody assembly 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-undef disable on a security-critical module reduces linting coverage. If this is needed for process, 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

📥 Commits

Reviewing files that changed from the base of the PR and between c95d485 and 4f5d199.

📒 Files selected for processing (23)
  • src/app.js
  • src/controllers/apiContentController.js
  • src/controllers/applicationsContentController.js
  • src/controllers/platformApiKeysContentController.js
  • src/controllers/subscriptionsContentController.js
  • src/defaultContent/pages/api-landing/page.hbs
  • src/defaultContent/pages/api-landing/partials/api-detail-banner.hbs
  • src/defaultContent/pages/api-platform-keys/page.hbs
  • src/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbs
  • src/defaultContent/pages/api-subscriptions/page.hbs
  • src/defaultContent/pages/docs/page.hbs
  • src/defaultContent/pages/mcp-landing/page.hbs
  • src/defaultContent/partials/sidebar.hbs
  • src/middlewares/csrfProtection.js
  • src/middlewares/registerPartials.js
  • src/pages/application/partials/manage-keys.hbs
  • src/routes/apiContentRoute.js
  • src/routes/devportalRoute.js
  • src/scripts/common.js
  • src/scripts/platform-api-keys-page.js
  • src/services/platformApiKeyService.js
  • src/services/platformApiKeysNavService.js
  • src/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

Comment thread src/controllers/apiContentController.js Outdated
Comment thread src/middlewares/csrfProtection.js
Comment thread src/middlewares/csrfProtection.js Outdated
Comment thread src/services/platformApiKeyService.js Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/app.js (1)

180-186: Unreachable Number.isNaN(n) check after regex validation.

The condition on lines 182-184 can never be true. Since s has already passed the /^\d+$/ regex test, parseInt(s, 10) will always return a valid number (never NaN). 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4f5d199 and 4b2134f.

📒 Files selected for processing (23)
  • src/app.js
  • src/controllers/apiContentController.js
  • src/controllers/applicationsContentController.js
  • src/controllers/platformApiKeysContentController.js
  • src/controllers/subscriptionsContentController.js
  • src/defaultContent/pages/api-landing/page.hbs
  • src/defaultContent/pages/api-landing/partials/api-detail-banner.hbs
  • src/defaultContent/pages/api-platform-keys/page.hbs
  • src/defaultContent/pages/api-platform-keys/partials/platform-api-key-list.hbs
  • src/defaultContent/pages/api-subscriptions/page.hbs
  • src/defaultContent/pages/docs/page.hbs
  • src/defaultContent/pages/mcp-landing/page.hbs
  • src/defaultContent/partials/sidebar.hbs
  • src/middlewares/csrfProtection.js
  • src/middlewares/registerPartials.js
  • src/pages/application/partials/manage-keys.hbs
  • src/routes/apiContentRoute.js
  • src/routes/devportalRoute.js
  • src/scripts/common.js
  • src/scripts/platform-api-keys-page.js
  • src/services/platformApiKeyService.js
  • src/services/platformApiKeysNavService.js
  • src/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

Comment thread src/services/platformApiKeyService.js
Comment thread src/services/platformApiKeysNavService.js Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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 either 204 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4b2134f and 9c96f99.

📒 Files selected for processing (2)
  • src/services/platformApiKeyService.js
  • src/services/platformApiKeysNavService.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/services/platformApiKeysNavService.js

@CrowleyRajapakse CrowleyRajapakse merged commit bb4b760 into wso2:main Mar 29, 2026
2 checks passed
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.

[Improvement]: Revamping the API Key Approach for Bijira Dev Portal (Self-Hosted Gateway APIs)

2 participants