[jira] [Updated] (ISIS-2844) With Secman, SudoService behaves differently from impersonation - should be consistent.

["Daniel Keir Haywood (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/ISIS-2844?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Daniel Keir Haywood updated ISIS-2844:
--------------------------------------
    Fix Version/s: 2.0.0-RC1
                       (was: 2.0.0-M8)
      Description: 
_*Analysis:*_

when sudo service runs and secman is configured, the effective permissions are obtained from the ApplicationUser object, and whichever ApplicationRoles that ApplicationUser happens to have.

in contrast, when impersonating then the permissions are obtained from the UserMemento + associated RoleMementos.

For consistency, I think that sudo service should also use the UserMemento to obtain the roles in effect.  

(Note: if not running under sudo service and not impersonating, then we also use the roles from usermemento; but these would have been copied from the ApplicatoinUser on login).

In terms of change to the user experience, because a `UserMemento` is immutable and is only populated on login from the `ApplicationUser`, and that it contains the roles, then the user will need to logout and login if they are added to any new roles while logged in.  I think this is acceptable.

~~~

_*Implementation:*_

In terms of code, it's pretty trivial I think; we just remove the check for userService.isImpersonating() below and always run the first branch, ie query `byUserMemento(...)`.  The `byUser(...)` method is probably therefore redundant and could be removed.  See code snippet below.

!image-2021-08-18-16-24-04-978.png|width=879,height=376!

  was:
I think that permissions should always be taken from the usermemento.  For the three use cases:
 * impersonation (as already is case): we use the roles specified on the usermemento
 * sudo service (new code): we instead use the roles  specified on the usermemento
 * no sudoservice, no impersonation : we obtain the roles from the usermemento, which would have been copied from the ApplicationUser on login.

In terms of code, I think we just remove the check for userService.isImpersonating() below and always run the first branch, ie query `byUserMemento(...)`.  The `byUser(...)` method is probably therefore redundant and could be removed.  See code snippet below.

In terms of change to the user experience, because a `UserMemento` is immutable and is only populated on login from the `ApplicationUser`, and that it contains the roles, then the user will need to logout and login if they are added to any new roles while logged in.  I think this is acceptable.

!image-2021-08-18-16-24-04-978.png|width=879,height=376!


> With Secman, SudoService behaves differently from impersonation - should be consistent.
> ---------------------------------------------------------------------------------------
>
>                 Key: ISIS-2844
>                 URL: https://issues.apache.org/jira/browse/ISIS-2844
>             Project: Isis
>          Issue Type: Improvement
>          Components: Isis Extensions SecMan
>    Affects Versions: 2.0.0-M6
>            Reporter: Daniel Keir Haywood
>            Priority: Minor
>             Fix For: 2.0.0-RC1
>
>         Attachments: image-2021-08-18-16-24-04-978.png
>
>
> _*Analysis:*_
> when sudo service runs and secman is configured, the effective permissions are obtained from the ApplicationUser object, and whichever ApplicationRoles that ApplicationUser happens to have.
> in contrast, when impersonating then the permissions are obtained from the UserMemento + associated RoleMementos.
> For consistency, I think that sudo service should also use the UserMemento to obtain the roles in effect.  
> (Note: if not running under sudo service and not impersonating, then we also use the roles from usermemento; but these would have been copied from the ApplicatoinUser on login).
> In terms of change to the user experience, because a `UserMemento` is immutable and is only populated on login from the `ApplicationUser`, and that it contains the roles, then the user will need to logout and login if they are added to any new roles while logged in.  I think this is acceptable.
> ~~~
> _*Implementation:*_
> In terms of code, it's pretty trivial I think; we just remove the check for userService.isImpersonating() below and always run the first branch, ie query `byUserMemento(...)`.  The `byUser(...)` method is probably therefore redundant and could be removed.  See code snippet below.
> !image-2021-08-18-16-24-04-978.png|width=879,height=376!



--
This message was sent by Atlassian Jira
(v8.20.10#820010)