ZCU-PUB/fix: shibboleth special groups lost in short-lived and refreshed tokens - 403 on download (backport #1347) - #1374
Conversation
…refreshed tokens Replicates #900: a bitstream restricted to the default shibboleth group (Authenticated) is readable with the login token, but the download via a short-lived token returns 403, because the special groups are recomputed from the (missing) servlet session instead of the user context when a new token is minted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ted (on token refresh) (ufal#1378) (#1347) * 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. --------- (cherry picked from commit 4c294b2) Co-authored-by: Milan Kuchtiak <kuchtiak@ufal.mff.cuni.cz> (cherry picked from commit 74f5862)
… raw-type read Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes loss of Shibboleth “special groups” (e.g., default Authenticated) across stateless REST requests so group-based authorization remains consistent for short-lived download tokens and refreshed login tokens (addresses #900).
Changes:
- Update
ClarinShibAuthentication.getSpecialGroups()to prefercontext.getSpecialGroups()(restored from the JWTsgclaim) and moveshib.authenticatedtracking from session to request scope. - Harden
Context.getSpecialGroups()to skip deleted (null) groups to avoid downstream NPEs. - Add an integration test covering restricted bitstream download via short-lived token and access persistence after login token refresh.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| dspace-server-webapp/src/test/java/org/dspace/app/rest/security/ClarinShibbolethSpecialGroupsIT.java | Adds IT coverage for short-lived token downloads and token refresh preserving Shibboleth special groups. |
| dspace-api/src/main/java/org/dspace/core/Context.java | Filters null (deleted) groups from getSpecialGroups() results. |
| dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java | Restores special groups from Context/JWT, and switches shib.authenticated to request scope to support stateless flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- reuse AUTHORIZATION_HEADER/AUTHORIZATION_TYPE from AbstractControllerIntegrationTest - assert the Authorization header and the token field are present before using them Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
Fixes #900.
After a Shibboleth login the user is implicitly a member of the special group
Authenticated(authentication-shibboleth.default.auth.group, clarin-dspace.cfg). An item restricted to this group is visible, but the bitstream download returns 403.Cause:
ClarinShibAuthentication.getSpecialGroups()reads the special groups from the servlet session attributeshib.authenticated, which is set only during the Shibboleth login request. Every newly minted token recomputes thesgJWT claim (SpecialGroupClaimProvider.getValue()), so on a stateless request:POST /api/authn/shortlivedtokens) is issued with an emptysgclaim →GET /api/core/bitstreams/{uuid}/content?authentication-token=…→ 403,POST /api/authn/loginwith Bearer) loses the special groups too, so the user loses the group-based access after the first token refresh.Fix
Backport of #1347 (
74f58627, port of ufal#1378, issue ufal#1373) fromdtq-dev:getSpecialGroups()first returnscontext.getSpecialGroups()(restored from the login token'ssgclaim),shib.authenticatedmoved from session attribute to request attribute (same as upstreamShibAuthentication, Removes ShibAuthentication session dependency for special groups. DSpace/DSpace#8160),Context.getSpecialGroups()skips null (deleted) groups.Behavior note: special groups are now echoed from the user context until re-login, so an affiliation revocation at the IdP takes effect at the next login (same semantics as upstream).
TDD
Commit 1 adds
ClarinShibbolethSpecialGroupsIT(restricted download via short-lived token + token refresh). Run locally on this branch without the fix — both tests fail with 403:Commit 2 is the backport — both tests pass:
(The images are rendered from the real local
mvn verifylogs. Note: the first commit intentionally fails CI when checked out alone - TDD ordering. The evidence images live on the deletable branchassets/zcu-shibboleth-pr-evidence.)How to replicate on current zcu-pub
Authenticated.API check:
POST /api/authn/shortlivedtokenswith the login Bearer token and decode the returned JWT — thesgclaim is empty.Related