Skip to content
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

[RFC] "action_execute" should also grant "execution_view" on all the corresponding executions #25

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Kami
Copy link
Member

@Kami Kami commented Aug 14, 2019

Closes #23

This pull request updates RBAC resolvers code and updates it so now action_execute permission either on the action directly or on a pack, implicitly grants execution_view permission for all the executions which belong to that particular action (or to all the actions which belong to a particular pack).

This was implemented, because of the discussion in #23.

It's worth noting that this is not a bug fix.

This is a new functionality / change of behavior which has security implications (see my comment here https://github.com/extremenetworks/st2-enterprise-rbac-backend/issues/23#issuecomment-521204374).


I think that change is reasonable since we already have some other implicit grants in other places, but it could surprise users so it's important all the implications are documented.

With this change, if user A has action_execute permission on Action 1, that user will also be able to view all the executions for that action, even the ones which are triggered by other users if rbac.permission_isolation is not enabled (it's disabled by default).

I personally think that's a reasonable behavior (since it's already the case for execution_re_run and execution_stop), but we should probably also enable rbac.permission_isolation by default at some point in the future.

What do others think?

TODO

  • Document this change of behavior in upgrade notes, add a note on user resource permission isolation
  • Document this behavior in the RBAC docs

directly on the action or on the pack implicity grants "action_execute"
to the corresponding action or all actions inside a particular pack.
action or pack also grants "execution_view" on all the executions
for that particular action / actions which are parent of that particular
pack.
@Kami Kami added this to the 3.2.0 milestone Aug 14, 2019
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this change 👍

However changing st2 CLI to make appropriate execution re-run API request (with no additional execution view API request) would be more appropriate IMO and less disturbing per https://github.com/extremenetworks/st2-enterprise-rbac-backend/issues/23#issuecomment-521213478.

@arm4b arm4b closed this Dec 2, 2020
@arm4b arm4b deleted the execute_grants_view_test_case branch December 2, 2020 20:03
@blag
Copy link
Contributor

blag commented Dec 2, 2020

@armab Was this merged in somewhere or no?

@arm4b
Copy link
Member

arm4b commented Dec 2, 2020

I think deleting the branch closed this PR automatically. Let me restore it.

With that, not sure if I feel lucky enough to merge this PR after 1yr of stale.

@arm4b arm4b restored the execute_grants_view_test_case branch December 2, 2020 20:46
@arm4b arm4b reopened this Dec 2, 2020
@cognifloyd cognifloyd modified the milestones: 3.2.0, 3.7.0 Apr 2, 2022
@cognifloyd cognifloyd added this to In progress in StackStorm v3.7.0 via automation Apr 2, 2022
@cognifloyd cognifloyd moved this from In progress to To do in StackStorm v3.7.0 Apr 2, 2022
@cognifloyd cognifloyd removed this from To do in StackStorm v3.7.0 Apr 27, 2022
@cognifloyd cognifloyd added this to In progress in StackStorm v3.8.0 via automation Apr 27, 2022
@cognifloyd cognifloyd modified the milestones: 3.7.0, 3.8.0 Apr 27, 2022
@cognifloyd cognifloyd moved this from In progress to To do in StackStorm v3.8.0 Apr 27, 2022
@cognifloyd
Copy link
Member

I merged in master.
But, CI is out-of-date so bumping to 3.8.0 milestone.

@CLAassistant
Copy link

CLAassistant commented May 11, 2022

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

action_view not included by default on action_execute
5 participants