rucio icon indicating copy to clipboard operation
rucio copied to clipboard

Allow permissions to optionally return a message

Open dynamic-entropy opened this issue 1 year ago • 4 comments

Description

The current permission checks only tell a user that they are not allowed to issue a specific command. However they do not explain any reason why. For e.g. a permission check if a issuer can or cannot create a rule returns Account joe cannot add a replication rule.

https://github.com/rucio/rucio/blob/dc97725de99d438966d412c81918a52755b63baa/lib/rucio/api/rule.py#L89-L90

This is often not enough when there are multiple criteria in an experiments policy package to determine if a rule can be created or not. This has become increasingly important for CMS due to user policy around rule creation that are AutoApproved.

We would like to optionally, attach a reason when the permission fails to let the user if there is an unintended mistake or in cases when they should get required permissions from ops.Joe

Motivation

No response

Change

Modify the has_permissions call to accept an optional (For backward compatibility and opt-in support) "Message" (reason for failure) in the return value and raise it with the Access Denied exception.

dynamic-entropy avatar Mar 18 '24 19:03 dynamic-entropy

I think this is a good idea and should not be too complicated to implement.

bari12 avatar Mar 20 '24 15:03 bari12

I've discussed this with Martin and I'm happy to take a look at it if people want me to? I think it should be possible to make it backwards compatible so that policy packages that implement their own permission checks and don't return a message will still work, probably by adding some code to the dispatcher in core/permission/__init__.py.

jamesp-epcc avatar May 03 '24 10:05 jamesp-epcc

Hello James It is good with us if you take this. I am not sure if @alexanderrichards has already started working on it.

dynamic-entropy avatar May 03 '24 10:05 dynamic-entropy

I am finally looking at this now. I plan to do the following:

  1. Add a new class PermissionResult containing a boolean and a string for the message. The return value from has_permission will be of this type.
  2. Implement __bool__() to maintain compatibility with old code that expects a raw boolean.
  3. The top level has_permission function in __init__.py will check the type returned by the policy packages. If it's a raw boolean, it will wrap the value in a PermissionResult object with an empty string, to maintain compatibility with old policy packages that return raw booleans.
  4. The code in the gateway layer (mostly) that calls has_permission can be modified to expect a PermissionResult object and report the message when it throws an exception. This could be done incrementally.

I've already done some tests to ensure this will work, but I wanted to check it's acceptable before I spend time updating all the callers to report the message.

jamesp-epcc avatar Jun 17 '24 10:06 jamesp-epcc