[Port to dtq-dev] Issue 1373: obtain special groups from user context when new token is generated (on token refresh)

[kosarko]

Port of ufal#1378 by @kuchtiak-ufal to dtq-dev.

Summary by CodeRabbit

• Bug Fixes
  • Improved authentication handling so sign-in status is tracked per request instead of persisting unexpectedly across sessions.
  • Fixed special-group lookup so only valid groups are returned, avoiding missing or null entries in group lists.
  • Tightened special-group access so results are only returned when a user is properly authenticated.

[coderabbitai]

📝 Walkthrough

Walkthrough

shib.authenticated is moved from HTTP session scope to request scope in authenticate, getSpecialGroups, and isUsed within ClarinShibAuthentication. getSpecialGroups also drops session caching, adds early-exit guards for null request/user, and skips null group lookups. Context.getSpecialGroups similarly adds a null check before adding groups.

Changes

Shibboleth Authentication Scope Fix

Layer / File(s)	Summary
Session → request attribute and null guards dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java, dspace-api/src/main/java/org/dspace/core/Context.java	authenticate and isUsed switch shib.authenticated from session to request attribute. getSpecialGroups removes session caching, gates on request attribute and current user, and skips null groups. Context.getSpecialGroups adds a null check to prevent null entries in the returned list.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is only a port note and omits the required Problem description, Analysis, Problems, and testing/review sections.	Fill the template sections with the problem, analysis, any issues encountered, manual testing, and Copilot review status.
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly names the token-refresh special-group fix and the dtq-dev port, matching the main change.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java`:
- Line 349: The special-group lookup in ClarinShibAuthentication.authenticate()
is still reading only servlet headers via new ShibHeaders(request), which breaks
verification-token flows where the data is stored in the shib.headers request
attribute. Update the group resolution call to use the same attribute-aware
header source as the rest of authenticate(), so the ShibGroup path can resolve
groups from either servlet headers or the request attribute after session
caching is removed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b2d62363-e88b-493e-8668-3ee31a20f575

📥 Commits

Reviewing files that changed from the base of the PR and between 867e43d and 6814843.

📒 Files selected for processing (2)

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

[Copilot]

Pull request overview

Ports upstream fix for Issue 1373 into dtq-dev, improving how Shibboleth-derived “special groups” are resolved during JWT generation/refresh and hardening special-group handling against deleted groups.

Changes:

• Filter out null entries from Context#getSpecialGroups() when a stored special-group UUID no longer resolves to an existing Group.
• Update ClarinShibAuthentication to use a request-scoped shib.authenticated flag (instead of session-scoped) and to prefer special groups already present in Context (e.g., from JWT claims) during token refresh.
• Replace legacy Collections.EMPTY_LIST usage with type-safe Collections.emptyList() and tighten special-group lookup null-handling.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
dspace-api/src/main/java/org/dspace/core/Context.java	Skips missing/deleted Groups so callers never receive null entries in special-group lists.
dspace-api/src/main/java/org/dspace/authenticate/clarin/ClarinShibAuthentication.java	Ensures special groups can be sourced from Context on token refresh and avoids session-persisted Shibboleth auth flags.