Draft
Conversation
- Added CSRF token management to DataverseSession for session-cookie authentication. - Introduced new feature flag `dataverse.feature.api-session-auth-hardening` to enable CSRF protections. - Enhanced AuthFilter to validate CSRF tokens and request origins for state-changing API calls. - Updated CompoundAuthMechanism to tag authentication mechanisms in requests. - Created new endpoint `GET /api/users/:csrf-token` for clients to retrieve CSRF tokens. - Added tests for new functionality and updated existing tests to cover new behavior. - Documented changes in release notes for clarity on new security features.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in “API session-cookie auth hardening” track to support JSESSIONID-based API usage with CSRF protections and endpoint guardrails, without changing default behavior when the flag is off.
Changes:
- Adds auth-mechanism tagging to the API auth flow (
apiKey,workflowKey,signedUrl,bearerToken,sessionCookie,none) and ensures guest sessions are not treated as session-cookie auth. - Implements session-scoped API CSRF token lifecycle in
DataverseSessionand introducesGET /api/users/:csrf-tokento retrieve it (auth required). - Enforces hardening rules in
AuthFilter(Origin/Referer +X-Dataverse-CSRF-Tokenfor mutating requests, plus/api/access/*and mutating-GET guardrails) and adds focused tests/docs.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/edu/harvard/iq/dataverse/api/auth/SessionCookieAuthMechanismTest.java | Adds coverage ensuring guest sessions don’t authenticate via session-cookie mechanism. |
| src/test/java/edu/harvard/iq/dataverse/api/auth/CompoundAuthMechanismTest.java | Updates tests to assert auth-mechanism tagging behavior. |
| src/test/java/edu/harvard/iq/dataverse/api/auth/AuthFilterTest.java | New tests for session-cookie hardening behavior (Origin/CSRF/access guardrails). |
| src/test/java/edu/harvard/iq/dataverse/DataverseSessionTest.java | New tests for CSRF token creation/reuse/clear lifecycle. |
| src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java | Adds API_SESSION_AUTH_HARDENING feature flag and updates related documentation. |
| src/main/java/edu/harvard/iq/dataverse/api/auth/SessionCookieAuthMechanism.java | Ensures only authenticated (non-guest) session users are treated as session-cookie auth. |
| src/main/java/edu/harvard/iq/dataverse/api/auth/CompoundAuthMechanism.java | Sets request auth-mechanism tag on ContainerRequestContext. |
| src/main/java/edu/harvard/iq/dataverse/api/auth/AuthFilter.java | Implements session-cookie CSRF/origin enforcement + /api/access/* and mutating-GET guardrails behind the hardening flag. |
| src/main/java/edu/harvard/iq/dataverse/api/Users.java | Adds GET /api/users/:csrf-token endpoint for session-cookie clients (hardening required). |
| src/main/java/edu/harvard/iq/dataverse/api/ApiConstants.java | Adds constants for auth-mechanism tagging and CSRF header name. |
| src/main/java/edu/harvard/iq/dataverse/DataverseSession.java | Adds per-session API CSRF token storage and clears it on user changes. |
| doc/sphinx-guides/source/installation/config.rst | Documents new hardening flag and deployment guidance for cookie attributes. |
| doc/sphinx-guides/source/api/native-api.rst | Documents CSRF token endpoint and usage for session-cookie clients. |
| doc/release-notes/12178-session-cookie-api-hardening.md | Adds release note entry for the new hardening track. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+131
to
+142
| public String getOrCreateApiCsrfToken() { | ||
| if (apiCsrfToken == null || apiCsrfToken.isBlank()) { | ||
| apiCsrfToken = UUID.randomUUID().toString(); | ||
| } | ||
| return apiCsrfToken; | ||
| } | ||
|
|
||
| public boolean matchesApiCsrfToken(String token) { | ||
| return token != null && apiCsrfToken != null && apiCsrfToken.equals(token); | ||
| } | ||
|
|
||
| public void clearApiCsrfToken() { |
There was a problem hiding this comment.
getOrCreateApiCsrfToken() lazily initializes apiCsrfToken without any synchronization. Because DataverseSession is session-scoped, concurrent requests on the same session can race and generate/overwrite different tokens, causing intermittent CSRF validation failures for legitimate clients. Consider making token initialization atomic (e.g., synchronize initialization, use an AtomicReference, or generate once in a @PostConstruct), so a token can’t be replaced mid-session by a parallel request.
Suggested change
| public String getOrCreateApiCsrfToken() { | |
| if (apiCsrfToken == null || apiCsrfToken.isBlank()) { | |
| apiCsrfToken = UUID.randomUUID().toString(); | |
| } | |
| return apiCsrfToken; | |
| } | |
| public boolean matchesApiCsrfToken(String token) { | |
| return token != null && apiCsrfToken != null && apiCsrfToken.equals(token); | |
| } | |
| public void clearApiCsrfToken() { | |
| public synchronized String getOrCreateApiCsrfToken() { | |
| if (apiCsrfToken == null || apiCsrfToken.isBlank()) { | |
| apiCsrfToken = UUID.randomUUID().toString(); | |
| } | |
| return apiCsrfToken; | |
| } | |
| public synchronized boolean matchesApiCsrfToken(String token) { | |
| return token != null && apiCsrfToken != null && apiCsrfToken.equals(token); | |
| } | |
| public synchronized void clearApiCsrfToken() { |
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This PR adds an opt-in hardening layer for API session-cookie auth (
JSESSIONID) so JSF/SAML-style deployments can use session auth with stronger CSRF protections, without breaking existing token-based API clients.Implemented changes:
dataverse.feature.api-session-auth-hardening(FeatureFlags.API_SESSION_AUTH_HARDENING).apiKey,workflowKey,signedUrl,bearerToken,sessionCookie,none) viaCompoundAuthMechanism.SessionCookieAuthMechanismto return only authenticated session users (guest session no longer tagged as session-cookie auth).DataverseSession(getOrCreateApiCsrfToken,matchesApiCsrfToken,clearApiCsrfToken).GET /api/users/:csrf-token(auth required) for session-cookie clients to fetch CSRF token.AuthFilter(only when hardening flag is enabled and auth mechanism issessionCookie):X-Dataverse-CSRF-Tokenfor state-changing methods (POST/PUT/PATCH/DELETE).GETendpoints:GET /api/datasets/{id}/uploadurlsGET /api/datasets/{id}/cleanStorage/api/access/*guardrails:GET/HEAD/OPTIONS) allowedPOST /api/access/datafilesallowed with same-origin validation/api/access/*for session-cookie authWhich issue(s) this PR closes:
Special notes for your reviewer:
GET /api/access/*and same-originPOST /api/access/datafiles).AuthFilterhandles optionalapi/prefix and trailing slashes for robust matching.@sincefor the new flag is set to Dataverse6.9to match current branch revision.Suggestions on how to test this:
AuthFilterTestDataverseSessionTestSessionCookieAuthMechanismTestCompoundAuthMechanismTestdataverse.feature.api-session-auth=truedataverse.feature.api-session-auth-hardening=trueGET /api/users/:csrf-tokenwith session cookie returns token.Origin+X-Dataverse-CSRF-Tokensucceeds.403.GET /api/datasets/{id}/uploadurlsandGET /api/datasets/{id}/cleanStoragerequire Origin + CSRF token under hardening.POST /api/access/datafilesis allowed for same-origin and blocked for cross-origin./api/access/*endpoints other than batch download POST are blocked for session-cookie auth.Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No direct UI change.
Is there a release notes update needed for this change?:
Yes, included:
doc/release-notes/12178-session-cookie-api-hardening.mdAdditional documentation:
doc/sphinx-guides/source/installation/config.rstdoc/sphinx-guides/source/api/native-api.rstSecurity model note: session-cookie hardening vs bearer token
This PR does not claim session-cookie auth is identical to bearer auth, but with hardening enabled it reaches a comparable security posture for first-party browser calls.
JSESSIONID) is normally vulnerable to CSRF because cookies are sent automatically by browsers.X-Dataverse-CSRF-Tokenvalidation for state-changing requests/api/access/*plus mutatingGETguardrailsComparison to bearer:
HttpOnlycan reduce JS-level token exposure compared to bearer tokens stored in browser-accessible storage.Additional documentation:
Secure+HttpOnly): https://github.com/IQSS/dataverse/blob/12178_CSRF_session_cookie_CSRF_protections/doc/sphinx-guides/source/installation/config.rst?plain=1#L3925