Allow to suppress peer IP in reflog records
This change adds new configuration option to suppress peer IP address
in reflog entry. For backwards compatibility, this option is enabled
per default, but the default can be swapped in future gerrit releases.
Bug: Issue 10810
Change-Id: I5645a4e68fb48ad155609573aea2518480074697
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 6c785f8..a144a67 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2296,6 +2296,12 @@
By default unset, meaning no bug report URL will be displayed. Administrators
should set this to the URL of their issue tracker, if necessary.
+[[gerrit.enablePeerIPInReflogRecord]]gerrit.enablePeerIPInReflogRecord::
+
+Record actual peer IP address in ref log entry for identified user.
+
+Defaults to true.
+
[[gerrit.secureStoreClass]]gerrit.secureStoreClass::
+
Use the secure store implementation from a specified class.
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 0e6bb29..508f96c 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -53,6 +53,8 @@
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.DefaultPreferencesCacheImpl;
import com.google.gerrit.server.config.DefaultUrlFormatter;
+import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
+import com.google.gerrit.server.config.EnablePeerIPInReflogRecordProvider;
import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
import com.google.gerrit.server.config.SysExecutorModule;
@@ -133,6 +135,10 @@
bind(String.class)
.annotatedWith(CanonicalWebUrl.class)
.toProvider(CanonicalWebUrlProvider.class);
+ bind(Boolean.class)
+ .annotatedWith(EnablePeerIPInReflogRecord.class)
+ .toProvider(EnablePeerIPInReflogRecordProvider.class)
+ .in(SINGLETON);
bind(Realm.class).to(FakeRealm.class);
bind(IdentifiedUser.class).toProvider(Providers.of(null));
bind(ReplacePatchSetSender.Factory.class).toProvider(Providers.of(null));
diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java
index 3968fc0..24ea9d2 100644
--- a/java/com/google/gerrit/server/IdentifiedUser.java
+++ b/java/com/google/gerrit/server/IdentifiedUser.java
@@ -36,6 +36,7 @@
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.inject.Inject;
import com.google.inject.OutOfScopeException;
@@ -68,6 +69,7 @@
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
+ private final Boolean enablePeerIPInReflogRecord;
@Inject
public GenericFactory(
@@ -75,6 +77,7 @@
Realm realm,
@AnonymousCowardName String anonymousCowardName,
@CanonicalWebUrl Provider<String> canonicalUrl,
+ @EnablePeerIPInReflogRecord Boolean enablePeerIPInReflogRecord,
AccountCache accountCache,
GroupBackend groupBackend) {
this.authConfig = authConfig;
@@ -83,6 +86,7 @@
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
+ this.enablePeerIPInReflogRecord = enablePeerIPInReflogRecord;
}
public IdentifiedUser create(AccountState state) {
@@ -93,6 +97,7 @@
canonicalUrl,
accountCache,
groupBackend,
+ enablePeerIPInReflogRecord,
Providers.of(null),
state,
null);
@@ -129,6 +134,7 @@
canonicalUrl,
accountCache,
groupBackend,
+ enablePeerIPInReflogRecord,
Providers.of(remotePeer),
id,
caller,
@@ -150,6 +156,7 @@
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
+ private final Boolean enablePeerIPInReflogRecord;
private final Provider<SocketAddress> remotePeerProvider;
@Inject
@@ -160,6 +167,7 @@
@CanonicalWebUrl Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
+ @EnablePeerIPInReflogRecord Boolean enablePeerIPInReflogRecord,
@RemotePeer Provider<SocketAddress> remotePeerProvider) {
this.authConfig = authConfig;
this.realm = realm;
@@ -167,6 +175,7 @@
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
+ this.enablePeerIPInReflogRecord = enablePeerIPInReflogRecord;
this.remotePeerProvider = remotePeerProvider;
}
@@ -182,6 +191,7 @@
canonicalUrl,
accountCache,
groupBackend,
+ enablePeerIPInReflogRecord,
remotePeerProvider,
id,
null,
@@ -196,6 +206,7 @@
canonicalUrl,
accountCache,
groupBackend,
+ enablePeerIPInReflogRecord,
remotePeerProvider,
id,
caller,
@@ -213,6 +224,7 @@
private final Realm realm;
private final GroupBackend groupBackend;
private final String anonymousCowardName;
+ 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.
@@ -231,6 +243,7 @@
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
+ Boolean enablePeerIPInReflogRecord,
@Nullable Provider<SocketAddress> remotePeerProvider,
AccountState state,
@Nullable CurrentUser realUser) {
@@ -241,6 +254,7 @@
canonicalUrl,
accountCache,
groupBackend,
+ enablePeerIPInReflogRecord,
remotePeerProvider,
state.account().id(),
realUser,
@@ -255,6 +269,7 @@
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
+ Boolean enablePeerIPInReflogRecord,
@Nullable Provider<SocketAddress> remotePeerProvider,
Account.Id id,
@Nullable CurrentUser realUser,
@@ -266,6 +281,7 @@
this.authConfig = authConfig;
this.realm = realm;
this.anonymousCowardName = anonymousCowardName;
+ this.enablePeerIPInReflogRecord = enablePeerIPInReflogRecord;
this.remotePeerProvider = remotePeerProvider;
this.accountId = id;
this.realUser = realUser != null ? realUser : this;
@@ -425,8 +441,20 @@
name = anonymousCowardName;
}
- String user = getUserName().orElse("") + "|account-" + ua.id().toString();
- return new PersonIdent(name, user + "@" + guessHost(), when, tz);
+ 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, tz);
+ }
+
+ private String constructMailAddress(Account ua, String host) {
+ return getUserName().orElse("") + "|account-" + ua.id().toString() + "@" + host;
}
public PersonIdent newCommitterIdent(Date when, TimeZone tz) {
@@ -503,6 +531,7 @@
Providers.of(canonicalUrl.get()),
accountCache,
groupBackend,
+ enablePeerIPInReflogRecord,
remotePeer,
state,
realUser);
diff --git a/java/com/google/gerrit/server/config/EnablePeerIPInReflogRecord.java b/java/com/google/gerrit/server/config/EnablePeerIPInReflogRecord.java
new file mode 100644
index 0000000..708bb15
--- /dev/null
+++ b/java/com/google/gerrit/server/config/EnablePeerIPInReflogRecord.java
@@ -0,0 +1,24 @@
+// Copyright (C) 2021 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.config;
+
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import com.google.inject.BindingAnnotation;
+import java.lang.annotation.Retention;
+
+@Retention(RUNTIME)
+@BindingAnnotation
+public @interface EnablePeerIPInReflogRecord {}
diff --git a/java/com/google/gerrit/server/config/EnablePeerIPInReflogRecordProvider.java b/java/com/google/gerrit/server/config/EnablePeerIPInReflogRecordProvider.java
new file mode 100644
index 0000000..07c1b4e
--- /dev/null
+++ b/java/com/google/gerrit/server/config/EnablePeerIPInReflogRecordProvider.java
@@ -0,0 +1,34 @@
+// Copyright (C) 2021 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.config;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import org.eclipse.jgit.lib.Config;
+
+public class EnablePeerIPInReflogRecordProvider implements Provider<Boolean> {
+ private final Boolean enablePeerIPInReflogRecord;
+
+ @Inject
+ EnablePeerIPInReflogRecordProvider(@GerritServerConfig Config config) {
+ enablePeerIPInReflogRecord =
+ config.getBoolean("gerrit", null, "enablePeerIPInReflogRecord", true);
+ }
+
+ @Override
+ public Boolean get() {
+ return enablePeerIPInReflogRecord;
+ }
+}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index b2f6d17..bb851e2 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -309,6 +309,10 @@
bind(SoySauce.class).annotatedWith(MailTemplates.class).toProvider(MailSoySauceProvider.class);
bind(FromAddressGenerator.class).toProvider(FromAddressGeneratorProvider.class).in(SINGLETON);
+ bind(Boolean.class)
+ .annotatedWith(EnablePeerIPInReflogRecord.class)
+ .toProvider(EnablePeerIPInReflogRecordProvider.class)
+ .in(SINGLETON);
bind(PatchSetInfoFactory.class);
bind(IdentifiedUser.GenericFactory.class).in(SINGLETON);
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
index d9cda26..8feac20 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -55,6 +56,7 @@
public class EmailIT extends AbstractDaemonTest {
@Inject private @AnonymousCowardName String anonymousCowardName;
@Inject private @CanonicalWebUrl Provider<String> canonicalUrl;
+ @Inject private @EnablePeerIPInReflogRecord boolean enablePeerIPInReflogRecord;
@Inject private @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider;
@Inject private AuthConfig authConfig;
@Inject private EmailExpander emailExpander;
@@ -274,7 +276,13 @@
private Context createContextWithCustomRealm(Realm realm) {
IdentifiedUser.GenericFactory userFactory =
new IdentifiedUser.GenericFactory(
- authConfig, realm, anonymousCowardName, canonicalUrl, accountCache, groupBackend);
+ authConfig,
+ realm,
+ anonymousCowardName,
+ canonicalUrl,
+ enablePeerIPInReflogRecord,
+ accountCache,
+ groupBackend);
return atrScope.set(atrScope.newContext(null, userFactory.create(admin.id())));
}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/ReflogIT.java b/javatests/com/google/gerrit/acceptance/server/project/ReflogIT.java
index 127f34b..a927ea4 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/ReflogIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/ReflogIT.java
@@ -23,6 +23,7 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.entities.AccountGroup;
@@ -65,6 +66,44 @@
}
@Test
+ public void peerIPIncludedInReflogRecord() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+
+ try (Repository repo = repoManager.openRepository(r.getChange().project())) {
+ File log = new File(repo.getDirectory(), "logs/" + changeMetaRef(id));
+ if (!log.exists()) {
+ log.getParentFile().mkdirs();
+ assertThat(log.createNewFile()).isTrue();
+ }
+
+ gApi.changes().id(id.get()).topic("foo");
+ ReflogEntry last = repo.getReflogReader(changeMetaRef(id)).getLastEntry();
+ assertThat(last.getWho().getEmailAddress())
+ .isEqualTo(admin.username() + "|account-" + admin.id() + "@unknown");
+ }
+ }
+
+ @Test
+ @GerritConfig(name = "gerrit.enablePeerIPInReflogRecord", value = "false")
+ public void emaiIncludedInReflogRecord() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+
+ try (Repository repo = repoManager.openRepository(r.getChange().project())) {
+ File log = new File(repo.getDirectory(), "logs/" + changeMetaRef(id));
+ if (!log.exists()) {
+ log.getParentFile().mkdirs();
+ assertThat(log.createNewFile()).isTrue();
+ }
+
+ gApi.changes().id(id.get()).topic("foo");
+ ReflogEntry last = repo.getReflogReader(changeMetaRef(id)).getLastEntry();
+ assertThat(last.getWho().getEmailAddress()).isEqualTo(admin.email());
+ }
+ }
+
+ @Test
public void reflogUpdatedBySubmittingChange() throws Exception {
BranchApi branchApi = gApi.projects().name(project.get()).branch("master");
List<ReflogEntryInfo> reflog = branchApi.reflog();
diff --git a/javatests/com/google/gerrit/server/IdentifiedUserTest.java b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
index 69bf3f3..217bec1 100644
--- a/javatests/com/google/gerrit/server/IdentifiedUserTest.java
+++ b/javatests/com/google/gerrit/server/IdentifiedUserTest.java
@@ -25,6 +25,7 @@
import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.util.time.TimeUtil;
@@ -78,6 +79,9 @@
new AbstractModule() {
@Override
protected void configure() {
+ bind(Boolean.class)
+ .annotatedWith(EnablePeerIPInReflogRecord.class)
+ .toInstance(Boolean.TRUE);
bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(config);
bind(String.class)
.annotatedWith(AnonymousCowardName.class)
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index d399ed4..94b6a19 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -45,6 +45,7 @@
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.DefaultUrlFormatter;
+import com.google.gerrit.server.config.EnablePeerIPInReflogRecord;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
@@ -151,6 +152,9 @@
bind(String.class)
.annotatedWith(CanonicalWebUrl.class)
.toInstance("http://localhost:8080/");
+ bind(Boolean.class)
+ .annotatedWith(EnablePeerIPInReflogRecord.class)
+ .toInstance(Boolean.TRUE);
bind(Realm.class).to(FakeRealm.class);
bind(GroupBackend.class).to(SystemGroupBackend.class).in(SINGLETON);
bind(AccountCache.class).toInstance(accountCache);
