Issue 1373: obtain special groups from user context when new token is generated (on token refresh) (ufal#1378)

* Issue 1373: obtain special groups from user context when new token is generated (on token refresh)

* resolve Copilot comments

* resolve Copilot Comments: compute special groups only when when user is authenticated

* Remove HttpSession dependency from ClarinShibAuthentication

Use request-scoped attributes for shib.authenticated instead of
HttpSession/JSESSIONID, aligning with upstream ShibAuthentication.
Follow-up to ufal#1373/ufal#1378.

* Guard against null special groups in Context.getSpecialGroups

A special-group UUID may reference a Group that has since been deleted;
GroupService.find returns null in that case. The list was built with an
unconditional add, so it could contain null elements, which caused an NPE
downstream (e.g. SpecialGroupClaimProvider.getValue maps group.getID()
while generating the JWT sg claim on token refresh). Filter nulls once
here so every caller is covered. Follow-up to ufal#1373/ufal#1378.

---------

Co-authored-by: Ondrej Kosarko <kosarko@ufal.mff.cuni.cz>
(cherry picked from commit 4c294b2)

Author: 2 people

dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java

@@ -277,7 +277,7 @@ public int authenticate(Context context, String username, String password,
 
             // Step 4: Log the user in.
             context.setCurrentUser(eperson);
-            request.getSession().setAttribute("shib.authenticated", true);
+            request.setAttribute("shib.authenticated", true);
             AuthenticateServiceFactory.getInstance().getAuthenticationService().initEPerson(context, request, eperson);
 
             log.info(eperson.getEmail() + " has been authenticated via shibboleth.");
@@ -330,42 +330,35 @@ public int authenticate(Context context, String username, String password,
     @Override
     public List<Group> getSpecialGroups(Context context, HttpServletRequest request) {
         try {
-            // User has not successfuly authenticated via shibboleth.
-            if (request == null ||
-                    context.getCurrentUser() == null ||
-                    request.getSession().getAttribute("shib.authenticated") == null) {
-                return Collections.EMPTY_LIST;
+            // User has not successfully authenticated via shibboleth.
+            if (request == null || context.getCurrentUser() == null) {
+                return Collections.emptyList();
             }
 
-            // If we have already calculated the special groups then return them.
-            if (request.getSession().getAttribute("shib.specialgroup") != null) {
-                log.debug("Returning cached special groups.");
-                List<UUID> sessionGroupIds = (List<UUID>) request.getSession().getAttribute("shib.specialgroup");
-                List<Group> result = new ArrayList<>();
-                for (UUID uuid : sessionGroupIds) {
-                    result.add(groupService.find(context, uuid));
-                }
-                return result;
+            List<Group> specialGroups = context.getSpecialGroups();
+            if (!specialGroups.isEmpty()) {
+                log.debug("Returning special groups from context.");
+                return specialGroups;
             }
 
+            if (request.getAttribute("shib.authenticated") == null) {
+                log.debug("User has not been authenticated via shibboleth, returning empty list of special groups.");
+                return Collections.emptyList();
+            }
 
             List<UUID> groupIds = new ShibGroup(new ShibHeaders(request), context).get();
-            // Cache the special groups, so we don't have to recalculate them again
-            // for this session.
-            request.getSession().setAttribute("shib.specialgroup", groupIds);
 
             List<Group> groups = new ArrayList<>();
             for (UUID uuid : groupIds) {
                 Group foundGroup = groupService.find(context, uuid);
-                if (Objects.isNull(foundGroup)) {
-                    continue;
+                if (foundGroup != null) {
+                    groups.add(foundGroup);
                 }
-                groups.add(foundGroup);
             }
             return groups;
         } catch (Throwable t) {
-            log.error("Unable to validate any sepcial groups this user may belong too because of an exception.", t);
-            return Collections.EMPTY_LIST;
+            log.error("Unable to validate any special groups this user may belong to because of an exception.", t);
+            return Collections.emptyList();
         }
     }
 
@@ -1291,7 +1284,7 @@ private String getShibURL(HttpServletRequest request) {
     public boolean isUsed(final Context context, final HttpServletRequest request) {
         if (request != null &&
                 context.getCurrentUser() != null &&
-                request.getSession().getAttribute("shib.authenticated") != null) {
+                request.getAttribute("shib.authenticated") != null) {
             return true;
         }
         return false;

dspace-api/src/main/java/org/dspace/core/Context.java

@@ -714,7 +714,12 @@ public boolean inSpecialGroup(UUID groupID) {
     public List<Group> getSpecialGroups() throws SQLException {
         List<Group> myGroups = new ArrayList<>();
         for (UUID groupId : specialGroups) {
-            myGroups.add(EPersonServiceFactory.getInstance().getGroupService().find(this, groupId));
+            Group group = EPersonServiceFactory.getInstance().getGroupService().find(this, groupId);
+            // A special group UUID may reference a group that has since been deleted; skip nulls
+            // so callers never receive a list containing null (avoids NPE downstream).
+            if (group != null) {
+                myGroups.add(group);
+            }
         }
 
         return myGroups;