Merge "Update _computeMessageContent to use optional properties"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 7ab97e65..313092f 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2512,6 +2512,18 @@
[[groups]]
=== Section groups
+[[groups.includeExternalUsersInRegisteredUsersGroup]]groups.includeExternalUsersInRegisteredUsersGroup::
++
+Controls whether external users (these are users we have sufficient
+knowledge about but who don't yet have a Gerrit account) are considered
+to be members of the `REGISTERED_USERS` group.
++
+This setting only makes sense if you run custom code (e.g. from a plugin
+or a custom authentication backend). By default, Gerrit core always requires
+users to register and doesn't use external users.
++
+By default, true.
+
[[groups.newGroupsVisibleToAll]]groups.newGroupsVisibleToAll::
+
Controls whether newly created groups should be by default visible to
diff --git a/WORKSPACE b/WORKSPACE
index f68ecf6..fd36850 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -137,36 +137,6 @@
sha1 = "83cd2cd674a217ade95a4bb83a8a14f351f48bd0",
)
-GUICE_VERS = "4.2.3"
-
-GUICE_LIBRARY_SHA256 = "5168f5e7383f978c1b4154ac777b78edd8ac214bb9f9afdb92921c8d156483d3"
-
-http_file(
- name = "guice-library-no-aop",
- canonical_id = "guice-library-no-aop-" + GUICE_VERS + ".jar-" + GUICE_LIBRARY_SHA256,
- downloaded_file_path = "guice-library-no-aop.jar",
- sha256 = GUICE_LIBRARY_SHA256,
- urls = [
- "https://repo1.maven.org/maven2/com/google/inject/guice/" +
- GUICE_VERS +
- "/guice-" +
- GUICE_VERS +
- "-no_aop.jar",
- ],
-)
-
-maven_jar(
- name = "guice-assistedinject",
- artifact = "com.google.inject.extensions:guice-assistedinject:" + GUICE_VERS,
- sha1 = "acbfddc556ee9496293ed1df250cc378f331d854",
-)
-
-maven_jar(
- name = "guice-servlet",
- artifact = "com.google.inject.extensions:guice-servlet:" + GUICE_VERS,
- sha1 = "8d6e7e35eac4fb5e7df19c55b3bc23fa51b10a11",
-)
-
maven_jar(
name = "javax_inject",
artifact = "javax.inject:javax.inject:1",
diff --git a/java/com/google/gerrit/server/CurrentUser.java b/java/com/google/gerrit/server/CurrentUser.java
index 825b34f..c85c389 100644
--- a/java/com/google/gerrit/server/CurrentUser.java
+++ b/java/com/google/gerrit/server/CurrentUser.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -128,6 +129,14 @@
getClass().getSimpleName() + " is not an IdentifiedUser");
}
+ /**
+ * Returns all {@link ExternalId.Key}s associated with this user. For {@link AnonymousUser} and
+ * other users that don't represent a person user or service account, this set will be empty.
+ */
+ public ImmutableSet<ExternalId.Key> getExternalIdKeys() {
+ return ImmutableSet.of();
+ }
+
/** Check if the CurrentUser is an InternalUser. */
public boolean isInternalUser() {
return false;
diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java
index 75c7cda..ca86434 100644
--- a/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/java/com/google/gerrit/server/IdentifiedUser.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server;
import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.flogger.LazyArgs.lazy;
import com.google.common.annotations.VisibleForTesting;
@@ -31,6 +32,7 @@
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.ListGroupMembership;
import com.google.gerrit.server.account.Realm;
+import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.CanonicalWebUrl;
@@ -388,6 +390,11 @@
return ImmutableSet.copyOf(validEmails);
}
+ @Override
+ public ImmutableSet<ExternalId.Key> getExternalIdKeys() {
+ return state().externalIds().stream().map(ExternalId::key).collect(toImmutableSet());
+ }
+
public String getName() {
return getAccount().getName();
}
diff --git a/java/com/google/gerrit/server/account/GroupBackend.java b/java/com/google/gerrit/server/account/GroupBackend.java
index 545da6e..d6360c5 100644
--- a/java/com/google/gerrit/server/account/GroupBackend.java
+++ b/java/com/google/gerrit/server/account/GroupBackend.java
@@ -19,7 +19,7 @@
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.entities.GroupReference;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
-import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.project.ProjectState;
import java.util.Collection;
@@ -42,7 +42,7 @@
Collection<GroupReference> suggest(String name, @Nullable ProjectState project);
/** @return the group membership checker for the backend. */
- GroupMembership membershipsOf(IdentifiedUser user);
+ GroupMembership membershipsOf(CurrentUser user);
/** @return {@code true} if the group with the given UUID is visible to all registered users. */
boolean isVisibleToAll(AccountGroup.UUID uuid);
diff --git a/java/com/google/gerrit/server/account/IncludingGroupMembership.java b/java/com/google/gerrit/server/account/IncludingGroupMembership.java
index 6dc7976..9cb11a6 100644
--- a/java/com/google/gerrit/server/account/IncludingGroupMembership.java
+++ b/java/com/google/gerrit/server/account/IncludingGroupMembership.java
@@ -14,11 +14,12 @@
package com.google.gerrit.server.account;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.entities.AccountGroup;
-import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.group.InternalGroup;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -40,18 +41,18 @@
*/
public class IncludingGroupMembership implements GroupMembership {
public interface Factory {
- IncludingGroupMembership create(IdentifiedUser user);
+ IncludingGroupMembership create(CurrentUser user);
}
private final GroupCache groupCache;
private final GroupIncludeCache includeCache;
- private final IdentifiedUser user;
+ private final CurrentUser user;
private final Map<AccountGroup.UUID, Boolean> memberOf;
private Set<AccountGroup.UUID> knownGroups;
@Inject
IncludingGroupMembership(
- GroupCache groupCache, GroupIncludeCache includeCache, @Assisted IdentifiedUser user) {
+ GroupCache groupCache, GroupIncludeCache includeCache, @Assisted CurrentUser user) {
this.groupCache = groupCache;
this.includeCache = includeCache;
this.user = user;
@@ -93,7 +94,7 @@
if (!group.isPresent()) {
continue;
}
- if (group.get().getMembers().contains(user.getAccountId())) {
+ if (user.isIdentifiedUser() && group.get().getMembers().contains(user.getAccountId())) {
memberOf.put(id, true);
return true;
}
@@ -124,7 +125,10 @@
private ImmutableSet<AccountGroup.UUID> computeKnownGroups() {
GroupMembership membership = user.getEffectiveGroups();
- Collection<AccountGroup.UUID> direct = includeCache.getGroupsWithMember(user.getAccountId());
+ Collection<AccountGroup.UUID> direct =
+ user.isIdentifiedUser()
+ ? includeCache.getGroupsWithMember(user.getAccountId())
+ : ImmutableList.of();
direct.forEach(groupUuid -> memberOf.put(groupUuid, true));
Set<AccountGroup.UUID> r = Sets.newHashSet(direct);
r.remove(null);
diff --git a/java/com/google/gerrit/server/account/InternalGroupBackend.java b/java/com/google/gerrit/server/account/InternalGroupBackend.java
index c520c96..8761081 100644
--- a/java/com/google/gerrit/server/account/InternalGroupBackend.java
+++ b/java/com/google/gerrit/server/account/InternalGroupBackend.java
@@ -20,7 +20,7 @@
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.entities.GroupReference;
-import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.InternalGroupDescription;
import com.google.gerrit.server.group.db.Groups;
@@ -97,7 +97,7 @@
}
@Override
- public GroupMembership membershipsOf(IdentifiedUser user) {
+ public GroupMembership membershipsOf(CurrentUser user) {
return groupMembershipFactory.create(user);
}
diff --git a/java/com/google/gerrit/server/account/UniversalGroupBackend.java b/java/com/google/gerrit/server/account/UniversalGroupBackend.java
index a35b0ac..5bd9bea 100644
--- a/java/com/google/gerrit/server/account/UniversalGroupBackend.java
+++ b/java/com/google/gerrit/server/account/UniversalGroupBackend.java
@@ -27,7 +27,7 @@
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.entities.GroupReference;
-import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.StartupCheck;
import com.google.gerrit.server.StartupException;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -94,14 +94,14 @@
}
@Override
- public GroupMembership membershipsOf(IdentifiedUser user) {
+ public GroupMembership membershipsOf(CurrentUser user) {
return new UniversalGroupMembership(user);
}
private class UniversalGroupMembership implements GroupMembership {
private final Map<GroupBackend, GroupMembership> memberships;
- private UniversalGroupMembership(IdentifiedUser user) {
+ private UniversalGroupMembership(CurrentUser user) {
ImmutableMap.Builder<GroupBackend, GroupMembership> builder = ImmutableMap.builder();
backends.runEach(g -> builder.put(g, g.membershipsOf(user)));
this.memberships = builder.build();
diff --git a/java/com/google/gerrit/server/auth/ldap/FakeLdapGroupBackend.java b/java/com/google/gerrit/server/auth/ldap/FakeLdapGroupBackend.java
index 63cd426..f806756 100644
--- a/java/com/google/gerrit/server/auth/ldap/FakeLdapGroupBackend.java
+++ b/java/com/google/gerrit/server/auth/ldap/FakeLdapGroupBackend.java
@@ -20,7 +20,7 @@
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.entities.GroupReference;
-import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.ListGroupMembership;
@@ -76,7 +76,7 @@
}
@Override
- public GroupMembership membershipsOf(IdentifiedUser user) {
+ public GroupMembership membershipsOf(CurrentUser user) {
return new ListGroupMembership(Collections.emptyList());
}
diff --git a/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java b/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
index 180612c..d5da406 100644
--- a/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
+++ b/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
@@ -29,7 +29,6 @@
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.entities.GroupReference;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -45,6 +44,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import javax.naming.InvalidNameException;
@@ -178,21 +178,13 @@
}
@Override
- public GroupMembership membershipsOf(IdentifiedUser user) {
- String id = findId(user.state().externalIds());
- if (id == null) {
+ public GroupMembership membershipsOf(CurrentUser user) {
+ Optional<ExternalId.Key> id =
+ user.getExternalIdKeys().stream().filter(e -> e.isScheme(SCHEME_GERRIT)).findAny();
+ if (!id.isPresent()) {
return GroupMembership.EMPTY;
}
- return new LdapGroupMembership(membershipCache, projectCache, id, gerritConfig);
- }
-
- private static String findId(Collection<ExternalId> extIds) {
- for (ExternalId extId : extIds) {
- if (extId.isScheme(SCHEME_GERRIT)) {
- return extId.key().id();
- }
- }
- return null;
+ return new LdapGroupMembership(membershipCache, projectCache, id.get().id(), gerritConfig);
}
private Set<GroupReference> suggestLdap(String name) {
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index b1b2711..d131cf5 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -179,6 +179,7 @@
import com.google.gerrit.server.query.change.ChangeIsVisibleToPredicate;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ConflictsCacheImpl;
+import com.google.gerrit.server.query.change.ExternalUser;
import com.google.gerrit.server.quota.QuotaEnforcer;
import com.google.gerrit.server.restapi.change.OnPostReview;
import com.google.gerrit.server.restapi.change.SuggestReviewers;
@@ -279,6 +280,7 @@
factory(ProjectState.Factory.class);
factory(RevisionJson.Factory.class);
factory(InboundEmailRejectionSender.Factory.class);
+ factory(ExternalUser.Factory.class);
bind(PermissionCollection.Factory.class);
bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON);
AccountDefaultDisplayName accountDefaultDisplayName =
diff --git a/java/com/google/gerrit/server/group/SystemGroupBackend.java b/java/com/google/gerrit/server/group/SystemGroupBackend.java
index b5ccb18..dfb42ff 100644
--- a/java/com/google/gerrit/server/group/SystemGroupBackend.java
+++ b/java/com/google/gerrit/server/group/SystemGroupBackend.java
@@ -24,6 +24,7 @@
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.entities.GroupReference;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StartupCheck;
import com.google.gerrit.server.StartupException;
@@ -33,6 +34,7 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.db.Groups;
import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.query.change.ExternalUser;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -90,6 +92,7 @@
private final SortedMap<String, GroupReference> namesToGroups;
private final ImmutableSet<String> names;
private final ImmutableMap<AccountGroup.UUID, GroupReference> uuids;
+ private final ImmutableSet<AccountGroup.UUID> externalUserMemberships;
@Inject
@VisibleForTesting
@@ -114,6 +117,10 @@
ImmutableSet.copyOf(
namesToGroups.values().stream().map(GroupReference::getName).collect(toSet()));
uuids = u.build();
+ externalUserMemberships =
+ cfg.getBoolean("groups", null, "includeExternalUsersInRegisteredUsersGroup", true)
+ ? ImmutableSet.of(ANONYMOUS_USERS, REGISTERED_USERS)
+ : ImmutableSet.of(ANONYMOUS_USERS);
}
public GroupReference getGroup(AccountGroup.UUID uuid) {
@@ -182,8 +189,14 @@
}
@Override
- public GroupMembership membershipsOf(IdentifiedUser user) {
- return new ListGroupMembership(ImmutableSet.of(ANONYMOUS_USERS, REGISTERED_USERS));
+ public GroupMembership membershipsOf(CurrentUser user) {
+ if (user instanceof ExternalUser) {
+ return new ListGroupMembership(externalUserMemberships);
+ }
+ if (user instanceof IdentifiedUser) {
+ return new ListGroupMembership(ImmutableSet.of(ANONYMOUS_USERS, REGISTERED_USERS));
+ }
+ return new ListGroupMembership(ImmutableSet.of(ANONYMOUS_USERS));
}
public static class NameCheck implements StartupCheck {
diff --git a/java/com/google/gerrit/server/group/testing/TestGroupBackend.java b/java/com/google/gerrit/server/group/testing/TestGroupBackend.java
index 35f18a2..601ac59 100644
--- a/java/com/google/gerrit/server/group/testing/TestGroupBackend.java
+++ b/java/com/google/gerrit/server/group/testing/TestGroupBackend.java
@@ -22,7 +22,7 @@
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.entities.GroupReference;
-import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.project.ProjectState;
@@ -122,7 +122,10 @@
}
@Override
- public GroupMembership membershipsOf(IdentifiedUser user) {
+ public GroupMembership membershipsOf(CurrentUser user) {
+ if (!user.isIdentifiedUser()) {
+ return GroupMembership.EMPTY;
+ }
return memberships.getOrDefault(user.getAccountId(), GroupMembership.EMPTY);
}
diff --git a/java/com/google/gerrit/server/query/change/ExternalUser.java b/java/com/google/gerrit/server/query/change/ExternalUser.java
new file mode 100644
index 0000000..f853a5c
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/ExternalUser.java
@@ -0,0 +1,77 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.query.change;
+
+import static com.google.common.flogger.LazyArgs.lazy;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.GroupBackend;
+import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import java.util.Collection;
+
+/**
+ * Represents a user that does not have a Gerrit account.
+ *
+ * <p>This user is limited in what they can do on Gerrit. For now, we only guarantee that permission
+ * checking - including ref filtering works.
+ *
+ * <p>This class is thread-safe.
+ */
+public class ExternalUser extends CurrentUser {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ public interface Factory {
+ ExternalUser create(Collection<ExternalId.Key> externalIdKeys);
+ }
+
+ private final GroupBackend groupBackend;
+ private final ImmutableSet<ExternalId.Key> externalIdKeys;
+
+ private GroupMembership effectiveGroups;
+
+ @Inject
+ public ExternalUser(
+ GroupBackend groupBackend, @Assisted Collection<ExternalId.Key> externalIdKeys) {
+ this.groupBackend = groupBackend;
+ this.externalIdKeys = ImmutableSet.copyOf(externalIdKeys);
+ }
+
+ @Override
+ public ImmutableSet<ExternalId.Key> getExternalIdKeys() {
+ return externalIdKeys;
+ }
+
+ @Override
+ public GroupMembership getEffectiveGroups() {
+ synchronized (this) {
+ if (effectiveGroups == null) {
+ effectiveGroups = groupBackend.membershipsOf(this);
+ logger.atFinest().log(
+ "Known groups of %s: %s", getLoggableName(), lazy(effectiveGroups::getKnownGroups));
+ }
+ }
+ return effectiveGroups;
+ }
+
+ @Override
+ public Object getCacheKey() {
+ return this; // Caching is tied to this exact instance.
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/GroupBackedUser.java b/java/com/google/gerrit/server/query/change/GroupBackedUser.java
index 3960813..d0d5735 100644
--- a/java/com/google/gerrit/server/query/change/GroupBackedUser.java
+++ b/java/com/google/gerrit/server/query/change/GroupBackedUser.java
@@ -23,21 +23,13 @@
/**
* Representation of a user that does not have a Gerrit account.
*
- * <p>This user representation is intended to be used for two purposes:
+ * <p>This user representation is intended to be used to check permissions for groups:
*
- * <ol>
- * <li>Checking permissions for groups: There are occasions where we need to check if a resource -
- * such as a change - is accessible by a group. Our entire {@link
- * com.google.gerrit.server.permissions.PermissionBackend} works solely with {@link
- * CurrentUser}. This class can be used to check permissions on a synthetic user with the
- * given group memberships. Any real Gerrit user with the same group memberships would receive
- * the same permission check results.
- * <li>Checking permissions for an external user: In installations with external group systems,
- * one might want to check what Gerrit permissions a user has, before or even without creating
- * a Gerrit account. Such an external user has external group memberships only as well as
- * internal groups that contain the user's external groups as subgroups. This class can be
- * used to represent such an external user.
- * </ol>
+ * <p>There are occasions where we need to check if a resource - such as a change - is accessible by
+ * a group. Our entire {@link com.google.gerrit.server.permissions.PermissionBackend} works solely
+ * with {@link CurrentUser}. This class can be used to check permissions on a synthetic user with
+ * the given group memberships. Any real Gerrit user with the same group memberships would receive
+ * the same permission check results.
*/
public final class GroupBackedUser extends CurrentUser {
private final GroupMembership groups;
diff --git a/java/com/google/gerrit/server/restapi/group/CreateGroup.java b/java/com/google/gerrit/server/restapi/group/CreateGroup.java
index 74ca721..613c805 100644
--- a/java/com/google/gerrit/server/restapi/group/CreateGroup.java
+++ b/java/com/google/gerrit/server/restapi/group/CreateGroup.java
@@ -54,6 +54,7 @@
import com.google.gerrit.server.notedb.Sequences;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.plugincontext.PluginSetContext;
+import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.server.validators.GroupCreationValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.inject.Inject;
@@ -66,6 +67,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Optional;
+import java.util.TimeZone;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
@@ -75,7 +77,7 @@
public class CreateGroup
implements RestCollectionCreateView<TopLevelResource, GroupResource, GroupInput> {
private final Provider<IdentifiedUser> self;
- private final PersonIdent serverIdent;
+ private final TimeZone serverTimeZone;
private final Provider<GroupsUpdate> groupsUpdateProvider;
private final GroupCache groupCache;
private final GroupResolver groups;
@@ -89,7 +91,7 @@
@Inject
CreateGroup(
Provider<IdentifiedUser> self,
- @GerritPersonIdent PersonIdent serverIdent,
+ @GerritPersonIdent Provider<PersonIdent> serverIdent,
@UserInitiated Provider<GroupsUpdate> groupsUpdateProvider,
GroupCache groupCache,
GroupResolver groups,
@@ -100,7 +102,7 @@
@GerritServerConfig Config cfg,
Sequences sequences) {
this.self = self;
- this.serverIdent = serverIdent;
+ this.serverTimeZone = serverIdent.get().getTimeZone();
this.groupsUpdateProvider = groupsUpdateProvider;
this.groupCache = groupCache;
this.groups = groups;
@@ -210,7 +212,7 @@
createGroupArgs.uuid,
GroupUuid.make(
createGroupArgs.getGroupName(),
- self.get().newCommitterIdent(serverIdent.getWhen(), serverIdent.getTimeZone())));
+ self.get().newCommitterIdent(TimeUtil.nowTs(), serverTimeZone)));
InternalGroupCreation groupCreation =
InternalGroupCreation.builder()
.setGroupUUID(uuid)
diff --git a/javatests/com/google/gerrit/acceptance/server/permissions/ExternalUserPermissionIT.java b/javatests/com/google/gerrit/acceptance/server/permissions/ExternalUserPermissionIT.java
new file mode 100644
index 0000000..fc7ab2f
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/permissions/ExternalUserPermissionIT.java
@@ -0,0 +1,298 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.server.permissions;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.permissionKey;
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.GroupDescription;
+import com.google.gerrit.entities.GroupReference;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Permission;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.GroupBackend;
+import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.query.change.ExternalUser;
+import com.google.inject.AbstractModule;
+import com.google.inject.Module;
+import java.util.Collection;
+import java.util.Set;
+import javax.inject.Inject;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Test;
+
+/** Tests that permission logic used by {@link ExternalUser} works as expected. */
+public class ExternalUserPermissionIT extends AbstractDaemonTest {
+ private static final AccountGroup.UUID EXTERNAL_GROUP =
+ AccountGroup.uuid("company-auth:it-department");
+
+ @Inject private ChangeOperations changeOperations;
+ @Inject private ProjectOperations projectOperations;
+ @Inject private PermissionBackend permissionBackend;
+ @Inject private ChangeNotes.Factory changeNotesFactory;
+ @Inject private ExternalUser.Factory externalUserFactory;
+ @Inject private GroupOperations groupOperations;
+
+ @Before
+ public void setUp() {
+ // Allow only read on refs/heads/master by default
+ projectOperations
+ .project(allProjects)
+ .forUpdate()
+ .remove(permissionKey(Permission.READ).ref("refs/heads/*").group(ANONYMOUS_USERS))
+ .add(allow(Permission.READ).ref("refs/heads/master").group(ANONYMOUS_USERS))
+ .update();
+ }
+
+ @Override
+ public Module createModule() {
+ /**
+ * Binding a {@link GroupBackend} that pretends a user is part of a group if the external ID
+ * starts with the group UUID.
+ *
+ * <p>Example: Users "company-auth:it-department-1" and "company-auth:it-department-2" are a
+ * member of the group "company-auth:it-department"
+ */
+ return new AbstractModule() {
+ @Override
+ protected void configure() {
+ DynamicSet.bind(binder(), GroupBackend.class)
+ .toInstance(
+ new GroupBackend() {
+ @Override
+ public boolean handles(AccountGroup.UUID uuid) {
+ return uuid.get().startsWith("company-auth:");
+ }
+
+ @Override
+ public GroupDescription.Basic get(AccountGroup.UUID uuid) {
+ return new GroupDescription.Basic() {
+ @Override
+ public AccountGroup.UUID getGroupUUID() {
+ return uuid;
+ }
+
+ @Override
+ public String getName() {
+ return uuid.get();
+ }
+
+ @Override
+ public String getEmailAddress() {
+ return uuid.get() + "@example.com";
+ }
+
+ @Override
+ public String getUrl() {
+ return null;
+ }
+ };
+ }
+
+ @Override
+ public Collection<GroupReference> suggest(String name, ProjectState project) {
+ throw new UnsupportedOperationException("not implemented");
+ }
+
+ @Override
+ public GroupMembership membershipsOf(CurrentUser user) {
+ return new GroupMembership() {
+ @Override
+ public boolean contains(AccountGroup.UUID groupId) {
+ return user.getExternalIdKeys().stream()
+ .anyMatch(e -> e.get().startsWith(groupId.get()));
+ }
+
+ @Override
+ public boolean containsAnyOf(Iterable<AccountGroup.UUID> groupIds) {
+ return ImmutableList.copyOf(groupIds).stream().anyMatch(g -> contains(g));
+ }
+
+ @Override
+ public Set<AccountGroup.UUID> intersection(
+ Iterable<AccountGroup.UUID> groupIds) {
+ return ImmutableList.copyOf(groupIds).stream()
+ .filter(g -> contains(g))
+ .collect(toImmutableSet());
+ }
+
+ @Override
+ public Set<AccountGroup.UUID> getKnownGroups() {
+ return ImmutableSet.of();
+ }
+ };
+ }
+
+ @Override
+ public boolean isVisibleToAll(AccountGroup.UUID uuid) {
+ return false;
+ }
+ });
+ }
+ };
+ }
+
+ @Test
+ public void defaultRefFilter_changeVisibilityIsAgnosticOfProvidedGroups() throws Exception {
+ ExternalUser user = createUserInGroup("1", "it-department");
+
+ Change.Id changeOnMaster = changeOperations.newChange().project(project).create();
+ Change.Id changeOnRefsMetaConfig =
+ changeOperations.newChange().project(project).branch("refs/meta/config").create();
+ // Check that only the change on the default branch is visible
+ assertThat(getVisibleRefNames(user))
+ .containsExactly(
+ "HEAD",
+ "refs/heads/master",
+ RefNames.changeMetaRef(changeOnMaster),
+ RefNames.patchSetRef(PatchSet.id(changeOnMaster, 1)));
+ // Grant access
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/meta/config").group(EXTERNAL_GROUP))
+ .update();
+ // Check that both changes are visible now
+ assertThat(getVisibleRefNames(user))
+ .containsExactly(
+ "HEAD",
+ "refs/heads/master",
+ "refs/meta/config",
+ RefNames.changeMetaRef(changeOnMaster),
+ RefNames.patchSetRef(PatchSet.id(changeOnMaster, 1)),
+ RefNames.changeMetaRef(changeOnRefsMetaConfig),
+ RefNames.patchSetRef(PatchSet.id(changeOnRefsMetaConfig, 1)));
+ }
+
+ @Test
+ public void defaultRefFilter_refVisibilityIsAgnosticOfProvidedGroups() throws Exception {
+ ExternalUser user = createUserInGroup("1", "it-department");
+ // Check that refs/meta/config isn't visible by default
+ assertThat(getVisibleRefNames(user)).containsExactly("HEAD", "refs/heads/master");
+ // Grant access
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref("refs/meta/config").group(EXTERNAL_GROUP))
+ .update();
+ // Check that refs/meta/config became visible
+ assertThat(getVisibleRefNames(user))
+ .containsExactly("HEAD", "refs/heads/master", "refs/meta/config");
+ }
+
+ @Test
+ public void changeVisibility_changeOnInvisibleBranchNotVisible() throws Exception {
+ // Create a change that is not visible to members of 'externalGroup'
+ Change.Id invisibleChange =
+ changeOperations.newChange().project(project).branch("refs/meta/config").create();
+ ExternalUser user = createUserInGroup("1", "it-department");
+ AuthException thrown =
+ assertThrows(
+ AuthException.class,
+ () ->
+ permissionBackend
+ .user(user)
+ .change(changeNotesFactory.create(project, invisibleChange))
+ .check(ChangePermission.READ));
+ assertThat(thrown).hasMessageThat().isEqualTo("read not permitted");
+ }
+
+ @Test
+ public void changeVisibility_changeOnBranchVisibleToAnonymousIsVisible() throws Exception {
+ Change.Id changeId = changeOperations.newChange().project(project).create();
+ ExternalUser user = createUserInGroup("1", "it-department");
+ permissionBackend
+ .user(user)
+ .change(changeNotesFactory.create(project, changeId))
+ .check(ChangePermission.READ);
+ }
+
+ @Test
+ public void changeVisibility_changeOnBranchVisibleToRegisteredUsersIsVisible() throws Exception {
+ Change.Id changeId = changeOperations.newChange().project(project).create();
+ ExternalUser user = createUserInGroup("1", "it-department");
+ blockAnonymousRead();
+ permissionBackend
+ .user(user)
+ .change(changeNotesFactory.create(project, changeId))
+ .check(ChangePermission.READ);
+ }
+
+ @Test
+ public void externalUser_isContainedInternalGroupThatContainsExternalGroup() {
+ AccountGroup.UUID internalGroup =
+ groupOperations.newGroup().addSubgroup(EXTERNAL_GROUP).create();
+ ExternalUser user = createUserInGroup("1", "it-department");
+ assertThat(user.getEffectiveGroups().contains(internalGroup)).isTrue();
+ assertThat(user.getEffectiveGroups().contains(EXTERNAL_GROUP)).isTrue();
+ assertThat(user.getEffectiveGroups().contains(REGISTERED_USERS)).isTrue();
+ assertThat(user.getEffectiveGroups().contains(ANONYMOUS_USERS)).isTrue();
+ }
+
+ @GerritConfig(name = "groups.includeExternalUsersInRegisteredUsersGroup", value = "true")
+ @Test
+ public void externalUser_isContainedInRegisteredUsersIfConfigured() {
+ ExternalUser user = createUserInGroup("1", "it-department");
+ assertThat(user.getEffectiveGroups().contains(REGISTERED_USERS)).isTrue();
+ }
+
+ @GerritConfig(name = "groups.includeExternalUsersInRegisteredUsersGroup", value = "false")
+ @Test
+ public void externalUser_isNotContainedInRegisteredUsersIfNotConfigured() {
+ ExternalUser user = createUserInGroup("1", "it-department");
+ assertThat(user.getEffectiveGroups().contains(REGISTERED_USERS)).isFalse();
+ }
+
+ private ImmutableList<String> getVisibleRefNames(CurrentUser user) throws Exception {
+ try (Repository repo = repoManager.openRepository(project)) {
+ return permissionBackend.user(user).project(project)
+ .filter(
+ repo.getRefDatabase().getRefs(), repo, PermissionBackend.RefFilterOptions.defaults())
+ .stream()
+ .map(Ref::getName)
+ .collect(toImmutableList());
+ }
+ }
+
+ ExternalUser createUserInGroup(String userId, String groupId) {
+ return externalUserFactory.create(
+ ImmutableSet.of(ExternalId.Key.parse("company-auth:" + groupId + "-" + userId)));
+ }
+}
diff --git a/lib/nongoogle_test.sh b/lib/nongoogle_test.sh
index e6b744c..8a6c282 100755
--- a/lib/nongoogle_test.sh
+++ b/lib/nongoogle_test.sh
@@ -23,6 +23,9 @@
flogger-log4j-backend
flogger-system-backend
guava
+guice-assistedinject
+guice-library-no-aop
+guice-servlet
httpasyncclient
httpcore-nio
j2objc
diff --git a/plugins/singleusergroup b/plugins/singleusergroup
index 58ee52a..a0c53c6 160000
--- a/plugins/singleusergroup
+++ b/plugins/singleusergroup
@@ -1 +1 @@
-Subproject commit 58ee52a8670e38f30785bfbb648ba27c61c3a202
+Subproject commit a0c53c6c5ad1ba8f8967ed6d2bcb18995f734cad
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index a5b1626..af5a900 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -1,4 +1,5 @@
load("//tools/bzl:maven_jar.bzl", "maven_jar")
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive", "http_file")
GUAVA_VERSION = "29.0-jre"
GUAVA_BIN_SHA1 = "801142b4c3d0f0770dd29abea50906cacfddd447"
@@ -145,6 +146,36 @@
sha1 = GUAVA_BIN_SHA1,
)
+ GUICE_VERS = "4.2.3"
+
+ GUICE_LIBRARY_SHA256 = "5168f5e7383f978c1b4154ac777b78edd8ac214bb9f9afdb92921c8d156483d3"
+
+ http_file(
+ name = "guice-library-no-aop",
+ canonical_id = "guice-library-no-aop-" + GUICE_VERS + ".jar-" + GUICE_LIBRARY_SHA256,
+ downloaded_file_path = "guice-library-no-aop.jar",
+ sha256 = GUICE_LIBRARY_SHA256,
+ urls = [
+ "https://repo1.maven.org/maven2/com/google/inject/guice/" +
+ GUICE_VERS +
+ "/guice-" +
+ GUICE_VERS +
+ "-no_aop.jar",
+ ],
+ )
+
+ maven_jar(
+ name = "guice-assistedinject",
+ artifact = "com.google.inject.extensions:guice-assistedinject:" + GUICE_VERS,
+ sha1 = "acbfddc556ee9496293ed1df250cc378f331d854",
+ )
+
+ maven_jar(
+ name = "guice-servlet",
+ artifact = "com.google.inject.extensions:guice-servlet:" + GUICE_VERS,
+ sha1 = "8d6e7e35eac4fb5e7df19c55b3bc23fa51b10a11",
+ )
+
# Test-only dependencies below.
maven_jar(
