Send email notification when SSH key or GPG key is removed We already send a notification when a key is added. Also sending a notification when a key is removed will help to alert the user if their account was compromised and the key removed by an attacker. Also-by: David Pursehouse <dpursehouse@collab.net> Change-Id: I84f4bc5df5b6a609c1a17bad7c4a327cd780aa10
diff --git a/Documentation/config-mail.txt b/Documentation/config-mail.txt index 9eb31bf..db5228d 100644 --- a/Documentation/config-mail.txt +++ b/Documentation/config-mail.txt
@@ -65,6 +65,12 @@ will be appended to emails related to a user submitting comments on changes. See `ChangeSubject.soy`, Comment and ChangeFooter. +=== DeleteKey.soy and DeleteKeyHtml.soy + +DeleteKey templates will determine the contents of the email related to SSH or GPG keys +being deleted from a user account. This notification is not sent when the key is +administratively deleted from another user account. + === DeleteVote.soy and DeleteVoteHtml.soy The DeleteVote templates will determine the contents of the email related to
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java index 80e43de..ec96c4b 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -1602,9 +1602,12 @@ addGpgKey(key.getPublicKeyArmored()); assertKeys(key); + sender.clear(); gApi.accounts().self().gpgKey(id).delete(); accountIndexedCounter.assertReindexOf(admin); assertKeys(); + assertThat(sender.getMessages()).hasSize(1); + assertThat(sender.getMessages().get(0).body()).contains("GPG keys have been deleted"); exception.expect(ResourceNotFoundException.class); exception.expectMessage(id); @@ -1707,12 +1710,15 @@ assertThat(sender.getMessages().get(0).body()).contains("new SSH keys have been added"); // Delete second key + sender.clear(); gApi.accounts().self().deleteSshKey(2); info = gApi.accounts().self().listSshKeys(); assertThat(info).hasSize(2); assertThat(info.get(0).seq).isEqualTo(1); assertThat(info.get(1).seq).isEqualTo(3); accountIndexedCounter.assertReindexOf(admin); + assertThat(sender.getMessages()).hasSize(1); + assertThat(sender.getMessages().get(0).body()).contains("SSH keys have been deleted"); } // reindex is tested by {@link AbstractQueryAccountsTest#reindex}
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/DeleteGpgKey.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/DeleteGpgKey.java index baf5a58..212b419 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/DeleteGpgKey.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/DeleteGpgKey.java
@@ -17,7 +17,9 @@ import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString; import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPGKEY; +import com.google.common.collect.ImmutableList; import com.google.common.io.BaseEncoding; +import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; @@ -26,6 +28,7 @@ import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.account.externalids.ExternalId; import com.google.gerrit.server.account.externalids.ExternalIdsUpdate; +import com.google.gerrit.server.mail.send.DeleteKeySender; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -36,22 +39,29 @@ import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.RefUpdate; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class DeleteGpgKey implements RestModifyView<GpgKey, Input> { + private static final Logger log = LoggerFactory.getLogger(DeleteGpgKey.class); + public static class Input {} private final Provider<PersonIdent> serverIdent; private final Provider<PublicKeyStore> storeProvider; private final ExternalIdsUpdate.User externalIdsUpdateFactory; + private final DeleteKeySender.Factory deleteKeySenderFactory; @Inject DeleteGpgKey( @GerritPersonIdent Provider<PersonIdent> serverIdent, Provider<PublicKeyStore> storeProvider, - ExternalIdsUpdate.User externalIdsUpdateFactory) { + ExternalIdsUpdate.User externalIdsUpdateFactory, + DeleteKeySender.Factory deleteKeySenderFactory) { this.serverIdent = serverIdent; this.storeProvider = storeProvider; this.externalIdsUpdateFactory = externalIdsUpdateFactory; + this.deleteKeySenderFactory = deleteKeySenderFactory; } @Override @@ -79,6 +89,16 @@ switch (saveResult) { case NO_CHANGE: case FAST_FORWARD: + try { + deleteKeySenderFactory + .create(rsrc.getUser(), ImmutableList.of(PublicKeyStore.keyToString(key))) + .send(); + } catch (EmailException e) { + log.error( + "Cannot send GPG key deletion message to {}", + rsrc.getUser().getAccount().getPreferredEmail(), + e); + } break; case FORCED: case IO_FAILURE:
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java index 77852ad..979691e 100644 --- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java +++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
@@ -49,6 +49,7 @@ import com.google.gerrit.server.account.externalids.ExternalIds; import com.google.gerrit.server.account.externalids.ExternalIdsUpdate; import com.google.gerrit.server.mail.send.AddKeySender; +import com.google.gerrit.server.mail.send.DeleteKeySender; import com.google.gerrit.server.query.account.InternalAccountQuery; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -87,7 +88,8 @@ private final Provider<CurrentUser> self; private final Provider<PublicKeyStore> storeProvider; private final GerritPublicKeyChecker.Factory checkerFactory; - private final AddKeySender.Factory addKeyFactory; + private final AddKeySender.Factory addKeySenderFactory; + private final DeleteKeySender.Factory deleteKeySenderFactory; private final Provider<InternalAccountQuery> accountQueryProvider; private final ExternalIds externalIds; private final ExternalIdsUpdate.User externalIdsUpdateFactory; @@ -98,7 +100,8 @@ Provider<CurrentUser> self, Provider<PublicKeyStore> storeProvider, GerritPublicKeyChecker.Factory checkerFactory, - AddKeySender.Factory addKeyFactory, + AddKeySender.Factory addKeySenderFactory, + DeleteKeySender.Factory deleteKeySenderFactory, Provider<InternalAccountQuery> accountQueryProvider, ExternalIds externalIds, ExternalIdsUpdate.User externalIdsUpdateFactory) { @@ -106,7 +109,8 @@ this.self = self; this.storeProvider = storeProvider; this.checkerFactory = checkerFactory; - this.addKeyFactory = addKeyFactory; + this.addKeySenderFactory = addKeySenderFactory; + this.deleteKeySenderFactory = deleteKeySenderFactory; this.accountQueryProvider = accountQueryProvider; this.externalIds = externalIds; this.externalIdsUpdateFactory = externalIdsUpdateFactory; @@ -225,13 +229,24 @@ case FORCED: if (!addedKeys.isEmpty()) { try { - addKeyFactory.create(user, addedKeys).send(); + addKeySenderFactory.create(user, addedKeys).send(); } catch (EmailException e) { log.error( "Cannot send GPG key added message to " + user.getAccount().getPreferredEmail(), e); } } + if (!toRemove.isEmpty()) { + try { + deleteKeySenderFactory + .create(user, toRemove.stream().map(Fingerprint::toString).collect(toList())) + .send(); + } catch (EmailException e) { + log.error( + "Cannot send GPG key deleted message to " + user.getAccount().getPreferredEmail(), + e); + } + } break; case NO_CHANGE: break;
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SitePathInitializer.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SitePathInitializer.java index 647bb6b..a633335 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SitePathInitializer.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/SitePathInitializer.java
@@ -115,6 +115,8 @@ extractMailExample("CommentHtml.soy"); extractMailExample("CommentFooter.soy"); extractMailExample("CommentFooterHtml.soy"); + extractMailExample("DeleteKey.soy"); + extractMailExample("DeleteKeyHtml.soy"); extractMailExample("DeleteReviewer.soy"); extractMailExample("DeleteReviewerHtml.soy"); extractMailExample("DeleteVote.soy");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteSshKey.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteSshKey.java index 74616bf..c22e345 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteSshKey.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteSshKey.java
@@ -14,11 +14,14 @@ package com.google.gerrit.server.account; +import com.google.gerrit.common.errors.EmailException; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.DeleteSshKey.Input; +import com.google.gerrit.server.mail.send.DeleteKeySender; import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -30,26 +33,33 @@ import java.io.IOException; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; @Singleton public class DeleteSshKey implements RestModifyView<AccountResource.SshKey, Input> { + private static final Logger log = LoggerFactory.getLogger(DeleteSshKey.class); + public static class Input {} private final Provider<CurrentUser> self; private final PermissionBackend permissionBackend; private final VersionedAuthorizedKeys.Accessor authorizedKeys; private final SshKeyCache sshKeyCache; + private final DeleteKeySender.Factory deleteKeySenderFactory; @Inject DeleteSshKey( Provider<CurrentUser> self, PermissionBackend permissionBackend, VersionedAuthorizedKeys.Accessor authorizedKeys, - SshKeyCache sshKeyCache) { + SshKeyCache sshKeyCache, + DeleteKeySender.Factory deleteKeySenderFactory) { this.self = self; this.permissionBackend = permissionBackend; this.authorizedKeys = authorizedKeys; this.sshKeyCache = sshKeyCache; + this.deleteKeySenderFactory = deleteKeySenderFactory; } @Override @@ -60,8 +70,17 @@ permissionBackend.user(self).check(GlobalPermission.ADMINISTRATE_SERVER); } - authorizedKeys.deleteKey(rsrc.getUser().getAccountId(), rsrc.getSshKey().getKey().get()); - sshKeyCache.evict(rsrc.getUser().getUserName()); + IdentifiedUser user = rsrc.getUser(); + authorizedKeys.deleteKey(user.getAccountId(), rsrc.getSshKey().getKey().get()); + + try { + deleteKeySenderFactory.create(user, rsrc.getSshKey()).send(); + } catch (EmailException e) { + log.error( + "Cannot send SSH key deletion message to {}", user.getAccount().getPreferredEmail(), e); + } + + sshKeyCache.evict(user.getUserName()); return Response.none(); }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailModule.java index 862293e..6b363c0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/EmailModule.java
@@ -20,6 +20,7 @@ import com.google.gerrit.server.mail.send.AddReviewerSender; import com.google.gerrit.server.mail.send.CommentSender; import com.google.gerrit.server.mail.send.CreateChangeSender; +import com.google.gerrit.server.mail.send.DeleteKeySender; import com.google.gerrit.server.mail.send.DeleteReviewerSender; import com.google.gerrit.server.mail.send.DeleteVoteSender; import com.google.gerrit.server.mail.send.MergedSender; @@ -37,6 +38,7 @@ factory(AddReviewerSender.Factory.class); factory(CommentSender.Factory.class); factory(CreateChangeSender.Factory.class); + factory(DeleteKeySender.Factory.class); factory(DeleteReviewerSender.Factory.class); factory(DeleteVoteSender.Factory.class); factory(MergedSender.Factory.class);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteKeySender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteKeySender.java new file mode 100644 index 0000000..c6a3532 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteKeySender.java
@@ -0,0 +1,150 @@ +// Copyright (C) 2019 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.mail.send; + +import com.google.common.base.Joiner; +import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.extensions.api.changes.RecipientType; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.reviewdb.client.AccountSshKey; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.mail.Address; +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; +import java.util.List; + +public class DeleteKeySender extends OutgoingEmail { + public interface Factory { + DeleteKeySender create(IdentifiedUser user, AccountSshKey sshKey); + + 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) { + super(ea, "deletekey"); + this.permissionBackend = permissionBackend; + this.callingUser = callingUser; + this.user = user; + this.gpgKeyFingerprints = Collections.emptyList(); + this.sshKey = sshKey; + } + + @AssistedInject + public DeleteKeySender( + EmailArguments ea, + PermissionBackend permissionBackend, + IdentifiedUser callingUser, + @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; + } + + @Override + protected void init() throws EmailException { + super.init(); + setHeader("Subject", String.format("[Gerrit Code Review] %s Keys Deleted", getKeyType())); + add(RecipientType.TO, new Address(getEmail())); + } + + @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; + } + } + + @Override + protected void format() throws EmailException { + appendText(textTemplate("DeleteKey")); + if (useHtml()) { + appendHtml(soyHtmlTemplate("DeleteKeyHtml")); + } + } + + public String getEmail() { + return user.getAccount().getPreferredEmail(); + } + + public String getUserNameEmail() { + return getUserNameEmailFor(user.getAccountId()); + } + + public String getKeyType() { + if (sshKey != null) { + return "SSH"; + } else if (gpgKeyFingerprints != null) { + return "GPG"; + } + throw new IllegalStateException("key type is not SSH or GPG"); + } + + public String getSshKey() { + return (sshKey != null) ? sshKey.getSshPublicKey() + "\n" : null; + } + + public String getGpgKeyFingerprints() { + if (!gpgKeyFingerprints.isEmpty()) { + return Joiner.on("\n").join(gpgKeyFingerprints); + } + return null; + } + + @Override + protected void setupSoyContext() { + super.setupSoyContext(); + soyContextEmailData.put("email", getEmail()); + soyContextEmailData.put("gpgKeyFingerprints", getGpgKeyFingerprints()); + soyContextEmailData.put("keyType", getKeyType()); + soyContextEmailData.put("sshKey", getSshKey()); + soyContextEmailData.put("userNameEmail", getUserNameEmail()); + } + + @Override + protected boolean supportsHtml() { + return true; + } +}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MailSoyTofuProvider.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MailSoyTofuProvider.java index b267275..81074fd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MailSoyTofuProvider.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MailSoyTofuProvider.java
@@ -47,6 +47,8 @@ "CommentHtml.soy", "CommentFooter.soy", "CommentFooterHtml.soy", + "DeleteKey.soy", + "DeleteKeyHtml.soy", "DeleteReviewer.soy", "DeleteReviewerHtml.soy", "DeleteVote.soy",
diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/DeleteKey.soy b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/DeleteKey.soy new file mode 100644 index 0000000..0fc4bf8 --- /dev/null +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/DeleteKey.soy
@@ -0,0 +1,72 @@ +/** + * Copyright (C) 2019 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. + */ + +{namespace com.google.gerrit.server.mail.template} + +/** + * The .DeleteKey template will determine the contents of the email related to + * deleting a SSH or GPG key. + * @param email + */ +{template .DeleteKey autoescape="strict" kind="text"} + One or more {$email.keyType} keys have been deleted on Gerrit Code Review at + {sp}{$email.gerritHost}: + + {\n} + {\n} + + {if $email.sshKey} + {$email.sshKey} + {elseif $email.gpgKeyFingerprints} + {$email.gpgKeyFingerprints} + {/if} + + {\n} + {\n} + + + If this is not expected, please contact your Gerrit Administrators + immediately. + + {\n} + {\n} + + You can also manage your {$email.keyType} keys by visiting + {\n} + {if $email.sshKey} + {$email.gerritUrl}#/settings/ssh-keys + {elseif $email.gpgKey} + {$email.gerritUrl}#/settings/gpg-keys + {/if} + {\n} + {if $email.userNameEmail} + (while signed in as {$email.userNameEmail}) + {else} + (while signed in as {$email.email}) + {/if} + + {\n} + {\n} + + If clicking the link above does not work, copy and paste the URL in a new + browser window instead. + + {\n} + {\n} + + This is a send-only email address. Replies to this message will not be read + or answered. +{/template}
diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/DeleteKeyHtml.soy b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/DeleteKeyHtml.soy new file mode 100644 index 0000000..acdadad --- /dev/null +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/DeleteKeyHtml.soy
@@ -0,0 +1,66 @@ +/** + * Copyright (C) 2019 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. + */ + +{namespace com.google.gerrit.server.mail.template} + +/** + * @param email + */ +{template .DeleteKeyHtml autoescape="strict" kind="html"} + <p> + One or more {$email.keyType} keys have been deleted on Gerrit Code Review + at {$email.gerritHost}: + </p> + + {let $keyStyle kind="css"} + background: #f0f0f0; + border: 1px solid #ccc; + color: #555; + padding: 12px; + width: 400px; + {/let} + + {if $email.sshKey} + <pre style="{$keyStyle}">{$email.sshKey}</pre> + {elseif $email.gpgKeyFingerprints} + <pre style="{$keyStyle}">{$email.gpgKeyFingerprints}</pre> + {/if} + + <p> + If this is not expected, please contact your Gerrit Administrators + immediately. + </p> + + <p> + You can also manage your {$email.keyType} keys by following{sp} + {if $email.sshKey} + <a href="{$email.gerritUrl}#/settings/ssh-keys">this link</a> + {elseif $email.gpgKeyFingerprints} + <a href="{$email.gerritUrl}#/settings/gpg-keys">this link</a> + {/if} + {sp} + {if $email.userNameEmail} + (while signed in as {$email.userNameEmail}) + {else} + (while signed in as {$email.email}) + {/if}. + </p> + + <p> + This is a send-only email address. Replies to this message will not be read + or answered. + </p> +{/template}