diff --git a/.bazelrc b/.bazelrc index d6d4ce6cd3..8b1abd60f6 100644 --- a/.bazelrc +++ b/.bazelrc @@ -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 diff --git a/.bazelversion b/.bazelversion new file mode 100644 index 0000000000..b2525e321c --- /dev/null +++ b/.bazelversion @@ -0,0 +1 @@ +0.27.0rc3 \ No newline at end of file diff --git a/WORKSPACE b/WORKSPACE index e4a4196047..e8162a4636 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -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 diff --git a/java/com/google/gerrit/server/mail/send/AddKeySender.java b/java/com/google/gerrit/server/mail/send/AddKeySender.java index da866f4ef4..6a4918cf36 100644 --- a/java/com/google/gerrit/server/mail/send/AddKeySender.java +++ b/java/com/google/gerrit/server/mail/send/AddKeySender.java @@ -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 gpgKey); } - private final PermissionBackend permissionBackend; - private final IdentifiedUser callingUser; private final IdentifiedUser user; private final AccountSshKey sshKey; private final List 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 gpgKeys) { + EmailArguments ea, @Assisted IdentifiedUser user, @Assisted List 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 diff --git a/java/com/google/gerrit/server/mail/send/DeleteKeySender.java b/java/com/google/gerrit/server/mail/send/DeleteKeySender.java index b95b59674e..39e21a52a2 100644 --- a/java/com/google/gerrit/server/mail/send/DeleteKeySender.java +++ b/java/com/google/gerrit/server/mail/send/DeleteKeySender.java @@ -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 gpgKeyFingerprints); } - private final PermissionBackend permissionBackend; - private final IdentifiedUser callingUser; private final IdentifiedUser user; private final AccountSshKey sshKey; private final List 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 gpgKeyFingerprints) { + EmailArguments ea, @Assisted IdentifiedUser user, @Assisted List 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 diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java index c8c1ae07f2..60a8290234 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java @@ -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 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 addGpgKey(String armored) throws Exception { + return addGpgKey(admin, armored); + } + + private Map addGpgKey(TestAccount account, String armored) throws Exception { Map 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.of()); + accountIndexedCounter.assertReindexOf(gApi.accounts().id(account.username()).get()); return gpgKeys; } diff --git a/tools/eclipse/project.py b/tools/eclipse/project.py index a32c3542af..478f448371 100755 --- a/tools/eclipse/project.py +++ b/tools/eclipse/project.py @@ -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