Allow to listen for account deactivations

Deactivating an account currently does not purge active SSH connections
(E.g.: long-running stream-events) and does not purge the web session
cache. So deactivating a user for real requires to also manually purge
these connections and caches, which is unnecessarily tedious.

Implementing such code directly in SetInactiveFlag is not straight
forward as com.google.server.account does not have access to the sshd
package, and naively adding it to the build introduces a circular
dependency. So we add an extension point for account deactivation.
Thereby, core components and plugins can self-cater cache purging and
other house-holding tasks upon user deactivation without messing with
the build.

Change-Id: I1dfa1779acfe56e5c7f1666b1e83e83f954083e1

8 files changed, 156 insertions, 8 deletions

diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 799c13c663..aad81bdfd9 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -390,6 +390,10 @@ Allows to listen to events visible to the specified user. These are the
 same link:cmd-stream-events.html#events[events] that are also streamed
 by the link:cmd-stream-events.html[gerrit stream-events] command.
 
+* `com.google.gerrit.extensions.events.AccountActivationListener`:
++
+User account got activated or deactivated
+
 * `com.google.gerrit.extensions.events.LifecycleListener`:
 +
 Plugin start and stop
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index f9116a19a1..eaf03b332c 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance;
 
 import com.google.gerrit.extensions.api.changes.ActionVisitor;
 import com.google.gerrit.extensions.config.DownloadScheme;
+import com.google.gerrit.extensions.events.AccountActivationListener;
 import com.google.gerrit.extensions.events.AccountIndexedListener;
 import com.google.gerrit.extensions.events.ChangeIndexedListener;
 import com.google.gerrit.extensions.events.CommentAddedListener;
