From 8a61badabc1f3f7e36af345e70906ea4d8394cef Mon Sep 17 00:00:00 2001 From: Tiago Salvado <9038083+tiagosalvado10@users.noreply.github.com> Date: Thu, 25 Jul 2024 21:23:51 +0100 Subject: [PATCH] [MNT-24513] Immutable user (IDS): allow to change enabled status (#2789) * [MNT-24513] Immutable user: allow enabled status change * [MNT-24513] Created 'allow.immutable.user.enabled.status.update' to control whether an immutabled user enabled status can be changed or not * [MNT-24513] Regardless user details enabled status, the person nodeRef enabled status is also checked * [MNT-24513] Prevent LDAP users from being disabled. Changed variable name. --- .../alfresco/rest/api/impl/PeopleImpl.java | 51 +++++++++++++--- .../alfresco/public-rest-context.xml | 1 + .../AuthenticationContextImpl.java | 61 ++++++++++++++++++- .../authentication-services-context.xml | 9 +++ .../resources/alfresco/repository.properties | 3 + 5 files changed, 116 insertions(+), 9 deletions(-) diff --git a/remote-api/src/main/java/org/alfresco/rest/api/impl/PeopleImpl.java b/remote-api/src/main/java/org/alfresco/rest/api/impl/PeopleImpl.java index 706e5238dd..d789a58d4e 100644 --- a/remote-api/src/main/java/org/alfresco/rest/api/impl/PeopleImpl.java +++ b/remote-api/src/main/java/org/alfresco/rest/api/impl/PeopleImpl.java @@ -125,7 +125,7 @@ public class PeopleImpl implements People protected ResetPasswordService resetPasswordService; protected UserRegistrySynchronizer userRegistrySynchronizer; protected Renditions renditions; - + private Boolean allowImmutableEnabledUpdate; private final static Map sort_params_to_qnames; static @@ -202,6 +202,11 @@ public class PeopleImpl implements People this.userRegistrySynchronizer = userRegistrySynchronizer; } + public void setAllowImmutableEnabledUpdate(Boolean allowImmutableEnabledUpdate) + { + this.allowImmutableEnabledUpdate = allowImmutableEnabledUpdate; + } + /** * Validate, perform -me- substitution and canonicalize the person ID. * @@ -708,16 +713,26 @@ public class PeopleImpl implements People // if requested, update password updatePassword(isAdmin, personIdToUpdate, person); - if (person.isEnabled() != null) + Set immutableProperties = userRegistrySynchronizer.getPersonMappedProperties(personIdToUpdate); + + Boolean isEnabled = person.isEnabled(); + if (isEnabled != null) { if (isAdminAuthority(personIdToUpdate)) { throw new PermissionDeniedException("Admin authority cannot be disabled."); } - // note: if current user is not an admin then permission denied exception is thrown - MutableAuthenticationService mutableAuthenticationService = (MutableAuthenticationService) authenticationService; - mutableAuthenticationService.setAuthenticationEnabled(personIdToUpdate, person.isEnabled()); + if (allowImmutableEnabledStatusUpdate(personIdToUpdate, isAdmin, immutableProperties)) + { + LOGGER.info("User " + personIdToUpdate + " is immutable but enabled status will be set to: " + isEnabled); + } + else + { + // note: if current user is not an admin then permission denied exception is thrown + MutableAuthenticationService mutableAuthenticationService = (MutableAuthenticationService) authenticationService; + mutableAuthenticationService.setAuthenticationEnabled(personIdToUpdate, person.isEnabled()); + } } NodeRef personNodeRef = personService.getPerson(personIdToUpdate, false); @@ -742,9 +757,7 @@ public class PeopleImpl implements People properties.putAll(nodes.mapToNodeProperties(customProps)); } - // MNT-21150 LDAP synced attributes can be changed using REST API - Set immutableProperties = userRegistrySynchronizer.getPersonMappedProperties(personIdToUpdate); - + // MNT-21150 LDAP synced attributes can't be changed using REST API immutableProperties.forEach(immutableProperty -> { if (properties.containsKey(immutableProperty)) { @@ -768,6 +781,28 @@ public class PeopleImpl implements People return getPerson(personId); } + private boolean allowImmutableEnabledStatusUpdate(String userId, boolean isAdmin, Set immutableProperties) + { + if (allowImmutableEnabledUpdate) + { + boolean containLdapUserAccountStatus = false; + QName propertyNameToCheck = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, "userAccountStatusProperty"); + + for (QName immutableProperty : immutableProperties) + { + if (immutableProperty.equals(propertyNameToCheck)) + { + containLdapUserAccountStatus = true; + break; + } + } + + return isAdmin && !containLdapUserAccountStatus && !isMutableAuthority(userId); + } + + return false; + } + private boolean checkCurrentUserOrAdmin(String personId) { boolean isAdmin = isAdminAuthority(); diff --git a/remote-api/src/main/resources/alfresco/public-rest-context.xml b/remote-api/src/main/resources/alfresco/public-rest-context.xml index 2f8497a061..a438e038a8 100644 --- a/remote-api/src/main/resources/alfresco/public-rest-context.xml +++ b/remote-api/src/main/resources/alfresco/public-rest-context.xml @@ -764,6 +764,7 @@ + diff --git a/repository/src/main/java/org/alfresco/repo/security/authentication/AuthenticationContextImpl.java b/repository/src/main/java/org/alfresco/repo/security/authentication/AuthenticationContextImpl.java index c9c4a84bb5..39b57d155f 100644 --- a/repository/src/main/java/org/alfresco/repo/security/authentication/AuthenticationContextImpl.java +++ b/repository/src/main/java/org/alfresco/repo/security/authentication/AuthenticationContextImpl.java @@ -36,7 +36,11 @@ import net.sf.acegisecurity.UserDetails; import net.sf.acegisecurity.providers.UsernamePasswordAuthenticationToken; import net.sf.acegisecurity.providers.dao.User; +import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.tenant.TenantService; +import org.alfresco.service.cmr.security.AuthenticationService; +import org.alfresco.service.cmr.security.MutableAuthenticationService; +import org.alfresco.service.cmr.security.PersonService; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -49,12 +53,30 @@ public class AuthenticationContextImpl implements AuthenticationContext private final Log logger = LogFactory.getLog(getClass()); private TenantService tenantService; + private PersonService personService; + private AuthenticationService authenticationService; + private Boolean allowImmutableEnabledUpdate; public void setTenantService(TenantService tenantService) { this.tenantService = tenantService; } + public void setPersonService(PersonService personService) + { + this.personService = personService; + } + + public void setAuthenticationService(AuthenticationService authenticationService) + { + this.authenticationService = authenticationService; + } + + public void setAllowImmutableEnabledUpdate(Boolean allowImmutableEnabledUpdate) + { + this.allowImmutableEnabledUpdate = allowImmutableEnabledUpdate; + } + /** * Explicitly set the given validated user details to be authenticated. * @@ -70,7 +92,7 @@ public class AuthenticationContextImpl implements AuthenticationContext { // Apply the same validation that ACEGI would have to the user details - we may be going through a 'back // door'. - if (!ud.isEnabled()) + if (isDisabled(userId, ud)) { throw new DisabledException("User is disabled"); } @@ -114,6 +136,43 @@ public class AuthenticationContextImpl implements AuthenticationContext } } + private boolean isDisabled(String userId, UserDetails ud) + { + boolean isDisabled = !ud.isEnabled(); + boolean isSystemUser = isSystemUserName(userId); + + if (allowImmutableEnabledUpdate && !isSystemUser) + { + try + { + boolean isImmutable = isImmutableAuthority(userId); + boolean isPersonEnabled = personService.isEnabled(userId); + isDisabled = isDisabled || (isImmutable && !isPersonEnabled); + } + catch (Exception e) + { + if (logger.isWarnEnabled()) + { + logger.warn("Failed to determine if person is enabled: " + userId + ", using user details status: " + isDisabled); + } + } + } + + return isDisabled; + } + + private boolean isImmutableAuthority(String authorityName) + { + return AuthenticationUtil.runAsSystem(new RunAsWork() + { + @Override public Boolean doWork() throws Exception + { + MutableAuthenticationService mutableAuthenticationService = (MutableAuthenticationService) authenticationService; + return !mutableAuthenticationService.isAuthenticationMutable(authorityName); + } + }); + } + public Authentication setSystemUserAsCurrentUser() { return setSystemUserAsCurrentUser(TenantService.DEFAULT_DOMAIN); diff --git a/repository/src/main/resources/alfresco/authentication-services-context.xml b/repository/src/main/resources/alfresco/authentication-services-context.xml index 1925c02d35..7d4f6d9666 100644 --- a/repository/src/main/resources/alfresco/authentication-services-context.xml +++ b/repository/src/main/resources/alfresco/authentication-services-context.xml @@ -274,6 +274,15 @@ + + + + + + + + ${allow.immutable.user.enabled.status.update} + diff --git a/repository/src/main/resources/alfresco/repository.properties b/repository/src/main/resources/alfresco/repository.properties index a6774373ca..a429143f61 100644 --- a/repository/src/main/resources/alfresco/repository.properties +++ b/repository/src/main/resources/alfresco/repository.properties @@ -435,6 +435,9 @@ repo.remote.endpoint=/service # persisted. create.missing.people=${server.transaction.allow-writes} +# Allow an immutable user to have its enabled status changed +allow.immutable.user.enabled.status.update=false + # Create home folders (unless disabled, see next property) as people are created (true) or create them lazily (false) home.folder.creation.eager=true # Disable home folder creation - if true then home folders are not created (neither eagerly nor lazily)