From 3c3afb10c7de96a201863096554eae9f300ed6dd Mon Sep 17 00:00:00 2001
From: Raluca Munteanu <raluca.munteanu@ness.com>
Date: Tue, 17 Oct 2017 10:48:18 +0300
Subject: [PATCH] REPO-1943: RestAPI: Deleting a non-existent group member does
 not return 404.    - Added verification for group membership existence.

---
 .../alfresco/rest/api/impl/GroupsImpl.java    | 11 ++++++-
 .../alfresco/rest/api/tests/GroupsTest.java   | 32 +++++++++++++++----
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/src/main/java/org/alfresco/rest/api/impl/GroupsImpl.java b/src/main/java/org/alfresco/rest/api/impl/GroupsImpl.java
index 22e824404e..737edad533 100644
--- a/src/main/java/org/alfresco/rest/api/impl/GroupsImpl.java
+++ b/src/main/java/org/alfresco/rest/api/impl/GroupsImpl.java
@@ -58,6 +58,7 @@ import org.alfresco.rest.api.model.GroupMember;
 import org.alfresco.rest.framework.core.exceptions.ConstraintViolatedException;
 import org.alfresco.rest.framework.core.exceptions.EntityNotFoundException;
 import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException;
+import org.alfresco.rest.framework.core.exceptions.NotFoundException;
 import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException;
 import org.alfresco.rest.framework.core.exceptions.UnsupportedResourceOperationException;
 import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo;
@@ -809,7 +810,15 @@ public class GroupsImpl implements Groups
         }
 
         validateGroupMemberId(groupMemberId);
-        // TODO: Verify if groupMemberId is member of groupId
+
+        // Verify if groupMemberId is member of groupId
+        AuthorityType authorityType = AuthorityType.getAuthorityType(groupMemberId);
+        Set<String> containedAuthorities = authorityService.getContainedAuthorities(authorityType, groupId, true);
+        if (!containedAuthorities.contains(groupMemberId))
+        {
+            throw new NotFoundException(groupMemberId + " is not member of " + groupId);
+        }
+
         authorityService.removeAuthority(groupId, groupMemberId);
     }
 
diff --git a/src/test/java/org/alfresco/rest/api/tests/GroupsTest.java b/src/test/java/org/alfresco/rest/api/tests/GroupsTest.java
index 117027fa63..e15c51650d 100644
--- a/src/test/java/org/alfresco/rest/api/tests/GroupsTest.java
+++ b/src/test/java/org/alfresco/rest/api/tests/GroupsTest.java
@@ -25,6 +25,23 @@
  */
 package org.alfresco.rest.api.tests;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
+
+import javax.servlet.http.HttpServletResponse;
+
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.rest.AbstractSingleNetworkSiteTest;
 import org.alfresco.rest.api.tests.client.PublicApiClient;
@@ -47,11 +64,6 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import javax.servlet.http.HttpServletResponse;
-import java.util.*;
-
-import static org.junit.Assert.*;
-
 /**
  * V1 REST API tests for managing Groups
  *
@@ -1780,6 +1792,12 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
                 groupsProxy.deleteGroupMembership(GROUP_EVERYONE, groupMemberA.getId(), HttpServletResponse.SC_CONFLICT);
             }
 
+            // Removing a group that is not a member (REPO-1943)
+            {
+                setRequestContext(user1);
+                groupsProxy.deleteGroupMembership(groupB.getId(), personMember.getId(), HttpServletResponse.SC_NOT_FOUND);
+            }
+
             // Authentication failed
             {
                 setRequestContext(networkOne.getId(), GUID.generate(), "password");
@@ -1788,8 +1806,10 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
 
             // User does not have permission to delete a group membership
             {
+                setRequestContext(networkOne.getId(), networkAdmin, DEFAULT_ADMIN_PWD);
+                groupsProxy.createGroupMember(groupA.getId(), personMember, HttpServletResponse.SC_CREATED);
                 setRequestContext(user1);
-                groupsProxy.deleteGroupMembership(groupA.getId(), groupMemberA.getId(), HttpServletResponse.SC_FORBIDDEN);
+                groupsProxy.deleteGroupMembership(groupA.getId(), personMember.getId(), HttpServletResponse.SC_FORBIDDEN);
             }
         }
         finally