Skip to content

Commit 7415b4c

Browse files
kosarkomilanmajchrak
authored andcommitted
UFAL/Obtain special groups from user context when new token is generated (on token refresh) (ufal#1378) (#1347)
(cherry picked from commit 74f5862 on dtq-dev) v9 adaptations / additions: - ClarinShibbolethSpecialGroupsIT.java added verbatim from the zcu backport branch head b7d3c78 (origin/zcu-pub/backport-1347-shib-special-groups) — the regression IT exists only on the 7.6 customer backport branches, not on dtq-dev; fix sourced from 74f5862 (canonical), test from b7d3c78 per the sync plan card. - No code adaptations: the pick applied conflict-free (method bodies only, jakarta imports untouched); Context.getSpecialGroups null-guard included. Fulfils CLARIN_V9_POST_SNAPSHOT_SYNC_ACCEPTANCE.md §5 / 74f5862 (BE-1, Vlna 1).
1 parent 255549b commit 7415b4c

3 files changed

Lines changed: 204 additions & 25 deletions

File tree

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

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ public int authenticate(Context context, String username, String password,
277277

278278
// Step 4: Log the user in.
279279
context.setCurrentUser(eperson);
280-
request.getSession().setAttribute("shib.authenticated", true);
280+
request.setAttribute("shib.authenticated", true);
281281
AuthenticateServiceFactory.getInstance().getAuthenticationService().initEPerson(context, request, eperson);
282282

283283
log.info(eperson.getEmail() + " has been authenticated via shibboleth.");
@@ -330,42 +330,35 @@ public int authenticate(Context context, String username, String password,
330330
@Override
331331
public List<Group> getSpecialGroups(Context context, HttpServletRequest request) {
332332
try {
333-
// User has not successfuly authenticated via shibboleth.
334-
if (request == null ||
335-
context.getCurrentUser() == null ||
336-
request.getSession().getAttribute("shib.authenticated") == null) {
337-
return Collections.EMPTY_LIST;
333+
// User has not successfully authenticated via shibboleth.
334+
if (request == null || context.getCurrentUser() == null) {
335+
return Collections.emptyList();
338336
}
339337

340-
// If we have already calculated the special groups then return them.
341-
if (request.getSession().getAttribute("shib.specialgroup") != null) {
342-
log.debug("Returning cached special groups.");
343-
List<UUID> sessionGroupIds = (List<UUID>) request.getSession().getAttribute("shib.specialgroup");
344-
List<Group> result = new ArrayList<>();
345-
for (UUID uuid : sessionGroupIds) {
346-
result.add(groupService.find(context, uuid));
347-
}
348-
return result;
338+
List<Group> specialGroups = context.getSpecialGroups();
339+
if (!specialGroups.isEmpty()) {
340+
log.debug("Returning special groups from context.");
341+
return specialGroups;
349342
}
350343

344+
if (request.getAttribute("shib.authenticated") == null) {
345+
log.debug("User has not been authenticated via shibboleth, returning empty list of special groups.");
346+
return Collections.emptyList();
347+
}
351348

352349
List<UUID> groupIds = new ShibGroup(new ShibHeaders(request), context).get();
353-
// Cache the special groups, so we don't have to recalculate them again
354-
// for this session.
355-
request.getSession().setAttribute("shib.specialgroup", groupIds);
356350

357351
List<Group> groups = new ArrayList<>();
358352
for (UUID uuid : groupIds) {
359353
Group foundGroup = groupService.find(context, uuid);
360-
if (Objects.isNull(foundGroup)) {
361-
continue;
354+
if (foundGroup != null) {
355+
groups.add(foundGroup);
362356
}
363-
groups.add(foundGroup);
364357
}
365358
return groups;
366359
} catch (Throwable t) {
367-
log.error("Unable to validate any sepcial groups this user may belong too because of an exception.", t);
368-
return Collections.EMPTY_LIST;
360+
log.error("Unable to validate any special groups this user may belong to because of an exception.", t);
361+
return Collections.emptyList();
369362
}
370363
}
371364

@@ -1291,7 +1284,7 @@ private String getShibURL(HttpServletRequest request) {
12911284
public boolean isUsed(final Context context, final HttpServletRequest request) {
12921285
if (request != null &&
12931286
context.getCurrentUser() != null &&
1294-
request.getSession().getAttribute("shib.authenticated") != null) {
1287+
request.getAttribute("shib.authenticated") != null) {
12951288
return true;
12961289
}
12971290
return false;

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,12 @@ public boolean inSpecialGroup(UUID groupID) {
686686
public List<Group> getSpecialGroups() throws SQLException {
687687
List<Group> myGroups = new ArrayList<>();
688688
for (UUID groupId : specialGroups) {
689-
myGroups.add(EPersonServiceFactory.getInstance().getGroupService().find(this, groupId));
689+
Group group = EPersonServiceFactory.getInstance().getGroupService().find(this, groupId);
690+
// A special group UUID may reference a group that has since been deleted; skip nulls
691+
// so callers never receive a list containing null (avoids NPE downstream).
692+
if (group != null) {
693+
myGroups.add(group);
694+
}
690695
}
691696

692697
return myGroups;
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
/**
2+
* The contents of this file are subject to the license and copyright
3+
* detailed in the LICENSE and NOTICE files at the root of the source
4+
* tree and available online at
5+
*
6+
* http://www.dspace.org/license/
7+
*/
8+
package org.dspace.app.rest.security;
9+
10+
import static org.junit.Assert.assertNotNull;
11+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
12+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
13+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
14+
15+
import java.io.InputStream;
16+
17+
import com.fasterxml.jackson.databind.JsonNode;
18+
import com.fasterxml.jackson.databind.ObjectMapper;
19+
import org.apache.commons.codec.CharEncoding;
20+
import org.apache.commons.io.IOUtils;
21+
import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
22+
import org.dspace.app.util.Util;
23+
import org.dspace.builder.BitstreamBuilder;
24+
import org.dspace.builder.CollectionBuilder;
25+
import org.dspace.builder.CommunityBuilder;
26+
import org.dspace.builder.EPersonBuilder;
27+
import org.dspace.builder.GroupBuilder;
28+
import org.dspace.builder.ItemBuilder;
29+
import org.dspace.content.Bitstream;
30+
import org.dspace.content.Collection;
31+
import org.dspace.content.Community;
32+
import org.dspace.content.Item;
33+
import org.dspace.core.I18nUtil;
34+
import org.dspace.eperson.EPerson;
35+
import org.dspace.eperson.Group;
36+
import org.dspace.services.ConfigurationService;
37+
import org.junit.Before;
38+
import org.junit.Test;
39+
import org.springframework.beans.factory.annotation.Autowired;
40+
import org.springframework.test.web.servlet.MvcResult;
41+
42+
/**
43+
* Integration test verifying that the Shibboleth special groups (e.g. the default `Authenticated` group)
44+
* survive into tokens which are minted on stateless REST requests after the login:
45+
* the short-lived token used for bitstream downloads and the refreshed login token.
46+
*
47+
* Replicates https://github.com/dataquest-dev/DSpace/issues/900 - a bitstream restricted to the
48+
* `Authenticated` group is visible after the Shibboleth login, but its download returns 403,
49+
* because the special groups are lost when the short-lived token is generated
50+
* (see ufal/clarin-dspace#1373).
51+
*
52+
* @author Milan Majchrak (milan.majchrak at dataquest.sk)
53+
*/
54+
public class ClarinShibbolethSpecialGroupsIT extends AbstractControllerIntegrationTest {
55+
56+
public static final String[] SHIB_ONLY = {"org.dspace.authenticate.clarin.ClarinShibAuthentication"};
57+
private static final String NET_ID_TEST_EPERSON = "123456789";
58+
private static final String IDP_TEST_EPERSON = "Test Idp";
59+
60+
private EPerson clarinEperson;
61+
private Bitstream restrictedBitstream;
62+
63+
@Autowired
64+
ConfigurationService configurationService;
65+
66+
@Before
67+
public void setup() throws Exception {
68+
super.setUp();
69+
70+
// Enable Shibboleth login for all tests
71+
configurationService.setProperty("plugin.sequence.org.dspace.authenticate.AuthenticationMethod", SHIB_ONLY);
72+
73+
context.turnOffAuthorisationSystem();
74+
75+
// Create an eperson with netID - that means the user already exists in the database
76+
clarinEperson = EPersonBuilder.createEPerson(context)
77+
.withCanLogin(false)
78+
.withEmail("clarin@email.com")
79+
.withNameInMetadata("first", "last")
80+
.withLanguage(I18nUtil.getDefaultLocale().getLanguage())
81+
.withNetId(Util.formatNetId(NET_ID_TEST_EPERSON, IDP_TEST_EPERSON))
82+
.build();
83+
84+
// The group every shibboleth-authenticated user is implicitly added to (as a special group)
85+
String defaultGroupName = configurationService.getProperty("authentication-shibboleth.default.auth.group");
86+
Group authenticatedGroup = GroupBuilder.createGroup(context)
87+
.withName(defaultGroupName)
88+
.build();
89+
90+
// A bitstream readable only by the shibboleth default special group
91+
Community community = CommunityBuilder.createCommunity(context)
92+
.withName("Community")
93+
.build();
94+
Collection collection = CollectionBuilder.createCollection(context, community)
95+
.withName("Collection")
96+
.build();
97+
Item item = ItemBuilder.createItem(context, collection)
98+
.withTitle("Item with a restricted bitstream")
99+
.build();
100+
try (InputStream is = IOUtils.toInputStream("Restricted content", CharEncoding.UTF_8)) {
101+
restrictedBitstream = BitstreamBuilder.createBitstream(context, item, is)
102+
.withName("restricted.txt")
103+
.withMimeType("text/plain")
104+
.withReaderGroup(authenticatedGroup)
105+
.build();
106+
}
107+
108+
context.restoreAuthSystemState();
109+
}
110+
111+
/**
112+
* Replication of the issue #900:
113+
* 1. Sign in via Shibboleth - the user is implicitly added into the `Authenticated` special group.
114+
* 2. The bitstream restricted to the `Authenticated` group is readable with the login token.
115+
* 3. The UI downloads the bitstream with a short-lived token minted on a separate stateless request
116+
* - the download must succeed too.
117+
*/
118+
@Test
119+
public void shouldDownloadRestrictedBitstreamWithShortLivedTokenAfterShibLogin() throws Exception {
120+
String loginToken = shibLogin();
121+
122+
// Sanity check: the login token keeps the special groups (its `sg` claim was computed
123+
// during the shibboleth login request), so the restricted bitstream is readable.
124+
getClient(loginToken).perform(get("/api/core/bitstreams/" + restrictedBitstream.getID() + "/content"))
125+
.andExpect(status().isOk());
126+
127+
// The short-lived token is minted on a stateless request - the special groups must be
128+
// obtained from the user context (restored from the login token), not from the session.
129+
String shortLivedToken = getShortLivedToken(loginToken);
130+
getClient().perform(get("/api/core/bitstreams/" + restrictedBitstream.getID()
131+
+ "/content?authentication-token=" + shortLivedToken))
132+
.andExpect(status().isOk());
133+
}
134+
135+
/**
136+
* The refreshed login token (POST /api/authn/login with the Bearer token, no shibboleth headers)
137+
* must keep the special groups too, otherwise the user loses the access after the first token refresh
138+
* (see ufal/clarin-dspace#1373).
139+
*/
140+
@Test
141+
public void shouldKeepSpecialGroupsAfterLoginTokenRefresh() throws Exception {
142+
String loginToken = shibLogin();
143+
144+
// Sanity check: the restricted bitstream is readable with the login token
145+
getClient(loginToken).perform(get("/api/core/bitstreams/" + restrictedBitstream.getID() + "/content"))
146+
.andExpect(status().isOk());
147+
148+
// Refresh the login token on a stateless request (no shibboleth session/headers)
149+
String refreshedAuthHeader = getClient(loginToken).perform(post("/api/authn/login"))
150+
.andExpect(status().isOk())
151+
.andReturn().getResponse().getHeader(AUTHORIZATION_HEADER);
152+
assertNotNull("The token refresh must return the Authorization header", refreshedAuthHeader);
153+
String refreshedToken = refreshedAuthHeader.replace(AUTHORIZATION_TYPE, "");
154+
155+
// The restricted bitstream must still be readable with the refreshed token
156+
getClient(refreshedToken).perform(get("/api/core/bitstreams/" + restrictedBitstream.getID() + "/content"))
157+
.andExpect(status().isOk());
158+
}
159+
160+
private String shibLogin() throws Exception {
161+
String authHeader = getClient().perform(get("/api/authn/shibboleth")
162+
.header("SHIB-MAIL", clarinEperson.getEmail())
163+
.header("Shib-Identity-Provider", IDP_TEST_EPERSON)
164+
.header("SHIB-NETID", NET_ID_TEST_EPERSON))
165+
.andExpect(status().is3xxRedirection())
166+
.andReturn().getResponse().getHeader(AUTHORIZATION_HEADER);
167+
assertNotNull("The shibboleth login must return the Authorization header", authHeader);
168+
return authHeader.replace(AUTHORIZATION_TYPE, "");
169+
}
170+
171+
private String getShortLivedToken(String loginToken) throws Exception {
172+
ObjectMapper mapper = new ObjectMapper();
173+
MvcResult mvcResult = getClient(loginToken).perform(post("/api/authn/shortlivedtokens"))
174+
.andExpect(status().isOk())
175+
.andReturn();
176+
String content = mvcResult.getResponse().getContentAsString();
177+
JsonNode token = mapper.readTree(content).get("token");
178+
assertNotNull("The shortlivedtokens response must contain the token field", token);
179+
return token.asText();
180+
}
181+
}

0 commit comments

Comments
 (0)