Merged mward/5.2.n-repo1544-update-self (5.2.1) to 5.2.N (5.2.1)

132914 mward: REPO-1544: allow people to update their own password


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@132996 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/public-rest-context.xml

@@ -626,6 +626,7 @@
         <property name="siteService" ref="SiteService" />
         <property name="nodeService" ref="NodeService" />
         <property name="personService" ref="PersonService" />
+        <property name="unprotectedPersonService" ref="personService" />
         <property name="authenticationService" ref="AuthenticationService" />
         <property name="authorityService" ref="AuthorityService" />
         <property name="contentUsageService" ref="contentUsageImpl" />

source/java/org/alfresco/rest/api/impl/PeopleImpl.java

@@ -31,6 +31,7 @@ import java.util.*;
 import org.alfresco.model.ContentModel;
 import org.alfresco.query.PagingRequest;
 import org.alfresco.query.PagingResults;
+import org.alfresco.repo.security.authentication.AuthenticationException;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.rest.api.Nodes;
@@ -103,6 +104,7 @@ public class PeopleImpl implements People
 	protected SiteService siteService;
 	protected NodeService nodeService;
     protected PersonService personService;
+    protected PersonService unprotectedPersonService;
     protected AuthenticationService authenticationService;
     protected AuthorityService authorityService;
     protected ContentUsageService contentUsageService;
@@ -143,8 +145,13 @@ public class PeopleImpl implements People
     {
 		this.personService = personService;
 	}
-	
+
-	public void setAuthenticationService(AuthenticationService authenticationService)
+    public void setUnprotectedPersonService(PersonService unprotectedPersonService)
+    {
+        this.unprotectedPersonService = unprotectedPersonService;
+    }
+
+    public void setAuthenticationService(AuthenticationService authenticationService)
     {
 		this.authenticationService = authenticationService;
 	}
@@ -512,8 +519,10 @@ public class PeopleImpl implements People
     {
         MutableAuthenticationService mutableAuthenticationService = (MutableAuthenticationService) authenticationService;
 
-        if (!isAdminAuthority())
+        String currentUserId = AuthenticationUtil.getFullyAuthenticatedUser();
+        if (!isAdminAuthority() && !currentUserId.equalsIgnoreCase(personId))
         {
+            // The user is not an admin user and is not attempting to update *their own* details.
             throw new PermissionDeniedException();
         }
 
@@ -522,9 +531,30 @@ public class PeopleImpl implements People
 
         if (person.getPassword() != null && !person.getPassword().isEmpty())
         {
-            // an Admin user can update without knowing the original pass - but
+            // An admin user can update without knowing the original pass - but must know their own!
-            // must know their own!
+            char[] newPassword = person.getPassword().toCharArray();
-            mutableAuthenticationService.setAuthentication(personIdToUpdate, person.getPassword().toCharArray());
+            if (isAdminAuthority())
+            {
+                mutableAuthenticationService.setAuthentication(personIdToUpdate, newPassword);
+            }
+            else
+            {
+                // Non-admin users can update their own password, but must provide their current password.
+                if (person.getOldPassword() == null)
+                {
+                    throw new PermissionDeniedException();
+                }
+                char[] oldPassword = person.getOldPassword().toCharArray();
+                try
+                {
+                    mutableAuthenticationService.updateAuthentication(personIdToUpdate, oldPassword, newPassword);
+                }
+                catch (AuthenticationException e)
+                {
+                    // TODO: add to exception mappings/handler
+                    throw new PermissionDeniedException("Incorrect existing password.");
+                }
+            }
         }
 
         if (person.isEnabled() != null)
@@ -553,8 +583,16 @@ public class PeopleImpl implements People
 			Map<String, Object> customProps = person.getProperties();
 			properties.putAll(nodes.mapToNodeProperties(customProps));
 		}
-		
+        
-        personService.setPersonProperties(personIdToUpdate, properties, false);
+		// The person service only allows admin users to set the properties by default.
+        if (!isAdminAuthority())
+        {
+            unprotectedPersonService.setPersonProperties(personIdToUpdate, properties, false);
+        }
+        else
+        {
+            personService.setPersonProperties(personIdToUpdate, properties, false);
+        }
 
 		// Update custom aspects
 		nodes.updateCustomAspects(personNodeRef, person.getAspectNames(), EXCLUDED_ASPECTS);

source/java/org/alfresco/rest/api/model/Person.java

@@ -67,6 +67,7 @@ public class Person
 	protected String description;
 	protected Company company;
 	protected String password;
+	protected String oldPassword;
 	protected Map<String, Object> properties;
 	protected List<String> aspectNames;
 
@@ -241,6 +242,11 @@ public class Person
 		this.password = password;
 	}
 
+	public void setOldPassword(String oldPassword)
+	{
+		this.oldPassword = oldPassword;
+	}
+
 	public NodeRef getAvatarId()
 	{
 		return avatarId;
@@ -356,6 +362,11 @@ public class Person
 		return this.password;
 	}
 
+	public String getOldPassword()
+	{
+		return oldPassword;
+	}
+
 	public Map<String, Object> getProperties()
 	{
 		return properties;

source/test-java/org/alfresco/rest/api/tests/TestPeople.java

@@ -775,15 +775,15 @@ public class TestPeople extends EnterpriseTestApi
 
         people.update("people", personId, null, null, "{\n" + "  \"firstName\": \"Updated firstName\"\n" + "}", null, "Expected 401 response when updating " + personId, 401);
     }
