Allow to bind a different implementation for creating reflog identities
In the reflog we record the identity of the user that updates the ref or
the user on whom's behalf the ref is updated.
By default the recorded identity consists out of the user's fullname and
the user's preferred email. This means when a user changes their
preferred email it changes their reflog identity. At Google we prefer to
keep the recorded reflog identity stable and always record the email
that matches the user's primary Google identity, regardless of which
email they have configured as preferred.
Release-Notes: skip
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Iddd712c256ae677aea413b4523b53ce1d31c111c
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index df2c5cb..7293f35 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -46,6 +46,7 @@
import com.google.gerrit.lucene.LuceneIndexModule;
import com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker;
import com.google.gerrit.pgm.util.LogFileCompressor.LogFileCompressorModule;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.LibModuleLoader;
import com.google.gerrit.server.LibModuleType;
import com.google.gerrit.server.ModuleOverloader;
@@ -310,6 +311,7 @@
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
modules.add(new GerritApiModule());
modules.add(new ProjectQueryBuilderModule());
+ modules.add(new DefaultRefLogIdentityProvider.Module());
modules.add(new PluginApiModule());
modules.add(new SearchingChangeCacheImplModule());
modules.add(new InternalAccountDirectoryModule());
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 0342fe5..845cc9a 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -55,6 +55,7 @@
import com.google.gerrit.pgm.util.LogFileCompressor.LogFileCompressorModule;
import com.google.gerrit.pgm.util.RuntimeShutdown;
import com.google.gerrit.pgm.util.SiteProgram;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.LibModuleLoader;
import com.google.gerrit.server.LibModuleType;
import com.google.gerrit.server.ModuleOverloader;
@@ -448,6 +449,7 @@
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
modules.add(new GerritApiModule());
modules.add(new ProjectQueryBuilderModule());
+ modules.add(new DefaultRefLogIdentityProvider.Module());
modules.add(new PluginApiModule());
modules.add(new SearchingChangeCacheImplModule(replica));
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 5bffce7..64b1def 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -28,6 +28,7 @@
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.LibModuleLoader;
@@ -127,6 +128,7 @@
modules.add(PatchListCacheImpl.module());
modules.add(new DefaultUrlFormatterModule());
modules.add(DiffOperationsImpl.module());
+ modules.add(new DefaultRefLogIdentityProvider.Module());
// There is the concept of LifecycleModule, in Gerrit's own extension to Guice, which has these:
// listener().to(SomeClassImplementingLifecycleListener.class);
diff --git a/java/com/google/gerrit/server/DefaultRefLogIdentityProvider.java b/java/com/google/gerrit/server/DefaultRefLogIdentityProvider.java
new file mode 100644
index 0000000..bef276a
--- /dev/null
+++ b/java/com/google/gerrit/server/DefaultRefLogIdentityProvider.java
@@ -0,0 +1,97 @@
+// Copyright (C) 2023 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;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.server.config.AnonymousCowardName;
+import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
+import java.time.Instant;
+import java.time.ZoneId;
+import org.eclipse.jgit.lib.PersonIdent;
+
+@Singleton
+public class DefaultRefLogIdentityProvider implements RefLogIdentityProvider {
+ public static class Module extends AbstractModule {
+ @Override
+ protected void configure() {
+ bind(RefLogIdentityProvider.class).to(DefaultRefLogIdentityProvider.class);
+ }
+ }
+
+ private final String anonymousCowardName;
+ private final Boolean enablePeerIPInReflogRecord;
+
+ @Inject
+ DefaultRefLogIdentityProvider(
+ @AnonymousCowardName String anonymousCowardName,
+ @EnablePeerIPInReflogRecord Boolean enablePeerIPInReflogRecord) {
+ this.anonymousCowardName = anonymousCowardName;
+ this.enablePeerIPInReflogRecord = enablePeerIPInReflogRecord;
+ }
+
+ @Override
+ public PersonIdent newRefLogIdent(IdentifiedUser user, Instant when, ZoneId zoneId) {
+ Account account = user.getAccount();
+
+ String name = account.fullName();
+ if (name == null || name.isEmpty()) {
+ name = account.preferredEmail();
+ }
+ if (name == null || name.isEmpty()) {
+ name = anonymousCowardName;
+ }
+
+ String email;
+ if (enablePeerIPInReflogRecord) {
+ email = constructMailAddress(user, guessHost(user));
+ } else {
+ email =
+ Strings.isNullOrEmpty(account.preferredEmail())
+ ? constructMailAddress(user, "unknown")
+ : account.preferredEmail();
+ }
+
+ return new PersonIdent(name, email, when, zoneId);
+ }
+
+ private String constructMailAddress(IdentifiedUser user, String host) {
+ return user.getUserName().orElse("")
+ + "|account-"
+ + user.getAccountId().toString()
+ + "@"
+ + host;
+ }
+
+ private String guessHost(IdentifiedUser user) {
+ String host = null;
+ SocketAddress remotePeer = user.getRemotePeer();
+ if (remotePeer instanceof InetSocketAddress) {
+ InetSocketAddress sa = (InetSocketAddress) remotePeer;
+ InetAddress in = sa.getAddress();
+ host = in != null ? in.getHostAddress() : sa.getHostName();
+ }
+ if (Strings.isNullOrEmpty(host)) {
+ return "unknown";
+ }
+ return host;
+ }
+}
diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java
index eda6e09..36d7888 100644
--- a/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/java/com/google/gerrit/server/IdentifiedUser.java
@@ -19,7 +19,6 @@
import static com.google.common.flogger.LazyArgs.lazy;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.common.flogger.FluentLogger;
@@ -44,8 +43,6 @@
import com.google.inject.ProvisionException;
import com.google.inject.Singleton;
import com.google.inject.util.Providers;
-import java.net.InetAddress;
-import java.net.InetSocketAddress;
import java.net.MalformedURLException;
import java.net.SocketAddress;
import java.net.URL;
@@ -66,6 +63,7 @@
private final AuthConfig authConfig;
private final Realm realm;
private final String anonymousCowardName;
+ private final RefLogIdentityProvider refLogIdentityProvider;
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
@@ -76,6 +74,7 @@
AuthConfig authConfig,
Realm realm,
@AnonymousCowardName String anonymousCowardName,
+ RefLogIdentityProvider refLogIdentityProvider,
@CanonicalWebUrl Provider<String> canonicalUrl,
@EnablePeerIPInReflogRecord Boolean enablePeerIPInReflogRecord,
AccountCache accountCache,
@@ -83,6 +82,7 @@
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
+ this.refLogIdentityProvider = refLogIdentityProvider;
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
@@ -94,6 +94,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -131,6 +132,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -153,6 +155,7 @@
private final AuthConfig authConfig;
private final Realm realm;
private final String anonymousCowardName;
+ private final RefLogIdentityProvider refLogIdentityProvider;
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
@@ -164,6 +167,7 @@
AuthConfig authConfig,
Realm realm,
@AnonymousCowardName String anonymousCowardName,
+ RefLogIdentityProvider refLogIdentityProvider,
@CanonicalWebUrl Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
@@ -172,6 +176,7 @@
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
+ this.refLogIdentityProvider = refLogIdentityProvider;
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
@@ -188,6 +193,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -203,6 +209,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -224,6 +231,7 @@
private final Realm realm;
private final GroupBackend groupBackend;
private final String anonymousCowardName;
+ private final RefLogIdentityProvider refLogIdentityProvider;
private final Boolean enablePeerIPInReflogRecord;
private final Set<String> validEmails = Sets.newTreeSet(String.CASE_INSENSITIVE_ORDER);
private final CurrentUser realUser; // Must be final since cached properties depend on it.
@@ -235,11 +243,13 @@
private boolean loadedAllEmails;
private Set<String> invalidEmails;
private GroupMembership effectiveGroups;
+ private PersonIdent refLogIdent;
private IdentifiedUser(
AuthConfig authConfig,
Realm realm,
String anonymousCowardName,
+ RefLogIdentityProvider refLogIdentityProvider,
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
@@ -251,6 +261,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
accountCache,
groupBackend,
@@ -266,6 +277,7 @@
AuthConfig authConfig,
Realm realm,
String anonymousCowardName,
+ RefLogIdentityProvider refLogIdentityProvider,
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
@@ -281,6 +293,7 @@
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
+ this.refLogIdentityProvider = refLogIdentityProvider;
this.enablePeerIPInReflogRecord = enablePeerIPInReflogRecord;
this.remotePeerProvider = remotePeerProvider;
this.accountId = id;
@@ -426,36 +439,27 @@
return getAccountId();
}
+ @Nullable
+ public SocketAddress getRemotePeer() {
+ try {
+ return remotePeerProvider.get();
+ } catch (OutOfScopeException | ProvisionException e) {
+ return null;
+ }
+ }
+
public PersonIdent newRefLogIdent() {
- return newRefLogIdent(Instant.now(), ZoneId.systemDefault());
+ return refLogIdentityProvider.newRefLogIdent(this);
}
public PersonIdent newRefLogIdent(Instant when, ZoneId zoneId) {
- final Account ua = getAccount();
-
- String name = ua.fullName();
- if (name == null || name.isEmpty()) {
- name = ua.preferredEmail();
+ if (refLogIdent != null) {
+ refLogIdent =
+ new PersonIdent(refLogIdent.getName(), refLogIdent.getEmailAddress(), when, zoneId);
+ return refLogIdent;
}
- if (name == null || name.isEmpty()) {
- name = anonymousCowardName;
- }
-
- String user;
- if (enablePeerIPInReflogRecord) {
- user = constructMailAddress(ua, guessHost());
- } else {
- user =
- Strings.isNullOrEmpty(ua.preferredEmail())
- ? constructMailAddress(ua, "unknown")
- : ua.preferredEmail();
- }
-
- return new PersonIdent(name, user, when, zoneId);
- }
-
- private String constructMailAddress(Account ua, String host) {
- return getUserName().orElse("") + "|account-" + ua.id().toString() + "@" + host;
+ refLogIdent = refLogIdentityProvider.newRefLogIdent(this, when, zoneId);
+ return refLogIdent;
}
public PersonIdent newCommitterIdent(PersonIdent ident) {
@@ -533,6 +537,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
Providers.of(canonicalUrl.get()),
accountCache,
groupBackend,
@@ -546,23 +551,4 @@
public boolean hasSameAccountId(CurrentUser other) {
return getAccountId().get() == other.getAccountId().get();
}
-
- private String guessHost() {
- String host = null;
- SocketAddress remotePeer = null;
- try {
- remotePeer = remotePeerProvider.get();
- } catch (OutOfScopeException | ProvisionException e) {
- // Leave null.
- }
- if (remotePeer instanceof InetSocketAddress) {
- InetSocketAddress sa = (InetSocketAddress) remotePeer;
- InetAddress in = sa.getAddress();
- host = in != null ? in.getHostAddress() : sa.getHostName();
- }
- if (Strings.isNullOrEmpty(host)) {
- return "unknown";
- }
- return host;
- }
}
diff --git a/java/com/google/gerrit/server/RefLogIdentityProvider.java b/java/com/google/gerrit/server/RefLogIdentityProvider.java
new file mode 100644
index 0000000..2a5d2b0
--- /dev/null
+++ b/java/com/google/gerrit/server/RefLogIdentityProvider.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2023 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;
+
+import java.time.Instant;
+import java.time.ZoneId;
+import org.eclipse.jgit.lib.PersonIdent;
+
+/**
+ * Extension point that allows to control which identity should be recorded in the reflog for ref
+ * updates done by a user or done on behalf of a user.
+ */
+public interface RefLogIdentityProvider {
+ /**
+ * Creates a {@link PersonIdent} for the given user that should be used as the user identity in
+ * the reflog for ref updates done by this user or done on behalf of this user.
+ *
+ * <p>The returned {@link PersonIdent} is created with the current timestamp and the system
+ * default timezone.
+ *
+ * @param user the user for which a reflog identity should be created
+ */
+ default PersonIdent newRefLogIdent(IdentifiedUser user) {
+ return newRefLogIdent(user, Instant.now(), ZoneId.systemDefault());
+ }
+
+ /**
+ * Creates a {@link PersonIdent} for the given user that should be used as the user identity in
+ * the reflog for ref updates done by this user or done on behalf of this user.
+ *
+ * @param user the user for which a reflog identity should be created
+ * @param when the timestamp that should be used to create the {@link PersonIdent}
+ * @param zoneId the zone ID identifying the timezone that should be used to create the {@link
+ * PersonIdent}
+ */
+ PersonIdent newRefLogIdent(IdentifiedUser user, Instant when, ZoneId zoneId);
+}
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index b828037..936b448 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -36,6 +36,7 @@
import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.CacheRefreshExecutor;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.GerritPersonIdentProvider;
@@ -193,6 +194,7 @@
install(new AuthModule(authConfig));
install(new GerritApiModule());
install(new ProjectQueryBuilderModule());
+ install(new DefaultRefLogIdentityProvider.Module());
factory(PluginUser.Factory.class);
install(new PluginApiModule());
install(new DefaultPermissionBackendModule());
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
index d055875..62ef118 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
@@ -33,6 +33,7 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.RefLogIdentityProvider;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.DefaultRealm;
@@ -57,6 +58,7 @@
public class EmailIT extends AbstractDaemonTest {
@Inject private @AnonymousCowardName String anonymousCowardName;
+ @Inject private RefLogIdentityProvider refLogIdentityProvider;
@Inject private @CanonicalWebUrl Provider<String> canonicalUrl;
@Inject private @EnablePeerIPInReflogRecord boolean enablePeerIPInReflogRecord;
@Inject private @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider;
@@ -283,6 +285,7 @@
authConfig,
realm,
anonymousCowardName,
+ refLogIdentityProvider,
canonicalUrl,
enablePeerIPInReflogRecord,
accountCache,
diff --git a/javatests/com/google/gerrit/server/IdentifiedUserTest.java b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
index 855a0bc..30ae4aa 100644
--- a/javatests/com/google/gerrit/server/IdentifiedUserTest.java
+++ b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
@@ -92,6 +92,7 @@
bind(AccountCache.class).toInstance(accountCache);
bind(GroupBackend.class).to(SystemGroupBackend.class).in(SINGLETON);
bind(Realm.class).toInstance(mockRealm);
+ install(new DefaultRefLogIdentityProvider.Module());
}
};
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index be8f1f9..1e6ba3a 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -31,6 +31,7 @@
import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.DefaultRefLogIdentityProvider;
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -180,6 +181,7 @@
install(new DefaultUrlFormatterModule());
install(NoteDbModule.forTest());
+ install(new DefaultRefLogIdentityProvider.Module());
bind(AllUsersName.class).toProvider(AllUsersNameProvider.class);
bind(String.class).annotatedWith(GerritServerId.class).toInstance(serverId);
bind(new TypeLiteral<ImmutableSet<String>>() {})
