Remove resource level permissions from default resources - #4368
Conversation
rajithacharith
added
the
trigger-pr-builder
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThe change removes granular system scopes and bootstrap resources, adds utilities for custom system-scoped resource servers, and updates group, OU, user, and user-type authorization tests to use resource-bound tokens and cleanup. ChangesScoped system authorization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AuthzSuite
participant ResourceServerHelpers
participant ThunderID
participant OAuthFlow
AuthzSuite->>ResourceServerHelpers: create scoped resource server
ResourceServerHelpers->>ThunderID: create resources and View actions
AuthzSuite->>ThunderID: assign role permissions
AuthzSuite->>OAuthFlow: request token with scoped resource identifier
OAuthFlow-->>AuthzSuite: return resource-bound access token
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.72.0)Trivy execution failed: 2026-07-27T09:16:46Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: kubernetes scan error: fs filter error: fs filter error: walk error range error: stat frontend/apps/console/doctor.config.json: no such file or directory: range error: stat frontend/apps/console/doctor.config.json: no such file or directory 🔧 ESLint
docs/content/deployment/configuration.mdxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. docs/content/guides/trusted-issuer.mdxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. frontend/apps/console/public/config.jsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. 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.
Actionable comments posted: 3
🤖 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 `@install/helm/values.yaml`:
- Line 204: Preserve the group scope in all OpenChoreo Console scope defaults:
update install/helm/values.yaml:204,
install/openchoreo/thunderid-oc-resourcetype/README.md:359,
install/openchoreo/thunderid-oc-resourcetype/samples/resource.yaml:132, and
install/openchoreo/thunderid-oc-resourcetype/templates/thunderid-resourcetype.yaml:220
so each default or example includes group.
In `@install/openchoreo/thunderid-oc-resourcetype/README.md`:
- Line 359: Update the runtime.console.scopes documentation entry in the README
to replace the em dash with an approved alternative such as a period, comma, or
rephrased wording, while preserving the existing meaning and default scope
value.
In `@tests/integration/testutils/api_utils.go`:
- Around line 1542-1556: Update the resource-creation error paths in the helper
containing the CreateResource and createActionUnderResource calls to capture
DeleteResourceServer failures instead of discarding them. Add a shared rollback
helper for these branches that combines the original creation error with the
cleanup error, while preserving the existing cleanup behavior and contextual
error messages.
🪄 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 Plus
Run ID: 88eb5fa2-e96d-494a-b09f-1ce2b0a486dd
📒 Files selected for processing (14)
backend/cmd/server/bootstrap/01-default-resources.yamldocs/content/deployment/configuration.mdxdocs/content/guides/trusted-issuer.mdxfrontend/apps/console/public/config.jsinstall/helm/values.yamlinstall/openchoreo/thunderid-oc-resourcetype/README.mdinstall/openchoreo/thunderid-oc-resourcetype/samples/resource.yamlinstall/openchoreo/thunderid-oc-resourcetype/templates/thunderid-resourcetype.yamltests/integration/group/group_authz_test.gotests/integration/ou/ou_authz_test.gotests/integration/testutils/api_utils.gotests/integration/testutils/oauth2_utils.gotests/integration/user/user_authz_test.gotests/integration/usertype/usertype_authz_test.go
💤 Files with no reviewable changes (2)
- frontend/apps/console/public/config.js
- backend/cmd/server/bootstrap/01-default-resources.yaml
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ae721d0 to
aa1e244
Compare
rajithacharith
added
trigger-pr-builder
aa1e244 to
1c09853
Compare
3 participants
Purpose
Make the default System resource server ship only the root
systemscope. The fine-grainedsystem:ou,system:user,system:group,system:usertype(and their:view) scopes are removed from the default (bootstrap) resource set.Resource-level permissions remain fully supported. The authorization mechanism is unchanged, so:
systemscope can do everything (root satisfies everysystem:*requirement).system:ou:view) still works when an operator declares and grants it.Approach
01-default-resources.yaml): the System resource server now declares only thesystemresource; theou/user/group/usertypesub-resources and theirviewactions were removed. The Administrator role already grantssystem, and the onboarding flow'srequiredScopes: [system:user]is left as-is (asystemtoken satisfies it hierarchically, and a configuredsystem:usertoken also works).internal/system/security/permissions.gostill maps management APIs to fine-grained permissions; hierarchical matching (HasSufficientPermission) already yields both behaviors above.system:<x>:viewhierarchy, grants those to the test role, and binds the token to that resource server via the RFC 8707resourceparameter. All original view-only and OU-scoping assertions are preserved, so the suites now serve as living proof that resource-level permissions still enforce when configured. New testutils helpers:CreateResource,createActionUnderResource,CreateSystemScopedResourceServer, and a resource-indicator override onObtainAccessTokenWithPassword.system): Consoleconfig.js, Helm values, OpenChoreo template/sample/README, and the trusted-issuer / configuration docs.Related Issues
Related PRs
Checklist
Security checks
Summary by CodeRabbit
system:*permissions, leaving a reduced set.system:*checks.