[
https://issues.apache.org/jira/browse/ISIS-2844?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andi Huber updated ISIS-2844:
-----------------------------
Fix Version/s: 2.0.0-RC1
(was: 2.0.0-M9)
> With Secman, SudoService behaves differently from impersonation - should be
> consistent in appropriate contexts...
> -----------------------------------------------------------------------------------------------------------------
>
> 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,
> image-2022-08-20-11-20-02-704.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 possibly sudo service should also use the
> UserMemento to obtain the roles in effect.... though see additional analysis
> below, I'm not 100% sure on this.
> (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.
>
> On the other hand... we do need to understand all of the code paths that are
> affected here. There are 4 callers to `ApplicationUser#getPermissionSet()`:
> !image-2022-08-20-11-20-02-704.png|width=601,height=177!
> This issue was raised originally in the context of `AuthorizorSecman#grants`
> to determine the effective permissoin, where it looks up the current
> `ApplicatoinUser` but the `ApplicationUser` (as currently implemented) is
> aware that impersonation can happen and if so to use the `UserMemento` rather
> than itself. But what of the other 3 callers? So it might be that the
> knowledge about impersonatoin should reside in the callers, eg in
> `AuthorizorSecman#grants` rather than in
> `ApplicationUser#getPermissionSet()`.
> So, more analysis needed, I think.
> ~~~
> _*Implementation (if decide to go ahead):*_
> 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)
