Merge "Add debug logs when checking account visibility"

java/com/google/gerrit/server/account/AccountControl.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.account;
 
 import static java.util.stream.Collectors.toSet;
 
+import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.entities.Account;
 import com.google.gerrit.entities.AccountGroup;
@@ -36,6 +37,8 @@ import java.util.Set;
 
 /** Access control management for one account's access to other accounts. */
 public class AccountControl {
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
   public static class Factory {
     private final PermissionBackend permissionBackend;
     private final ProjectCache projectCache;
@@ -145,11 +148,19 @@ public class AccountControl {
 
   private boolean canSee(OtherUser otherUser) {
     if (accountVisibility == AccountVisibility.ALL) {
+      logger.atFine().log(
+          "user %s can see account %d (accountVisibility = %s)",
+          user.getLoggableName(), otherUser.getId().get(), AccountVisibility.ALL);
       return true;
     } else if (user.isIdentifiedUser() && user.getAccountId().equals(otherUser.getId())) {
       // I can always see myself.
+      logger.atFine().log(
+          "user %s can see own account %d", user.getLoggableName(), otherUser.getId().get());
       return true;
     } else if (viewAll()) {
+      logger.atFine().log(
+          "user %s can see account %d (view all accounts = true)",
+          user.getLoggableName(), otherUser.getId().get());
       return true;
     }
 
@@ -159,14 +170,32 @@ public class AccountControl {
           Set<AccountGroup.UUID> usersGroups = groupsOf(otherUser.getUser());
           for (PermissionRule rule : accountsSection.getSameGroupVisibility()) {
             if (rule.isBlock() || rule.isDeny()) {
+              logger.atFine().log(
+                  "ignoring group %s of user %s for %s account visibility check"
+                      + " because there is a blocked/denied sameGroupVisibility rule: %s",
+                  rule.getGroup().getUUID(),
+                  otherUser.getUser().getLoggableName(),
+                  AccountVisibility.SAME_GROUP,
+                  rule);
               usersGroups.remove(rule.getGroup().getUUID());
             }
           }
 
           if (user.getEffectiveGroups().containsAnyOf(usersGroups)) {
+            logger.atFine().log(
+                "user %s can see account %d because they share a group (accountVisibility = %s)",
+                user.getLoggableName(), otherUser.getId().get(), AccountVisibility.SAME_GROUP);
             return true;
           }
-          break;
+
+          logger.atFine().log(
+              "user %s cannot see account %d because they don't share a group"
+                  + " (accountVisibility = %s)",
+              user.getLoggableName(), otherUser.getId().get(), AccountVisibility.SAME_GROUP);
+          logger.atFine().log("groups of user %s: %s", user.getLoggableName(), groupsOf(user));
+          logger.atFine().log(
+              "groups of other user %s: %s", otherUser.getUser().getLoggableName(), usersGroups);
+          return false;
         }
       case VISIBLE_GROUP:
         {
@@ -174,21 +203,37 @@ public class AccountControl {
           for (AccountGroup.UUID usersGroup : usersGroups) {
             try {
               if (groupControlFactory.controlFor(usersGroup).isVisible()) {
+                logger.atFine().log(
+                    "user %s can see account %d because it is member of the visible group %s"
+                        + " (accountVisibility = %s)",
+                    user.getLoggableName(),
+                    otherUser.getId().get(),
+                    usersGroup.get(),
+                    AccountVisibility.VISIBLE_GROUP);
                 return true;
               }
             } catch (NoSuchGroupException e) {
               continue;
             }
           }
-          break;
+
+          logger.atFine().log(
+              "user %s cannot see account %d because none of its groups are visible"
+                  + " (accountVisibility = %s)",
+              user.getLoggableName(), otherUser.getId().get(), AccountVisibility.VISIBLE_GROUP);
+          logger.atFine().log(
+              "groups of other user %s: %s", otherUser.getUser().getLoggableName(), usersGroups);
+          return false;
         }
       case NONE:
-        break;
+        logger.atFine().log(
+            "user %s cannot see account %d (accountVisibility = %s)",
+            user.getLoggableName(), otherUser.getId().get(), AccountVisibility.NONE);
+        return false;
       case ALL:
       default:
         throw new IllegalStateException("Bad AccountVisibility " + accountVisibility);
     }
-    return false;
   }
 
   private boolean viewAll() {
@@ -196,14 +241,19 @@ public class AccountControl {
       try {
         perm.check(GlobalPermission.VIEW_ALL_ACCOUNTS);
         viewAll = true;
-      } catch (AuthException | PermissionBackendException e) {
+      } catch (AuthException e) {
+        viewAll = false;
+      } catch (PermissionBackendException e) {
+        logger.atFine().withCause(e).log(
+            "Failed to check %s global capability for user %s",
+            GlobalPermission.VIEW_ALL_ACCOUNTS, user.getLoggableName());
         viewAll = false;
       }
     }
     return viewAll;
   }
 
-  private Set<AccountGroup.UUID> groupsOf(IdentifiedUser user) {
+  private Set<AccountGroup.UUID> groupsOf(CurrentUser user) {
     return user.getEffectiveGroups().getKnownGroups().stream()
         .filter(a -> !SystemGroupBackend.isSystemGroup(a))
         .collect(toSet());