opendev
/
gerrit
Code Issues Proposed changes

Add validation listener for account activation 

Change-Id: Ib238701b2a9563157f37834fa0944b8e40db3701
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2018-04-20 13:03:34 +02:00
parent 6d8a2d0ff5
commit f0d71a822d
6 changed files with 183 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
Documentation/config-validation.txt
View File

@ -114,6 +114,14 @@ This interface provides a low-level e-mail filtering API for plugins.
Plugins implementing the `OutgoingEmailValidationListener` interface can perform
filtering of outgoing e-mails just before they are sent.
[[account-activation-validation]]
== Account activation validation
Plugins implementing the `AccountActivationValidationListener` interface can
perform validation when an account is activated or deactivated via the Gerrit
REST API or the Java extension API.
GERRIT
------

39
java/com/google/gerrit/server/account/SetInactiveFlag.java
View File

@ -14,33 +14,43 @@
package com.google.gerrit.server.account;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.ServerInitiated;
import com.google.gerrit.server.validators.AccountActivationValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
public class SetInactiveFlag {
private final DynamicSet<AccountActivationValidationListener>
accountActivationValidationListeners;
private final Provider<AccountsUpdate> accountsUpdateProvider;
@Inject
SetInactiveFlag(@ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider) {
SetInactiveFlag(
DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners,
@ServerInitiated Provider<AccountsUpdate> accountsUpdateProvider) {
this.accountActivationValidationListeners = accountActivationValidationListeners;
this.accountsUpdateProvider = accountsUpdateProvider;
}
public Response<?> deactivate(Account.Id accountId)
throws RestApiException, IOException, ConfigInvalidException, OrmException {
AtomicBoolean alreadyInactive = new AtomicBoolean(false);
AtomicReference<Optional<RestApiException>> exception = new AtomicReference<>(Optional.empty());
accountsUpdateProvider
.get()
.update(
@ -50,10 +60,21 @@ public class SetInactiveFlag {
if (!a.getAccount().isActive()) {
alreadyInactive.set(true);
} else {
for (AccountActivationValidationListener l : accountActivationValidationListeners) {
try {
l.validateDeactivation(a);
} catch (ValidationException e) {
exception.set(Optional.of(new ResourceConflictException(e.getMessage(), e)));
return;
}
}
u.setActive(false);
}
})
.orElseThrow(() -> new ResourceNotFoundException("account not found"));
if (exception.get().isPresent()) {
throw exception.get().get();
}
if (alreadyInactive.get()) {
throw new ResourceConflictException("account not active");
}
@ -61,8 +82,9 @@ public class SetInactiveFlag {
}
public Response<String> activate(Account.Id accountId)
throws ResourceNotFoundException, IOException, ConfigInvalidException, OrmException {
throws RestApiException, IOException, ConfigInvalidException, OrmException {
AtomicBoolean alreadyActive = new AtomicBoolean(false);
AtomicReference<Optional<RestApiException>> exception = new AtomicReference<>(Optional.empty());
accountsUpdateProvider
.get()
.update(
@ -72,10 +94,21 @@ public class SetInactiveFlag {
if (a.getAccount().isActive()) {
alreadyActive.set(true);
} else {
for (AccountActivationValidationListener l : accountActivationValidationListeners) {
try {
l.validateActivation(a);
} catch (ValidationException e) {
exception.set(Optional.of(new ResourceConflictException(e.getMessage(), e)));
return;
}
}
u.setActive(true);
}
})
.orElseThrow(() -> new ResourceNotFoundException("account not found"));
if (exception.get().isPresent()) {
throw exception.get().get();
}
return alreadyActive.get() ? Response.ok("") : Response.created("");
}
}

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

@ -184,6 +184,7 @@ import com.google.gerrit.server.tools.ToolsCatalog;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.util.IdGenerator;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gerrit.server.validators.AccountActivationValidationListener;
import com.google.gerrit.server.validators.AssigneeValidationListener;
import com.google.gerrit.server.validators.GroupCreationValidationListener;
import com.google.gerrit.server.validators.HashtagValidationListener;
@ -365,6 +366,7 @@ public class GerritGlobalModule extends FactoryModule {
DynamicSet.setOf(binder(), GroupCreationValidationListener.class);
DynamicSet.setOf(binder(), HashtagValidationListener.class);
DynamicSet.setOf(binder(), OutgoingEmailValidationListener.class);
DynamicSet.setOf(binder(), AccountActivationValidationListener.class);
DynamicItem.itemOf(binder(), AvatarProvider.class);
DynamicSet.setOf(binder(), LifecycleListener.class);
DynamicSet.setOf(binder(), TopMenu.class);

4
java/com/google/gerrit/server/restapi/account/PutActive.java
View File

@ -17,8 +17,8 @@ package com.google.gerrit.server.restapi.account;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.SetInactiveFlag;
@ -41,7 +41,7 @@ public class PutActive implements RestModifyView<AccountResource, Input> {
@Override
public Response<String> apply(AccountResource rsrc, Input input)
throws ResourceNotFoundException, OrmException, IOException, ConfigInvalidException {
throws RestApiException, OrmException, IOException, ConfigInvalidException {
return setInactiveFlag.activate(rsrc.getUser().getAccountId());
}
}

41
java/com/google/gerrit/server/validators/AccountActivationValidationListener.java Normal file
View File

@ -0,0 +1,41 @@
// Copyright (C) 2018 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.validators;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.server.account.AccountState;
/**
* Validator that is invoked when an account activated or deactivated via the Gerrit REST API or the
* Java extension API.
*/
@ExtensionPoint
public interface AccountActivationValidationListener {
/**
* Called when an account should be activated to allow validation of the account activation.
*
* @param account the account that should be activated
* @throws ValidationException if validation fails
*/
void validateActivation(AccountState account) throws ValidationException;
/**
* Called when an account should be deactivated to allow validation of the account deactivation.
*
* @param account the account that should be deactivated
* @throws ValidationException if validation fails
*/
void validateDeactivation(AccountState account) throws ValidationException;
}

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

@ -117,6 +117,8 @@ import com.google.gerrit.server.project.RefPattern;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.util.MagicBranch;
import com.google.gerrit.server.validators.AccountActivationValidationListener;
import com.google.gerrit.server.validators.ValidationException;
import com.google.gerrit.testing.ConfigSuite;
import com.google.gerrit.testing.FakeEmailSender.Message;
import com.google.gerrit.testing.TestTimeUtil;
@ -216,6 +218,9 @@ public class AccountIT extends AbstractDaemonTest {
@Inject private AccountOperations accountOperations;
@Inject
private DynamicSet<AccountActivationValidationListener> accountActivationValidationListeners;
private AccountIndexedCounter accountIndexedCounter;
private RegistrationHandle accountIndexEventCounterHandle;
private RefUpdateCounter refUpdateCounter;
@ -518,6 +523,95 @@ public class AccountIT extends AbstractDaemonTest {
accountIndexedCounter.assertReindexOf(user);
}
@Test
public void validateAccountActivation() throws Exception {
com.google.gerrit.acceptance.testsuite.account.TestAccount activatableAccount =
accountOperations.newAccount().inactive().preferredEmail("foo@activatable.com").create();
com.google.gerrit.acceptance.testsuite.account.TestAccount deactivatableAccount =
accountOperations.newAccount().preferredEmail("foo@deactivatable.com").create();
RegistrationHandle registrationHandle =
accountActivationValidationListeners.add(
new AccountActivationValidationListener() {
@Override
public void validateActivation(AccountState account) throws ValidationException {
String preferredEmail = account.getAccount().getPreferredEmail();
if (preferredEmail == null || !preferredEmail.endsWith("@activatable.com")) {
throw new ValidationException("not allowed to active account");
}
}
@Override
public void validateDeactivation(AccountState account) throws ValidationException {
String preferredEmail = account.getAccount().getPreferredEmail();
if (preferredEmail == null || !preferredEmail.endsWith("@deactivatable.com")) {
throw new ValidationException("not allowed to deactive account");
}
}
});
try {
/* Test account that can be activated, but not deactivated */
// Deactivate account that is already inactive
try {
gApi.accounts().id(activatableAccount.accountId().get()).setActive(false);
fail("Expected exception");
} catch (ResourceConflictException e) {
assertThat(e.getMessage()).isEqualTo("account not active");
}
assertThat(accountOperations.account(activatableAccount.accountId()).get().active())
.isFalse();
// Activate account that can be activated
gApi.accounts().id(activatableAccount.accountId().get()).setActive(true);
assertThat(accountOperations.account(activatableAccount.accountId()).get().active()).isTrue();
// Activate account that is already active
gApi.accounts().id(activatableAccount.accountId().get()).setActive(true);
assertThat(accountOperations.account(activatableAccount.accountId()).get().active()).isTrue();
// Try deactivating account that cannot be deactivated
try {
gApi.accounts().id(activatableAccount.accountId().get()).setActive(false);
fail("Expected exception");
} catch (ResourceConflictException e) {
assertThat(e.getMessage()).isEqualTo("not allowed to deactive account");
}
assertThat(accountOperations.account(activatableAccount.accountId()).get().active()).isTrue();
/* Test account that can be deactivated, but not activated */
// Activate account that is already inactive
gApi.accounts().id(deactivatableAccount.accountId().get()).setActive(true);
assertThat(accountOperations.account(deactivatableAccount.accountId()).get().active())
.isTrue();
// Deactivate account that can be deactivated
gApi.accounts().id(deactivatableAccount.accountId().get()).setActive(false);
assertThat(accountOperations.account(deactivatableAccount.accountId()).get().active())
.isFalse();
// Deactivate account that is already inactive
try {
gApi.accounts().id(deactivatableAccount.accountId().get()).setActive(false);
fail("Expected exception");
} catch (ResourceConflictException e) {
assertThat(e.getMessage()).isEqualTo("account not active");
}
assertThat(accountOperations.account(deactivatableAccount.accountId()).get().active())
.isFalse();
// Try activating account that cannot be activated
try {
gApi.accounts().id(deactivatableAccount.accountId().get()).setActive(true);
fail("Expected exception");
} catch (ResourceConflictException e) {
assertThat(e.getMessage()).isEqualTo("not allowed to active account");
}
assertThat(accountOperations.account(deactivatableAccount.accountId()).get().active())
.isFalse();
} finally {
registrationHandle.remove();
}
}
@Test
public void deactivateSelf() throws Exception {
exception.expect(ResourceConflictException.class);