Skip to content

Enhance key manager handling by fetching production and sandbox key managers in parallel #648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Thushani-Jayasekera wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Enhance key manager handling by fetching production and sandbox key managers in parallel #648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b575f2
Enhance key manager handling by fetching production and sandbox key m…
Thushani-Jayasekera Apr 3, 2026
5617f0c
Refactor key manager handling to use Promise.allSettled for independe…
Thushani-Jayasekera Apr 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion sample_config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@
"url": "https://sts.preview-dv.choreo.dev/api/am/devportal/v2",
"graphqlURL": "https://app.preview-dv.choreo.dev/graphql",
"disableCertValidation": true,
"pathToCertificate": "./resources/security/client-truststore.pem"
"pathToCertificate": "./resources/security/client-truststore.pem",
"residentKeyManager": {
"tokenEndpoint": "https://sts.choreo.dev/oauth2/token",
"authorizeEndpoint": "https://sts.choreo.dev/oauth2/authorize",
"revokeEndpoint": "https://sts.choreo.dev/oauth2/revoke"
}
},
"aiSDKService": {
"url": "http://localhost:5001",
Expand Down
136 changes: 84 additions & 52 deletions src/controllers/applicationsContentController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,35 +173,26 @@ const loadApplicationData = async (req, orgName, applicationId, viewName) => {
api.subscriptionPolicyDetails = await util.appendSubscriptionPlanDetails(orgID, api.subscriptionPolicies);
}));

let kMmetaData = await getAPIMKeyManagers(req);

// Ensure kMmetaData is an array before filtering
if (!Array.isArray(kMmetaData)) {
kMmetaData = [];
}

kMmetaData = kMmetaData.filter(keyManager => keyManager.enabled);

// TODO: Instead of using priority-based filtering, we should identify the key manager
// configured for the production environment from the Bijira console configuration.
// This temporary priority-based approach should be replaced with a proper configuration-based selection.
if (Array.isArray(kMmetaData) && kMmetaData.length > 1) {
kMmetaData = kMmetaData.filter(keyManager =>
keyManager.name.includes("_internal_key_manager_") ||
(!kMmetaData.some(km => km.name.includes("_internal_key_manager_")) && keyManager.name.includes("Resident Key Manager")) ||
(!kMmetaData.some(km => km.name.includes("_internal_key_manager_") || km.name.includes("Resident Key Manager")) && keyManager.name.includes("_appdev_sts_key_manager_") && keyManager.name.endsWith("_prod"))
);
}

for (const keyManager of kMmetaData) {
if (keyManager.name === 'Resident Key Manager') {
keyManager.tokenEndpoint = 'https://sts.choreo.dev/oauth2/token';
keyManager.authorizeEndpoint = 'https://sts.choreo.dev/oauth2/authorize';
keyManager.revokeEndpoint = 'https://sts.choreo.dev/oauth2/revoke';
}
keyManager.availableGrantTypes = await mapGrants(keyManager.availableGrantTypes);
keyManager.applicationConfiguration = await mapDefaultValues(keyManager.applicationConfiguration);
}
// Fetch prod and sandbox key managers in parallel; each falls back to [] independently
const [prodResult, sandboxResult] = await Promise.allSettled([
getAPIMKeyManagers(req, 'prod'),
getAPIMKeyManagers(req, 'sandbox')
]);
const rawProdKeyManagers = prodResult.status === 'fulfilled' ? prodResult.value : [];
const rawSandboxKeyManagers = sandboxResult.status === 'fulfilled' ? sandboxResult.value : [];

const prodKeyManagers = filterKeyManagers(rawProdKeyManagers, '_prod');
const sandboxKeyManagers = filterKeyManagers(rawSandboxKeyManagers, '_sandbox');

// Enrich all key managers with endpoints, grant types, and configuration
await Promise.all([
...prodKeyManagers.map(enrichKeyManager),
...sandboxKeyManagers.map(enrichKeyManager)
]);

