Skip to content

Conversation

@NipuniBhagya
Copy link
Contributor

@NipuniBhagya NipuniBhagya commented Nov 25, 2025

This pull request introduces new API resource collections and access control definitions for user credential management, both at the tenant and organization levels. The changes add definitions for these APIs, specify their scopes, and update access control rules to ensure proper authorization for viewing, updating, and deleting user credentials.

API Resource Definitions

  • Added user_credentials and org_user_credentials collections to api-resource-collection.xml and its template, specifying relevant scopes for feature, update, delete, create, and read actions. [1] [2]

  • Defined new API resources for user credential management in system-api-resource.xml and its template, including endpoints and authorization requirements for both tenant and organization contexts. [1] [2]

Access Control Updates

  • Updated resource-access-control-v2.xml and its template to add access control entries for the new user credential management APIs, specifying scope requirements for GET and DELETE methods for both tenant and organization endpoints. [1] [2]### Proposed changes in this pull request

[List all changes you want to add here. If you fixed an issue, please
add a reference to that issue as well.]

When should this PR be merged

[Please describe any preconditions that need to be addressed before we
can merge this pull request.]

Follow up actions

[List any possible follow-up actions here; for instance, testing data
migrations, software that we need to install on staging and production
environments.]

Developer Checklist (Mandatory)

  • Complete the Developer Checklist in the related product-is issue to track any behavioral change or migration impact.

Checklist (for reviewing)

General

  • Is this PR explained thoroughly? All code changes must be accounted for in the PR description.
  • Is the PR labeled correctly?

Functionality

  • Are all requirements met? Compare implemented functionality with the requirements specification.
  • Does the UI work as expected? There should be no Javascript errors in the console; all resources should load. There should be no unexpected errors. Deliberately try to break the feature to find out if there are corner cases that are not handled.

Code

  • Do you fully understand the introduced changes to the code? If not ask for clarification, it might uncover ways to solve a problem in a more elegant and efficient way.
  • Does the PR introduce any inefficient database requests? Use the debug server to check for duplicate requests.
  • Are all necessary strings marked for translation? All strings that are exposed to users via the UI must be marked for translation.

Tests

  • Are there sufficient test cases? Ensure that all components are tested individually; models, forms, and serializers should be tested in isolation even if a test for a view covers these components.
  • If this is a bug fix, are tests for the issue in place? There must be a test case for the bug to ensure the issue won’t regress. Make sure that the tests break without the new code to fix the issue.
  • If this is a new feature or a significant change to an existing feature? has the manual testing spreadsheet been updated with instructions for manual testing?

Security

  • Confirm this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.
  • Are all UI and API inputs run through forms or serializers?
  • Are all external inputs validated and sanitized appropriately?
  • Does all branching logic have a default case?
  • Does this solution handle outliers and edge cases gracefully?
  • Are all external communications secured and restricted to SSL?

Documentation

  • Are changes to the UI documented in the platform docs? If this PR introduces new platform site functionality or changes existing ones, the changes should be documented.
  • Are changes to the API documented in the API docs? If this PR introduces new API functionality or changes existing ones, the changes must be documented.
  • Are reusable components documented? If this PR introduces components that are relevant to other developers (for instance a mixin for a view or a generic form) they should be documented in the Wiki.

Summary by CodeRabbit

  • New Features
    • Added credential management API endpoints for retrieving and deleting user credentials with enhanced authorization controls.
    • Extended credential management operations to support both tenant and organization-level contexts with role-based access scopes.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings November 25, 2025 16:59
@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

Walkthrough

Two templated XML configuration files are updated to introduce new Credential Management API resources. These additions define credentials endpoints for both organization-scoped and tenant-scoped contexts, each paired with appropriate authorization scopes and access control rules.

Changes

Cohort / File(s) Summary
API Resource Definitions
features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml.j2
Added two new APIResource entries for credentials management: tenant-scoped (/api/server/v1/users/(.*)/credentials) and organization-scoped (/o/api/server/v1/users/(.*)/credentials) with corresponding internal management scopes.
Access Control Rules
features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/resource-access-control-v2.xml.j2
Added four new credential endpoint rules: GET and DELETE methods for both organization (/o/api/server/v1/users/(.*)/credentials) and non-organization (/api/server/v1/users/(.*)/credentials) contexts with view and delete scopes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify consistency of API endpoint patterns and regex expressions across both configuration files
  • Validate scope naming convention alignment (internal_user_mgt_* vs. internal_org_user_mgt_*)
  • Confirm authorization flags and metadata are correctly specified in API resource definitions
  • Check that organization and tenant-scoped resource separation is properly maintained

Poem

🐰 With whiskers twitched and paws so quick,
New credentials flow through API tricks—
Scopes aligned in organization's way,
And tenant realms now have their say.
Management magic, secured just right,
The rabbit hops into the night! 🌙

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers API resource definitions, access control updates, and references relevant diff hunks, but lacks required template sections like Purpose/Goals, Release notes, Security checks, and Test environment details. Complete all mandatory template sections including Purpose, Goals, Release note, Security checks confirmation (secure coding, FindSecurityBugs, secrets verification), and Test environment details.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding end-user credential management API resources at both tenant and organization levels.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b472d5 and 42af47d.

