Fix URL signing api for URLs containing special characters

[ErykKul]

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

[coveralls]

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

[github-actions]

Test Results

404 tests   389 ✅  37m 2s ⏱️
 55 suites   15 💤
 55 files      0 ❌

Results for commit 3fb4cc1.

♻️ This comment has been updated with latest results.

[pdurbin]

@ErykKul I'm putting this on hold for now since I heard you're on vacation until Aug 4. Please resolve merge conflicts when you get back. Thanks!

[ErykKul]

@pdurbin, I did the merge. This one was easy. If there is something bigger, then I will pick it up after Aug 4.

[qqmyers]

Thanks for working with the existing code. FWIW: I think it would be cleaner overall for this general utility to not try fixing a caller's mistakes - perhaps it should throw an exception if one of the four params needed by the algorithm are sent (and never touch the url on the good path and thus not needing to strip anything). Similarly flagging/removing signed and key here because the caller doesn't strip them seems backwards.

[ErykKul]

This wasn't introduced in this PR. It came in with the 6.10 guestbook changes (#12001), where the ?signed=true flow signs the incoming request URI, which can carry key=<apitoken> and signed=true. But agreed it's worth picking up here: I'll make signUrl throw if any of the four algorithm params are present and never touch the URL otherwise, and move the (byte-exact) stripping helper out of signUrl to the callers that build URLs from incoming requests.

[ErykKul]

Done in 2e0749f: signUrl now throws an IllegalArgumentException if the base URL already contains one of the four signing params and never touches the URL otherwise. The byte-exact stripping helper moved out of signUrl to the ?signed=true flow in Access (the caller that actually receives those params in the request URI), and /api/admin/requestSignedUrl now returns a 400 naming the offending parameter instead of silently rewriting the supplied URL. Release note, docs and tests updated accordingly.

[qqmyers]

@ErykKul - are you around to be able to fix the minor merge conflict? (If not, I can fix it and go forward with QA/merge). Also - do I understand correctly there is no visible effect in 6.10 - the URLs it creates internally don't have characters that are affected by the bug, so the only thing to QA is regression testing and trying the create signed url api with something like a doi (with : and / ).

[ErykKul]

The conflict should be gone already, I merged develop this morning before pushing (GitHub shows the branch as mergeable now).

And yes, that's my understanding too: the URLs Dataverse creates internally only contain numeric ids, so the only visible breakage in 6.10 is for requestSignedUrl with special characters in the url, like a doi (that's how rdm-integration ran into it). Besides that and regression testing, two things from this PR worth a quick try in QA: requestSignedUrl now returns 400 when the url already contains a reserved param, and the signing endpoints now error without dataverse.api.signing-secret instead of falling back to a weak key.

[sonarqubecloud]

Quality Gate failed

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

See analysis details on SonarQube Cloud

[ErykKul]

@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]

📦 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.