opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.16' into stable-3.0

Browse Source 
* stable-2.16:
  Fix eclipse project generation
  Always send an email when adding or deleting an SSH key
  AccountIT: Add test for adding GPG key to another account
  AccountIT: Add tests for modifying SSH keys of another account
  Update rules_closure to latest version
  Use bazelisk to switch between used bazel version
  Allow load labels to cross package boundaries by default

Change-Id: I42abaad38c838772234acc78f2e4de19551fa1e4
This commit is contained in:
David Ostrovsky
2019-06-07 20:36:36 +02:00
parent 443b0674ad 56dfccdf61
commit e2794d533e
7 changed files with 89 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.bazelrc
View File

@@ -4,6 +4,7 @@ build --experimental_strict_action_env
build --action_env=PATH
build --disk_cache=~/.gerritcodereview/bazel-cache/cas
build --java_toolchain //tools:error_prone_warnings_toolchain
build --incompatible_disallow_load_labels_to_cross_package_boundaries=false
test --build_tests_only
test --test_output=errors

1
.bazelversion Normal file
View File

@@ -0,0 +1 @@
0.27.0rc3

6
WORKSPACE
View File

@@ -49,9 +49,9 @@ http_archive(
http_archive(
name = "io_bazel_rules_closure",
sha256 = "75c58680af5f7b938ce9fe2abe8ecd9d24c698d160c0b71a945bd100fa77632b",
strip_prefix = "rules_closure-10cb1a78bd6cc8927eb39c2644c0369934f4aed6",
urls = ["https://github.com/bazelbuild/rules_closure/archive/10cb1a78bd6cc8927eb39c2644c0369934f4aed6.tar.gz"],
sha256 = "d075b084e6f4109d1b1ab877495ac72c1a6c4dbc593980967e0b7359f4254d7e",
strip_prefix = "rules_closure-78f1192664acf66ca1de24116cbcc98e1698f26b",
urls = ["https://github.com/bazelbuild/rules_closure/archive/78f1192664acf66ca1de24116cbcc98e1698f26b.tar.gz"],
)
# File is specific to Polymer and copied from the Closure Github -- should be

37
java/com/google/gerrit/server/mail/send/AddKeySender.java
View File

@@ -17,13 +17,9 @@ package com.google.gerrit.server.mail.send;
import com.google.common.base.Joiner;
import com.google.gerrit.exceptions.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.mail.Address;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountSshKey;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.util.List;
@@ -35,22 +31,14 @@ public class AddKeySender extends OutgoingEmail {
AddKeySender create(IdentifiedUser user, List<String> gpgKey);
}
private final PermissionBackend permissionBackend;
private final IdentifiedUser callingUser;
private final IdentifiedUser user;
private final AccountSshKey sshKey;
private final List<String> gpgKeys;
@AssistedInject
public AddKeySender(
EmailArguments ea,
PermissionBackend permissionBackend,
IdentifiedUser callingUser,
@Assisted IdentifiedUser user,
@Assisted AccountSshKey sshKey) {
EmailArguments ea, @Assisted IdentifiedUser user, @Assisted AccountSshKey sshKey) {
super(ea, "addkey");
this.permissionBackend = permissionBackend;
this.callingUser = callingUser;
this.user = user;
this.sshKey = sshKey;
this.gpgKeys = null;
@@ -58,14 +46,8 @@ public class AddKeySender extends OutgoingEmail {
@AssistedInject
public AddKeySender(
EmailArguments ea,
PermissionBackend permissionBackend,
IdentifiedUser callingUser,
@Assisted IdentifiedUser user,
@Assisted List<String> gpgKeys) {
EmailArguments ea, @Assisted IdentifiedUser user, @Assisted List<String> gpgKeys) {
super(ea, "addkey");
this.permissionBackend = permissionBackend;
this.callingUser = callingUser;
this.user = user;
this.sshKey = null;
this.gpgKeys = gpgKeys;
@@ -85,20 +67,7 @@ public class AddKeySender extends OutgoingEmail {
return false;
}
if (user.equals(callingUser)) {
// Send email if the user self-added a key; this notification is necessary to alert
// the user if their account was compromised and a key was unexpectedly added.
return true;
}
try {
// Don't email if an administrator added a key on behalf of the user.
permissionBackend.user(callingUser).check(GlobalPermission.ADMINISTRATE_SERVER);
return false;
} catch (AuthException | PermissionBackendException e) {
// Send email if a non-administrator modified the keys, e.g. by MODIFY_ACCOUNT.
return true;
}
return true;
}
@Override

37
java/com/google/gerrit/server/mail/send/DeleteKeySender.java
View File

@@ -17,13 +17,9 @@ package com.google.gerrit.server.mail.send;
import com.google.common.base.Joiner;
import com.google.gerrit.exceptions.EmailException;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.mail.Address;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountSshKey;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.util.Collections;
@@ -36,22 +32,14 @@ public class DeleteKeySender extends OutgoingEmail {
DeleteKeySender create(IdentifiedUser user, List<String> gpgKeyFingerprints);
}
private final PermissionBackend permissionBackend;
private final IdentifiedUser callingUser;
private final IdentifiedUser user;
private final AccountSshKey sshKey;
private final List<String> gpgKeyFingerprints;
@AssistedInject
public DeleteKeySender(
EmailArguments ea,
PermissionBackend permissionBackend,
IdentifiedUser callingUser,
@Assisted IdentifiedUser user,
@Assisted AccountSshKey sshKey) {
EmailArguments ea, @Assisted IdentifiedUser user, @Assisted AccountSshKey sshKey) {
super(ea, "deletekey");
this.permissionBackend = permissionBackend;
this.callingUser = callingUser;
this.user = user;
this.gpgKeyFingerprints = Collections.emptyList();
this.sshKey = sshKey;
@@ -59,14 +47,8 @@ public class DeleteKeySender extends OutgoingEmail {
@AssistedInject
public DeleteKeySender(
EmailArguments ea,
PermissionBackend permissionBackend,
IdentifiedUser callingUser,
@Assisted IdentifiedUser user,
@Assisted List<String> gpgKeyFingerprints) {
EmailArguments ea, @Assisted IdentifiedUser user, @Assisted List<String> gpgKeyFingerprints) {
super(ea, "deletekey");
this.permissionBackend = permissionBackend;
this.callingUser = callingUser;
this.user = user;
this.gpgKeyFingerprints = gpgKeyFingerprints;
this.sshKey = null;
@@ -81,20 +63,7 @@ public class DeleteKeySender extends OutgoingEmail {
@Override
protected boolean shouldSendMessage() {
if (user.equals(callingUser)) {
// Send email if the user self-removed a key; this notification is necessary to alert
// the user if their account was compromised and a key was unexpectedly deleted.
return true;
}
try {
// Don't email if an administrator removed a key on behalf of the user.
permissionBackend.user(callingUser).check(GlobalPermission.ADMINISTRATE_SERVER);
return false;
} catch (AuthException | PermissionBackendException e) {
// Send email if a non-administrator modified the keys, e.g. by MODIFY_ACCOUNT.
return true;
}
return true;
}
@Override

80
javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
View File

@@ -1943,6 +1943,17 @@ public class AccountIT extends AbstractDaemonTest {
gApi.accounts().self().gpgKey(id).get();
}
@Test
public void adminCannotAddGpgKeyToOtherAccount() throws Exception {
TestKey key = validKeyWithoutExpiration();
addExternalIdEmail(user, "test1@example.com");
sender.clear();
requestScopeOperations.setApiUser(admin.id());
exception.expect(ResourceNotFoundException.class);
addGpgKey(user, key.getPublicKeyArmored());
}
@Test
public void reAddExistingGpgKey() throws Exception {
addExternalIdEmail(admin, "test5@example.com");
@@ -1985,7 +1996,7 @@ public class AccountIT extends AbstractDaemonTest {
exception.expect(ResourceConflictException.class);
exception.expectMessage("GPG key already associated with another account");
addGpgKey(key.getPublicKeyArmored());
addGpgKey(user, key.getPublicKeyArmored());
}
@Test
@@ -2137,6 +2148,63 @@ public class AccountIT extends AbstractDaemonTest {
accountIndexedCounter.assertReindexOf(admin);
}
@Test
@UseSsh
public void adminCanAddOrRemoveSshKeyOnOtherAccount() throws Exception {
// The test account should initially have exactly one ssh key
List<SshKeyInfo> info = gApi.accounts().self().listSshKeys();
assertThat(info).hasSize(1);
assertSequenceNumbers(info);
SshKeyInfo key = info.get(0);
KeyPair keyPair = sshKeys.getKeyPair(admin);
String initial = TestSshKeys.publicKey(keyPair, admin.email());
assertThat(key.sshPublicKey).isEqualTo(initial);
accountIndexedCounter.assertNoReindex();
// Add a new key
sender.clear();
String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), user.email());
gApi.accounts().id(user.username()).addSshKey(newKey);
info = gApi.accounts().id(user.username()).listSshKeys();
assertThat(info).hasSize(2);
assertSequenceNumbers(info);
accountIndexedCounter.assertReindexOf(user);
assertThat(sender.getMessages()).hasSize(1);
Message message = sender.getMessages().get(0);
assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
assertThat(message.body()).contains("new SSH keys have been added");
// Delete key
sender.clear();
gApi.accounts().id(user.username()).deleteSshKey(1);
info = gApi.accounts().id(user.username()).listSshKeys();
assertThat(info).hasSize(1);
accountIndexedCounter.assertReindexOf(user);
assertThat(sender.getMessages()).hasSize(1);
message = sender.getMessages().get(0);
assertThat(message.rcpt()).containsExactly(user.getEmailAddress());
assertThat(message.body()).contains("SSH keys have been deleted");
}
@Test
@UseSsh
public void userCannotAddSshKeyToOtherAccount() throws Exception {
String newKey = TestSshKeys.publicKey(TestSshKeys.genSshKey(), admin.email());
requestScopeOperations.setApiUser(user.id());
exception.expect(AuthException.class);
gApi.accounts().id(admin.username()).addSshKey(newKey);
}
@Test
@UseSsh
public void userCannotDeleteSshKeyOfOtherAccount() throws Exception {
requestScopeOperations.setApiUser(user.id());
exception.expect(ResourceNotFoundException.class);
gApi.accounts().id(admin.username()).deleteSshKey(0);
}
// reindex is tested by {@link AbstractQueryAccountsTest#reindex}
@Test
public void reindexPermissions() throws Exception {
@@ -3011,9 +3079,15 @@ public class AccountIT extends AbstractDaemonTest {
}
private Map<String, GpgKeyInfo> addGpgKey(String armored) throws Exception {
return addGpgKey(admin, armored);
}
private Map<String, GpgKeyInfo> addGpgKey(TestAccount account, String armored) throws Exception {
Map<String, GpgKeyInfo> gpgKeys =
gApi.accounts().self().putGpgKeys(ImmutableList.of(armored), ImmutableList.of());
accountIndexedCounter.assertReindexOf(gApi.accounts().self().get());
gApi.accounts()
.id(account.username())
.putGpgKeys(ImmutableList.of(armored), ImmutableList.<String>of());
accountIndexedCounter.assertReindexOf(gApi.accounts().id(account.username()).get());
return gpgKeys;
}

1
tools/eclipse/project.py
View File

@@ -202,6 +202,7 @@ def gen_classpath(ext):
# Exceptions: both source and lib
if p.endswith('libquery_parser.jar') or \
p.endswith('libgerrit-prolog-common.jar') or \
p.endswith('com_google_protobuf/libprotobuf_java.jar') or \
p.endswith('lucene-core-and-backward-codecs__merged.jar'):
lib.add(p)
# JGit dependency from external repository