Implement key manager config UI#2493
Conversation
📝 WalkthroughWalkthroughKey manager type handling is standardized to server-fixed ChangesKey manager type standardization
Estimated code review effort: 3 (Moderate) | ~30 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
| logger.warn('Failed to load key managers for settings page', { error: err.message }); | ||
| } | ||
| templateContent.keyManagers = keyManagers; | ||
| templateContent.keyManagerTypes = ['ASGARDEO', 'WSO2IS', 'KEYCLOAK', 'GENERIC_OIDC']; |
There was a problem hiding this comment.
Do we need to special case Asgardeo and WSO2IS over GENERIC_OIDC?
There was a problem hiding this comment.
Currently we don't have any vendor-specific key manager configurations in the devportal. So we can use GENERIC_OIDC as the default and remove other types.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
portals/developer-portal/docs/administer/key-manager-integration.md (1)
24-45: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExample needs
spec.displayName;metadata.nameonly sets the handle.
This manifest will fail withMissing required fields: displayNameunlessspec.displayName: Asgardeois added. Also update themetadata.namerow — it describes the handle, not the developer-facing display name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/docs/administer/key-manager-integration.md` around lines 24 - 45, Update the key manager manifest example to include the required spec.displayName field, using Asgardeo as its value. Revise the metadata.name table description to identify it as the unique handle, and document spec.displayName as the developer-facing display name.
🧹 Nitpick comments (1)
portals/developer-portal/it/rest-api/key-managers/key-managers.spec.js (1)
59-61: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding an update-path counterpart to the "ignores client-supplied type" test.
The new test at lines 91-99 covers
POSTforcingGENERIC_OIDC, butupdateKeyManager'sdelete payload.type(keyManagerService.js lines 184-185) has no dedicated test asserting aPUTwith a client-suppliedtypestill returnsGENERIC_OIDCunchanged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/developer-portal/it/rest-api/key-managers/key-managers.spec.js` around lines 59 - 61, Extend the key-manager test suite with a PUT/update counterpart to the existing POST test that supplies a client-provided type and asserts the response remains GENERIC_OIDC. Target the updateKeyManager path and verify its delete payload.type behavior without changing the existing creation test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@portals/developer-portal/src/services/keyManagerService.js`:
- Around line 144-151: Update the handle validation in the create flow around
hadExplicitHandle, resolvedHandle, and kmDao.create to validate resolvedHandle
unconditionally after trimming or generating it. Reject empty or
HANDLE_PATTERN-invalid generated handles with the existing 400 response before
calling kmDao.create, while preserving the current behavior for valid explicit
and generated handles.
---
Outside diff comments:
In `@portals/developer-portal/docs/administer/key-manager-integration.md`:
- Around line 24-45: Update the key manager manifest example to include the
required spec.displayName field, using Asgardeo as its value. Revise the
metadata.name table description to identify it as the unique handle, and
document spec.displayName as the developer-facing display name.
---
Nitpick comments:
In `@portals/developer-portal/it/rest-api/key-managers/key-managers.spec.js`:
- Around line 59-61: Extend the key-manager test suite with a PUT/update
counterpart to the existing POST test that supplies a client-provided type and
asserts the response remains GENERIC_OIDC. Target the updateKeyManager path and
verify its delete payload.type behavior without changing the existing creation
test.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 04f4ffbc-6e1e-4257-a8cb-687c5f47f90a
📒 Files selected for processing (12)
docs/rest-apis/devportal/key-managers.mddocs/rest-apis/devportal/schemas.mdportals/developer-portal/docs/administer/key-manager-integration.mdportals/developer-portal/docs/devportal-openapi-spec-v0.9.yamlportals/developer-portal/it/rest-api/key-managers/key-managers.spec.jsportals/developer-portal/src/controllers/applicationsContentController.jsportals/developer-portal/src/controllers/viewConfigureController.jsportals/developer-portal/src/pages/application/partials/manage-keys-km-card.hbsportals/developer-portal/src/pages/application/partials/manage-keys.hbsportals/developer-portal/src/pages/settings/page.hbsportals/developer-portal/src/services/keyManagerService.jsportals/developer-portal/src/styles/manage-keys.css
💤 Files with no reviewable changes (5)
- portals/developer-portal/src/styles/manage-keys.css
- portals/developer-portal/src/pages/application/partials/manage-keys-km-card.hbs
- portals/developer-portal/src/pages/application/partials/manage-keys.hbs
- portals/developer-portal/src/controllers/applicationsContentController.js
- portals/developer-portal/src/controllers/viewConfigureController.js
2 participants
Purpose
Fix #2527
Approach
Implemented the key manager config UI
Samples
Related PRs
N/A