Skip to content

Fix URL signing api for URLs containing special characters - #12435

Open
ErykKul wants to merge 31 commits into
developfrom
fix-url-signing-special-characters
Open

Fix URL signing api for URLs containing special characters#12435
ErykKul wants to merge 31 commits into
developfrom
fix-url-signing-special-characters

Conversation

@ErykKul

@ErykKul ErykKul commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:
Bug fix; URL signing for URLs containing special characters broken in 6.10, this PR fixes the introduced bug.

Which issue(s) this PR closes:
Issue not created, bug fixed directly here.

Special notes for your reviewer:
Added tests to prevent regression. Also, API requires now the signing secret to be set, otherwise it will not work.

Suggestions on how to test this:
No special testing needed, the unit tests should be sufficient.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No

Is there a release notes update needed for this change?:
Yes, included.

Additional documentation:
No

@ErykKul ErykKul added Size: 3 A percentage of a sprint. 2.1 hours. Type: Bug - Blocker Bug is blocking user work, no workaround available labels Jun 4, 2026
@ErykKul ErykKul added this to the 6.11 milestone Jun 4, 2026
@ErykKul ErykKul moved this to Ready for Review ⏩ in IQSS Dataverse Project Jun 4, 2026
@coveralls

coveralls commented Jun 4, 2026

Copy link
Copy Markdown

Coverage Status

Coverage is 25.061%fix-url-signing-special-characters into develop. No base build found for develop.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@ErykKul ErykKul moved this from Ready for Review ⏩ to In Progress 💻 in IQSS Dataverse Project Jun 4, 2026
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

Test Results

404 tests   389 ✅  32m 52s ⏱️
 55 suites   15 💤
 55 files      0 ❌

Results for commit 4fef358.

♻️ This comment has been updated with latest results.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Comment thread doc/sphinx-guides/source/installation/config.rst Outdated
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@pdurbin pdurbin removed this from the 6.11 milestone Jun 8, 2026
@ErykKul

ErykKul commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

@qqmyers Heads up before QA: a few more commits landed since my comments above. While testing against a real client I also made validation verbatim-first — the signature is checked against the raw request URI first, with the URL-decoded check kept as fallback — so a URL signed exactly as it will be used now works as received, and everything that validated before still validates. A final review pass added a couple of fixes on top: crafted user=!... values get a proper 401 instead of a 500, and tool-manifest urlTemplates get reserved params stripped before signing again like 6.10 did (otherwise key={apiToken} in a template would sign the user's real API token into the URL). All with unit tests; docs and release note updated. That's everything from my side.

@github-actions

This comment has been minimized.

@qqmyers

qqmyers commented Aug 18, 2026

Copy link
Copy Markdown
Member

@ErykKul - can we coordinate in slack and discuss this PR?

@pdurbin pdurbin removed the Status: Merge Conflicts Merge conflicts must be resolved. label Aug 19, 2026
@qqmyers qqmyers added Original size: 3 Size: 10 A percentage of a sprint. 7 hours. and removed Size: 3 A percentage of a sprint. 2.1 hours. labels Aug 19, 2026
@pdurbin pdurbin added the Status: Needs Input Applied to issues in need of input from someone currently unavailable label Aug 25, 2026
@pdurbin

pdurbin commented Aug 26, 2026

Copy link
Copy Markdown
Member

@ErykKul just a reminder that we'd like your input and code freeze for 6.12 is next Thursday! 🥶

@pdurbin

pdurbin commented Aug 26, 2026

Copy link
Copy Markdown
Member

We think only @ErykKul is actively using this but we'd like to get it into 6.12.

@cmbz cmbz added the FY27 Sprint 5 FY27 Sprint 5 (2026-08-26 - 2026-09-09) label Aug 26, 2026
@ErykKul

ErykKul commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@pdurbin I am on it. Trying to wrap it up. Not ready yet, I will try to have it ready by tomorrow evening.

…eserved params instead of stripping, constant-time signature check #12435
Comment thread src/main/java/edu/harvard/iq/dataverse/api/Admin.java Fixed
@github-actions

This comment has been minimized.

…chronized, no per-node cache), 400s instead of 500s for smuggled or manifest reserved params, constructor injection, coverage tests #12435
Comment thread src/main/java/edu/harvard/iq/dataverse/util/URLTokenUtil.java Fixed
@github-actions

This comment has been minimized.

2 similar comments
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
61.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@github-actions

Copy link
Copy Markdown

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:fix-url-signing-special-characters
ghcr.io/gdcc/configbaker:fix-url-signing-special-characters

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

FY26 Sprint 25 FY26 Sprint 25 (2026-06-03 - 2026-06-17) FY26 Sprint 26 FY26 Sprint 26 (2026-06-17 - 2026-07-01) FY27 Sprint 1 FY27 Sprint 1 (2026-07-01 - 2026-07-15) FY27 Sprint 2 FY27 Sprint 2 (2026-07-15 - 2026-07-29) FY27 Sprint 3 FY27 Sprint 3 (2026-07-29 - 2026-08-12) FY27 Sprint 4 FY27 Sprint 4 (2026-08-12 - 2026-08-26) FY27 Sprint 5 FY27 Sprint 5 (2026-08-26 - 2026-09-09) Original size: 3 Size: 10 A percentage of a sprint. 7 hours. Status: Needs Input Applied to issues in need of input from someone currently unavailable Type: Bug - Blocker Bug is blocking user work, no workaround available

Projects

Status: QA ✅

Development

Successfully merging this pull request may close these issues.

7 participants