Authorization: Apply filters on hierarchy elements based on access rights #3802
Conversation
oheger-bosch
force-pushed
the
oheger-bosch/db_authorization_element_filters
branch
3 times, most recently
from
41bd030 to
68cdf8f
Compare
oheger-bosch
requested review from
MarcelBochtler,
bs-ondem,
mnonnenmacher,
nnobelis,
sschuberth and
wkl3nk
as code owners
| * the `WRITER` role on an organization, they automatically have `WRITER` permissions on all products and | ||
| * repositories within that organization. | ||
| * - Role assignments on lower levels in the hierarchy override those from higher levels. For example, a `WRITER` role | ||
| * assignment for a user on product level could be turned into a `READER` role assignment on repository level. |
There was a problem hiding this comment.
I'm really against implementing it that way because of the added complexity as explained here:
#3799 (reply in thread)
In addition, this also can create a deadlock situation, for example, when there is only one organization ADMIN who makes himself a READER of a repository, then there is no one left with ADMIN permission for that repository.
There was a problem hiding this comment.
Chiming in as the one who wrote "Inherited permissions can be overridden on lower levels" over here: While I still think it's a nice-to-have feature, it's not a blocker for us to not have that feature.
There was a problem hiding this comment.
Actually, the complexity is not that much increased. Role assignments on lower levels of the hierarchy need to be handled in any case to support widening of access rights. So, why not take that additional step?
The mentioned problems have partly been addressed by the function of Authorization service that constructs a HierarchyFilter. Such filters can be applied when selecting elements from the hierarchy. The argument that it is hard to understand which user has which rights is not specific to this solution. It is also hard to understand that a role assignment made explicitly on a lower level does not have any effect when rights are inherited from higher levels.
There are easier ways to shoot yourself in the foot than your deadlock scenario. An admin could as well remove their own user from the organization or self-assign a lower role.
There was a problem hiding this comment.
Ok, forget about it. There is obviously no chance to get this in. So I am going to change the implementation as requested.
There was a problem hiding this comment.
Fortunately, it turned out to be not that difficult to change the behavior, since the major part of the logic how roles are evaluated in the hierarchy is centralized in the HierarchyPermissions class.
There was a problem hiding this comment.
Actually, the complexity is not that much increased.
Maybe not in this component, but for the consumers of the component the situation is different as I explained in my other post.
The argument that it is hard to understand which user has which rights is not specific to this solution. It is also hard to understand that a role assignment made explicitly on a lower level does not have any effect when rights are inherited from higher levels.
I think the principle that roles can only grant permissions but not remove them is easy to understand. I doubt that users will be surprised that getting a role has no effect if it grants them permissions they already had anyway.
There are easier ways to shoot yourself in the foot than your deadlock scenario. An admin could as well remove their own user from the organization or self-assign a lower role.
Yes, but losing permissions by removing a role is intuitive, losing permissions by getting a role much less so.
oheger-bosch
force-pushed
the
oheger-bosch/db_authorization_element_filters
branch
from
68cdf8f to
38ff20c
Compare
components/authorization/backend/src/test/kotlin/rights/HierarchyPermissionsTest.kt
Outdated
Show resolved
Hide resolved
components/authorization/backend/src/main/kotlin/rights/OrganizationRole.kt
Show resolved
Hide resolved
oheger-bosch
force-pushed
the
oheger-bosch/db_authorization_element_filters
branch
from
38ff20c to
2cbd90d
Compare
components/authorization/backend/src/main/kotlin/rights/HierarchyPermissions.kt
Outdated
Show resolved
Hide resolved
components/authorization/backend/src/main/kotlin/rights/HierarchyPermissions.kt
Show resolved
Hide resolved
components/authorization/backend/src/main/kotlin/rights/HierarchyPermissions.kt
Show resolved
Hide resolved
components/authorization/backend/src/main/kotlin/rights/HierarchyPermissions.kt
Outdated
Show resolved
Hide resolved
This interface is going to be used in different use cases to check whether specific permissions are available on concrete hierarchy elements. The implementation contains the logic how permissions are inherited through the hierarchy. Signed-off-by: Oliver Heger <[email protected]>
Introduce new `checkPermissions` functions that are based on `HierarchyPermissions` and check whether specific permissions are granted. These are going to be used for permission checks in the REST API. Rework the `getEffectiveRole` functions in `DbAuthorizationService` to use `HierarchyPermissions` as well. Modify the permission sets in the role classes, so that permissions granted on higher levels of the hierarchy are inherited downwards. Signed-off-by: Oliver Heger <[email protected]>
This class allows defining filters for elements in the hierarchy based on element IDs. This is going to be used to select only the elements a user has access to. Signed-off-by: Oliver Heger <[email protected]>
Add a function to `AuthorizationService` that computes a `HierarchyFilter` for the IDs of elements in the hierarchy on which a user has specific access rights. This can then be used to generate filter conditions for database queries. Signed-off-by: Oliver Heger <[email protected]>
Extend the `RepositoryRepository.list()` function to support a `HierarchyFiler`. This allows to query a specific set of repositories; for instance, all those a user can access. To make this possible, add some more helper extension functions for the `dao` module. Signed-off-by: Oliver Heger <[email protected]>
Extend the `ProductRepository.list()` function to support a `HierarchyFiler`. This allows to query a specific set of products; for instance, all those a user can access. Signed-off-by: Oliver Heger <[email protected]>
Extend the `OrganizationRepository.list()` function to support a `HierarchyFiler`. This allows to query a specific set of organizations; for instance, all those a user can access. Signed-off-by: Oliver Heger <[email protected]>
To perform authorization checks in routes automatically, now call the `checkPermissions` function of the authorization service. Simplify the `AuthorizationChecker` interface. The two-step logic of first loading the effective role and then checking permissions is not necessary; a single function to load and check permissions is sufficient. Signed-off-by: Oliver Heger <[email protected]>
oheger-bosch
force-pushed
the
oheger-bosch/db_authorization_element_filters
branch
from
2cbd90d to
e115557
Compare
oheger-bosch
deleted the
oheger-bosch/db_authorization_element_filters
branch
3 participants
This PR adds functionality to the new authorization component to compute the elements in the hierarchy a user has access to based on the following criteria:
These rules are applied for permission checks. They can also be used to generate filters to load only the elements from the database a user can access.