Merge branch 'main' into DS-2058-7x

dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java

@@ -60,7 +60,9 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
     public void addMember(Context context, Group group, EPerson e);
 
     /**
-     * add group to this group
+     * add group to this group. Be sure to call the {@link #update(Context, Group)}
+     * method once that all the membership are set to trigger the rebuild of the
+     * group2group cache table
      *
      * @param context     DSpace context object
      * @param groupParent parent group
@@ -80,7 +82,9 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
 
 
     /**
-     * remove group from this group
+     * remove group from this group. Be sure to call the {@link #update(Context, Group)}
+     * method once that all the membership are set to trigger the rebuild of the
+     * group2group cache table
      *
      * @param context     DSpace context object
      * @param groupParent parent group

dspace-server-webapp/src/main/java/org/dspace/app/rest/GroupRestController.java

@@ -95,7 +95,8 @@ public class GroupRestController {
         for (Group childGroup : childGroups) {
             groupService.addMember(context, parentGroup, childGroup);
         }
-
+        // this is required to trigger the rebuild of the group2group cache
+        groupService.update(context, parentGroup);
         context.complete();
 
         response.setStatus(SC_NO_CONTENT);
@@ -203,7 +204,8 @@ public class GroupRestController {
         }
 
         groupService.removeMember(context, parentGroup, childGroup);
-
+        // this is required to trigger the rebuild of the group2group cache
+        groupService.update(context, parentGroup);
         context.complete();
 
         response.setStatus(SC_NO_CONTENT);

dspace-server-webapp/src/main/java/org/dspace/app/rest/RestResourceController.java

@@ -1021,15 +1021,18 @@ public class RestResourceController implements InitializingBean {
 
     /**
      * Internal method to convert the parameters provided as a MultivalueMap as a string to use in the self-link.
+     * This function will exclude all "embed" parameters and parameters starting with "embed."
      * @param parameters
-     * @return encoded uriString containing request parameters
+     * @return encoded uriString containing request parameters without embed parameter
      */
     private String getEncodedParameterStringFromRequestParams(
             @RequestParam MultiValueMap<String, Object> parameters) {
         UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.newInstance();
 
         for (String key : parameters.keySet()) {
-            uriComponentsBuilder.queryParam(key, parameters.get(key));
+            if (!StringUtils.equals(key, "embed") && !StringUtils.startsWith(key, "embed.")) {
+                uriComponentsBuilder.queryParam(key, parameters.get(key));
+            }
         }
         return uriComponentsBuilder.encode().build().toString();
     }

dspace-server-webapp/src/main/webapp/js/hal/http/client.js

@@ -92,7 +92,7 @@ function downloadFile(url) {
 HAL.Http.Client.prototype.get = function(url) {
     var self = this;
     this.vent.trigger('location-change', { url: url });
-    var jqxhr = $.ajax({
+    $.ajax({
         url: url,
         dataType: 'json',
         xhrFields: {
@@ -109,15 +109,18 @@ HAL.Http.Client.prototype.get = function(url) {
                 headers: jqXHR.getAllResponseHeaders()
             });
         },
-        error: function() {
+        error: function(jqXHR, textStatus, errorThrown) {
-            self.vent.trigger('fail-response', { jqxhr: jqxhr });
+            // Also check for updated token during errors. E.g. when a login failure occurs, token may be changed.
-            var contentTypeResponseHeader = jqxhr.getResponseHeader("content-type");
+            checkForUpdatedCSRFTokenInResponse(jqXHR);
+            self.vent.trigger('fail-response', { jqxhr: jqXHR });
+            var contentTypeResponseHeader = jqXHR.getResponseHeader("content-type");
             if (contentTypeResponseHeader != undefined
                     && !contentTypeResponseHeader.startsWith("application/hal")
                     && !contentTypeResponseHeader.startsWith("application/json")) {
                 downloadFile(url);
             }
-        }});
+        }
+    });
 };
 
 HAL.Http.Client.prototype.request = function(opts) {
@@ -137,6 +140,11 @@ HAL.Http.Client.prototype.request = function(opts) {
         checkForUpdatedCSRFTokenInResponse(jqXHR);
     };
 
+    // Also check error responses to see if CSRF Token has been updated
+    opts.error = function(jqXHR, textStatus, errorThrown) {
+         checkForUpdatedCSRFTokenInResponse(jqXHR);
+    };
+
     self.vent.trigger('location-change', { url: opts.url });
     return jqxhr = $.ajax(opts);
 };

