From c24f7246ddae57d0f60d568a377cb09952ec4a53 Mon Sep 17 00:00:00 2001 From: Owen Li Date: Wed, 14 Jun 2017 10:04:00 -0400 Subject: [PATCH] Synchronize account inactive flag with LDAP auth Implement the capability to automatically synchronize an account's active/inactive flag with the authentication back-end. This change is intended to remove the manual steps involved with activating/deactivating Gerrit accounts when their status changes in the authentication back-end. Upon interactive login, an account's inactive flag should be updated accordingly, and the login attempt should succeed/fail accordingly. To maintain backwards compatibility, this feature is by default disabled, and can be enabled within gerrit.config for supported authentication back-ends. Currently, it is implemented only for LDAP. Change-Id: I9dc124473ec6c83c369a9eee278bc07fa7cf3d4c --- Documentation/config-gerrit.txt | 12 +++++++ .../gerrit/server/account/AccountManager.java | 32 +++++++++++++++++-- .../gerrit/server/account/AuthRequest.java | 18 +++++++++++ .../gerrit/server/auth/ldap/LdapRealm.java | 11 ++++++- 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 6e1f3941f6..b21501834a 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -628,6 +628,18 @@ enable registration of new email addresses. + By default, true. +[[auth.autoUpdateAccountActiveStatus]]auth.autoUpdateAccountActiveStatus:: ++ +Whether to allow automatic synchronization of an account's inactive flag upon login. +If set to true, upon login, if the authentication back-end reports the account as active, +the account's inactive flag in the internal Gerrit database will be updated to be active. +If the authentication back-end reports the account as inactive, the account's flag will be +updated to be inactive and the login attempt will be blocked. Users enabling this feature +should ensure that their authentication back-end is supported. Currently, only +strict 'LDAP' authentication is supported. ++ +By default, false. + [[cache]] === Section cache diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java index ec756bcd49..046b60a925 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java @@ -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 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"); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java index e654b8d737..6647ca4806 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AuthRequest.java @@ -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; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java index a34e3fcadb..179944428e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java @@ -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