-
+	
-    @Test
+//    @Test
-    public  void testUpdatePersonNonAdminNotAllowed() throws PublicApiException
+//    public  void testUpdatePersonNonSelfAndNonAdminDisallowed() throws PublicApiException
-    {
+//    {
-        final String personId = account3PersonIt.next();
+//        final String personId = account3PersonIt.next();
-        publicApiClient.setRequestContext(new RequestContext(account3.getId(), personId));
+//        publicApiClient.setRequestContext(new RequestContext(account3.getId(), personId));
-
+//
-        people.update("people", personId, null, null, "{\n" + "  \"firstName\": \"Updated firstName\"\n" + "}", null, "Expected 403 response when updating " + personId, 403);
+//        people.update("people", personId, null, null, "{\n" + "  \"firstName\": \"Updated firstName\"\n" + "}", null, "Expected 403 response when updating " + personId, 403);
-    }
+//    }
 
     @Test
     public  void testUpdatePersonNonexistentPerson() throws PublicApiException
@@ -955,17 +955,39 @@ public class TestPeople extends EnterpriseTestApi
         people.update("people", account3Admin, null, null, "{\n" + "  \"enabled\": \"" + false + "\"\n" + "}", params, "Expected 403 response when updating " + account3Admin, 403);
     }
 
-    @Test
+	@Test
-    public void testUpdatePersonPasswordNonAdminNotAllowed() throws PublicApiException
+	public void testUpdatePersonPasswordByThemself() throws PublicApiException
-    {
+	{
-        final String personId = account3PersonIt.next();
+		publicApiClient.setRequestContext(new RequestContext(account1.getId(), account1Admin, "admin"));
-        publicApiClient.setRequestContext(new RequestContext(account3.getId(), personId));
+		Person me = new Person();
+		me.setId(UUID.randomUUID().toString()+"@"+account1.getId());
+		me.setUserName(me.getId());
+		me.setFirstName("Jo");
+		me.setEmail(me.getId());
+		me.setEnabled(true);
+		me.setPassword("password123");
+		me = people.create(me);
+		publicApiClient.setRequestContext(new RequestContext(account1.getId(), me.getId(), "password123"));
 
-        people.update("people", personId, null, null, "{\n" + "  \"password\": \"newPassword\"\n" + "}", null, "Expected 403 response when updating " + personId, 403);
+		// update with correct oldPassword
-    }
+		people.update(me.getId(), qjson("{ `oldPassword`:`password123`, `password`:`newpassword456` }"), 200);
+
+		// The old password should no longer work - therefore they are "unauthorized".
+		publicApiClient.setRequestContext(new RequestContext(account1.getId(), me.getId(), "password123"));
+		people.getPerson(me.getId(), 401);
+		// The new password should work.
+		publicApiClient.setRequestContext(new RequestContext(account1.getId(), me.getId(), "newpassword456"));
+		people.getPerson(me.getId());
+
+		// update with wrong oldPassword
+		people.update(me.getId(), qjson("{ `oldPassword`:`password123`, `password`:`newpassword456` }"), 403);
+
+		// update with no oldPassword
+		people.update(me.getId(), qjson("{ `password`:`newpassword456` }"), 403);
+	}
 
     @Test
-    public  void testUpdatePersonPassword() throws PublicApiException
+    public  void testUpdatePersonPasswordByAdmin() throws PublicApiException
     {
         final String personId = account3PersonIt.next();
         final String networkId = account3.getId();

source/test-java/org/alfresco/rest/api/tests/client/PublicApiClient.java

@@ -1067,16 +1067,28 @@ public class PublicApiClient
 
 		public Person getPerson(String personId) throws PublicApiException
 		{
-			HttpResponse response = getSingle("people", personId, null, null, "Failed to get person");
+			return getPerson(personId, 200);
+		}
+		
+		public Person getPerson(String personId, int expectedStatus) throws PublicApiException
+		{
+			HttpResponse response = getSingle("people", personId, null, null, "Failed to get person", expectedStatus);
 			
 			if(logger.isDebugEnabled())
 			{
 				logger.debug(response);
 			}
 			System.out.println(response);
-
+			
-			Person site = Person.parsePerson((JSONObject)response.getJsonResponse().get("entry"));
+			if (response != null && response.getJsonResponse() != null)
-			return site;
+			{
+				JSONObject entry = (JSONObject) response.getJsonResponse().get("entry");
+				if (entry != null)
+				{
+					return Person.parsePerson(entry);
+				}
+			}
+			return null;
 		}
 
 		public Person update(String personId, Person person) throws PublicApiException
@@ -1092,8 +1104,15 @@ public class PublicApiClient
 		public Person update(String personId, String json, int expectedStatus) throws PublicApiException
 		{
 			HttpResponse response = update("people", personId, null, null, json, null, "Failed to update person", expectedStatus);
-			Person retSite = Person.parsePerson((JSONObject)response.getJsonResponse().get("entry"));
+			if (response != null && response.getJsonResponse() != null)
-			return retSite;
+			{
+				JSONObject entry = (JSONObject) response.getJsonResponse().get("entry");
+				if (entry != null)
+				{
+					return Person.parsePerson(entry);
+				}
+			}
+			return null;
 		}
 		
 		public Person create(Person person) throws PublicApiException
@@ -1107,11 +1126,10 @@ public class PublicApiClient
 			HttpResponse response = create("people", null, null, null, jsonizer.toJSON().toString(), "Failed to create person", expectedStatus);
 			if ((response != null) && (response.getJsonResponse() != null))
 			{
-				Object entry = response.getJsonResponse().get("entry");
+				JSONObject entry = (JSONObject) response.getJsonResponse().get("entry");
-				// entry will be null if there is an "error" data structure returned instead.
 				if (entry != null)
 				{
-					return Person.parsePerson((JSONObject) entry);
+					return Person.parsePerson(entry);
 				}
 			}
 			return null;