dspace-server-webapp/src/main/webapp/login.html

@@ -73,11 +73,8 @@
         var successHandler = function(result, status, xhr) {
             // look for Authorization header & save to a MyHalBrowserToken cookie
             document.cookie = "MyHalBrowserToken=" + xhr.getResponseHeader('Authorization').split(" ")[1];
-            // look for DSpace-XSRF-TOKEN header & save to a MyHalBrowserCsrfToken cookie (if found)
+            // Check for an update to the CSRF Token & save to a MyHalBrowserCsrfToken cookie (if found)
-            var csrfToken = xhr.getResponseHeader('DSPACE-XSRF-TOKEN');
+            checkForUpdatedCSRFTokenInResponse(xhr);
-            if (csrfToken!=null) {
-                document.cookie = "MyHalBrowserCsrfToken=" + csrfToken;
-            }
             toastr.success('You are now logged in. Please wait while we redirect you...', 'Login Successful');
             setTimeout(function() {
                 window.location.href = window.location.pathname.replace("login.html", "");
@@ -112,9 +109,13 @@
                }
           },
           success : successHandler,
-          error : function(result, status, xhr) {
+          error : function(xhr, textStatus, errorThrown) {
-              if (result.status === 401) {
+              // Check for an update to the CSRF Token & save to a MyHalBrowserCsrfToken cookie (if found)
-                  var authenticate = result.getResponseHeader("WWW-Authenticate");
+              checkForUpdatedCSRFTokenInResponse(xhr);
+
+              // If 401 Unauthorized, check WWW-Authenticate for authentication options
+              if (xhr.status === 401) {
+                  var authenticate = xhr.getResponseHeader("WWW-Authenticate");
                   var element = $('div.other-login-methods');
                   if(authenticate !== null) {
                       var realms = authenticate.match(/(\w+ (\w+=((".*?")|[^,]*)(, )?)*)/g);
@@ -146,6 +147,18 @@
             return string.charAt(0).toUpperCase() + string.slice(1);
         }
 
+       /**
+        * Check current response headers to see if the CSRF Token has changed. If a new value is found in headers,
+        * save the new value into our "MyHalBrowserCsrfToken" cookie.
+        **/
+        function checkForUpdatedCSRFTokenInResponse(jqxhr) {
+            // look for DSpace-XSRF-TOKEN header & save to our MyHalBrowserCsrfToken cookie (if found)
+            var updatedCsrfToken = jqxhr.getResponseHeader('DSPACE-XSRF-TOKEN');
+            if (updatedCsrfToken != null) {
+               document.cookie = "MyHalBrowserCsrfToken=" + updatedCsrfToken;
+            }
+        }
+
        /**
         * Get CSRF Token by parsing it out of the "MyHalBrowserCsrfToken" cookie.
         * This cookie is set in login.html after a successful login occurs.

dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java

@@ -629,57 +629,67 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest {
     public void addChildGroupTest() throws Exception {
 
         GroupService groupService = EPersonServiceFactory.getInstance().getGroupService();
+        context.turnOffAuthorisationSystem();
+        EPerson member = EPersonBuilder.createEPerson(context).build();
+        Group parentGroup = GroupBuilder.createGroup(context).build();
+        Group parentGroupWithPreviousSubgroup = GroupBuilder.createGroup(context).build();
+        Group subGroup = GroupBuilder.createGroup(context).withParent(parentGroupWithPreviousSubgroup)
+                .addMember(eperson).build();
+        Group childGroup1 = GroupBuilder.createGroup(context).addMember(member).build();
+        Group childGroup2 = GroupBuilder.createGroup(context).build();
+        context.restoreAuthSystemState();
 
-        Group parentGroup = null;
+        String authToken = getAuthToken(admin.getEmail(), password);
-        Group childGroup1 = null;
+        getClient(authToken).perform(
-        Group childGroup2 = null;
+                post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups")
+                        .contentType(parseMediaType(TEXT_URI_LIST_VALUE))
+                        .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n"
+                                + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID()
+                        )
+        ).andExpect(status().isNoContent());
+        getClient(authToken).perform(
+                post("/api/eperson/groups/" + parentGroupWithPreviousSubgroup.getID() + "/subgroups")
+                        .contentType(parseMediaType(TEXT_URI_LIST_VALUE))
+                        .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n"
+                                + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID()
+                        )
+        ).andExpect(status().isNoContent());
 
-        try {
+        parentGroup = context.reloadEntity(parentGroup);
-            context.turnOffAuthorisationSystem();
+        parentGroupWithPreviousSubgroup = context.reloadEntity(parentGroupWithPreviousSubgroup);
+        subGroup = context.reloadEntity(subGroup);
+        childGroup1 = context.reloadEntity(childGroup1);
+        childGroup2 = context.reloadEntity(childGroup2);
 
-            parentGroup = groupService.create(context);
+        assertTrue(
-            childGroup1 = groupService.create(context);
+                groupService.isMember(parentGroup, childGroup1)
-            childGroup2 = groupService.create(context);
+        );
+        assertTrue(
+                groupService.isMember(parentGroup, childGroup2)
+        );
+        // member of the added groups should be member of the group now
+        assertTrue(
+                groupService.isMember(context, member, parentGroup)
+        );
 
-            context.commit();
+        // verify that the previous subGroup is still here
+        assertTrue(
+                groupService.isMember(parentGroupWithPreviousSubgroup, childGroup1)
+        );
+        assertTrue(
+                groupService.isMember(parentGroupWithPreviousSubgroup, childGroup2)
+        );
+        assertTrue(
+                groupService.isMember(parentGroupWithPreviousSubgroup, subGroup)
+        );
+        // and that both the member of the added groups than existing ones are still member
+        assertTrue(
+                groupService.isMember(context, member, parentGroupWithPreviousSubgroup)
+        );
+        assertTrue(
+                groupService.isMember(context, eperson, parentGroupWithPreviousSubgroup)
+        );
 
-            parentGroup = context.reloadEntity(parentGroup);
-            childGroup1 = context.reloadEntity(childGroup1);
-            childGroup2 = context.reloadEntity(childGroup2);
-
-            context.restoreAuthSystemState();
-            String authToken = getAuthToken(admin.getEmail(), password);
-            getClient(authToken).perform(
-                    post("/api/eperson/groups/" + parentGroup.getID() + "/subgroups")
-                            .contentType(parseMediaType(TEXT_URI_LIST_VALUE))
-                            .content(REST_SERVER_URL + "eperson/groups/" + childGroup1.getID() + "/\n"
-                                    + REST_SERVER_URL + "eperson/groups/" + childGroup2.getID()
-                            )
-            ).andExpect(status().isNoContent());
-
-            parentGroup = context.reloadEntity(parentGroup);
-            childGroup1 = context.reloadEntity(childGroup1);
-            childGroup2 = context.reloadEntity(childGroup2);
-
-            assertTrue(
-                    groupService.isMember(parentGroup, childGroup1)
-            );
-
-            assertTrue(
-                    groupService.isMember(parentGroup, childGroup2)
-            );
-
-        } finally {
-            if (parentGroup != null) {
-                GroupBuilder.deleteGroup(parentGroup.getID());
-            }
-            if (childGroup1 != null) {
-                GroupBuilder.deleteGroup(childGroup1.getID());
-            }
-            if (childGroup2 != null) {
-                GroupBuilder.deleteGroup(childGroup2.getID());
-            }
-        }
     }
 
     @Test
@@ -942,60 +952,64 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest {
 
     @Test
     public void addMemberTest() throws Exception {
-
         GroupService groupService = EPersonServiceFactory.getInstance().getGroupService();
-        EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService();
+        context.turnOffAuthorisationSystem();
+        Group parentGroup = GroupBuilder.createGroup(context).build();
+        Group parentGroupWithPreviousMember = GroupBuilder.createGroup(context).addMember(eperson).build();
+        Group groupWithSubgroup = GroupBuilder.createGroup(context).build();
+        Group subGroup = GroupBuilder.createGroup(context).withParent(groupWithSubgroup).build();
+        EPerson member1 = EPersonBuilder.createEPerson(context).build();
+        EPerson member2 = EPersonBuilder.createEPerson(context).build();
+        context.restoreAuthSystemState();
 
-        Group parentGroup = null;
+        String authToken = getAuthToken(admin.getEmail(), password);
-        EPerson member1 = null;
+        getClient(authToken).perform(
-        EPerson member2 = null;
+                post("/api/eperson/groups/" + parentGroup.getID() + "/epersons")
+                        .contentType(parseMediaType(TEXT_URI_LIST_VALUE))
+                        .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n"
+                                + REST_SERVER_URL + "eperson/groups/" + member2.getID()
+                        )
+        ).andExpect(status().isNoContent());
+        getClient(authToken).perform(
+                post("/api/eperson/groups/" + parentGroupWithPreviousMember.getID() + "/epersons")
+                        .contentType(parseMediaType(TEXT_URI_LIST_VALUE))
+                        .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n"
+                                + REST_SERVER_URL + "eperson/groups/" + member2.getID()
+                        )
+        ).andExpect(status().isNoContent());
+        getClient(authToken).perform(
+                post("/api/eperson/groups/" + subGroup.getID() + "/epersons")
+                        .contentType(parseMediaType(TEXT_URI_LIST_VALUE))
+                        .content(REST_SERVER_URL + "eperson/groups/" + member1.getID()
+                        )
+        ).andExpect(status().isNoContent());
+        parentGroup = context.reloadEntity(parentGroup);
+        parentGroupWithPreviousMember = context.reloadEntity(parentGroupWithPreviousMember);
+        groupWithSubgroup = context.reloadEntity(groupWithSubgroup);
+        member1 = context.reloadEntity(member1);
+        member2 = context.reloadEntity(member2);
+        eperson = context.reloadEntity(eperson);
 
-        try {
+        assertTrue(
-            context.turnOffAuthorisationSystem();
+                groupService.isMember(context, member1, parentGroup)
+        );
+        assertTrue(
+                groupService.isMember(context, member2, parentGroup)
+        );
 
-            parentGroup = groupService.create(context);
+        assertTrue(
-            member1 = ePersonService.create(context);
+                groupService.isMember(context, member1, parentGroupWithPreviousMember)
-            member2 = ePersonService.create(context);
+        );
+        assertTrue(
+                groupService.isMember(context, member2, parentGroupWithPreviousMember)
+        );
+        assertTrue(
+                groupService.isMember(context, eperson, parentGroupWithPreviousMember)
+        );
 
-            context.commit();
+        assertTrue(
-
+                groupService.isMember(context, member1, groupWithSubgroup)
-            parentGroup = context.reloadEntity(parentGroup);
+        );
-            member1 = context.reloadEntity(member1);
-            member2 = context.reloadEntity(member2);
-
-            context.restoreAuthSystemState();
-            String authToken = getAuthToken(admin.getEmail(), password);
-            getClient(authToken).perform(
-                    post("/api/eperson/groups/" + parentGroup.getID() + "/epersons")
-                            .contentType(parseMediaType(TEXT_URI_LIST_VALUE))
-                            .content(REST_SERVER_URL + "eperson/groups/" + member1.getID() + "/\n"
-                                    + REST_SERVER_URL + "eperson/groups/" + member2.getID()
-                            )
-            ).andExpect(status().isNoContent());
-
-            parentGroup = context.reloadEntity(parentGroup);
-            member1 = context.reloadEntity(member1);
-            member2 = context.reloadEntity(member2);
-
-            assertTrue(
-                    groupService.isMember(context, member1, parentGroup)
-            );
-
-            assertTrue(
-                    groupService.isMember(context, member2, parentGroup)
-            );
-
-        } finally {
-            if (parentGroup != null) {
-                GroupBuilder.deleteGroup(parentGroup.getID());
-            }
-            if (member1 != null) {
-                EPersonBuilder.deleteEPerson(member1.getID());
-            }
-            if (member2 != null) {
-                EPersonBuilder.deleteEPerson(member2.getID());
-            }
-        }
     }
 
     @Test
@@ -1253,43 +1267,47 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest {
     public void removeChildGroupTest() throws Exception {
 
         GroupService groupService = EPersonServiceFactory.getInstance().getGroupService();
+        context.turnOffAuthorisationSystem();
+        Group parentGroup = GroupBuilder.createGroup(context).build();
+        Group childGroup = GroupBuilder.createGroup(context).withParent(parentGroup).build();
+        Group childGroupWithMember = GroupBuilder.createGroup(context).addMember(eperson).withParent(parentGroup)
+                .build();
+        context.restoreAuthSystemState();
+        String authToken = getAuthToken(admin.getEmail(), password);
+        getClient(authToken).perform(
+                delete("/api/eperson/groups/" + parentGroup.getID() + "/subgroups/" + childGroup.getID())
+        ).andExpect(status().isNoContent());
 
-        Group parentGroup = null;
+        parentGroup = context.reloadEntity(parentGroup);
-        Group childGroup = null;
+        childGroup = context.reloadEntity(childGroup);
+        childGroupWithMember = context.reloadEntity(childGroupWithMember);
+        eperson = context.reloadEntity(eperson);
 
-        try {
+        assertFalse(
-            context.turnOffAuthorisationSystem();
+                groupService.isMember(parentGroup, childGroup)
+        );
+        assertTrue(
+                groupService.isMember(parentGroup, childGroupWithMember)
+        );
+        assertTrue(
+                groupService.isMember(context, eperson, parentGroup)
+        );
 
-            parentGroup = groupService.create(context);
+        getClient(authToken).perform(
-            childGroup = groupService.create(context);
+                delete("/api/eperson/groups/" + parentGroup.getID() + "/subgroups/" + childGroupWithMember.getID())
-            groupService.addMember(context, parentGroup, childGroup);
+        ).andExpect(status().isNoContent());
 
-            context.commit();
+        parentGroup = context.reloadEntity(parentGroup);
+        childGroup = context.reloadEntity(childGroup);
+        childGroupWithMember = context.reloadEntity(childGroupWithMember);
+        eperson = context.reloadEntity(eperson);
+        assertFalse(
+                groupService.isMember(parentGroup, childGroupWithMember)
+        );
+        assertFalse(
+                groupService.isMember(context, eperson, parentGroup)
+        );
 
-            parentGroup = context.reloadEntity(parentGroup);
-            childGroup = context.reloadEntity(childGroup);
-
-            context.restoreAuthSystemState();
-            String authToken = getAuthToken(admin.getEmail(), password);
-            getClient(authToken).perform(
-                    delete("/api/eperson/groups/" + parentGroup.getID() + "/subgroups/" + childGroup.getID())
-            ).andExpect(status().isNoContent());
-
-            parentGroup = context.reloadEntity(parentGroup);
-            childGroup = context.reloadEntity(childGroup);
-
-            assertFalse(
-                    groupService.isMember(parentGroup, childGroup)
-            );
-
-        } finally {
-            if (parentGroup != null) {
-                GroupBuilder.deleteGroup(parentGroup.getID());
-            }
-            if (childGroup != null) {
-                GroupBuilder.deleteGroup(childGroup.getID());
-            }
-        }
     }
 
     @Test
@@ -1493,45 +1511,38 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest {
     public void removeMemberTest() throws Exception {
 
         GroupService groupService = EPersonServiceFactory.getInstance().getGroupService();
-        EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService();
+        context.turnOffAuthorisationSystem();
+        EPerson member = EPersonBuilder.createEPerson(context).build();
+        EPerson member2 = EPersonBuilder.createEPerson(context).build();
+        Group parentGroup = GroupBuilder.createGroup(context).addMember(member).build();
+        Group childGroup = GroupBuilder.createGroup(context).withParent(parentGroup).addMember(member2).build();
+        context.restoreAuthSystemState();
 
-        Group parentGroup = null;
+        assertTrue(
-        EPerson member = null;
+                groupService.isMember(context, member, parentGroup)
+        );
+        // member2 is member of the parentGroup via the childGroup
+        assertTrue(
+                groupService.isMember(context, member2, parentGroup)
+        );
 
-        try {
+        String authToken = getAuthToken(admin.getEmail(), password);
-            context.turnOffAuthorisationSystem();
+        getClient(authToken).perform(
+                delete("/api/eperson/groups/" + parentGroup.getID() + "/epersons/" + member.getID())
+        ).andExpect(status().isNoContent());
+        // remove the member2 from the children group
+        getClient(authToken).perform(
+                delete("/api/eperson/groups/" + childGroup.getID() + "/epersons/" + member2.getID())
+        ).andExpect(status().isNoContent());
+        parentGroup = context.reloadEntity(parentGroup);
+        member = context.reloadEntity(member);
 
-            parentGroup = groupService.create(context);
+        assertFalse(
-            member = ePersonService.create(context);
+                groupService.isMember(context, member, parentGroup)
-            groupService.addMember(context, parentGroup, member);
+        );
-            assertTrue(groupService.isMember(context, member, parentGroup));
+        assertFalse(
-
+                groupService.isMember(context, member2, parentGroup)
-            context.commit();
+        );
-
-            parentGroup = context.reloadEntity(parentGroup);
-            member = context.reloadEntity(member);
-
-            context.restoreAuthSystemState();
-            String authToken = getAuthToken(admin.getEmail(), password);
-            getClient(authToken).perform(
-                    delete("/api/eperson/groups/" + parentGroup.getID() + "/epersons/" + member.getID())
-            ).andExpect(status().isNoContent());
-
-            parentGroup = context.reloadEntity(parentGroup);
-            member = context.reloadEntity(member);
-
-            assertFalse(
-                    groupService.isMember(context, member, parentGroup)
-            );
-
-        } finally {
-            if (parentGroup != null) {
-                GroupBuilder.deleteGroup(parentGroup.getID());
-            }
-            if (member != null) {
-                EPersonBuilder.deleteEPerson(member.getID());
-            }
-        }
     }
 
     @Test

dspace-server-webapp/src/test/java/org/dspace/app/rest/RestResourceControllerIT.java

@@ -91,6 +91,20 @@ public class RestResourceControllerIT extends AbstractControllerIntegrationTest
                                        endsWith("/api/core/metadatafields/search/byFieldName")));
     }
 
+    @Test
+    public void selfLinkContainsRequestParametersAndEmbedsWhenProvided() throws Exception {
+        // When we call a search endpoint with additional parameters and an embed parameter
+        getClient().perform(get("/api/core/metadatafields/search/byFieldName?schema=dc&offset=0&embed=schema"))
+                   // The self link should contain those same parameters
+                   .andExpect(jsonPath("$._links.self.href", endsWith(
+                           "/api/core/metadatafields/search/byFieldName?schema=dc&offset=0")));
+
+        getClient().perform(get("/api/core/metadatafields/search/byFieldName?schema=dc&offset=0&embed.size=schema=5"))
+                   // The self link should contain those same parameters
+                   .andExpect(jsonPath("$._links.self.href", endsWith(
+                           "/api/core/metadatafields/search/byFieldName?schema=dc&offset=0")));
+    }
+
 
 
 }