fix: possible locally fair dataset download fix - #12581
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a UI download failure (404) for files in datasets under a Locally FAIR-enabled Dataverse by ensuring Locally FAIR visibility checks are performed with the session-backed DataverseRequest rather than a GuestUser request derived from the JAX-RS ContainerRequestContext.
Changes:
- Adjust
findDataFileUserCanSeeOrDieWrapperto swap indvRequestService.getDataverseRequest()when the incoming request user isGuestUser. - Ensures Locally FAIR access checks in
AbstractApiBean.findDataFileUserCanSeeOrDie(...)receive an authenticated/session user where applicable, preventing false “not found” outcomes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (req.getUser() instanceof GuestUser) { | ||
| req = dvRequestService.getDataverseRequest(); | ||
| } |
There was a problem hiding this comment.
Pending IQSS team review and suggestions.
There was a problem hiding this comment.
The fix takes the right approach, but I think this is too broad, as it affects all APIs and is essentially the same as https://guides.dataverse.org/en/latest/installation/config.html#dataverse-feature-api-session-auth, with the same risk noted in the Guides. (Nominally a work-around for testing LFAIR though). The access apis are special in that they are the only ones called from the JSF UI directly, so we need to allow session auth just for them even when the flag is off. This is currently being handled in a few related Access methods - see
dataverse/src/main/java/edu/harvard/iq/dataverse/api/Access.java
Lines 2284 to 2299 in 0a49c4c
dataverse/src/main/java/edu/harvard/iq/dataverse/api/Access.java
Lines 2237 to 2268 in 0a49c4c
We probably need to discuss this more, but I might suggest we simplify by just exempting the access apis in https://github.com/IQSS/dataverse/blob/develop/src/main/java/edu/harvard/iq/dataverse/api/auth/SessionCookieAuthMechanism.java which should resolve this issue, and the private URL one and clean up the code. I'll put this in triage so this gets on the radar Monday.
There was a problem hiding this comment.
Thanks for the detailed review and context!
My original fix was definitely intended as a minimal proof-of-concept to get the root cause and a workaround on the team's radar.
Your suggestion to handle this at the SessionCookieAuthMechanism level makes a lot of sense. I am happy to pause or close this PR and let the team discuss the best path forward.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
may also close #12563 |
What this PR does / why we need it:
Fixes the locally FAIR dataset download issue outlined in #12579 that prevents dataset files within a locally FAIR dataverse from being downloaded
Which issue(s) this PR closes:
Special notes for your reviewer:
N/A
Suggestions on how to test this:
Spin up a docker container on develop, verify that dataset downloads don't work. Spin up this branch and verify that dataset downloads start working once again
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
N/A
Is there a release notes update needed for this change?:
N/A
Additional documentation:
N/A