Recursively resolve holder feature action scopes to internal_ scopes#8206
Closed
mpmadhavig wants to merge 2 commits into
Closed
Recursively resolve holder feature action scopes to internal_ scopes#8206mpmadhavig wants to merge 2 commits into
mpmadhavig wants to merge 2 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughChangesThe change adds configurable granular console permissions with separate create, update, and delete scope buckets. Collection models and parsers support action-specific scopes and holder resolution, runtime APIs populate granular resources, and tenant, organization, system, template, and compatibility configurations expose the new scopes. Granular Console Permissions
🚥 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/api-resource-collection.xml (1)
804-807: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMirror the holder-scope additions in the static XML
These 8 blocks still only reference
*_edit, whileapi-resource-collection.xml.j2also adds the matching*_create/*_update/*_deleteholders. KeepapplicationAuthenticationScript,applicationClientSecretManagement,applicationInternalAPIAuthorization,userSharingV2, and theirorg_*counterparts aligned with the template so permission resolution stays consistent.🤖 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 `@features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/api-resource-collection.xml` around lines 804 - 807, Update the static XML blocks for applicationAuthenticationScript, applicationClientSecretManagement, applicationInternalAPIAuthorization, userSharingV2, and their org_* counterparts to mirror the template’s matching *_create, *_update, and *_delete holder scopes alongside the existing *_edit scopes. Keep the static api-resource collection aligned with api-resource-collection.xml.j2.
🧹 Nitpick comments (1)
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java (1)
298-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct coverage for cyclic holder references.
The implementation explicitly short-circuits cycles, but this suite only tests acyclic nesting. Add two mutually referencing holders and verify termination, removal of holder names, and preservation of non-cyclic leaves.
🤖 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 `@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java` around lines 298 - 313, Extend testHolderResolutionRecursesThroughNestedHolders or add a focused holder-resolution test that defines two holders referencing each other and includes a non-cyclic leaf scope. Resolve a collection using that cycle, assert resolution terminates, verify both holder names are absent from the resulting scopes, and assert the unrelated leaf scope is preserved.
🤖 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
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionManagementUtil.java`:
- Around line 73-76: Normalize the property value in
isGranularConsolePermissionsEnabled by trimming the non-null result from
IdentityUtil.getProperty before passing it to Boolean.parseBoolean. In
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionManagementUtil.java
lines 73-76, apply this change while preserving null handling. In
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagementUtilTest.java
lines 40-47, extend the data provider with whitespace-padded true and false
values.
- Around line 68-72: Complete the public method documentation for the granular
console permission check by removing the dangling “Controlled” wording and
naming the configuration property that controls the feature. Ensure the summary
and return description are grammatically complete and end with periods.
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionMgtConfigBuilder.java`:
- Around line 174-176: Update the holder-scope resolution branches in
APIResourceCollectionMgtConfigBuilder, including the readScopeSet handling and
the corresponding branches around lines 181-185, 191-195, 201-205, and 375-381,
so missing owners and cyclic holder scopes are not silently converted to empty
sets. Emit a WARN containing only the safe scope identifier at each unresolved
or cycle decision point, or reject the invalid configuration, while preserving
normal resolution behavior.
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagerTest.java`:
- Around line 248-261: Update the APIResourceCollection fixture builder in
APIResourceCollectionManagerTest so createScopes, updateScopes, and deleteScopes
contain only their respective action and feature scopes, removing the shared
read scopes from each list; keep readScopes populated separately with all read
scopes.
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java`:
- Line 146: Update the comment at
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java:146
to capitalize “Write” while preserving the existing wording and period. Rewrite
the comment at
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagementUtilTest.java:39
as a capitalized sentence ending with a period, removing the brace notation.
- Line 2: Update the Java license header year in
APIResourceCollectionMgtConfigBuilderTest to 2026, preserving the existing
copyright holder and header format.
In
`@features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml.j2`:
- Around line 2068-2073: Update the descriptions for the tenant-level
console:userSharing_create and console:userSharing_delete Scope entries in the
user sharing management block to use distinct Create and Delete wording,
matching the corresponding org-level and system-api-resource entries; leave
console:userSharing_update unchanged.
---
Outside diff comments:
In
`@features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/api-resource-collection.xml`:
- Around line 804-807: Update the static XML blocks for
applicationAuthenticationScript, applicationClientSecretManagement,
applicationInternalAPIAuthorization, userSharingV2, and their org_* counterparts
to mirror the template’s matching *_create, *_update, and *_delete holder scopes
alongside the existing *_edit scopes. Keep the static api-resource collection
aligned with api-resource-collection.xml.j2.
---
Nitpick comments:
In
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java`:
- Around line 298-313: Extend testHolderResolutionRecursesThroughNestedHolders
or add a focused holder-resolution test that defines two holders referencing
each other and includes a non-cyclic leaf scope. Resolve a collection using that
cycle, assert resolution terminates, verify both holder names are absent from
the resulting scopes, and assert the unrelated leaf scope is preserved.
🪄 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.yml
Review profile: CHILL
Plan: Pro
Run ID: dd58ec52-12b4-43ba-b5fa-446589950b37
📒 Files selected for processing (20)
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/pom.xmlcomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagerImpl.javacomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/constant/APIResourceCollectionManagementConstants.javacomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/model/APIResourceCollection.javacomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionManagementUtil.javacomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionMgtConfigBuilder.javacomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagementUtilTest.javacomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagerTest.javacomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.javacomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionTest.javacomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/resources/repository/conf/api-resource-collection.xmlcomponents/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/resources/testng.xmlfeatures/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/api-resource-collection.xmlfeatures/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/api-resource-collection.xml.j2features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xmlfeatures/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml.j2features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xmlfeatures/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/org.wso2.carbon.identity.core.server.feature.default.jsonfeatures/identity-core/org.wso2.carbon.identity.core.server.feature/resources/org.wso2.carbon.identity.core.server.feature.infer.json
Comment on lines
+68
to
+72
| /** | ||
| * Check whether the granular console permission model (create/update/delete feature scopes) is enabled. Controlled | ||
| * | ||
| * @return True if granular console permissions are enabled. | ||
| */ |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the public method documentation.
The summary ends with the dangling word “Controlled” and does not identify the controlling property.
Proposed fix
- * Check whether the granular console permission model (create/update/delete feature scopes) is enabled. Controlled
+ * Check whether the granular console permission model (create/update/delete feature scopes) is enabled.
+ * Controlled by {`@code` USE_GRANULAR_CONSOLE_PERMISSIONS_CONFIG}.As per coding guidelines, “All public methods should have a docstring” and comments should end with a period.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * Check whether the granular console permission model (create/update/delete feature scopes) is enabled. Controlled | |
| * | |
| * @return True if granular console permissions are enabled. | |
| */ | |
| /** | |
| * Check whether the granular console permission model (create/update/delete feature scopes) is enabled. | |
| * Controlled by {`@code` USE_GRANULAR_CONSOLE_PERMISSIONS_CONFIG}. | |
| * | |
| * `@return` True if granular console permissions are enabled. | |
| */ |
🤖 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
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionManagementUtil.java`
around lines 68 - 72, Complete the public method documentation for the granular
console permission check by removing the dangling “Controlled” wording and
naming the configuration property that controls the feature. Ensure the summary
and return description are grammatically complete and end with periods.
Source: Coding guidelines
Comment on lines
+73
to
+76
| public static boolean isGranularConsolePermissionsEnabled() { | ||
|
|
||
| return Boolean.parseBoolean(IdentityUtil.getProperty( | ||
| APIResourceCollectionManagementConstants.USE_GRANULAR_CONSOLE_PERMISSIONS_CONFIG)); |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Normalize whitespace when reading the granular-permissions flag. Surrounding whitespace currently changes a valid true value into false, disabling the granular authorization model.
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionManagementUtil.java#L73-L76: trim the non-null property before callingBoolean.parseBoolean.components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagementUtilTest.java#L40-L47: add whitespace-padded true and false values to the data provider.
📍 Affects 2 files
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionManagementUtil.java#L73-L76(this comment)components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagementUtilTest.java#L40-L47
🤖 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
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionManagementUtil.java`
around lines 73 - 76, Normalize the property value in
isGranularConsolePermissionsEnabled by trimming the non-null result from
IdentityUtil.getProperty before passing it to Boolean.parseBoolean. In
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionManagementUtil.java
lines 73-76, apply this change while preserving null handling. In
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagementUtilTest.java
lines 40-47, extend the data provider with whitespace-padded true and false
values.
Comment on lines
+174
to
+176
| if (isHolderScope(scopeName)) { | ||
| readScopeSet.addAll(holderResolutionMap.getOrDefault( | ||
| scopeName, Collections.emptySet())); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Report unresolved and cyclic holder scopes instead of silently dropping them.
These branches convert missing owners and cycles into empty sets, silently removing permissions from collection buckets. Emit a WARN with the safe scope identifier, or reject the invalid configuration, so incomplete authorization mappings are diagnosable.
As per path instructions, logs are required around meaningful decision points and exceptional-but-handled scenarios.
Also applies to: 181-185, 191-195, 201-205, 375-381
🤖 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
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/main/java/org/wso2/carbon/identity/api/resource/collection/mgt/util/APIResourceCollectionMgtConfigBuilder.java`
around lines 174 - 176, Update the holder-scope resolution branches in
APIResourceCollectionMgtConfigBuilder, including the readScopeSet handling and
the corresponding branches around lines 181-185, 191-195, 201-205, and 375-381,
so missing owners and cyclic holder scopes are not silently converted to empty
sets. Emit a WARN containing only the safe scope identifier at each unresolved
or cycle decision point, or reject the invalid configuration, while preserving
normal resolution behavior.
Source: Path instructions
Comment on lines
+248
to
+261
| APIResourceCollection collection = new APIResourceCollection.APIResourceCollectionBuilder() | ||
| .id(getEncodedName(name)) | ||
| .name(name) | ||
| .displayName("Display Name " + name) | ||
| .type("tenant") | ||
| .readScopes(Arrays.asList(READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | ||
| .writeScopes(Arrays.asList(CREATE_NORMAL_SCOPE, UPDATE_NORMAL_SCOPE, DELETE_NORMAL_SCOPE, | ||
| EDIT_FEATURE_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | ||
| .createScopes(Arrays.asList(CREATE_NORMAL_SCOPE, CREATE_FEATURE_SCOPE, | ||
| READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | ||
| .updateScopes(Arrays.asList(UPDATE_NORMAL_SCOPE, UPDATE_FEATURE_SCOPE, | ||
| READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | ||
| .deleteScopes(Arrays.asList(DELETE_NORMAL_SCOPE, DELETE_FEATURE_SCOPE, | ||
| READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not pre-merge read scopes in the test fixture.
The create/update/delete lists already contain every read scope, so this test still passes if the manager stops merging read scopes into granular buckets. Populate each action list only with its own action and feature scopes.
Proposed fix
- .writeScopes(Arrays.asList(CREATE_NORMAL_SCOPE, UPDATE_NORMAL_SCOPE, DELETE_NORMAL_SCOPE,
- EDIT_FEATURE_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE))
- .createScopes(Arrays.asList(CREATE_NORMAL_SCOPE, CREATE_FEATURE_SCOPE,
- READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE))
- .updateScopes(Arrays.asList(UPDATE_NORMAL_SCOPE, UPDATE_FEATURE_SCOPE,
- READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE))
- .deleteScopes(Arrays.asList(DELETE_NORMAL_SCOPE, DELETE_FEATURE_SCOPE,
- READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE))
+ .writeScopes(Arrays.asList(CREATE_NORMAL_SCOPE, UPDATE_NORMAL_SCOPE,
+ DELETE_NORMAL_SCOPE, EDIT_FEATURE_SCOPE))
+ .createScopes(Arrays.asList(CREATE_NORMAL_SCOPE, CREATE_FEATURE_SCOPE))
+ .updateScopes(Arrays.asList(UPDATE_NORMAL_SCOPE, UPDATE_FEATURE_SCOPE))
+ .deleteScopes(Arrays.asList(DELETE_NORMAL_SCOPE, DELETE_FEATURE_SCOPE))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| APIResourceCollection collection = new APIResourceCollection.APIResourceCollectionBuilder() | |
| .id(getEncodedName(name)) | |
| .name(name) | |
| .displayName("Display Name " + name) | |
| .type("tenant") | |
| .readScopes(Arrays.asList(READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | |
| .writeScopes(Arrays.asList(CREATE_NORMAL_SCOPE, UPDATE_NORMAL_SCOPE, DELETE_NORMAL_SCOPE, | |
| EDIT_FEATURE_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | |
| .createScopes(Arrays.asList(CREATE_NORMAL_SCOPE, CREATE_FEATURE_SCOPE, | |
| READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | |
| .updateScopes(Arrays.asList(UPDATE_NORMAL_SCOPE, UPDATE_FEATURE_SCOPE, | |
| READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | |
| .deleteScopes(Arrays.asList(DELETE_NORMAL_SCOPE, DELETE_FEATURE_SCOPE, | |
| READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | |
| APIResourceCollection collection = new APIResourceCollection.APIResourceCollectionBuilder() | |
| .id(getEncodedName(name)) | |
| .name(name) | |
| .displayName("Display Name " + name) | |
| .type("tenant") | |
| .readScopes(Arrays.asList(READ_NORMAL_SCOPE, VIEW_FEATURE_SCOPE, FEATURE_SCOPE)) | |
| .writeScopes(Arrays.asList(CREATE_NORMAL_SCOPE, UPDATE_NORMAL_SCOPE, | |
| DELETE_NORMAL_SCOPE, EDIT_FEATURE_SCOPE)) | |
| .createScopes(Arrays.asList(CREATE_NORMAL_SCOPE, CREATE_FEATURE_SCOPE)) | |
| .updateScopes(Arrays.asList(UPDATE_NORMAL_SCOPE, UPDATE_FEATURE_SCOPE)) | |
| .deleteScopes(Arrays.asList(DELETE_NORMAL_SCOPE, DELETE_FEATURE_SCOPE)) |
🤖 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
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagerTest.java`
around lines 248 - 261, Update the APIResourceCollection fixture builder in
APIResourceCollectionManagerTest so createScopes, updateScopes, and deleteScopes
contain only their respective action and feature scopes, removing the shared
read scopes from each list; keep readScopes populated separately with all read
scopes.
| @@ -1,34 +1,362 @@ | |||
| /* | |||
| * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com). | |||
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Update the license year to 2026.
Proposed fix
- * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com).
+ * Copyright (c) 2025-2026, WSO2 LLC. (http://www.wso2.com).As per coding guidelines, Java license headers must use the current year or a range ending in the current year.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| * Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com). | |
| * Copyright (c) 2025-2026, WSO2 LLC. (http://www.wso2.com). |
🤖 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
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java`
at line 2, Update the Java license header year in
APIResourceCollectionMgtConfigBuilderTest to 2026, preserving the existing
copyright holder and header format.
Source: Coding guidelines
|
|
||
| APIResourceCollection collection = getApiResourcesCollection(); | ||
|
|
||
| // write feature + write normal scopes. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Bring changed Java comments into the repository convention.
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java#L146-L146: capitalize “Write”.components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagementUtilTest.java#L39-L39: rewrite the brace notation as a capitalized sentence ending with a period.
As per coding guidelines, comments must begin with a capitalized first letter and end with a period.
📍 Affects 2 files
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java#L146-L146(this comment)components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagementUtilTest.java#L39-L39
🤖 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
`@components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java`
at line 146, Update the comment at
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionMgtConfigBuilderTest.java:146
to capitalize “Write” while preserving the existing wording and period. Rewrite
the comment at
components/api-resource-mgt/org.wso2.carbon.identity.api.resource.collection.mgt/src/test/java/org/wso2/carbon/identity/api/resource/collection/mgt/APIResourceCollectionManagementUtilTest.java:39
as a capitalized sentence ending with a period, removing the brace notation.
Source: Coding guidelines
Comment on lines
+2068
to
+2073
| <Scope displayName="User Sharing Management Update Feature" name="console:userSharing_update" | ||
| description="Update user sharing status from the Console"/> | ||
| <Scope displayName="User Sharing Management Create Feature" name="console:userSharing_create" | ||
| description="Update user sharing status from the Console"/> | ||
| <Scope displayName="User Sharing Management Delete Feature" name="console:userSharing_delete" | ||
| description="Update user sharing status from the Console"/> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Copy-pasted description for console:userSharing_create/_delete — both say "Update...".
The Update, Create, and Delete Feature scopes for console:userSharing all share the same description text ("Update user sharing status from the Console"). The org-level counterpart (console:org:userSharing, lines 2090-2095) and the equivalent entries in system-api-resource.xml correctly use distinct Create/Update/Delete wording — only this tenant-level block has the mismatch.
💚 Proposed fix
<Scope displayName="User Sharing Management Update Feature" name="console:userSharing_update"
description="Update user sharing status from the Console"/>
<Scope displayName="User Sharing Management Create Feature" name="console:userSharing_create"
- description="Update user sharing status from the Console"/>
+ description="Create user sharing status from the Console"/>
<Scope displayName="User Sharing Management Delete Feature" name="console:userSharing_delete"
- description="Update user sharing status from the Console"/>
+ description="Delete user sharing status from the Console"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Scope displayName="User Sharing Management Update Feature" name="console:userSharing_update" | |
| description="Update user sharing status from the Console"/> | |
| <Scope displayName="User Sharing Management Create Feature" name="console:userSharing_create" | |
| description="Update user sharing status from the Console"/> | |
| <Scope displayName="User Sharing Management Delete Feature" name="console:userSharing_delete" | |
| description="Update user sharing status from the Console"/> | |
| <Scope displayName="User Sharing Management Update Feature" name="console:userSharing_update" | |
| description="Update user sharing status from the Console"/> | |
| <Scope displayName="User Sharing Management Create Feature" name="console:userSharing_create" | |
| description="Create user sharing status from the Console"/> | |
| <Scope displayName="User Sharing Management Delete Feature" name="console:userSharing_delete" | |
| description="Delete user sharing status from the Console"/> |
🤖 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
`@features/api-resource-mgt/org.wso2.carbon.identity.api.resource.mgt.server.feature/resources/system-api-resource.xml.j2`
around lines 2068 - 2073, Update the descriptions for the tenant-level
console:userSharing_create and console:userSharing_delete Scope entries in the
user sharing management block to use distinct Create and Delete wording,
matching the corresponding org-level and system-api-resource entries; leave
console:userSharing_update unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Stacked on top of #8178. Until that merges, this PR's diff includes the granular-scope-feature commit as a base; after it merges, only the nested-scope resolution commit remains.
When a console feature action scope (e.g. `console:applications_edit`, `console:applications_create`) appears inside another collection's `` / `` / `` / `` block, treat it as a holder and substitute it with the leaf scopes from the matching action block of the owning collection. The holder name itself is dropped from the bucket so the `/api-resource-collections/{id}` response surfaces the underlying internal scopes (and their owning API resource) instead of an opaque console scope that no API resource owns.
Changes
Related PRs
Depends on #8178.