Skip to content

OSAC-3372: refactor role model from DemoShellRole to UserRole - #109

Merged
openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
rawagner:feature/OSAC-3372-user-role-refactor
Aug 3, 2026
Merged

OSAC-3372: refactor role model from DemoShellRole to UserRole#109
openshift-merge-bot[bot] merged 1 commit into
osac-project:mainfrom
rawagner:feature/OSAC-3372-user-role-refactor

Conversation

@rawagner

@rawagner rawagner commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Rename DemoShellRole to UserRole ('admin' | 'tenant-idp-manager' | 'tenant-admin' | 'tenant-user') and update roleFromRoles to resolve
in priority order: admin (group 'admins') > tenant-admin >
tenant-idp-manager > tenant-user (default).

Update all consumers across shell navigation, routing, labels, catalog display, catalog management panels, session context, and test files. Add unit tests for roleFromRoles priority logic.

Assisted-by: Claude Code noreply@anthropic.com

Summary by CodeRabbit

  • New Features

    • Introduced clearer role names for administrators, tenant administrators, identity-provider managers, and tenant users.
    • Added translated labels for each role.
    • Updated administration navigation and catalog access to reflect the new role permissions.
  • Bug Fixes

    • Corrected role-based catalog visibility and scope handling across administration pages.
  • Tests

    • Expanded coverage for role resolution, navigation visibility, routing, and catalog access scenarios.

@openshift-ci-robot

openshift-ci-robot commented Jul 30, 2026

Copy link
Copy Markdown

@rawagner: This pull request references OSAC-3372 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Rename DemoShellRole to UserRole ('admin' | 'tenant-idp-manager' | 'tenant-admin' | 'tenant-user') and update roleFromRoles to resolve
in priority order: admin (group 'admins') > tenant-admin >
tenant-idp-manager > tenant-user (default).

Update all consumers across shell navigation, routing, labels, catalog display, catalog management panels, session context, and test files. Add unit tests for roleFromRoles priority logic.

Assisted-by: Claude Code noreply@anthropic.com

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@rawagner, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7d8339f2-2200-492d-8d0c-e82ba7a92935

📥 Commits

Reviewing files that changed from the base of the PR and between c383bf3 and 738f835.

📒 Files selected for processing (12)
  • apps/app-frontend/src/hooks/oidc-login.test.ts
  • apps/app-frontend/src/hooks/oidc-login.tsx
  • apps/app-frontend/src/shell/AppShell.tsx
  • apps/app-frontend/src/shell/ShellMasthead.tsx
  • apps/app-frontend/src/shell/shellLabels.ts
  • apps/app-frontend/src/shell/shellNav.test.ts
  • apps/app-frontend/src/shell/shellNav.ts
  • apps/app-frontend/src/shell/shellRoutes.test.ts
  • apps/app-frontend/src/shell/shellRoutes.ts
  • libs/i18n/locales/en/translation.json
  • libs/ui-components/src/hooks/use-session.tsx
  • libs/ui-components/src/shellTypes.ts

Walkthrough

The frontend replaces legacy shell role literals with UserRole values, updates OIDC role resolution and access checks, adds translated role labels, adjusts catalog behavior, and updates related tests and fixtures.

Changes

User role migration

Layer / File(s) Summary
Role contracts and OIDC derivation
libs/ui-components/src/shellTypes.ts, apps/app-frontend/src/hooks/oidc-login.tsx, libs/ui-components/src/hooks/use-session.tsx, libs/ui-components/src/test-utils/*
Introduces the UserRole union, updates session and test-provider contracts, and maps OIDC roles to admin, tenant-admin, tenant-idp-manager, and tenant-user.
Shell access and translated labels
apps/app-frontend/src/shell/*, apps/app-frontend/src/hooks/oidc-login.test.ts, libs/i18n/locales/en/translation.json
Updates administration route and navigation gates, replaces shell role labels with translated UserRole labels, removes the legacy label module, and expands role-resolution/navigation tests.
Catalog role behavior
libs/ui-components/src/components/catalog/*, libs/ui-components/src/pages/admin/*, libs/ui-components/src/api/v1/**/*test.ts
Uses admin for provider-admin catalog behavior, updates catalog scope handling for admin and tenant-admin, and aligns catalog tests with the new role values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant OIDC
  participant useOIDCLogin
  participant SessionContext
  participant Shell
  participant Catalog
  OIDC->>useOIDCLogin: provide roles and groups
  useOIDCLogin->>SessionContext: store UserRole
  SessionContext->>Shell: expose role
  Shell->>Catalog: allow admin catalog routes
  Catalog->>Catalog: select scope and public/private data by role
Loading

Possibly related PRs

Suggested labels: lgtm

Suggested reviewers: elayaharoni, batzionb

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main change: renaming the role model from DemoShellRole to UserRole.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets ✅ Passed Scanned all changed files; found no hardcoded credentials, secrets, base64 blobs, or embedded-credential URLs. Only benign labels like 'pull secret'/'password' in i18n text.
No-Weak-Crypto ✅ Passed Checked the touched role/login/catalog files; no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret/token comparisons were introduced.
No-Injection-Vectors ✅ Passed No SQL/shell/eval/yaml/pickle/innerHTML sinks appear in the touched files; the new role logic uses equality checks and encoded fetch params only.
Container-Privileges ✅ Passed No container/K8s manifests were changed, and the patch contains no privileged/hostPID/hostNetwork/hostIPC/SYS_ADMIN/allowPrivilegeEscalation/root-related settings.
No-Sensitive-Data-In-Logs ✅ Passed No new sensitive logging was added; the only console.warn in oidc-login.tsx pre-existed and was unchanged.
Ai-Attribution ✅ Passed AI use is attributed with an Assisted-by trailer in the commit; no Co-Authored-By AI trailer was found.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@rawagner
rawagner force-pushed the feature/OSAC-3372-user-role-refactor branch from c383bf3 to 1aa4a22 Compare July 30, 2026 10:42
Rename DemoShellRole to UserRole ('admin' | 'tenant-idp-manager' |
'tenant-admin' | 'tenant-user') and update roleFromRoles to resolve
in priority order: admin (group 'admins') > tenant-admin >
tenant-idp-manager > tenant-user (default).

Update all consumers across shell navigation, routing, labels, catalog
display, catalog management panels, session context, and test files.
Add unit tests for roleFromRoles priority logic.

Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: rawagner <rawagner@redhat.com>
@rawagner
rawagner force-pushed the feature/OSAC-3372-user-role-refactor branch from 1aa4a22 to 738f835 Compare July 30, 2026 10:44
@openshift-ci

openshift-ci Bot commented Aug 3, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: batzionb, rawagner

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit 6f85d18 into osac-project:main Aug 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants