opendev/gerrit
Code Issues Proposed changes

Merge changes I4ecdc08d,I9dc12447,I173903d2

Browse Source 
* changes:
  Create a scheduled task to automatically deactivate accounts
  Synchronize account inactive flag with LDAP auth
  Change SetInactiveFlag to accept account id instead of IdentifiedUser
This commit is contained in:
Dave Borowitz
2017-10-01 13:31:04 +00:00
committed by Gerrit Code Review
parent 7881fbe5bb e2877bb28d
commit 297bb9c95b
12 changed files with 271 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

121
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDeactivator.java Normal file
View File

@@ -0,0 +1,121 @@
// Copyright (C) 2017 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.account;
import static com.google.gerrit.server.config.ScheduleConfig.MISSING_CONFIG;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.ScheduleConfig;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.query.account.AccountPredicates;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.Config;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Runnable to enable scheduling account deactivations to run periodically */
public class AccountDeactivator implements Runnable {
private static final Logger log = LoggerFactory.getLogger(AccountDeactivator.class);
public static class Module extends LifecycleModule {
@Override
protected void configure() {
listener().to(Lifecycle.class);
}
}
static class Lifecycle implements LifecycleListener {
private final WorkQueue queue;
private final AccountDeactivator deactivator;
private final boolean supportAutomaticAccountActivityUpdate;
private final ScheduleConfig scheduleConfig;
@Inject
Lifecycle(WorkQueue queue, AccountDeactivator deactivator, @GerritServerConfig Config cfg) {
this.queue = queue;
this.deactivator = deactivator;
scheduleConfig = new ScheduleConfig(cfg, "accountDeactivation");
supportAutomaticAccountActivityUpdate =
cfg.getBoolean("auth", "autoUpdateAccountActiveStatus", false);
}
@Override
public void start() {
if (!supportAutomaticAccountActivityUpdate) {
return;
}
long interval = scheduleConfig.getInterval();
long delay = scheduleConfig.getInitialDelay();
if (delay == MISSING_CONFIG && interval == MISSING_CONFIG) {
log.info("Ignoring missing accountDeactivator schedule configuration");
} else if (delay < 0 || interval <= 0) {
log.warn(
String.format(
"Ignoring invalid accountDeactivator schedule configuration: %s", scheduleConfig));
} else {
queue
.getDefaultQueue()
.scheduleAtFixedRate(deactivator, delay, interval, TimeUnit.MILLISECONDS);
}
}
@Override
public void stop() {
// handled by WorkQueue.stop() already
}
}
private final Provider<InternalAccountQuery> accountQueryProvider;
private final Realm realm;
private final SetInactiveFlag sif;
@Inject
AccountDeactivator(
Provider<InternalAccountQuery> accountQueryProvider, SetInactiveFlag sif, Realm realm) {
this.accountQueryProvider = accountQueryProvider;
this.sif = sif;
this.realm = realm;
}
@Override
public void run() {
log.debug("Running account deactivations");
try {
int numberOfAccountsDeactivated = 0;
for (AccountState acc : accountQueryProvider.get().query(AccountPredicates.isActive())) {
log.debug("processing account " + acc.getUserName());
if (acc.getUserName() != null && !realm.isActive(acc.getUserName())) {
sif.deactivate(acc.getAccount().getId());
log.debug("deactivated accout " + acc.getUserName());
numberOfAccountsDeactivated++;
}
}
log.info(
"Deactivations complete, {} account(s) were deactivated", numberOfAccountsDeactivated);
} catch (Exception e) {
log.error("Failed to deactivate inactive accounts " + e.getMessage(), e);
}
}
@Override
public String toString() {
return "account deactivator";
}
}

32
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
View File

@@ -22,6 +22,8 @@ import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.errors.NameAlreadyUsedException;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.client.AccountFieldName;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb;
@@ -70,6 +72,8 @@ public class AccountManager {
private final ExternalIds externalIds;
private final ExternalIdsUpdate.Server externalIdsUpdateFactory;
private final GroupsUpdate.Factory groupsUpdateFactory;
private final boolean autoUpdateAccountActiveStatus;
private final SetInactiveFlag setInactiveFlag;
@Inject
AccountManager(
@@ -86,7 +90,8 @@ public class AccountManager {
Provider<InternalAccountQuery> accountQueryProvider,
ExternalIds externalIds,
ExternalIdsUpdate.Server externalIdsUpdateFactory,
GroupsUpdate.Factory groupsUpdateFactory) {
GroupsUpdate.Factory groupsUpdateFactory,
SetInactiveFlag setInactiveFlag) {
this.schema = schema;
this.sequences = sequences;
this.accounts = accounts;
@@ -102,6 +107,9 @@ public class AccountManager {
this.externalIds = externalIds;
this.externalIdsUpdateFactory = externalIdsUpdateFactory;
this.groupsUpdateFactory = groupsUpdateFactory;
this.autoUpdateAccountActiveStatus =
cfg.getBoolean("auth", "autoUpdateAccountActiveStatus", false);
this.setInactiveFlag = setInactiveFlag;
}
/** @return user identified by this external identity string */
@@ -122,8 +130,8 @@ public class AccountManager {
* @param who identity of the user, with any details we received about them.
* @return the result of authenticating the user.
* @throws AccountException the account does not exist, and cannot be created, or exists, but
* cannot be located, or is inactive, or cannot be added to the admin group (only for the
* first account).
* cannot be located, is unable to be activated or deactivated, or is inactive, or cannot be
* added to the admin group (only for the first account).
*/
public AuthResult authenticate(AuthRequest who) throws AccountException, IOException {
who = realm.authenticate(who);
@@ -138,6 +146,24 @@ public class AccountManager {
// Account exists
Account act = byIdCache.get(id.accountId()).getAccount();
if (autoUpdateAccountActiveStatus && who.authProvidesAccountActiveStatus()) {
if (who.isActive() && !act.isActive()) {
try {
setInactiveFlag.activate(act.getId());
act = byIdCache.get(id.accountId()).getAccount();
} catch (ResourceNotFoundException e) {
throw new AccountException("Unable to activate account " + act.getId(), e);
}
} else if (!who.isActive() && act.isActive()) {
try {
setInactiveFlag.deactivate(act.getId());
act = byIdCache.get(id.accountId()).getAccount();
} catch (RestApiException e) {
throw new AccountException("Unable to deactivate account " + act.getId(), e);
}
}
}
if (!act.isActive()) {
throw new AccountException("Authentication error, account inactive");
}

18
gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java
View File

@@ -63,6 +63,8 @@ public class AuthRequest {
private boolean skipAuthentication;
private String authPlugin;
private String authProvider;
private boolean authProvidesAccountActiveStatus;
private boolean active;
public AuthRequest(ExternalId.Key externalId) {
this.externalId = externalId;
@@ -140,4 +142,20 @@ public class AuthRequest {
public void setAuthProvider(String authProvider) {
this.authProvider = authProvider;
}
public boolean authProvidesAccountActiveStatus() {
return authProvidesAccountActiveStatus;
}
public void setAuthProvidesAccountActiveStatus(boolean authProvidesAccountActiveStatus) {
this.authProvidesAccountActiveStatus = authProvidesAccountActiveStatus;
}
public boolean isActive() {
return active;
}
public void setActive(Boolean isActive) {
this.active = isActive;
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
View File

@@ -49,6 +49,6 @@ public class DeleteActive implements RestModifyView<AccountResource, Input> {
if (self.get() == rsrc.getUser()) {
throw new ResourceConflictException("cannot deactivate own account");
}
return setInactiveFlag.deactivate(rsrc.getUser());
return setInactiveFlag.deactivate(rsrc.getUser().getAccountId());
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
View File

@@ -41,6 +41,6 @@ public class PutActive implements RestModifyView<AccountResource, Input> {
@Override
public Response<String> apply(AccountResource rsrc, Input input)
throws ResourceNotFoundException, OrmException, IOException, ConfigInvalidException {
return setInactiveFlag.activate(rsrc.getUser());
return setInactiveFlag.activate(rsrc.getUser().getAccountId());
}
}

13
gerrit-server/src/main/java/com/google/gerrit/server/account/Realm.java
View File

@@ -19,6 +19,8 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.server.IdentifiedUser;
import java.io.IOException;
import java.util.Set;
import javax.naming.NamingException;
import javax.security.auth.login.LoginException;
public interface Realm {
/** Can the end-user modify this field of their own account? */
@@ -45,4 +47,15 @@ public interface Realm {
* into an email address, and then locate the user by that email address.
*/
Account.Id lookup(String accountName) throws IOException;
/**
* @return true if the account is active.
* @throws NamingException
* @throws LoginException
* @throws AccountException
*/
default boolean isActive(@SuppressWarnings("unused") String username)
throws LoginException, NamingException, AccountException {
return true;
}
}

9
gerrit-server/src/main/java/com/google/gerrit/server/account/SetInactiveFlag.java
View File

@@ -19,7 +19,6 @@ 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.IdentifiedUser;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -36,14 +35,14 @@ public class SetInactiveFlag {
this.accountsUpdate = accountsUpdate;
}
public Response<?> deactivate(IdentifiedUser user)
public Response<?> deactivate(Account.Id accountId)
throws RestApiException, IOException, ConfigInvalidException {
AtomicBoolean alreadyInactive = new AtomicBoolean(false);
Account account =
accountsUpdate
.create()
.update(
user.getAccountId(),
accountId,
a -> {
if (!a.isActive()) {
alreadyInactive.set(true);
@@ -60,14 +59,14 @@ public class SetInactiveFlag {
return Response.none();
}
public Response<String> activate(IdentifiedUser user)
public Response<String> activate(Account.Id accountId)
throws ResourceNotFoundException, IOException, ConfigInvalidException {
AtomicBoolean alreadyActive = new AtomicBoolean(false);
Account account =
accountsUpdate
.create()
.update(
user.getAccountId(),
accountId,
a -> {
if (a.isActive()) {
alreadyActive.set(true);

24
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java
View File

@@ -33,6 +33,7 @@ import com.google.gerrit.server.account.GroupBackends;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.auth.AuthenticationUnavailableException;
import com.google.gerrit.server.auth.NoSuchUserException;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
@@ -232,7 +233,15 @@ class LdapRealm extends AbstractRealm {
}
try {
final Helper.LdapSchema schema = helper.getSchema(ctx);
final LdapQuery.Result m = helper.findAccount(schema, ctx, username, fetchMemberOfEagerly);
LdapQuery.Result m;
who.setAuthProvidesAccountActiveStatus(true);
try {
m = helper.findAccount(schema, ctx, username, fetchMemberOfEagerly);
who.setActive(true);
} catch (NoSuchUserException e) {
who.setActive(false);
return who;
}
if (authConfig.getAuthType() == AuthType.LDAP && !who.isSkipAuthentication()) {
// We found the user account, but we need to verify
@@ -314,6 +323,19 @@ class LdapRealm extends AbstractRealm {
}
}
@Override
public boolean isActive(String username)
throws LoginException, NamingException, AccountException {
try {
DirContext ctx = helper.open();
Helper.LdapSchema schema = helper.getSchema(ctx);
helper.findAccount(schema, ctx, username, false);
} catch (NoSuchUserException e) {
return false;
}
return true;
}
static class UserLoader extends CacheLoader<String, Optional<Account.Id>> {
private final ExternalIds externalIds;

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

@@ -81,6 +81,7 @@ import com.google.gerrit.server.PluginUser;
import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.account.AccountCacheImpl;
import com.google.gerrit.server.account.AccountControl;
import com.google.gerrit.server.account.AccountDeactivator;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.AccountVisibilityProvider;
@@ -279,6 +280,7 @@ public class GerritGlobalModule extends FactoryModule {
bind(GcConfig.class);
bind(ChangeCleanupConfig.class);
bind(AccountDeactivator.class);
bind(ApprovalsUtil.class);