From 78f1e6a2b0f8920dbd5e738ed9633d2056eaa094 Mon Sep 17 00:00:00 2001 From: Piumal Rathnayake Date: Tue, 4 Aug 2026 13:44:57 +0530 Subject: [PATCH 1/6] Make idp org ref id editable through config --- .../api-portal/configs/config-template.toml | 19 ++++ .../organizations/organizations.spec.js | 7 +- .../api-portal/src/config/configDefaults.js | 15 +++ portals/api-portal/src/dao/organizationDao.js | 37 ++++++++ .../partials/cfg-organization-panel.hbs | 2 +- .../src/scripts/settings-organization.js | 3 + .../api-portal/src/services/adminService.js | 10 +- .../api-portal/src/services/seederService.js | 68 +++++++++++++- .../api-portal/src/utils/idpOrgIdPolicy.js | 53 +++++++++++ .../src/utils/idpOrgIdPolicy.test.js | 91 +++++++++++++++++++ portals/api-portal/src/utils/orgContext.js | 45 +++++++++ 11 files changed, 344 insertions(+), 6 deletions(-) create mode 100644 portals/api-portal/src/utils/idpOrgIdPolicy.js create mode 100644 portals/api-portal/src/utils/idpOrgIdPolicy.test.js diff --git a/portals/api-portal/configs/config-template.toml b/portals/api-portal/configs/config-template.toml index 82dcf86fc..a6dbdb3d9 100644 --- a/portals/api-portal/configs/config-template.toml +++ b/portals/api-portal/configs/config-template.toml @@ -148,6 +148,21 @@ session_secret = "" # 64-char hex — express-session signing secret # ([api_portal.auth.local] below). "idp" — external OIDC IDP (auth.idp below). mode = "local" # local | idp +# `idp_org_id` is your IdP's identifier for this organization — the VALUE expected in +# the claim named by claim_mappings.organization below, which is what incoming tokens +# are matched against. It defaults to organization.handle (leave it unset for the +# common case, including local/platform-api-managed mode where the org_handle claim IS +# the handle). Set it only when your IdP's org claim differs from the URL handle. +# +# This setting owns the value — stored as the organization's idp_ref_id, the name the +# REST API uses. It is seeded on first boot and re-applied on every restart, so +# correcting it here and restarting is how you change it. The admin API refuses to +# change it, since a write there would be reverted at the next restart. Leaving it +# unset does NOT reset a previously configured value back to the handle — remove it +# only along with the IdP claim it mirrored. Anyone already logged in with the previous +# claim value must log in again after a change. +# idp_org_id = "default" + # JWT claim name mappings — which token claim carries each field. # Dot-notation supported for nested claims (e.g. "realm_access.roles"). [api_portal.auth.claim_mappings] @@ -284,6 +299,10 @@ subscriber = "ap_subscriber" # [platform_api.auth.file.organization] id — that is what the Platform API puts in # the org_handle claim of the tokens this portal verifies. A mismatch means every # login is rejected with 403. +# +# The org identifier your IdP asserts for this organization is configured separately, +# as `idp_org_id` in [api_portal.auth] — it belongs with the claim mapping that names +# the claim it arrives in. [api_portal.organization] handle = "default" # URL slug: /{handle}/views/{viewName} diff --git a/portals/api-portal/it/rest-api/organizations/organizations.spec.js b/portals/api-portal/it/rest-api/organizations/organizations.spec.js index 04e789df9..3c219a703 100644 --- a/portals/api-portal/it/rest-api/organizations/organizations.spec.js +++ b/portals/api-portal/it/rest-api/organizations/organizations.spec.js @@ -98,11 +98,14 @@ describe('organizations', () => { }); }); - describe('identity fields are immutable', () => { + describe('identity fields are not writable through this API', () => { // The handle and idp_ref_id are what page URLs and incoming token // organization claims are matched against. Renaming either would leave the // running instance unable to find its own organization — every page 404ing - // and every login 403ing until config was edited to match. + // and every login 403ing until config was edited to match. idp_ref_id is + // additionally re-applied from auth.idp_org_id by the startup seeder + // (seederService.reconcileIdpOrgId), so a write accepted here would be + // reverted on the next restart; config stays its single writer. it('rejects changing the organization handle', async () => { const res = await client.as('admin').put(`/organizations/${OWN_ORG}`, { id: 'renamed-org', diff --git a/portals/api-portal/src/config/configDefaults.js b/portals/api-portal/src/config/configDefaults.js index b7c0d6b60..71bb72960 100644 --- a/portals/api-portal/src/config/configDefaults.js +++ b/portals/api-portal/src/config/configDefaults.js @@ -83,6 +83,21 @@ const DEFAULTS = { // "local" — username/password validated against the Platform API control // plane (auth.local below). "idp" — external OIDC IDP (auth.idp below). mode: 'local', // local | idp + // The org identifier the IdP asserts at SSO login — the expected VALUE of the + // claim named by claimMappings.organization below, which is what incoming + // tokens are matched against (ensureAuthenticated.belongsToTargetOrg). Stored + // on the organization row as idp_ref_id, where token claims are compared + // against it. + // + // Applied at startup only: seeded with the organization and re-applied on + // every later boot (seederService.reconcileIdpOrgId), which makes this setting + // the single writer of that field — the admin API refuses to change it. Empty + // means "use organization.handle" for a fresh organization (the common case, + // incl. the platform-api-managed mode where the org_handle claim IS the + // handle) and "leave the stored value alone" for an existing one, so dropping + // the setting never silently resets it. Matched verbatim against the claim, so + // it is NOT lowercased, unlike the handle. + idpOrgId: '', // JWT claim name mappings — which token claim carries each field. // Dot-notation supported for nested claims (e.g. "realm_access.roles"). claimMappings: { diff --git a/portals/api-portal/src/dao/organizationDao.js b/portals/api-portal/src/dao/organizationDao.js index b3692830c..db80898f4 100644 --- a/portals/api-portal/src/dao/organizationDao.js +++ b/portals/api-portal/src/dao/organizationDao.js @@ -170,6 +170,41 @@ const update = async (orgData, t) => { return [rowCount, [updatedOrg]]; }; +/** + * Narrow, targeted write of idp_ref_id alone — used by the startup seeder to + * reconcile the stored value with auth.idp_org_id in config. update() above writes + * display_name, the business_owner fields, and cp_ref_id unconditionally, so + * reusing it here would clear whatever an operator set through the settings UI. + */ +const updateIdpRefId = async (orgUuid, idpRefId, actor, t) => { + const exec = t || db; + const { rowCount } = await exec.execute( + `UPDATE ${ORG_TABLE} SET idp_ref_id = ?, updated_by = ?, updated_at = ? WHERE uuid = ?`, + [idpRefId, actor, new Date(), orgUuid] + ); + if (rowCount < 1) { + throw new NotFoundError('Organization not found'); + } +}; + +/** + * Returns another organization that findOrgByIdentifier would resolve `value` to — + * i.e. one whose handle, display_name, or idp_ref_id already equals it — or null. + * + * A shared multi-organization database is the case this guards: pointing this + * instance's idp_ref_id at a value another organization already answers to would + * shadow that organization's own identifier resolution, so the seeder refuses the + * change rather than breaking a neighbouring tenant. + */ +const findOtherOrgClaimingIdentifier = async (value, excludeUuid, t) => { + const exec = t || db; + const rows = await exec.query( + `SELECT * FROM ${ORG_TABLE} WHERE (handle = ? OR display_name = ? OR idp_ref_id = ?) AND uuid <> ?`, + [String(value).toLowerCase(), value, value, excludeUuid] + ); + return rows.length ? normalizeOrgRow(rows[0]) : null; +}; + // Tables whose org_uuid FK is ON DELETE NO ACTION (database/schema.*.sql) block // deleting the organization row unless their rows are removed first. Tables with // ON DELETE CASCADE/SET NULL (api_metadata, subscription_plans, audit, @@ -350,6 +385,8 @@ module.exports = { getId, list, update, + updateIdpRefId, + findOtherOrgClaimingIdentifier, delete: deleteOrg, createContent, updateContent, diff --git a/portals/api-portal/src/pages/settings/partials/cfg-organization-panel.hbs b/portals/api-portal/src/pages/settings/partials/cfg-organization-panel.hbs index 8116be906..56e50d1f3 100644 --- a/portals/api-portal/src/pages/settings/partials/cfg-organization-panel.hbs +++ b/portals/api-portal/src/pages/settings/partials/cfg-organization-panel.hbs @@ -45,7 +45,7 @@
- +
diff --git a/portals/api-portal/src/scripts/settings-organization.js b/portals/api-portal/src/scripts/settings-organization.js index 0528d63f9..0411a8770 100644 --- a/portals/api-portal/src/scripts/settings-organization.js +++ b/portals/api-portal/src/scripts/settings-organization.js @@ -41,6 +41,9 @@ var oe = g('org-owner-email').value.trim(); if (oe && !emailRe.test(oe)) { await showAlert('Business owner email is not a valid email address.', 'error'); return; } // displayName, id and idpRefId are required by the update schema; send them always. + // idpRefId is read back from its read-only input rather than being editable: it is + // owned by the auth.idp_org_id configuration and the API rejects any change with + // 400, so echoing the current value is what lets the rest of this form save. var body = { displayName: name, id: handle, diff --git a/portals/api-portal/src/services/adminService.js b/portals/api-portal/src/services/adminService.js index 0de694a96..f87f9f5e0 100644 --- a/portals/api-portal/src/services/adminService.js +++ b/portals/api-portal/src/services/adminService.js @@ -285,12 +285,18 @@ const updateOrganization = async (req, res) => { `The organization handle cannot be changed; it is fixed to '${currentHandle}' by this ` + "portal's organization.handle configuration."); } + // idp_ref_id stays config-owned rather than immutable: auth.idp_org_id is + // re-applied by the startup seeder (seederService.reconcileIdpOrgId), so a + // write accepted here would be silently reverted on the next restart. Keeping + // config the single writer also means the value cannot drift between the file an + // operator reads and the row incoming token claims are matched against. if (payload.idpRefId !== undefined) { const existingIdpRefId = (await orgDao.getByHandle(currentHandle)).idp_ref_id; if (payload.idpRefId !== existingIdpRefId) { return util.sendError(res, 400, - 'The organization IDP reference cannot be changed; it is what incoming ' + - 'tokens are matched against.'); + 'The organization IDP reference cannot be changed through this API; it is what ' + + "incoming tokens are matched against and is set by this portal's auth.idp_org_id " + + 'configuration.'); } } diff --git a/portals/api-portal/src/services/seederService.js b/portals/api-portal/src/services/seederService.js index 6ddb04177..e8c9a754c 100644 --- a/portals/api-portal/src/services/seederService.js +++ b/portals/api-portal/src/services/seederService.js @@ -23,11 +23,55 @@ const viewDao = require('../dao/viewDao'); const subscriptionPlanDao = require('../dao/subscriptionPlanDao'); const { config } = require('../config/configLoader'); const orgContext = require('../utils/orgContext'); +const { planIdpOrgIdReconcile } = require('../utils/idpOrgIdPolicy'); const constants = require('../utils/constants'); const logger = require('../config/logger'); const db = require('../db/driver'); const { NotFoundError } = require('../utils/errors/customErrors'); +/** + * Brings the organization's stored idp_ref_id in line with auth.idp_org_id in config. + * + * Config is the only writer of this field — the admin API refuses to change it + * (adminService.updateOrganization) — so there is no operator edit here to clobber, + * and without this reconcile a value that was wrong on first boot, or an IdP that + * changed the org claim it asserts, could only be repaired with direct SQL. + * + * Sessions already holding the previous org claim stop passing the org check + * (ensureAuthenticated.belongsToTargetOrg) and their owners have to log in again — + * hence the warn-level log recording both values. + */ +async function reconcileIdpOrgId(org) { + const configured = orgContext.getConfiguredIdpOrgId(); + const stored = org.idp_ref_id || ''; + if (planIdpOrgIdReconcile({ configured, stored }).action === 'skip') return; + + const conflict = await orgDao.findOtherOrgClaimingIdentifier(configured, org.uuid); + const { action } = planIdpOrgIdReconcile({ + configured, + stored, + conflictingOrgHandle: conflict?.handle, + }); + if (action === 'conflict') { + logger.error('Org: configured auth.idp_org_id is already claimed by another organization — keeping the stored value', { + handle: org.handle, + configured, + stored, + claimedBy: conflict.handle, + operation: 'reconcileIdpOrgId', + }); + return; + } + + await orgDao.updateIdpRefId(org.uuid, configured, constants.SYSTEM_ACTOR); + logger.warn('Org: IdP organization id updated from configuration — existing sessions carrying the previous org claim must log in again', { + handle: org.handle, + previous: stored, + current: configured, + operation: 'reconcileIdpOrgId', + }); +} + /** * Seeds this instance's organization and its dependent resources on startup. * Each resource is checked/created individually so an existing org with @@ -39,6 +83,10 @@ const { NotFoundError } = require('../utils/errors/customErrors'); * multi-organization database the looser match could resolve to a *different* * organization that happens to carry this handle as its display name, and the * seeder would then adopt that row as this instance's org. + * + * An organization that already exists is left as the operator has since configured it + * through the settings UI, with one exception: idp_ref_id, which auth.idp_org_id owns + * outright and reconcileIdpOrgId re-applies on every boot. */ async function seedDefaultOrg() { const orgName = orgContext.getHandle(); @@ -47,7 +95,11 @@ async function seedDefaultOrg() { const payload = { displayName: orgContext.getDisplayName(), handle: orgName, - idpRefId: orgName, + // Defaults to the handle (getIdpOrgId falls back when unset) — override via + // auth.idp_org_id when the IdP asserts an org claim that differs from the URL + // handle. Config owns this field: the admin API refuses to change it, and + // reconcileIdpOrgId re-applies the configured value on later boots. + idpRefId: orgContext.getIdpOrgId(), configuration: {}, createdBy: constants.SYSTEM_ACTOR, }; @@ -56,6 +108,20 @@ async function seedDefaultOrg() { try { const existing = await orgDao.getByHandle(orgName); orgId = existing.uuid; + try { + await reconcileIdpOrgId(existing); + } catch (error) { + // Non-fatal, unlike a failed lookup or create: the organization exists and + // the portal can serve it with the previously stored value. Logins whose + // org claim only matches the newly configured value will fail until the + // write succeeds, which the operator needs to see rather than have startup + // aborted underneath a working deployment. + logger.error('Failed to reconcile organization idp_ref_id from configuration', { + error: error.message, + handle: orgName, + operation: 'seedDefaultOrg', + }); + } } catch (notFound) { if (!(notFound instanceof NotFoundError)) { // Rethrow rather than continue: without this row the portal has no diff --git a/portals/api-portal/src/utils/idpOrgIdPolicy.js b/portals/api-portal/src/utils/idpOrgIdPolicy.js new file mode 100644 index 000000000..05d17b2dd --- /dev/null +++ b/portals/api-portal/src/utils/idpOrgIdPolicy.js @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.com) All Rights Reserved. + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +'use strict'; + +/* + * What the startup seeder should do with the organization's stored IdP org + * identifier — auth.idp_org_id in config, the idp_ref_id column on the row. + * + * Kept as a standalone, dependency-free module — not a helper inside + * seederService.js — so the policy can be unit-tested without loading the config + * layer and database driver that requiring the seeder would pull in. + */ + +/** + * Decides what the startup reconcile should do with the stored value, given the + * explicitly configured auth.idp_org_id (orgContext.getConfiguredIdpOrgId), what is + * stored on the organization row, and any other organization already answering to it. + * + * - 'skip' — nothing configured (never rewrite a stored value back to the + * handle just because the setting is absent), or already in sync. + * - 'conflict' — another organization in a shared database already resolves this + * value via its handle/display_name/idp_ref_id; taking it would + * shadow that organization, so the stored value is left as-is. + * - 'update' — write the configured value. + * + * @param {{configured: string, stored: string, conflictingOrgHandle?: string}} input + * @returns {{action: 'skip'|'update'|'conflict'}} + */ +function planIdpOrgIdReconcile({ configured, stored, conflictingOrgHandle }) { + if (!configured) return { action: 'skip' }; + // Compared verbatim: the stored value is matched case-sensitively against the token + // claim, so a case-only difference is a real difference worth writing. + if (configured === stored) return { action: 'skip' }; + if (conflictingOrgHandle) return { action: 'conflict' }; + return { action: 'update' }; +} + +module.exports = { planIdpOrgIdReconcile }; diff --git a/portals/api-portal/src/utils/idpOrgIdPolicy.test.js b/portals/api-portal/src/utils/idpOrgIdPolicy.test.js new file mode 100644 index 000000000..56e386fb6 --- /dev/null +++ b/portals/api-portal/src/utils/idpOrgIdPolicy.test.js @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (http://www.wso2.com) All Rights Reserved. + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +'use strict'; + +/* + * The startup idp_ref_id reconcile policy (idpOrgIdPolicy.planIdpOrgIdReconcile), + * applied by seederService.reconcileIdpOrgId on every boot. + * + * idp_ref_id is what incoming token org claims are matched against + * (ensureAuthenticated.belongsToTargetOrg), and auth.idp_org_id in config is + * its only writer — so the two failure modes worth pinning down are a boot that + * silently rewrites a working value, and one that refuses a legitimate correction. + */ + +const test = require('node:test'); +const assert = require('node:assert'); + +const { planIdpOrgIdReconcile } = require('./idpOrgIdPolicy'); + +test('unset configuration leaves the stored value alone', () => { + // The regression this guards: getIdpOrgId() falls back to the handle, so a + // reconcile keyed on it would rewrite a deliberately-set 'ACME-PROD' back to + // 'acme' on the first boot after the setting was dropped from config. + assert.deepStrictEqual( + planIdpOrgIdReconcile({ configured: '', stored: 'ACME-PROD' }), + { action: 'skip' } + ); +}); + +test('a value already in sync is not rewritten', () => { + assert.deepStrictEqual( + planIdpOrgIdReconcile({ configured: 'ACME-PROD', stored: 'ACME-PROD' }), + { action: 'skip' } + ); +}); + +test('a changed configured value is written', () => { + assert.deepStrictEqual( + planIdpOrgIdReconcile({ configured: 'ACME-PROD', stored: 'acme' }), + { action: 'update' } + ); +}); + +test('a case-only difference counts as a change', () => { + // idp_ref_id is compared verbatim against the token claim, unlike the handle, + // so 'acme' and 'ACME' are genuinely different matching keys. + assert.deepStrictEqual( + planIdpOrgIdReconcile({ configured: 'ACME', stored: 'acme' }), + { action: 'update' } + ); +}); + +test('an empty stored value is filled in from configuration', () => { + assert.deepStrictEqual( + planIdpOrgIdReconcile({ configured: 'ACME-PROD', stored: '' }), + { action: 'update' } + ); +}); + +test('a value another organization already answers to is refused', () => { + assert.deepStrictEqual( + planIdpOrgIdReconcile({ configured: 'globex', stored: 'acme', conflictingOrgHandle: 'globex' }), + { action: 'conflict' } + ); +}); + +test('a conflict on an already-in-sync value is still a no-op, not an error', () => { + // Ordering matters here: the in-sync check comes first, so a neighbouring + // organization that happens to share this value cannot turn every boot of an + // already-correct deployment into a logged conflict. + assert.deepStrictEqual( + planIdpOrgIdReconcile({ configured: 'acme', stored: 'acme', conflictingOrgHandle: 'other' }), + { action: 'skip' } + ); +}); diff --git a/portals/api-portal/src/utils/orgContext.js b/portals/api-portal/src/utils/orgContext.js index 49f62e100..428206471 100644 --- a/portals/api-portal/src/utils/orgContext.js +++ b/portals/api-portal/src/utils/orgContext.js @@ -75,6 +75,49 @@ function getDisplayName() { return config.organization?.displayName || getHandle(); } +/** + * The explicitly configured auth.idp_org_id, or '' when unset. + * + * Kept separate from getIdpOrgId() because the two questions differ: "what should a + * brand-new organization row be seeded with" (getIdpOrgId, which falls back to the + * handle) versus "did the operator actually ask for a specific value" (this one). The + * startup reconcile in seederService.js needs the latter — with only the falling-back + * form it could not tell an unset setting from one deliberately set to the handle, and + * would silently rewrite a stored idp_ref_id back to the handle whenever the setting + * was absent. + * + * @returns {string} + */ +function getConfiguredIdpOrgId() { + const configured = config.auth?.idpOrgId; + return (typeof configured === 'string' && configured.trim()) || ''; +} + +/** + * The IdP's organization identifier for this instance's organization — the value of + * the org claim the IdP asserts at SSO login, which is what incoming tokens are + * matched against (see organizationDao.findOrgByIdentifier). Falls back to the handle + * when unset, so a deployment whose IdP claim equals the handle needs no extra config. + * + * Read from [api_portal.auth] rather than [api_portal.organization]: it describes the + * identity provider's naming of this organization, and pairs with + * auth.claim_mappings.organization — that names the claim, this is the value expected + * in it. It is persisted as the organization row's idp_ref_id column, which is the + * name the REST API and database schema use for the same thing. + * + * NOT lowercased, unlike the handle: the stored value is compared verbatim + * (case-sensitive) against the token claim, so config must be preserved exactly. + * + * Consulted at startup only: the seeder writes it when creating the organization and + * reconciles it on later boots (seederService.js). The admin API never changes it, so + * config stays the single writer of this field. + * + * @returns {string} + */ +function getIdpOrgId() { + return getConfiguredIdpOrgId() || getHandle(); +} + /** * Resolves — and caches — the uuid of this instance's organization. * @@ -180,6 +223,8 @@ async function requirePinnedOrg(identifier) { module.exports = { getHandle, getDisplayName, + getConfiguredIdpOrgId, + getIdpOrgId, getOrgUuid, isPinnedOrg, requirePinnedOrg, From b7acf95d154a89c60764a65acd2532b3bbb6d861 Mon Sep 17 00:00:00 2001 From: Piumal Rathnayake Date: Tue, 4 Aug 2026 14:52:45 +0530 Subject: [PATCH 2/6] Fix multiple km handling issue in app overview --- .../e2e/applications/application-flows.cy.js | 6 +- .../applications/key-managers-multiple.cy.js | 203 ++++++++++++++++++ .../applicationsContentController.js | 7 +- .../partials/keys-instructions.hbs | 5 +- .../pages/application/partials/keys-token.hbs | 28 ++- .../partials/manage-keys-km-card.hbs | 31 +-- .../src/scripts/oauth2-key-generation.js | 82 ++++--- 7 files changed, 308 insertions(+), 54 deletions(-) create mode 100644 portals/api-portal/it/ui/cypress/e2e/applications/key-managers-multiple.cy.js diff --git a/portals/api-portal/it/ui/cypress/e2e/applications/application-flows.cy.js b/portals/api-portal/it/ui/cypress/e2e/applications/application-flows.cy.js index b0add2118..85a857b17 100644 --- a/portals/api-portal/it/ui/cypress/e2e/applications/application-flows.cy.js +++ b/portals/api-portal/it/ui/cypress/e2e/applications/application-flows.cy.js @@ -148,13 +148,15 @@ describe('Applications', () => { // 2. Generate a token with the consumer secret (portal → mock key manager). cy.get(`#tab-btn-token-${KM_ID}-PRODUCTION`).click(); - cy.get('#tokenKeyBtn-PRODUCTION').click(); + // Every id on the Manage Keys page is scoped by key manager as well as key + // type — see key-managers-multiple.cy.js for why. + cy.get(`#tokenKeyBtn-${KM_ID}-PRODUCTION`).click(); cy.get('#generateTokenPromptModal').should('be.visible'); cy.get('#generateTokenPromptSecretInput').type(mockToken.secret); cy.get('#generateTokenPromptConfirmBtn').click(); cy.get(`#token_${KM_ID}_PRODUCTION`, { timeout: 15000 }) .should('contain', mockToken.accessToken); - cy.get('[data-cyid="keysTokenModal-PRODUCTION-close"]').click(); + cy.get(`[data-cyid="keysTokenModal-${KM_ID}-PRODUCTION-close"]`).click(); // 3. Revoke the keys — confirm in the shared delete-confirmation modal. // Scope the revoke button to the Production pane — the Sandbox card diff --git a/portals/api-portal/it/ui/cypress/e2e/applications/key-managers-multiple.cy.js b/portals/api-portal/it/ui/cypress/e2e/applications/key-managers-multiple.cy.js new file mode 100644 index 000000000..2824028eb --- /dev/null +++ b/portals/api-portal/it/ui/cypress/e2e/applications/key-managers-multiple.cy.js @@ -0,0 +1,203 @@ +// -------------------------------------------------------------------- +// Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +// -------------------------------------------------------------------- + +// Manage Keys with MORE THAN ONE key manager in the organization. +// +// manage-keys.hbs renders one KM card per enabled key manager per key type, and +// keys-token.hbs one result modal per the same. Ids used to be keyed on the key type +// alone, so a second key manager produced duplicate ids and every +// document.getElementById in oauth2-key-generation.js resolved to the FIRST-rendered +// key manager. Generating a token from the second card then wrote the token into that +// card's modal but opened the first card's — which reads as "generate access token +// fails" — and spinners, errors and scope chips all landed on the wrong card. With +// only one key manager configured nothing collides, which is why the single-KM spec +// (application-flows.cy.js) never caught it. +// +// Everything here therefore asserts the KM the interaction *came from*, never just +// the key type. + +describe('Applications — multiple key managers', () => { + const APP = 'IT Multi KM App'; + // Alphabetical handles, because the cards render in whatever order the DAO + // returns: 'alpha' being first makes "the second card" unambiguous below. + const KM_A = { id: 'it-km-alpha', displayName: 'IT KM Alpha' }; + const KM_B = { id: 'it-km-beta', displayName: 'IT KM Beta' }; + const CLIENT_ID_A = 'it-client-id-alpha'; + const CLIENT_ID_B = 'it-client-id-beta'; + + let appHandle; + let mockToken; + + before(() => { + cy.login(); + cy.createApplication(APP, 'Used by the multiple-key-manager tests') + .then((handle) => { appHandle = handle; }); + // One mock token endpoint serves both key managers — the portal reaches it + // server-side, and which KM a token came from is established by the card the + // click started in, not by the endpoint. + cy.task('startMockTokenServer').then((mock) => { + mockToken = mock; + cy.seedKeyManager({ ...KM_A, tokenEndpoint: mock.endpoint }); + cy.seedKeyManager({ ...KM_B, tokenEndpoint: mock.endpoint }); + }); + }); + + after(() => { + cy.login(); + cy.request({ + method: 'DELETE', + url: `/api/v0.9/applications/${appHandle}`, + failOnStatusCode: false, + }).its('status').should('be.oneOf', [200, 404]); + cy.deleteKeyManager(KM_A.id); + cy.deleteKeyManager(KM_B.id); + cy.task('stopMockTokenServer'); + }); + + it('renders one card per key manager, with no duplicate element ids', () => { + cy.login(); + cy.visitPortal(`/applications/${appHandle}`); + + cy.get('#production').find('.mk-km-card').should('have.length', 2); + cy.get('.mk-km-name').should('contain', KM_A.displayName); + cy.get('.mk-km-name').should('contain', KM_B.displayName); + + // Each card owns its own generate button and result modal. + cy.get(`#tokenKeyBtn-${KM_A.id}-PRODUCTION`).should('exist'); + cy.get(`#tokenKeyBtn-${KM_B.id}-PRODUCTION`).should('exist'); + cy.get(`#keysTokenModal-${KM_A.id}-PRODUCTION`).should('exist'); + cy.get(`#keysTokenModal-${KM_B.id}-PRODUCTION`).should('exist'); + + // The regression guard proper: any id repeated on this page is a getElementById + // that silently resolves to whichever key manager happens to render first. + cy.document().then((doc) => { + const ids = Array.from(doc.querySelectorAll('[id]')).map((el) => el.id); + const duplicates = [...new Set(ids.filter((id, i) => ids.indexOf(id) !== i))]; + expect(duplicates, 'duplicate element ids on the application page').to.deep.equal([]); + }); + }); + + it('links a separate client ID to each key manager', () => { + cy.login(); + cy.visitPortal(`/applications/${appHandle}`); + + cy.get(`#addClientIdInput-${KM_A.id}-PRODUCTION`).type(CLIENT_ID_A); + cy.get(`#addClientIdBtn-${KM_A.id}-PRODUCTION`).click(); + cy.get(`#consumer-key-${KM_A.id}-PRODUCTION-view`, { timeout: 15000 }) + .should('have.value', CLIENT_ID_A); + + cy.get(`#addClientIdInput-${KM_B.id}-PRODUCTION`).type(CLIENT_ID_B); + cy.get(`#addClientIdBtn-${KM_B.id}-PRODUCTION`).click(); + cy.get(`#consumer-key-${KM_B.id}-PRODUCTION-view`, { timeout: 15000 }) + .should('have.value', CLIENT_ID_B); + + // Each card keeps its own credentials — the first card's client id must not + // have been overwritten by, or copied onto, the second. + cy.get(`#consumer-key-${KM_A.id}-PRODUCTION-view`).should('have.value', CLIENT_ID_A); + // appRefId used to be appKeyMappings[0].asClientId for every card. + cy.get(`#app-ref-${KM_A.id}-PRODUCTION`).should('have.value', CLIENT_ID_A); + cy.get(`#app-ref-${KM_B.id}-PRODUCTION`).should('have.value', CLIENT_ID_B); + // Distinct mappings, so distinct mapping ids. + cy.get(`#key-map-${KM_A.id}-PRODUCTION`).invoke('val').then((mappingA) => { + cy.get(`#key-map-${KM_B.id}-PRODUCTION`).invoke('val').should('not.eq', mappingA); + }); + }); + + it('generates a token from the second key manager into that key manager\'s own modal', () => { + cy.login(); + cy.visitPortal(`/applications/${appHandle}`); + + cy.get(`#tab-btn-token-${KM_B.id}-PRODUCTION`).click(); + cy.get(`#tokenKeyBtn-${KM_B.id}-PRODUCTION`).click(); + cy.get('#generateTokenPromptModal').should('be.visible'); + cy.get('#generateTokenPromptSecretInput').type(mockToken.secret); + cy.get('#generateTokenPromptConfirmBtn').click(); + + // The token lands in the second key manager's modal, and that modal is the one + // shown. Before the fix the token was written here but the FIRST key manager's + // (empty) modal was opened instead. + cy.get(`#keysTokenModal-${KM_B.id}-PRODUCTION`, { timeout: 15000 }) + .should('be.visible'); + cy.get(`#token_${KM_B.id}_PRODUCTION`, { timeout: 15000 }) + .should('contain', mockToken.accessToken); + + // The other key manager's modal stays closed and empty. + cy.get(`#keysTokenModal-${KM_A.id}-PRODUCTION`).should('not.be.visible'); + cy.get(`#token_${KM_A.id}_PRODUCTION`).should('not.contain', mockToken.accessToken); + + // The clicked button returns to its normal state (the spinner used to be + // applied to, and left on, the first card's button). + cy.get(`[data-cyid="keysTokenModal-${KM_B.id}-PRODUCTION-close"]`).click(); + cy.get(`#tokenKeyBtn-${KM_B.id}-PRODUCTION`) + .should('not.be.disabled') + .find('.button-normal-state').should('be.visible'); + }); + + it('generates a token from the first key manager as well', () => { + cy.login(); + cy.visitPortal(`/applications/${appHandle}`); + + cy.get(`#tab-btn-token-${KM_A.id}-PRODUCTION`).click(); + cy.get(`#tokenKeyBtn-${KM_A.id}-PRODUCTION`).click(); + cy.get('#generateTokenPromptModal').should('be.visible'); + cy.get('#generateTokenPromptSecretInput').type(mockToken.secret); + cy.get('#generateTokenPromptConfirmBtn').click(); + + cy.get(`#keysTokenModal-${KM_A.id}-PRODUCTION`, { timeout: 15000 }) + .should('be.visible'); + cy.get(`#token_${KM_A.id}_PRODUCTION`, { timeout: 15000 }) + .should('contain', mockToken.accessToken); + cy.get(`#keysTokenModal-${KM_B.id}-PRODUCTION`).should('not.be.visible'); + cy.get(`[data-cyid="keysTokenModal-${KM_A.id}-PRODUCTION-close"]`).click(); + }); + + it('shows a token failure in the card it came from, not the first one', () => { + cy.login(); + cy.visitPortal(`/applications/${appHandle}`); + + // The mock rejects any secret but its own, so this is a genuine upstream 401 + // surfaced through the portal. + cy.get(`#tab-btn-token-${KM_B.id}-PRODUCTION`).click(); + cy.get(`#tokenKeyBtn-${KM_B.id}-PRODUCTION`).click(); + cy.get('#generateTokenPromptSecretInput').type('it-wrong-secret'); + cy.get('#generateTokenPromptConfirmBtn').click(); + + cy.get(`#keyGenerationErrorContainer-${KM_B.id}-PRODUCTION`, { timeout: 15000 }) + .should('be.visible') + .and('contain', 'Failed to generate access token'); + cy.get(`#keyGenerationErrorContainer-${KM_A.id}-PRODUCTION`).should('not.be.visible'); + cy.get(`#keysTokenModal-${KM_B.id}-PRODUCTION`).should('not.be.visible'); + }); + + it('revokes one key manager\'s keys without touching the other\'s', () => { + cy.login(); + cy.visitPortal(`/applications/${appHandle}`); + + cy.get(`#tab-btn-creds-${KM_B.id}-PRODUCTION`).click(); + // Scope the revoke click to the second card — every card renders its own. + cy.get(`#keyActionsContainer-${KM_B.id}-PRODUCTION`).find('.mk-btn-danger').click(); + cy.get('#deleteConfirmation').should('be.visible'); + cy.get('#deleteConfirmationBtn').click(); + + // The second key manager is back to the empty state; the first still holds its + // own client id (the unscoped fallback used to delete the first card's mapping). + cy.get(`#addClientIdBtn-${KM_B.id}-PRODUCTION`, { timeout: 15000 }).should('exist'); + cy.get(`#consumer-key-${KM_B.id}-PRODUCTION-view`).should('have.value', ''); + cy.get(`#consumer-key-${KM_A.id}-PRODUCTION-view`).should('have.value', CLIENT_ID_A); + }); +}); diff --git a/portals/api-portal/src/controllers/applicationsContentController.js b/portals/api-portal/src/controllers/applicationsContentController.js index 98c87dc07..6aaf08bdd 100644 --- a/portals/api-portal/src/controllers/applicationsContentController.js +++ b/portals/api-portal/src/controllers/applicationsContentController.js @@ -69,10 +69,8 @@ const loadApplicationData = async (req, orgName, applicationHandle, viewName) => const applicationId = appRecord.uuid; const applicationList = await adminService.getApplicationKeyMap(orgId, applicationId, userId); - let applicationReference = ""; let applicationKeyList; if (Array.isArray(applicationList.appKeyMappings) && applicationList.appKeyMappings.length > 0) { - applicationReference = applicationList.appKeyMappings[0].asClientId; try { const localMappings = await appDao.getKeyMappings(orgId, applicationId); const keyList = []; @@ -127,7 +125,10 @@ const loadApplicationData = async (req, orgName, applicationHandle, viewName) => consumerKey: key.consumerKey, keyMappingId: key.keyMappingId, keyType: key.keyType, - appRefId: applicationReference + // This key set's own client id. Previously appKeyMappings[0].asClientId, + // which stamped the first key manager's client id onto every key manager's + // card once an organization had more than one. + appRefId: key.consumerKey }; if (key.keyType === constants.KEY_TYPE.PRODUCTION) { productionKeys.push(keyData); diff --git a/portals/api-portal/src/pages/application/partials/keys-instructions.hbs b/portals/api-portal/src/pages/application/partials/keys-instructions.hbs index fe20aaa64..b8b42dab8 100644 --- a/portals/api-portal/src/pages/application/partials/keys-instructions.hbs +++ b/portals/api-portal/src/pages/application/partials/keys-instructions.hbs @@ -1,13 +1,14 @@ {{#each keyManagersMetadata}} {{#if enabled}} {{#each applicationKeys}} -