This is an automated email from the ASF dual-hosted git repository.
enorman pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-jackrabbit-usermanager.git
The following commit(s) were added to refs/heads/master by this push:
new 84c75c5 SLING-12196 allowSelfChangePassword config key does not match
(#23)
84c75c5 is described below
commit 84c75c5f242db21d199ca472b65929f3f8d85bc3
Author: Eric Norman <[email protected]>
AuthorDate: Sun Dec 17 16:21:32 2023 -0800
SLING-12196 allowSelfChangePassword config key does not match (#23)
---
.../impl/AuthorizablePrivilegesInfoImpl.java | 14 +-
.../impl/post/ChangeUserPasswordServlet.java | 12 +-
.../it/AuthorizablePrivilegesInfoIT.java | 85 +++++++++++
.../usermanager/it/post/ChangeUserPasswordIT.java | 159 +++++++++++++++++++--
4 files changed, 248 insertions(+), 22 deletions(-)
diff --git
a/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/AuthorizablePrivilegesInfoImpl.java
b/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/AuthorizablePrivilegesInfoImpl.java
index 066bf9d..3e0edbb 100644
---
a/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/AuthorizablePrivilegesInfoImpl.java
+++
b/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/AuthorizablePrivilegesInfoImpl.java
@@ -97,15 +97,21 @@ public class AuthorizablePrivilegesInfoImpl implements
AuthorizablePrivilegesInf
private String usersPath;
private String groupsPath;
private boolean selfRegistrationEnabled;
- private boolean alwaysAllowSelfChangePassword = false;
+ private boolean allowSelfChangePassword = false;
@Reference(cardinality=ReferenceCardinality.OPTIONAL, policy =
ReferencePolicy.DYNAMIC)
private void bindChangeUserPassword(ChangeUserPassword changeUserPassword,
Map<String, Object> properties) {
- alwaysAllowSelfChangePassword =
OsgiUtil.toBoolean(properties.get("alwaysAllowSelfChangePassword"), false);
+ if (properties.containsKey("alwaysAllowSelfChangePassword")) {
+ // log warning about the wrong property name
+ log.warn("Obsolete 'alwaysAllowSelfChangePassword' configuration
key was detected for the bound ChangeUserPassword component. Please change that
key in your configuration to 'allowSelfChangePassword'.");
+ allowSelfChangePassword =
OsgiUtil.toBoolean(properties.get("alwaysAllowSelfChangePassword"), false);
+ } else {
+ allowSelfChangePassword =
OsgiUtil.toBoolean(properties.get("allowSelfChangePassword"), false);
+ }
}
@SuppressWarnings("unused")
private void unbindChangeUserPassword(ChangeUserPassword
changeUserPassword, Map<String, Object> properties) {
- alwaysAllowSelfChangePassword = false;
+ allowSelfChangePassword = false;
}
@Reference(cardinality=ReferenceCardinality.OPTIONAL, policy =
ReferencePolicy.DYNAMIC)
@@ -365,7 +371,7 @@ public class AuthorizablePrivilegesInfoImpl implements
AuthorizablePrivilegesInf
if (!allowed && jcrSession.getUserID().equals(userId))
{
// check if the ChangeUserPassword service is
configured to always allow
// a user to change their own password.
- allowed = alwaysAllowSelfChangePassword;
+ allowed = allowSelfChangePassword;
}
return allowed;
});
diff --git
a/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/post/ChangeUserPasswordServlet.java
b/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/post/ChangeUserPasswordServlet.java
index 8722d47..743b161 100644
---
a/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/post/ChangeUserPasswordServlet.java
+++
b/src/main/java/org/apache/sling/jackrabbit/usermanager/impl/post/ChangeUserPasswordServlet.java
@@ -146,7 +146,7 @@ public class ChangeUserPasswordServlet extends
AbstractAuthorizablePostServlet i
private String userAdminGroupName = DEFAULT_USER_ADMIN_GROUP_NAME;
- private boolean alwaysAllowSelfChangePassword = true;
+ private boolean allowSelfChangePassword = true;
/**
* The JCR Repository we access to resolve resources
@@ -170,7 +170,13 @@ public class ChangeUserPasswordServlet extends
AbstractAuthorizablePostServlet i
protected void activate(final Map<String, Object> props) {
super.activate(props);
- alwaysAllowSelfChangePassword =
OsgiUtil.toBoolean(props.get("alwaysAllowSelfChangePassword"), false);
+ if (props.containsKey("alwaysAllowSelfChangePassword")) {
+ // log warning about the wrong property name
+ log.warn("Obsolete 'alwaysAllowSelfChangePassword' configuration
key was detected. Please change the key name in your configuration to
'allowSelfChangePassword'");
+ allowSelfChangePassword =
OsgiUtil.toBoolean(props.get("alwaysAllowSelfChangePassword"), false);
+ } else {
+ allowSelfChangePassword =
OsgiUtil.toBoolean(props.get("allowSelfChangePassword"), false);
+ }
this.userAdminGroupName =
OsgiUtil.toString(props.get(PAR_USER_ADMIN_GROUP_NAME),
DEFAULT_USER_ADMIN_GROUP_NAME);
@@ -301,7 +307,7 @@ public class ChangeUserPasswordServlet extends
AbstractAuthorizablePostServlet i
if (oldPassword != null && oldPassword.length() > 0) {
// verify old password
- if (alwaysAllowSelfChangePassword &&
jcrSession.getUserID().equals(name)) {
+ if (allowSelfChangePassword &&
jcrSession.getUserID().equals(name)) {
// first check if the current user has enough permissions to
do this without
// the aid of a service session
AccessControlManager acm =
jcrSession.getAccessControlManager();
diff --git
a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/AuthorizablePrivilegesInfoIT.java
b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/AuthorizablePrivilegesInfoIT.java
index b3759eb..f239503 100644
---
a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/AuthorizablePrivilegesInfoIT.java
+++
b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/AuthorizablePrivilegesInfoIT.java
@@ -24,8 +24,10 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Dictionary;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.atomic.AtomicLong;
@@ -65,6 +67,8 @@ import org.ops4j.pax.exam.junit.PaxExam;
import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
import org.ops4j.pax.exam.spi.reactors.PerClass;
import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceReference;
+import org.osgi.service.cm.ConfigurationAdmin;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -83,6 +87,9 @@ public class AuthorizablePrivilegesInfoIT extends
UserManagerTestSupport {
@Inject
protected SlingRepository repository;
+ @Inject
+ protected ConfigurationAdmin configAdmin;
+
@Inject
private AuthorizablePrivilegesInfo privilegesInfo;
@@ -965,6 +972,84 @@ public class AuthorizablePrivilegesInfoIT extends
UserManagerTestSupport {
}
}
+ /**
+ * Checks whether the current user can change their own password
+ */
+ @Test
+ public void canChangePasswordForSelf() throws RepositoryException {
+ assertNotNull("Expected privilegesInfo to not be null",
privilegesInfo);
+
+ assertTrue("Should be allowed to change the user password",
+ privilegesInfo.canChangePassword(user1Session, user1.getID()));
+ }
+
+ /**
+ * Checks whether the current user can change their own password using the
obsolete configuration key
+ */
+ @Test
+ public void canChangePasswordForSelfWithObsoleteConfigurationKey() throws
RepositoryException, IOException {
+ // deny user1 rights to their own profile
+ Map<String, String> privileges = new HashMap<>();
+ privileges.put(String.format("privilege@%s",
PrivilegeConstants.REP_USER_MANAGEMENT), "denied");
+ modifyAce.modifyAce(adminSession, user1.getPath(), user1.getID(),
+ privileges,
+ "first");
+
+ // first try with the allowSelfChangePassword set to false
+ org.osgi.service.cm.Configuration configuration =
configAdmin.getConfiguration("org.apache.sling.jackrabbit.usermanager.impl.post.ChangeUserPasswordServlet",
null);
+ Dictionary<String, Object> originalServiceProps =
configuration.getProperties();
+ ServiceReference<ChangeUserPassword> serviceReference = null;
+ try {
+ // update the service configuration to ensure the option is
disabled
+ Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "alwaysAllowSelfChangePassword",
Boolean.FALSE);
+ configuration.update(newServiceProps);
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class,
+ "alwaysAllowSelfChangePassword", Boolean.FALSE);
+
+ assertNotNull("Expected privilegesInfo to not be null",
privilegesInfo);
+
+ assertFalse("Should be not allowed to change the user password",
+ privilegesInfo.canChangePassword(user1Session,
user1.getID()));
+ } finally {
+ if (serviceReference != null) {
+ // done with this.
+ bundleContext.ungetService(serviceReference);
+ }
+
+ //put the original config back
+ configuration.update(originalServiceProps);
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "alwaysAllowSelfChangePassword",
+ originalServiceProps == null ? null
:originalServiceProps.get("alwaysAllowSelfChangePassword"));
+ }
+
+ // second try with the allowSelfChangePassword set to true
+ configuration =
configAdmin.getConfiguration("org.apache.sling.jackrabbit.usermanager.impl.post.ChangeUserPasswordServlet",
null);
+ originalServiceProps = configuration.getProperties();
+ serviceReference = null;
+ try {
+ // update the service configuration to ensure the option is
disabled
+ Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "alwaysAllowSelfChangePassword",
Boolean.TRUE);
+ configuration.update(newServiceProps);
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class,
+ "alwaysAllowSelfChangePassword", Boolean.TRUE);
+
+ assertNotNull("Expected privilegesInfo to not be null",
privilegesInfo);
+
+ assertTrue("Should be allowed to change the user password",
+ privilegesInfo.canChangePassword(user1Session,
user1.getID()));
+ } finally {
+ if (serviceReference != null) {
+ // done with this.
+ bundleContext.ungetService(serviceReference);
+ }
+
+ //put the original config back
+ configuration.update(originalServiceProps);
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "alwaysAllowSelfChangePassword",
+ originalServiceProps == null ? null
:originalServiceProps.get("alwaysAllowSelfChangePassword"));
+ }
+ }
+
/**
* SLING-9814 Checks whether the current user has been granted privileges
* to change the anonymous user's password.
diff --git
a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/ChangeUserPasswordIT.java
b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/ChangeUserPasswordIT.java
index f439fbc..5f39bb4 100644
---
a/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/ChangeUserPasswordIT.java
+++
b/src/test/java/org/apache/sling/jcr/jackrabbit/usermanager/it/post/ChangeUserPasswordIT.java
@@ -170,13 +170,13 @@ public class ChangeUserPasswordIT extends
UserManagerTestSupport {
ServiceReference<ChangeUserPassword> serviceReference = null;
try {
// update the service configuration to ensure the option is enabled
- Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "alwaysAllowSelfChangePassword",
Boolean.TRUE);
+ Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "allowSelfChangePassword",
Boolean.TRUE);
configuration.update(newServiceProps);
new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class,
- "alwaysAllowSelfChangePassword", Boolean.TRUE);
+ "allowSelfChangePassword", Boolean.TRUE);
serviceReference =
bundleContext.getServiceReference(ChangeUserPassword.class);
- assertEquals(Boolean.TRUE,
serviceReference.getProperty("alwaysAllowSelfChangePassword"));
+ assertEquals(Boolean.TRUE,
serviceReference.getProperty("allowSelfChangePassword"));
ChangeUserPassword changeUserPassword =
bundleContext.getService(serviceReference);
assertNotNull(changeUserPassword);
@@ -204,8 +204,8 @@ public class ChangeUserPasswordIT extends
UserManagerTestSupport {
//put the original config back
configuration.update(originalServiceProps);
- new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "alwaysAllowSelfChangePassword",
- originalServiceProps == null ? null
:originalServiceProps.get("alwaysAllowSelfChangePassword"));
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "allowSelfChangePassword",
+ originalServiceProps == null ? null
:originalServiceProps.get("allowSelfChangePassword"));
}
}
@@ -219,13 +219,13 @@ public class ChangeUserPasswordIT extends
UserManagerTestSupport {
ServiceReference<ChangeUserPassword> serviceReference = null;
try {
// update the service configuration to ensure the option is
disabled
- Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "alwaysAllowSelfChangePassword",
Boolean.FALSE);
+ Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "allowSelfChangePassword",
Boolean.FALSE);
configuration.update(newServiceProps);
new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class,
- "alwaysAllowSelfChangePassword", Boolean.FALSE);
+ "allowSelfChangePassword", Boolean.FALSE);
serviceReference =
bundleContext.getServiceReference(ChangeUserPassword.class);
- assertEquals(Boolean.FALSE,
serviceReference.getProperty("alwaysAllowSelfChangePassword"));
+ assertEquals(Boolean.FALSE,
serviceReference.getProperty("allowSelfChangePassword"));
ChangeUserPassword changeUserPassword =
bundleContext.getService(serviceReference);
assertNotNull(changeUserPassword);
@@ -257,8 +257,8 @@ public class ChangeUserPasswordIT extends
UserManagerTestSupport {
//put the original config back
configuration.update(originalServiceProps);
- new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "alwaysAllowSelfChangePassword",
- originalServiceProps == null ? null
:originalServiceProps.get("alwaysAllowSelfChangePassword"));
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "allowSelfChangePassword",
+ originalServiceProps == null ? null
:originalServiceProps.get("allowSelfChangePassword"));
}
}
@@ -272,13 +272,13 @@ public class ChangeUserPasswordIT extends
UserManagerTestSupport {
ServiceReference<ChangeUserPassword> serviceReference = null;
try {
// update the service configuration to ensure the option is enabled
- Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "alwaysAllowSelfChangePassword",
Boolean.TRUE);
+ Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "allowSelfChangePassword",
Boolean.TRUE);
configuration.update(newServiceProps);
new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class,
- "alwaysAllowSelfChangePassword", Boolean.TRUE);
+ "allowSelfChangePassword", Boolean.TRUE);
serviceReference =
bundleContext.getServiceReference(ChangeUserPassword.class);
- assertEquals(Boolean.TRUE,
serviceReference.getProperty("alwaysAllowSelfChangePassword"));
+ assertEquals(Boolean.TRUE,
serviceReference.getProperty("allowSelfChangePassword"));
ChangeUserPassword changeUserPassword =
bundleContext.getService(serviceReference);
assertNotNull(changeUserPassword);
@@ -324,8 +324,8 @@ public class ChangeUserPasswordIT extends
UserManagerTestSupport {
//put the original config back
configuration.update(originalServiceProps);
- new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "alwaysAllowSelfChangePassword",
- originalServiceProps == null ? null
:originalServiceProps.get("alwaysAllowSelfChangePassword"));
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "allowSelfChangePassword",
+ originalServiceProps == null ? null
:originalServiceProps.get("allowSelfChangePassword"));
}
}
@@ -431,4 +431,133 @@ public class ChangeUserPasswordIT extends
UserManagerTestSupport {
}
}
+ /**
+ * For code coverage, test that changing another user's password works
while
+ * the allowSelfChangePassword config is true
+ */
+ @Test
+ public void changePasswordAsSelfGrantedForOtherUserWithOldPassword()
throws RepositoryException, IOException {
+ org.osgi.service.cm.Configuration configuration =
configAdmin.getConfiguration("org.apache.sling.jackrabbit.usermanager.impl.post.ChangeUserPasswordServlet",
null);
+ Dictionary<String, Object> originalServiceProps =
configuration.getProperties();
+ ServiceReference<ChangeUserPassword> serviceReference = null;
+ User user2 = null;
+ try {
+ // create a second user to attempt the change password on
+ user2 = createUser.createUser(adminSession,
createUniqueName("user"), "testPwd", "testPwd",
+ Collections.emptyMap(), new ArrayList<>());
+ if (adminSession.hasPendingChanges()) {
+ adminSession.save();
+ }
+
+ // update the service configuration to ensure the option is enabled
+ Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "allowSelfChangePassword",
Boolean.TRUE);
+ configuration.update(newServiceProps);
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class,
+ "allowSelfChangePassword", Boolean.TRUE);
+
+ serviceReference =
bundleContext.getServiceReference(ChangeUserPassword.class);
+ assertEquals(Boolean.TRUE,
serviceReference.getProperty("allowSelfChangePassword"));
+ ChangeUserPassword changeUserPassword =
bundleContext.getService(serviceReference);
+ assertNotNull(changeUserPassword);
+
+ //make sure the user1 has the expected privileges granted
+ Map<String, String> privileges = new HashMap<>();
+ privileges.put(String.format("privilege@%s", Privilege.JCR_READ),
"granted");
+ privileges.put(String.format("privilege@%s",
PrivilegeConstants.REP_USER_MANAGEMENT), "granted");
+ modifyAce.modifyAce(adminSession, user2.getPath(), user1.getID(),
+ privileges,
+ "first");
+
+
+ // create a fresh session so changes from the adminSession are
picked up
+ user1Session.logout();
+ user1Session = repository.login(new
SimpleCredentials(user1.getID(), "testPwd".toCharArray()));
+ assertNotNull("Expected user1Session to not be null",
user1Session);
+
+ // user can do the operation
+ assertTrue("Should be allowed to change the user password",
+ privilegesInfo.canChangePassword(user1Session,
user2.getID()));
+
+ // no oldPassword submitted
+ try {
+ changeUserPassword.changePassword(user1Session,
+ user2.getID(),
+ "testPwd",
+ "testPwdChanged",
+ "testPwdChanged",
+ new ArrayList<>());
+ } catch (RepositoryException e) {
+ fail("Did not expect a RepositoryException when changing user
passsword.");
+ }
+ try {
+ user1Session.save();
+ } catch (AccessDeniedException e) {
+ logger.error(String.format("Did not expect
AccessDeniedException when changing user passsword: %s", e.getMessage()), e);
+ fail("Did not expect AccessDeniedException when changing user
passsword: " + e.getMessage());
+ }
+ } finally {
+ if (user2 != null) {
+ deleteUser.deleteUser(adminSession, user2.getID(), new
ArrayList<>());
+ }
+
+ if (serviceReference != null) {
+ // done with this.
+ bundleContext.ungetService(serviceReference);
+ }
+
+ //put the original config back
+ configuration.update(originalServiceProps);
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "allowSelfChangePassword",
+ originalServiceProps == null ? null
:originalServiceProps.get("allowSelfChangePassword"));
+ }
+ }
+
+ /**
+ */
+ @Test
+ public void changePasswordAsSelfGrantedWithObsoleteConfigurationKey()
throws Exception {
+ org.osgi.service.cm.Configuration configuration =
configAdmin.getConfiguration("org.apache.sling.jackrabbit.usermanager.impl.post.ChangeUserPasswordServlet",
null);
+ Dictionary<String, Object> originalServiceProps =
configuration.getProperties();
+ ServiceReference<ChangeUserPassword> serviceReference = null;
+ try {
+ // update the service configuration to ensure the option is
disabled
+ Dictionary<String, Object> newServiceProps =
replaceConfigProp(originalServiceProps, "alwaysAllowSelfChangePassword",
Boolean.TRUE);
+ configuration.update(newServiceProps);
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class,
+ "alwaysAllowSelfChangePassword", Boolean.TRUE);
+
+ serviceReference =
bundleContext.getServiceReference(ChangeUserPassword.class);
+ assertEquals(Boolean.TRUE,
serviceReference.getProperty("alwaysAllowSelfChangePassword"));
+ ChangeUserPassword changeUserPassword =
bundleContext.getService(serviceReference);
+ assertNotNull(changeUserPassword);
+
+ // user can do the operation
+ assertTrue("Should be allowed to change the user password",
+ privilegesInfo.canChangePassword(user1Session,
user1.getID()));
+
+ changeUserPassword.changePassword(user1Session,
+ user1.getID(),
+ "testPwd",
+ "testPwdChanged",
+ "testPwdChanged",
+ new ArrayList<>());
+ try {
+ user1Session.save();
+ } catch (AccessDeniedException e) {
+ logger.error(String.format("Did not expect
AccessDeniedException when changing user passsword: %s", e.getMessage()), e);
+ fail("Did not expect AccessDeniedException when changing user
passsword: " + e.getMessage());
+ }
+ } finally {
+ if (serviceReference != null) {
+ // done with this.
+ bundleContext.ungetService(serviceReference);
+ }
+
+ //put the original config back
+ configuration.update(originalServiceProps);
+ new WaitForServiceUpdated(5000, 100, bundleContext,
ChangeUserPassword.class, "alwaysAllowSelfChangePassword",
+ originalServiceProps == null ? null
:originalServiceProps.get("alwaysAllowSelfChangePassword"));
+ }
+ }
+
}
