antoniu98 commented on code in PR #862:
URL: https://github.com/apache/jackrabbit-oak/pull/862#discussion_r1183440243
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/ImpersonationImpl.java:
##########
@@ -173,13 +182,49 @@ private void updateImpersonatorNames(@NotNull Tree
userTree, @NotNull Set<String
private boolean isAdmin(@NotNull Principal principal) {
if (principal instanceof AdminPrincipal) {
return true;
- } else if (GroupPrincipals.isGroup(principal)) {
+ }
+ return Utils.isAdmin(principal, user.getUserManager());
+ }
+
+ private boolean isImpersonator(@NotNull Set<String> principalNames) {
+ Set<String> impersonatorGroups =
Set.of(user.getUserManager().getConfig().getConfigValue(
+ PARAM_IMPERSONATOR_GROUPS_ID,
+ new String[]{}));
+
+ if (impersonatorGroups.isEmpty()) {
return false;
- } else {
- return Utils.canImpersonateAllUsers(principal,
user.getUserManager());
}
+ return principalNames.stream()
+ .anyMatch(impersonatorGroups::contains);
}
+ public boolean isImpersonator(@NotNull Authorizable authorizable) {
+ Set<String> impersonatorGroups =
Set.of(user.getUserManager().getConfig().getConfigValue(
+ PARAM_IMPERSONATOR_GROUPS_ID,
+ new String[]{}));
+
+ if (impersonatorGroups.isEmpty()) {
+ return false;
+ }
+
+ try {
+ Iterator<Group> it = authorizable.memberOf();
Review Comment:
I like the edge case that your proposal is also covering, might be something
wanted but nobody thought of.
I believe the number of groups somebody is member of is also small most of
the times, so it comes up to what is more expensive:
a)"memberOf" + looping through list of groups a principal is member of
b) getting groups from the config and comparing them with the principal
I believe b) is faster.
_I have a question_: shouldn't we loop through the "principals of the
authorizable" when evaluating membership? By this I mean getting the subject
for the current `authorizable.getPrincipal()` and then get all the principals
associated to that subject ? If YES, I didn't know a way of getting the
subject, this is why I chose looping through the groups of a principal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