// Tag each key manager with its environment for template-side filtering
prodKeyManagers.forEach(km => { km.devPortalAppEnv = 'PROD'; });
sandboxKeyManagers.forEach(km => { km.devPortalAppEnv = 'SANDBOX'; });

let productionKeys = [];
let sandboxKeys = [];
Expand Down Expand Up @@ -231,30 +222,26 @@ const loadApplicationData = async (req, orgName, applicationId, viewName) => {
return keyData;
}) || [];

kMmetaData.forEach(keyManager => {
productionKeys.forEach(productionKey => {
if (productionKey.keyManager === keyManager.name) {
keyManager.productionKeys = productionKey;
}
// Match production keys to prod key managers; each prod KM only holds production applicationKeys
prodKeyManagers.forEach(km => {
productionKeys.forEach(pk => {
if (pk.keyManager === km.name) km.productionKeys = pk;
});
sandboxKeys.forEach(sandboxKey => {
if (sandboxKey.keyManager === keyManager.name) {
keyManager.sandboxKeys = sandboxKey;
}
km.applicationKeys = [{ keys: km.productionKeys || {}, keyType: constants.KEY_TYPE.PRODUCTION }];
});

// Match sandbox keys to sandbox key managers; each sandbox KM only holds sandbox applicationKeys
sandboxKeyManagers.forEach(km => {
sandboxKeys.forEach(sk => {
if (sk.keyManager === km.name) km.sandboxKeys = sk;
});
// Build applicationKeys per keyManager with single objects (not arrays)
keyManager.applicationKeys = [
{
keys: keyManager.productionKeys || {},
keyType: 'PRODUCTION'
},
{
keys: keyManager.sandboxKeys || {},
keyType: 'SANDBOX'
}
];
km.applicationKeys = [{ keys: km.sandboxKeys || {}, keyType: constants.KEY_TYPE.SANDBOX }];
});

const kMmetaData = [...prodKeyManagers, ...sandboxKeyManagers];
const hasProdKeyManagers = prodKeyManagers.length > 0;
const hasSandboxKeyManagers = sandboxKeyManagers.length > 0;

let subscriptionScopes = [];
if (applicationReference) {
let cpApplication = await getAPIMApplication(req, applicationReference);
Expand Down Expand Up @@ -417,6 +404,8 @@ const loadApplicationData = async (req, orgName, applicationId, viewName) => {
orgID,
applicationList,
keyManagersMetadata: kMmetaData,
hasProdKeyManagers,
hasSandboxKeyManagers,
subAPIs: subList,
subAPIsForApplicationKeys,
platformSubscriptionsForApplicationKeys: [],
Expand Down Expand Up @@ -529,6 +518,8 @@ const loadApplication = async (req, res) => {
templateContent = {
applicationMetadata: metaData,
keyManagersMetadata: kMmetaData,
hasProdKeyManagers: kMmetaData.some(km => km.devPortalAppEnv === 'PROD'),
hasSandboxKeyManagers: kMmetaData.some(km => km.devPortalAppEnv === 'SANDBOX'),
baseUrl: baseURLDev + viewName,
features: {
sdkGeneration: config.features?.sdkGeneration?.enabled || false
Expand All @@ -547,6 +538,8 @@ const loadApplication = async (req, res) => {
subscriptionCount: data.subAPIs.length
},
keyManagersMetadata: kMmetaData,
hasProdKeyManagers: data.hasProdKeyManagers,
hasSandboxKeyManagers: data.hasSandboxKeyManagers,
baseUrl: '/' + orgName + constants.ROUTE.VIEWS_PATH + viewName,
subAPIs: data.subAPIs,
nonSubAPIs: data.nonSubAPIs,
Expand Down Expand Up @@ -624,6 +617,8 @@ const loadApplicationKeys = async (req, res) => {
templateContent = {
applicationMetadata: metaData,
keyManagersMetadata: kMmetaData,
hasProdKeyManagers: kMmetaData.some(km => km.devPortalAppEnv === 'PROD'),
hasSandboxKeyManagers: kMmetaData.some(km => km.devPortalAppEnv === 'SANDBOX'),
baseUrl: baseURLDev + viewName,
productionKeys: [],
sandboxKeys: [],
Expand Down Expand Up @@ -652,6 +647,8 @@ const loadApplicationKeys = async (req, res) => {
subscriptionCount: data.subAPIs.length
},
keyManagersMetadata: kMmetaData,
hasProdKeyManagers: data.hasProdKeyManagers,
hasSandboxKeyManagers: data.hasSandboxKeyManagers,
baseUrl: '/' + orgName + constants.ROUTE.VIEWS_PATH + viewName,
subAPIs: data.subAPIs,
nonSubAPIs: data.nonSubAPIs,
Expand Down Expand Up @@ -776,9 +773,44 @@ async function getAPIMApplication(req, applicationId) {
return responseData;
}

async function getAPIMKeyManagers(req) {
const responseData = await invokeApiRequest(req, 'GET', controlPlaneUrl + '/key-managers?devPortalAppEnv=prod', null, null);
return responseData.list;
/**
* Selects the appropriate key manager from a list using a priority-based approach:
* 1. Internal key manager (highest priority)
* 2. Resident Key Manager
* 3. AppDev STS key manager matching the given environment suffix (e.g. '_prod' or '_sandbox')
*/
function filterKeyManagers(keyManagers, envSuffix) {
if (!Array.isArray(keyManagers) || keyManagers.length === 0) return [];

const enabled = keyManagers.filter(km => km.enabled);
if (enabled.length <= 1) return enabled;

const hasInternal = enabled.some(km => km.name.includes(constants.KEY_MANAGERS.INTERNAL_KEY_MANAGER));
const hasResident = enabled.some(km => km.name.includes(constants.KEY_MANAGERS.RESIDENT_KEY_MANAGER));

return enabled.filter(km =>
km.name.includes(constants.KEY_MANAGERS.INTERNAL_KEY_MANAGER) ||
(!hasInternal && km.name.includes(constants.KEY_MANAGERS.RESIDENT_KEY_MANAGER)) ||
(!hasInternal && !hasResident &&
km.name.includes(constants.KEY_MANAGERS.APP_DEV_STS_KEY_MANAGER) &&
km.name.endsWith(envSuffix))
);
}

async function enrichKeyManager(keyManager) {
if (keyManager.name === constants.KEY_MANAGERS.RESIDENT_KEY_MANAGER) {
const residentKMConfig = config.controlPlane?.residentKeyManager || {};
keyManager.tokenEndpoint = residentKMConfig.tokenEndpoint || 'https://sts.choreo.dev/oauth2/token';
keyManager.authorizeEndpoint = residentKMConfig.authorizeEndpoint || 'https://sts.choreo.dev/oauth2/authorize';
keyManager.revokeEndpoint = residentKMConfig.revokeEndpoint || 'https://sts.choreo.dev/oauth2/revoke';
}
keyManager.availableGrantTypes = await mapGrants(keyManager.availableGrantTypes);
keyManager.applicationConfiguration = await mapDefaultValues(keyManager.applicationConfiguration);
}
Comment on lines +800 to +809

@coderabbitai coderabbitai Bot Apr 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hardcoded Choreo-specific URLs may break non-Choreo deployments.

The fallback URLs (https://sts.choreo.dev/oauth2/...) are Choreo-specific. If config.controlPlane.residentKeyManager is not configured in a non-Choreo environment, users will see incorrect endpoint URLs.

Consider either:

  1. Making these URLs required configuration (no fallback)
  2. Logging a warning when using fallback URLs
🔧 Suggested improvement 
 async function enrichKeyManager(keyManager) {
     if (keyManager.name === constants.KEY_MANAGERS.RESIDENT_KEY_MANAGER) {
         const residentKMConfig = config.controlPlane?.residentKeyManager || {};
-        keyManager.tokenEndpoint = residentKMConfig.tokenEndpoint || 'https://sts.choreo.dev/oauth2/token';
-        keyManager.authorizeEndpoint = residentKMConfig.authorizeEndpoint || 'https://sts.choreo.dev/oauth2/authorize';
-        keyManager.revokeEndpoint = residentKMConfig.revokeEndpoint || 'https://sts.choreo.dev/oauth2/revoke';
+        if (!residentKMConfig.tokenEndpoint) {
+            logger.warn('Resident Key Manager token endpoint not configured, using Choreo default');
+        }
+        keyManager.tokenEndpoint = residentKMConfig.tokenEndpoint || 'https://sts.choreo.dev/oauth2/token';
+        keyManager.authorizeEndpoint = residentKMConfig.authorizeEndpoint || 'https://sts.choreo.dev/oauth2/authorize';
+        keyManager.revokeEndpoint = residentKMConfig.revokeEndpoint || 'https://sts.choreo.dev/oauth2/revoke';
     }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/controllers/applicationsContentController.js` around lines 800 - 809, The
enrichKeyManager function currently fills resident KM endpoints with hardcoded
Choreo URLs when config.controlPlane.residentKeyManager is missing; update
enrichKeyManager (and the branch that checks
constants.KEY_MANAGERS.RESIDENT_KEY_MANAGER) to stop silently defaulting to
Choreo-specific URLs: either require the endpoints (throw or return an error if
residentKMConfig.tokenEndpoint/authorizeEndpoint/revokeEndpoint are not present)
or, if you prefer a fallback, keep the assignment but add a clear warning via
the logger (e.g., processLogger.warn) whenever any of the
tokenEndpoint/authorizeEndpoint/revokeEndpoint values are missing and the
fallback is used; leave the subsequent calls to mapGrants and mapDefaultValues
untouched.


async function getAPIMKeyManagers(req, env) {
const responseData = await invokeApiRequest(req, 'GET', `${controlPlaneUrl}/key-managers?devPortalAppEnv=${env}`, null, null);
return Array.isArray(responseData.list) ? responseData.list : [];
}

async function getAPIDetails(req, apiId) {
Expand Down
8 changes: 6 additions & 2 deletions src/pages/application/partials/manage-keys.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@
<div id="collapseProductionOauth2" class="accordion-collapse collapse show"
aria-labelledby="production-oauth2">
<div class="accordion-body">
{{#if keyManagersMetadata}}
{{#if hasProdKeyManagers}}
{{#each keyManagersMetadata}}
{{#if (eq devPortalAppEnv 'PROD')}}
{{#in name values="_internal_key_manager_,Resident Key Manager,_appdev_sts_key_manager_"}}
{{#if enabled}}
{{#let "keys" productionKeys}}
Expand Down Expand Up @@ -259,6 +260,7 @@
<!-- Error message container for key generation and token generation -->
<div id="keyGenerationErrorContainer-PRODUCTION" class="mt-2 text-danger"
style="display: none; font-size: 0.75rem; word-wrap: break-word;"></div>
{{/if}}
{{/each}}
Comment on lines 260 to 264

@coderabbitai coderabbitai Bot Apr 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Verify error container placement relative to new conditionals.

The error container at lines 261-262 is now inside the {{/in}} block but outside the {{#if (eq devPortalAppEnv 'PROD')}} closing tag at line 263. This means the error container will render for every key manager in keyManagersMetadata regardless of environment, which may cause duplicate error containers.

🔧 Move error container inside the devPortalAppEnv check 
                                {{/in}}
                                <!-- Error message container for key generation and token generation -->
                                <div id="keyGenerationErrorContainer-PRODUCTION" class="mt-2 text-danger"
                                    style="display: none; font-size: 0.75rem; word-wrap: break-word;"></div>
+                               {{/if}}
-                               {{/if}}
                                {{/each}}

The error container should be moved inside the {{#if (eq devPortalAppEnv 'PROD')}} block to avoid duplicate rendering.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/pages/application/partials/manage-keys.hbs` around lines 260 - 264, The
error container div with id "keyGenerationErrorContainer-PRODUCTION" is
currently rendered for every key manager because it's placed outside the {{`#if`
(eq devPortalAppEnv 'PROD')}} block; move that <div> so it sits inside the {{`#if`
(eq devPortalAppEnv 'PROD')}}...{{/if}} conditional (and still inside the
{{`#each` keyManagersMetadata}} / corresponding {{/in}} block) so the
keyGenerationErrorContainer-PRODUCTION is only emitted when devPortalAppEnv
equals 'PROD' and avoids duplicate containers per key manager.

{{else}}
<div style="margin-left: 1rem;">
Expand Down Expand Up @@ -607,8 +609,9 @@
<div id="collapseSandboxOAuth2" class="accordion-collapse collapse show"
aria-labelledby="sandbox-oauth2">
<div class="accordion-body">
{{#if keyManagersMetadata}}
{{#if hasSandboxKeyManagers}}
{{#each keyManagersMetadata}}
{{#if (eq devPortalAppEnv 'SANDBOX')}}
{{#in name values="_internal_key_manager_,Resident Key Manager,_appdev_sts_key_manager_"}}
{{#if enabled}}
{{#let "keys" sandboxKeys}}
Expand Down Expand Up @@ -826,6 +829,7 @@
<!-- Error message container for key generation and token generation -->
<div id="keyGenerationErrorContainer-SANDBOX" class="mt-2 text-danger"
style="display: none; font-size: 0.75rem; word-wrap: break-word;"></div>
{{/if}}
{{/each}}
{{else}}
<div style="margin-left: 1rem;">
Expand Down
19 changes: 11 additions & 8 deletions src/services/adminService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1597,16 +1597,19 @@ const createAppKeyMappingOnBehalfOfUser = async (cpAppID, keymanager, clientId,
}

const getAPIMKeyManagersBehalfOfUser = async (cpOrgId, patToken) => {

let headers = {
const headers = {
'Content-Type': 'application/json',
Authorization: `Bearer ${patToken}`
}
let url = `${controlPlaneGwUrl}/key-managers?devPortalAppEnv=prod`;
const keymanagersResponse = await util.apiRequest('GET', url, headers, null, cpOrgId);

return keymanagersResponse.data.list;
}
};
const [prodResponse, sandboxResponse] = await Promise.all([
util.apiRequest('GET', `${controlPlaneGwUrl}/key-managers?devPortalAppEnv=prod`, headers, null, cpOrgId),
util.apiRequest('GET', `${controlPlaneGwUrl}/key-managers?devPortalAppEnv=sandbox`, headers, null, cpOrgId)
]);
return [
...(prodResponse.data.list || []),
...(sandboxResponse.data.list || [])
];
Comment on lines +1604 to +1611

@coderabbitai coderabbitai Bot Apr 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep the environment on each returned key manager.

This helper now flattens prod and sandbox into one anonymous list. Callers like src/controllers/devportalController.js:472-481 only validate by km.name, so a match from the wrong environment can still pass. Return devPortalAppEnv with each item, or keep separate arrays, so downstream validation stays environment-aware.

Possible direction 
-    return [
-        ...(prodResponse.data.list || []),
-        ...(sandboxResponse.data.list || [])
-    ];
+    return [
+        ...((prodResponse.data.list || []).map(km => ({ ...km, devPortalAppEnv: 'PROD' }))),
+        ...((sandboxResponse.data.list || []).map(km => ({ ...km, devPortalAppEnv: 'SANDBOX' }))),
+    ];
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/services/adminService.js` around lines 1604 - 1611, The current merge of
prodResponse and sandboxResponse into a single anonymous list loses each
key-manager's environment, which breaks downstream validation (e.g.,
devportalController.js). Fix by preserving the devPortalAppEnv for each item:
map over prodResponse.data.list and sandboxResponse.data.list (from the
util.apiRequest calls) and add a property like devPortalAppEnv: 'prod' or
'sandbox' (or alternatively return an object with separate prod and sandbox
arrays), then return the combined list so every returned key-manager includes
its originating environment for correct environment-aware validation.

};

const createCPApplication = async (req, cpApplicationName) => {
logger.info('Creating control plane application', {
Expand Down