opendev
/
gerrit
Code Issues Proposed changes

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
This commit is contained in:
David Ostrovsky 2021-04-14 20:00:37 +02:00 committed by Luca Milanesio
parent 245e41b353
commit 8e6acc97c2
10 changed files with 161 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
Documentation/config-gerrit.txt
View File

@ -2296,6 +2296,12 @@ URL to direct users to when they need to report a bug.
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.

6
java/com/google/gerrit/pgm/util/BatchProgramModule.java
View File

@ -53,6 +53,8 @@ import com.google.gerrit.server.config.CanonicalWebUrl;
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 @@ public class BatchProgramModule extends FactoryModule {
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));

33
java/com/google/gerrit/server/IdentifiedUser.java
View File

@ -36,6 +36,7 @@ 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;
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 @@ public class IdentifiedUser extends CurrentUser {
private final Provider<String> canonicalUrl;
private final AccountCache accountCache;
private final GroupBackend groupBackend;
private final Boolean enablePeerIPInReflogRecord;
@Inject
public GenericFactory(
@ -75,6 +77,7 @@ public class IdentifiedUser extends CurrentUser {
Realm realm,
@AnonymousCowardName String anonymousCowardName,
@CanonicalWebUrl Provider<String> canonicalUrl,
@EnablePeerIPInReflogRecord Boolean enablePeerIPInReflogRecord,
AccountCache accountCache,
GroupBackend groupBackend) {
this.authConfig = authConfig;
@ -83,6 +86,7 @@ public class IdentifiedUser extends CurrentUser {
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
this.enablePeerIPInReflogRecord = enablePeerIPInReflogRecord;
}
public IdentifiedUser create(AccountState state) {
@ -93,6 +97,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
enablePeerIPInReflogRecord,
Providers.of(null),
state,
null);
@ -129,6 +134,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
enablePeerIPInReflogRecord,
Providers.of(remotePeer),
id,
caller,
@ -150,6 +156,7 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
@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 @@ public class IdentifiedUser extends CurrentUser {
this.canonicalUrl = canonicalUrl;
this.accountCache = accountCache;
this.groupBackend = groupBackend;
this.enablePeerIPInReflogRecord = enablePeerIPInReflogRecord;
this.remotePeerProvider = remotePeerProvider;
}
@ -182,6 +191,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
enablePeerIPInReflogRecord,
remotePeerProvider,
id,
null,
@ -196,6 +206,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
enablePeerIPInReflogRecord,
remotePeerProvider,
id,
caller,
@ -213,6 +224,7 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
Boolean enablePeerIPInReflogRecord,
@Nullable Provider<SocketAddress> remotePeerProvider,
AccountState state,
@Nullable CurrentUser realUser) {
@ -241,6 +254,7 @@ public class IdentifiedUser extends CurrentUser {
canonicalUrl,
accountCache,
groupBackend,
enablePeerIPInReflogRecord,
remotePeerProvider,
state.account().id(),
realUser,
@ -255,6 +269,7 @@ public class IdentifiedUser extends CurrentUser {
Provider<String> canonicalUrl,
AccountCache accountCache,
GroupBackend groupBackend,
Boolean enablePeerIPInReflogRecord,
@Nullable Provider<SocketAddress> remotePeerProvider,
Account.Id id,
@Nullable CurrentUser realUser,
@ -266,6 +281,7 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
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 @@ public class IdentifiedUser extends CurrentUser {
Providers.of(canonicalUrl.get()),
accountCache,
groupBackend,
enablePeerIPInReflogRecord,
remotePeer,
state,
realUser);

24
java/com/google/gerrit/server/config/EnablePeerIPInReflogRecord.java Normal file
View File

@ -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 {}

34
java/com/google/gerrit/server/config/EnablePeerIPInReflogRecordProvider.java Normal file
View File

@ -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;
}
}

4
java/com/google/gerrit/server/config/GerritGlobalModule.java
View File

@ -309,6 +309,10 @@ public class GerritGlobalModule extends FactoryModule {
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);

10
javatests/com/google/gerrit/acceptance/rest/account/EmailIT.java
View File

@ -44,6 +44,7 @@ import com.google.gerrit.server.account.externalids.ExternalIds;
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 @@ import org.junit.Test;
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 @@ public class EmailIT extends AbstractDaemonTest {
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())));
}

39
javatests/com/google/gerrit/acceptance/server/project/ReflogIT.java
View File

@ -23,6 +23,7 @@ import static com.google.gerrit.testing.GerritJUnit.assertThrows;
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;
@ -64,6 +65,44 @@ public class ReflogIT extends AbstractDaemonTest {
}
}
@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");

4
javatests/com/google/gerrit/server/IdentifiedUserTest.java
View File

@ -25,6 +25,7 @@ import com.google.gerrit.server.account.Realm;
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 @@ public class IdentifiedUserTest {
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)

4
javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
View File

@ -45,6 +45,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.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 @@ public abstract class AbstractChangeNotesTest {
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);