Add granular console scopes integration tests - #28189
Conversation
📝 WalkthroughSummary
WalkthroughAdds Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java (1)
117-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
Files.createTempFileand ensure deletion.For better safety and to ensure cleanup, use
Files.createTempFileand register it for deletion on exit. As per path instructions, this focuses on correctness and best practices.♻️ Proposed fix
- File enabledTomlFile = File.createTempFile("console-granular-enabled", ".toml"); - Files.write(enabledTomlFile.toPath(), content.getBytes(StandardCharsets.UTF_8)); + File enabledTomlFile = Files.createTempFile("console-granular-enabled", ".toml").toFile(); + enabledTomlFile.deleteOnExit(); + Files.write(enabledTomlFile.toPath(), content.getBytes(StandardCharsets.UTF_8));🤖 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 `@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java` around lines 117 - 118, Update the temporary-file setup in ConsoleGranularScopeTestCase to use Files.createTempFile instead of File.createTempFile, and register the created path for deletion on exit before writing content. Preserve the existing UTF-8 file contents and enabled TOML test flow.Sources: Path instructions, Linters/SAST tools
🤖 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.
Nitpick comments:
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java`:
- Around line 117-118: Update the temporary-file setup in
ConsoleGranularScopeTestCase to use Files.createTempFile instead of
File.createTempFile, and register the created path for deletion on exit before
writing content. Preserve the existing UTF-8 file contents and enabled TOML test
flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dffca029-62bf-4964-aabd-b99be0def367
📒 Files selected for processing (3)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/SCIM2RestClient.javamodules/integration/tests-integration/tests-backend/src/test/resources/testng.xml
Ported from wso2-support/product-is#2175. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java (1)
386-401: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated permission-extraction logic.
This re-implements the same JSON extraction already in
SCIM2RestClient.getV2RolePermissions(), just returning aListto preserve duplicates. Consider adding a list-returning counterpart onSCIM2RestClientto keep both implementations in sync.🤖 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 `@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java` around lines 386 - 401, Add a list-returning counterpart to SCIM2RestClient.getV2RolePermissions() that extracts permission values from the v2 role response while preserving duplicates, then update rolePermissionList to delegate to it. Remove the duplicated JSON parsing from rolePermissionList and keep its existing exception behavior.
🤖 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
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java`:
- Around line 121-122: Update the temporary file setup in
ConsoleGranularScopeTestCase so disabledTomlFile is restricted to the test
process and scheduled for deletion when the JVM exits. Preserve the existing
content-writing behavior while applying both access restriction and
delete-on-exit handling immediately after File.createTempFile.
- Around line 110-120: Update the config replacement logic in
ConsoleGranularScopeTestCase to detect whether the
use_granular_console_permissions pattern matched explicitly, rather than using
disabledContent.equals(content). Replace the property value when matched,
including when already false; append the [console.console_settings] section only
when no matching property exists.
---
Nitpick comments:
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java`:
- Around line 386-401: Add a list-returning counterpart to
SCIM2RestClient.getV2RolePermissions() that extracts permission values from the
v2 role response while preserving duplicates, then update rolePermissionList to
delegate to it. Remove the duplicated JSON parsing from rolePermissionList and
keep its existing exception behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 211b7350-c963-4825-9c27-a6c205f246fe
📒 Files selected for processing (1)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java
645b6e7 to
c67be49
Compare
|
PR builder started |
|
PR builder completed |
|
|
PR builder started |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java`:
- Around line 114-122: Update the configuration-editing logic around knobPattern
and knobMatcher so that when use_granular_console_permissions is absent, it
inserts the property into an existing [console.console_settings] table instead
of appending another table. Append the table only when neither the property nor
the table exists, while preserving the existing replacement behavior when the
property is present.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3750c7b4-5cd0-44f2-8fa7-9e609589e545
📒 Files selected for processing (2)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/restclients/SCIM2RestClient.java
| Pattern knobPattern = Pattern.compile( | ||
| "(?m)^(\\s*use_granular_console_permissions\\s*=\\s*)(true|false)(\\s*(#.*)?)$"); | ||
| Matcher knobMatcher = knobPattern.matcher(content); | ||
| if (knobMatcher.find()) { | ||
| content = knobMatcher.replaceFirst("$1false$3"); | ||
| } else { | ||
| content += System.lineSeparator() + "[console.console_settings]" + System.lineSeparator() | ||
| + "use_granular_console_permissions = false" + System.lineSeparator(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle an existing [console.console_settings] table.
If the property is absent but the table already exists, this branch appends a second table. The configuration parser may reject the duplicate definition during restart. Insert the property into the existing table, and append a new table only when the table is also absent.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 122-122: Invalid permissions for temporary file
Context: File disabledTomlFile = File.createTempFile("console-granular-disabled", ".toml");
Note: [CWE-378] Creation of Temporary File With Insecure Permissions. Security best practice.
(tempfile-permissions)
🤖 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
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/roles/v2/ConsoleGranularScopeTestCase.java`
around lines 114 - 122, Update the configuration-editing logic around
knobPattern and knobMatcher so that when use_granular_console_permissions is
absent, it inserts the property into an existing [console.console_settings]
table instead of appending another table. Append the table only when neither the
property nor the table exists, while preserving the existing replacement
behavior when the property is present.
|
PR builder completed |
3 participants
Ports the granular console scopes integration tests
Changes
ConsoleGranularScopeTestCase— covers Console role permission resolution for granular application feature scopes (console:applications_create/_update/_delete) withuse_granular_console_permissionstoggled off and on.getV2Role(...)andgetV2RolePermissions(...)helpers.Notes
pom.xmldependency bumps from the source PR were not ported — those are 7.1.0-line patch builds. This branch is on a newer dependency line assumed to already contain the feature.🤖 Generated with Claude Code