StackStorm / st2-rbac-backend

RBAC backend for StackStorm (previously part of EWC aka StackStorm Enteprise)
https://docs.stackstorm.com/latest/rbac.html
Apache License 2.0
5 stars 12 forks source link

action_view not included by default on action_execute #23

Open m4dcoder opened 5 years ago

m4dcoder commented 5 years ago

This issue is triaged on st2 v3.1. The action_view permission is not implicitly granted to user with action_execution permission.

Kami commented 5 years ago

Can you please provide some more context and details?

I tried to reproduce this issue, but I wasn't able to. I started by adding a new test case where we only grant "action_execute" to a specific action and then verify action_view is also granted on that action and it works fine (and in fact, we already had a test case which verifies that, but I added another one just in case to make sure existing one wasn't broken in some way).

So I assume it's related to some edge case where perhaps action_execute is granted to a pack and not action directly or similar?

Kami commented 5 years ago

I tried to reproduce it using both scenarios:

  1. action_execute is granted directly to the action -> this also implicitly grants action_view on that action
  2. action_execute is granted on a pack -> this also implicitly grants action_view to all actions inside that pack

And both of the scenarios seem to work correctly - https://github.com/extremenetworks/st2-enterprise-rbac-backend/commit/43b6110455039b242f6bbeb647a247e54d3e9a97.

Kami commented 5 years ago

Per discussion with @armab on Slack, it looks like people are mixing up two different permission types - action_view and execution_view.

Screenshot from 2019-08-14 13-02-17

So those are two different permission and resource types.

I believe we don't claim anywhere that granting action_execute on a pack also grants execution_view to all the executions for actions which belong to that pack.

We only claim that for action_execute and action_view as documented in the docs and described in my comment above.

Having said that, does it make sense for action_execute permission on a particular pack to also grant execution_view for all executions for actions which belong to a particular pack?

Perhaps, but we need to think about it some more since it has potential to change things.

Let's take the following scenario for example:

  1. User A has permission to execute action action_5
  2. Should this user be able to view all the executions for that action (or potentially executions for all the actions inside the pack if action_execute is granted on the pack level)?

I think it's reasonable to do that, but if user resource permission isolation functionality is not enabled (it's disabled by default), user A will also be able to see executions for action_5 triggered by the other users by default.

Is this a problem? It depends on the use case and user setup. In any case, we need to document that well.

/cc @VineeshJain

arm4b commented 5 years ago

I think the customer had similar example from https://docs.stackstorm.com/rbac.html#defining-roles-and-permission-grants

        -
            # Here we grant "action_execute" to "dummy_pack_2" which means user can execute (run)
            # all the actions inside this pack. Execute permission also grants "execution_re_run"
            # and "execution_stop" which means user can also re-run and stop (cancel) all the
            # executions which are triggered for the actions which belong to this pack.
            resource_uid: "pack:dummy_pack_2"
            permission_types:
               - "action_execute"

Doc example/comment says that action_execute also grants execution_re_run permission.

Despite that, customer was able to run the action, but wasn’t able to re-run it as the rbac gave him execution_view permission. That was unexpected.

Kami commented 5 years ago

Correct.

action_execute does implicitly grant execution_re_run, but it currently doesn't grant execution_view.

That's because of how CLI command is implemented - CLI first retrieves the original execution before performing the re-run operation.

To re-run an execution, it's not strictly necessary to retrieve the original execution first (which requires execution_view permission).

That's simply an implementation detail of CLI command.

If you use the API directly, execution_view is not required in such scenario.