@@ -68,6 +69,7 @@ public class ExtensionRegistry {
   private final DynamicSet<GroupBackend> groupBackends;
   private final DynamicSet<AccountActivationValidationListener>
       accountActivationValidationListeners;
+  private final DynamicSet<AccountActivationListener> accountActivationListeners;
   private final DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners;
 
   @Inject
@@ -93,6 +95,7 @@ public class ExtensionRegistry {
       DynamicSet<RevisionCreatedListener> revisionCreatedListeners,
       DynamicSet<GroupBackend> groupBackends,
       DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners,
+      DynamicSet<AccountActivationListener> accountActivationListeners,
       DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners) {
     this.accountIndexedListeners = accountIndexedListeners;
     this.changeIndexedListeners = changeIndexedListeners;
@@ -115,6 +118,7 @@ public class ExtensionRegistry {
     this.revisionCreatedListeners = revisionCreatedListeners;
     this.groupBackends = groupBackends;
     this.accountActivationValidationListeners = accountActivationValidationListeners;
+    this.accountActivationListeners = accountActivationListeners;
     this.onSubmitValidationListeners = onSubmitValidationListeners;
   }
 
@@ -215,6 +219,10 @@ public class ExtensionRegistry {
       return add(accountActivationValidationListeners, accountActivationValidationListener);
     }
 
+    public Registration add(AccountActivationListener accountDeactivatedListener) {
+      return add(accountActivationListeners, accountDeactivatedListener);
+    }
+
     public Registration add(OnSubmitValidationListener onSubmitValidationListener) {
       return add(onSubmitValidationListeners, onSubmitValidationListener);
     }
diff --git a/java/com/google/gerrit/extensions/events/AccountActivationListener.java b/java/com/google/gerrit/extensions/events/AccountActivationListener.java
new file mode 100644
index 0000000000..b45533b8bf
--- /dev/null
+++ b/java/com/google/gerrit/extensions/events/AccountActivationListener.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2020 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.extensions.events;
+
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+
+/**
+ * Notified whenever an account got activated or deactivated.
+ *
+ * <p>This listener is called only after an account got (de)activated and hence cannot cancel the
+ * (de)activation. See {@link
+ * com.google.gerrit.server.validators.AccountActivationValidationListener} for a listener that can
+ * cancel a (de)activation.
+ */
+@ExtensionPoint
+public interface AccountActivationListener {
+  /**
+   * Invoked after an account got activated
+   *
+   * @param id of the account
+   */
+  default void onAccountActivated(int id) {}
+
+  /**
+   * Invoked after an account got deactivated
+   *
+   * @param id of the account
+   */
+  default void onAccountDeactivated(int id) {}
+}
diff --git a/java/com/google/gerrit/server/account/SetInactiveFlag.java b/java/com/google/gerrit/server/account/SetInactiveFlag.java
index fb3d4ea00d..40cc185ff4 100644
--- a/java/com/google/gerrit/server/account/SetInactiveFlag.java
+++ b/java/com/google/gerrit/server/account/SetInactiveFlag.java
@@ -15,6 +15,7 @@
 package com.google.gerrit.server.account;
 
 import com.google.gerrit.entities.Account;
+import com.google.gerrit.extensions.events.AccountActivationListener;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
@@ -37,13 +38,16 @@ public class SetInactiveFlag {
   private final PluginSetContext<AccountActivationValidationListener>
       accountActivationValidationListeners;
   private final Provider<AccountsUpdate> accountsUpdateProvider;
+  private final PluginSetContext<AccountActivationListener> accountActivationListeners;
 
   @Inject
   SetInactiveFlag(
       PluginSetContext<AccountActivationValidationListener> accountActivationValidationListeners,
-      @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider) {
+      @ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider,
+      PluginSetContext<AccountActivationListener> accountActivationListeners) {
     this.accountActivationValidationListeners = accountActivationValidationListeners;
     this.accountsUpdateProvider = accountsUpdateProvider;
+    this.accountActivationListeners = accountActivationListeners;
   }
 
   public Response<?> deactivate(Account.Id accountId)
@@ -76,6 +80,12 @@ public class SetInactiveFlag {
     if (alreadyInactive.get()) {
       throw new ResourceConflictException("account not active");
     }
+
+    // At this point the account got set inactive and no errors occurred
+
+    int id = accountId.get();
+    accountActivationListeners.runEach(l -> l.onAccountDeactivated(id));
+
     return Response.none();
   }
 
@@ -106,6 +116,16 @@ public class SetInactiveFlag {
     if (exception.get().isPresent()) {
       throw exception.get().get();
     }
-    return alreadyActive.get() ? Response.ok("") : Response.created("");
+
+    Response<String> res;
+    if (alreadyActive.get()) {
+      res = Response.ok("");
+    } else {
+      res = Response.created("");
+
+      int id = accountId.get();
+      accountActivationListeners.runEach(l -> l.onAccountActivated(id));
+    }
+    return res;
   }
 }
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index e17fb02a05..25f2b20269 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -30,6 +30,7 @@ import com.google.gerrit.extensions.config.DownloadScheme;
 import com.google.gerrit.extensions.config.ExternalIncludedIn;
 import com.google.gerrit.extensions.config.FactoryModule;
 import com.google.gerrit.extensions.config.PluginProjectPermissionDefinition;
+import com.google.gerrit.extensions.events.AccountActivationListener;
 import com.google.gerrit.extensions.events.AccountIndexedListener;
 import com.google.gerrit.extensions.events.AgreementSignupListener;
 import com.google.gerrit.extensions.events.AssigneeChangedListener;
@@ -332,6 +333,7 @@ public class GerritGlobalModule extends FactoryModule {
     DynamicSet.setOf(binder(), PostReceiveHook.class);
     DynamicSet.setOf(binder(), PreUploadHook.class);
     DynamicSet.setOf(binder(), PostUploadHook.class);
+    DynamicSet.setOf(binder(), AccountActivationListener.class);
     DynamicSet.setOf(binder(), AccountIndexedListener.class);
     DynamicSet.setOf(binder(), ChangeIndexedListener.class);
     DynamicSet.setOf(binder(), GroupIndexedListener.class);
diff --git a/java/com/google/gerrit/server/validators/AccountActivationValidationListener.java b/java/com/google/gerrit/server/validators/AccountActivationValidationListener.java
index bc52308609..9fdc9e6d71 100644
--- a/java/com/google/gerrit/server/validators/AccountActivationValidationListener.java
+++ b/java/com/google/gerrit/server/validators/AccountActivationValidationListener.java
@@ -26,6 +26,9 @@ public interface AccountActivationValidationListener {
   /**
    * Called when an account should be activated to allow validation of the account activation.
    *
+   * <p>See {@link com.google.gerrit.extensions.events.AccountActivationListener} for a listener
+   * that's run after the account got activated.
+   *
    * @param account the account that should be activated
    * @throws ValidationException if validation fails
    */
@@ -34,6 +37,9 @@ public interface AccountActivationValidationListener {
   /**
    * Called when an account should be deactivated to allow validation of the account deactivation.
    *
+   * <p>See {@link com.google.gerrit.extensions.events.AccountActivationListener} for a listener
+   * that's run after the account got deactivated.
+   *
    * @param account the account that should be deactivated
    * @throws ValidationException if validation fails
    */
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index ac007827c5..5ba6d395b5 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -44,6 +44,10 @@ import static java.util.concurrent.TimeUnit.SECONDS;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
 
 import com.github.rholder.retry.StopStrategies;
 import com.google.common.cache.LoadingCache;
@@ -104,6 +108,7 @@ import com.google.gerrit.extensions.common.EmailInfo;
 import com.google.gerrit.extensions.common.GpgKeyInfo;
 import com.google.gerrit.extensions.common.GroupInfo;
 import com.google.gerrit.extensions.common.SshKeyInfo;
+import com.google.gerrit.extensions.events.AccountActivationListener;
 import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -589,7 +594,7 @@ public class AccountIT extends AbstractDaemonTest {
     Account.Id deactivatableAccountId =
         accountOperations.newAccount().preferredEmail("foo@deactivatable.com").create();
 
-    AccountActivationValidationListener listener =
+    AccountActivationValidationListener validationListener =
         new AccountActivationValidationListener() {
           @Override
           public void validateActivation(AccountState account) throws ValidationException {
@@ -607,7 +612,11 @@ public class AccountIT extends AbstractDaemonTest {
             }
           }
         };
-    try (Registration registration = extensionRegistry.newRegistration().add(listener)) {
+
+    AccountActivationListener listener = mock(AccountActivationListener.class);
+
+    try (Registration registration =
+        extensionRegistry.newRegistration().add(validationListener).add(listener)) {
       /* Test account that can be activated, but not deactivated */
       // Deactivate account that is already inactive
       ResourceConflictException thrown =
@@ -616,14 +625,18 @@ public class AccountIT extends AbstractDaemonTest {
               () -> gApi.accounts().id(activatableAccountId.get()).setActive(false));
       assertThat(thrown).hasMessageThat().isEqualTo("account not active");
       assertThat(accountOperations.account(activatableAccountId).get().active()).isFalse();
+      verifyZeroInteractions(listener);
 
       // Activate account that can be activated
       gApi.accounts().id(activatableAccountId.get()).setActive(true);
       assertThat(accountOperations.account(activatableAccountId).get().active()).isTrue();
+      verify(listener).onAccountActivated(activatableAccountId.get());
+      verifyNoMoreInteractions(listener);
 
       // Activate account that is already active
       gApi.accounts().id(activatableAccountId.get()).setActive(true);
       assertThat(accountOperations.account(activatableAccountId).get().active()).isTrue();
+      verifyZeroInteractions(listener);
 
       // Try deactivating account that cannot be deactivated
       thrown =
@@ -632,15 +645,19 @@ public class AccountIT extends AbstractDaemonTest {
               () -> gApi.accounts().id(activatableAccountId.get()).setActive(false));
       assertThat(thrown).hasMessageThat().isEqualTo("not allowed to deactive account");
       assertThat(accountOperations.account(activatableAccountId).get().active()).isTrue();
+      verifyZeroInteractions(listener);
 
       /* Test account that can be deactivated, but not activated */
       // Activate account that is already inactive
       gApi.accounts().id(deactivatableAccountId.get()).setActive(true);
       assertThat(accountOperations.account(deactivatableAccountId).get().active()).isTrue();
+      verifyZeroInteractions(listener);
 
       // Deactivate account that can be deactivated
       gApi.accounts().id(deactivatableAccountId.get()).setActive(false);
       assertThat(accountOperations.account(deactivatableAccountId).get().active()).isFalse();
+      verify(listener).onAccountDeactivated(deactivatableAccountId.get());
+      verifyNoMoreInteractions(listener);
 
       // Deactivate account that is already inactive
       thrown =
@@ -649,6 +666,7 @@ public class AccountIT extends AbstractDaemonTest {
               () -> gApi.accounts().id(deactivatableAccountId.get()).setActive(false));
       assertThat(thrown).hasMessageThat().isEqualTo("account not active");
       assertThat(accountOperations.account(deactivatableAccountId).get().active()).isFalse();
+      verifyZeroInteractions(listener);
 
       // Try activating account that cannot be activated
       thrown =
@@ -657,6 +675,7 @@ public class AccountIT extends AbstractDaemonTest {
               () -> gApi.accounts().id(deactivatableAccountId.get()).setActive(true));
       assertThat(thrown).hasMessageThat().isEqualTo("not allowed to active account");
       assertThat(accountOperations.account(deactivatableAccountId).get().active()).isFalse();
+      verifyZeroInteractions(listener);
     }
   }
 
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountListenersIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountListenersIT.java
index d37f019b19..80eff9606a 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountListenersIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountListenersIT.java
@@ -20,6 +20,7 @@ import static com.google.gerrit.testing.GerritJUnit.assertThrows;
 import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
 import com.google.gerrit.acceptance.TestPlugin;
 import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.extensions.events.AccountActivationListener;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.RestApiException;
@@ -48,14 +49,18 @@ public class AccountListenersIT extends LightweightPluginDaemonTest {
     @Override
     protected void configure() {
       DynamicSet.bind(binder(), AccountActivationValidationListener.class).to(Validator.class);
+      DynamicSet.bind(binder(), AccountActivationListener.class).to(Listener.class);
     }
   }
 
   Validator validator;
+  Listener listener;
 
   @Before
   public void setUp() {
     validator = plugin.getSysInjector().getInstance(Validator.class);
+
+    listener = plugin.getSysInjector().getInstance(Listener.class);
   }
 
   @Test
@@ -65,7 +70,8 @@ public class AccountListenersIT extends LightweightPluginDaemonTest {
     gApi.accounts().id(id).setActive(true);
 
     validator.assertActivationValidation(id);
-    validator.assertNoMoreEvents();
+    listener.assertActivated(id);
+    assertNoMoreEvents();
     assertThat(gApi.accounts().id(id).getActive()).isTrue();
   }
 
@@ -82,7 +88,8 @@ public class AccountListenersIT extends LightweightPluginDaemonTest {
         });
 
     validator.assertActivationValidation(id);
-    validator.assertNoMoreEvents();
+    // No call to activation listener as validation failed
+    assertNoMoreEvents();
     assertThat(gApi.accounts().id(id).getActive()).isFalse();
   }
 
@@ -93,7 +100,8 @@ public class AccountListenersIT extends LightweightPluginDaemonTest {
     gApi.accounts().id(id).setActive(false);
 
     validator.assertDeactivationValidation(id);
-    validator.assertNoMoreEvents();
+    listener.assertDeactivated(id);
+    assertNoMoreEvents();
     assertThat(gApi.accounts().id(id).getActive()).isFalse();
   }
 
@@ -110,10 +118,16 @@ public class AccountListenersIT extends LightweightPluginDaemonTest {
         });
 
     validator.assertDeactivationValidation(id);
-    validator.assertNoMoreEvents();
+    // No call to activation listener as validation failed
+    assertNoMoreEvents();
     assertThat(gApi.accounts().id(id).getActive()).isTrue();
   }
 
+  private void assertNoMoreEvents() {
+    validator.assertNoMoreEvents();
+    listener.assertNoMoreEvents();
+  }
+
   @Singleton
   public static class Validator implements AccountActivationValidationListener {
     private Integer lastIdActivationValidation;
@@ -162,4 +176,37 @@ public class AccountListenersIT extends LightweightPluginDaemonTest {
       lastIdDeactivationValidation = null;
     }
   }
+
+  @Singleton
+  public static class Listener implements AccountActivationListener {
+    private Integer lastIdActivated;
+    private Integer lastIdDeactivated;
+
+    @Override
+    public void onAccountActivated(int id) {
+      assertThat(lastIdActivated).isNull();
+      lastIdActivated = id;
+    }
+
+    @Override
+    public void onAccountDeactivated(int id) {
+      assertThat(lastIdDeactivated).isNull();
+      lastIdDeactivated = id;
+    }
+
+    private void assertNoMoreEvents() {
+      assertThat(lastIdActivated).isNull();
+      assertThat(lastIdDeactivated).isNull();
+    }
+
+    private void assertDeactivated(int id) {
+      assertThat(lastIdDeactivated).isEqualTo(id);
+      lastIdDeactivated = null;
+    }
+
+    private void assertActivated(int id) {
+      assertThat(lastIdActivated).isEqualTo(id);
+      lastIdActivated = null;
+    }
+  }
 }