diff --git a/CHANGELOG.md b/CHANGELOG.md index d0ee1edda5..eb7118546d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,11 @@ and this project adheres to ### Fixed +- Return a 400 instead of a 500 on malformed SSO callbacks. A request to + `/authenticate/callback` with no query string or an unexpected param + combination no longer raises `Phoenix.ActionClauseError`. + [#4806](https://github.com/OpenFn/lightning/issues/4806) + ## [2.16.8-pre] - 2026-06-18 ### Added diff --git a/lib/lightning_web/controllers/oidc_controller.ex b/lib/lightning_web/controllers/oidc_controller.ex index 4d8c8dc470..5ef555f23a 100644 --- a/lib/lightning_web/controllers/oidc_controller.ex +++ b/lib/lightning_web/controllers/oidc_controller.ex @@ -89,6 +89,16 @@ defmodule LightningWeb.OidcController do close_browser_window(conn) end + # A callback that matches none of the expected shapes (no query string, an + # unexpected param combination, or bare-URL hits from scanners and crawlers) + # can't be tied back to a flow we started. Return a plain 400 rather than + # letting Phoenix raise an ActionClauseError and 500. + def new(conn, _params) do + conn + |> put_status(:bad_request) + |> text("Bad Request") + end + @doc """ Renders the confirmation page shown after a successful SSO callback that would create a brand-new account. We ask the user to confirm before diff --git a/test/lightning_web/controllers/oidc_controller_test.exs b/test/lightning_web/controllers/oidc_controller_test.exs index 85bcc50cc7..15eb1d7ae6 100644 --- a/test/lightning_web/controllers/oidc_controller_test.exs +++ b/test/lightning_web/controllers/oidc_controller_test.exs @@ -1014,6 +1014,20 @@ defmodule LightningWeb.OidcControllerTest do ) end + test "returns 400 for a callback with no params", %{conn: conn} do + response = get(conn, "/authenticate/callback") + + assert response.status == 400 + assert response.resp_body =~ "Bad Request" + end + + test "returns 400 for an unexpected param combination", %{conn: conn} do + response = get(conn, "/authenticate/callback?foo=bar") + + assert response.status == 400 + assert response.resp_body =~ "Bad Request" + end + defp perform_broadcast_test(conn, state, component_id, type, value, key) do response = conn