Skip to content

Reflect RBAC rules from backend in GUI #2058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

kvklink
Copy link
Member

@kvklink kvklink commented Jul 8, 2025

Closes #2013 and #1730
Depends on: workfloworchestrator/orchestrator-core#990

Adds support for reflecting RBAC rules from the backend in the frontend. Depends on a backend update.

When a user lacks permission for an operation, buttons and input fields (if applicable) are grayed out

Unable to create specific products:
image

Unable to start a specific task:
image

Unable to resume a workflow:
image

Unable to run a specific workflow on a subscription:
image

Copy link

changeset-bot bot commented Jul 8, 2025

⚠️ No Changeset found

Latest commit: 3f9372b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@DutchBen
Copy link
Collaborator

DutchBen commented Jul 8, 2025

I'm not sure about this approach. Greyed out entries in the dropdown list ok but don't you just want to restrict the whole page instead of disabling the Submit button?

@kvklink
Copy link
Member Author

kvklink commented Jul 8, 2025

In the current implementation, the entire input form is disabled, hence the str changing to a read_only_field. Or did you have something else in mind?

@DutchBen
Copy link
Collaborator

DutchBen commented Jul 8, 2025

In the current implementation, the entire input form is disabled, hence the str changing to a read_only_field. Or did you have something else in mind?

Why show the form at all though. In WfoStepForm you could show a message instead of showing the disabled form.

               {if(!allowed) ... show message } else { <UserInputFormWizard
                    stepUserInput={userInputForm}
                    stepSubmit={submitForm}
                    hasNext={false}
                    isTask={isTask}
                    isResuming={true}
                    allowSubmit={userPermissions.inputAllowed}
                /> }

Have you guys considered extending the WfoIsAllowedToRender wrapper to handle this case aswell? This feels like we are putting auth code in more places now.

@Mark90
Copy link
Contributor

Mark90 commented Jul 9, 2025

@DutchBen

Why show the form at all though. In WfoStepForm you could show a message instead of showing the disabled form

The idea discussed with Wouter was to make it visible when a user lacks permissions to do something, preferably without altering the look and feel of the interface. So if there's a dropdown that has options a user cannot select; the options are shown greyed out. If there's a button the user cannot press; the button is greyed out. All of the greyed out items are meant to have an on-hover text informing the user that they lack permissions - similar to the on-hover text for disabled actions on an out-of-sync subscription.
To be honest I forgot about input fields, but disabling them with an on-hover explanation seems fine to me.

If you have concerns about the design though then we should reconsider it :)

Replacing the components with a different text is an option.. doesn't sound very pretty though.
Perhaps we could add small "lock" icons to disabled components (those which support it, anyway).
Or add a banner/info box somewhere on the workflow detail page that contains something like "You don't have enough permissions to perform all actions on this page"

@DutchBen
Copy link
Collaborator

DutchBen commented Jul 9, 2025

After some reflection and input from Mark and Wouter I understand the reasoning for not hiding the forms but instead show them with messages about the missing priviliges. Also the reason for not handling these priviliges on opa level as we do in other places is that the rules on the roles are handles in the orchestrator core layer specifically and not in the - in the case of Surf - in the oidc/opa layer. The last thing to mention is that these check need to be implemented in the pydantic forms implemenation of the form handling seperately.

@Mark90
Copy link
Contributor

Mark90 commented Jul 10, 2025

Wait with merging until orchestrator-core 4.2.0 has been released.

@Mark90 Mark90 added the blocked label Jul 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Add Role based access control to start workflows - Implement updated endpoint in UI
3 participants