[WHIT-3439] Harden link-checker-api callback endpoint#11484
Open
TonyGDS wants to merge 3 commits into
Open
Conversation
3af1036 to
369fc4e
Compare
Admin::ErrorsController inherited authenticate_user! from Admin::BaseController. When Rails routed an unhandled exception through exceptions_app to an error action, the filter ran during error rendering and surfaced as a bare 500 for unauthenticated clients. Error pages shouldn't require authentication.
verify_signature returned silently when the configured webhook secret was nil, allowing any unsigned POST to mark a LinkCheckerApiReport as completed. Reply 503 instead so the callback action never runs without a secret.
369fc4e to
4989970
Compare
A correctly-formed request with the wrong signature is an authentication failure, not a malformed request.
4989970 to
e15fee8
Compare
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.
Summary
Three independent fixes to the
/government/admin/link-checker-api-callbackendpoint:Admin::ErrorsController. Error pages inheritedauthenticate_user!fromAdmin::BaseController, so when Rails routed an unhandled exception throughexceptions_appto an error action, the filter ran during error rendering and surfaced as a bare 500 for unauthenticated clients (e.g.GETon this URL).verify_signaturereturned silently when the configured secret wasnil, letting any unsignedPOSTmark aLinkCheckerApiReportas completed. Reply 503 instead so the callback action never runs without a secret.intercept_401, which would otherwise turn the 401 into a redirect to/auth/gds; the controller opts out viawarden.custom_failure!. Added an integration test so the full middleware stack is exercised.Jira
Test plan
Run against integration (
whitehall-admin.integration.publishing.service.gov.uk).GET on the callback URL (errors-controller fix). Before:
HTTP/2 500. After:HTTP/2 404.Expected: status
404, body is the admin "There is a mistake in the URL" page.POST with no signature header (existing 400, unchanged).
Expected: status
400, empty body.POST with a wrong signature (was 400, now 401).
Expected: status
401, empty body.POST with a valid signature (happy path, unchanged). Trigger a real link-check from the Whitehall integration admin UI (edit a publication, save). In the link-checker-api logs, confirm the outbound webhook returns
204and the correspondingLinkCheckerApiReportrow updates tostatus: "completed".