diff --git a/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java index ba5d8cd65bfa..ef0b9e4b0a82 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java +++ b/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 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 sessionGroupIds = (List) request.getSession().getAttribute("shib.specialgroup"); - List result = new ArrayList<>(); - for (UUID uuid : sessionGroupIds) { - result.add(groupService.find(context, uuid)); - } - return result; + List 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 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 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; diff --git a/dspace-api/src/main/java/org/dspace/core/Context.java b/dspace-api/src/main/java/org/dspace/core/Context.java index e721deff5e71..3f615d4ebfb2 100644 --- a/dspace-api/src/main/java/org/dspace/core/Context.java +++ b/dspace-api/src/main/java/org/dspace/core/Context.java @@ -714,7 +714,12 @@ public boolean inSpecialGroup(UUID groupID) { public List getSpecialGroups() throws SQLException { List 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;