📒 Files selected for processing (2)
  • features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml.j2 (1 hunks)
  • features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/resource-access-control-v2.xml.j2 (1 hunks)
🔇 Additional comments (2)
features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml.j2 (1)

2166-2185: Credential Management APIResource definitions are consistent and aligned

The new tenant- and org-scoped Credential Management API resources use the expected identifiers, types (TENANT / ORGANIZATION), and reuse the existing internal_user_mgt_view/delete and internal_org_user_mgt_view/delete scopes in a way that matches surrounding user-management APIs. I don’t see any blocking issues here.

features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/resource-access-control-v2.xml.j2 (1)

2064-2078: Access-control rules for credential APIs correctly mirror the system API definitions

The new org and tenant credential endpoints use path patterns consistent with the APIResource identifiers, and map GET to the respective *_user_mgt_view scopes and DELETE to *_user_mgt_delete, matching how user-management is scoped elsewhere. Placement before the wildcard /api/server/v1/... rules also looks correct. No changes needed from my side.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

@sonarqubecloud
Copy link

Copilot finished reviewing on behalf of NipuniBhagya November 25, 2025 17:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds API resource definitions and access control rules for end-user credential management at both tenant and organization levels. The changes enable proper authorization for viewing and deleting user credentials through new REST API endpoints.

Key Changes:

  • Added two new API resources for credential management (tenant and organization contexts) with view and delete scopes
  • Configured access control rules with regex patterns to secure GET and DELETE operations on credential endpoints
  • Reused existing internal_user_mgt_* and internal_org_user_mgt_* scopes for consistency with SCIM2 user management APIs

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml.j2 Defines API resources for credential management with view and delete scopes for both tenant and organization contexts
features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/resource-access-control-v2.xml.j2 Adds access control entries to secure credential endpoints with appropriate scope requirements for GET and DELETE methods

Comment on lines +2167 to +2168
requiresAuthorization="true"
description="API representation of the Credential Management API" type="TENANT">
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

Inconsistent indentation. The requiresAuthorization attribute should align with the name attribute on the previous line, similar to other APIResource elements in this file (e.g., lines 2087, 2101, 2115).

The line currently has excessive spacing (29 spaces) instead of the standard 17 spaces used throughout the file.

Suggested change
requiresAuthorization="true"
description="API representation of the Credential Management API" type="TENANT">
requiresAuthorization="true"
description="API representation of the Credential Management API" type="TENANT">

Copilot uses AI. Check for mistakes.
Comment on lines +2171 to +2173
description="View credentials of users in the organization"/>
<Scope displayName="Delete Credential" name="internal_user_mgt_delete"
description="Delete credentials of users in the organization"/>
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The scope description should follow the established pattern for TENANT type APIs. Based on similar APIs in this file (e.g., SCIM2 Users API at lines 772, 774, 776, 778, 780), TENANT type scope descriptions use "in the organization (root)" instead of just "in the organization". This helps distinguish between tenant-level and organization-level scopes.

Suggested: description="View credentials of users in the organization (root)"

Suggested change
description="View credentials of users in the organization"/>
<Scope displayName="Delete Credential" name="internal_user_mgt_delete"
description="Delete credentials of users in the organization"/>
description="View credentials of users in the organization (root)"/>
<Scope displayName="Delete Credential" name="internal_user_mgt_delete"
description="Delete credentials of users in the organization (root)"/>

Copilot uses AI. Check for mistakes.
Comment on lines +2171 to +2173
description="View credentials of users in the organization"/>
<Scope displayName="Delete Credential" name="internal_user_mgt_delete"
description="Delete credentials of users in the organization"/>
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The scope description should follow the established pattern for TENANT type APIs. Based on similar APIs in this file (e.g., SCIM2 Users API at lines 776, 778, 780), TENANT type scope descriptions use "in the organization (root)" instead of just "in the organization". This helps distinguish between tenant-level and organization-level scopes.

Suggested: description="Delete credentials of users in the organization (root)"

Suggested change
description="View credentials of users in the organization"/>
<Scope displayName="Delete Credential" name="internal_user_mgt_delete"
description="Delete credentials of users in the organization"/>
description="View credentials of users in the organization (root)"/>
<Scope displayName="Delete Credential" name="internal_user_mgt_delete"
description="Delete credentials of users in the organization (root)"/>

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.46%. Comparing base (66b9294) to head (42af47d).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #7633      +/-   ##
============================================
+ Coverage     50.02%   50.46%   +0.44%     
+ Complexity    19784    18903     -881     
============================================
  Files          2121     2121              
  Lines        129971   127831    -2140     
  Branches      27031    26494     -537     
============================================
- Hits          65015    64514     -501     
+ Misses        56633    55084    -1549     
+ Partials       8323     8233      -90     
Flag Coverage Δ
unit 35.60% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

1 participant