OpenFn / OpenFn/lightning

`/authenticate/callback` raises ActionClauseError → 500 on empty or unexpected params

Open
#4,806 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug Monitoring Sentry
Dominant language
Elixir
Stars
296
Forks
86
Avg merge
1d 13h
Merged PRs (30d)
50

Description

Describe the bug

GET /authenticate/callback raises Phoenix.ActionClauseError whenever the
request arrives without the params that one of the existing
OidcController.new/2 clauses expects. The three clauses match
%{"provider" => _, "code" => _}, %{"state" => _, "code" => _}, and
%{"error" => _, "state" => _} — so a request with no query string (or
any combination of unexpected params) reaches the action, no clause matches,
Phoenix raises ActionClauseError, and the request returns a 500 with an
unhandled stack.

In practice the bare URL gets hit fairly often by scanners and crawlers
(stale UAs, no referer, no cookies, no repeat IPs), which means a steady
trickle of unhandled exceptions in the logs that aren't user-affecting but
also shouldn't be 500s. The controller should never raise on a malformed
callback regardless of who's calling.

Version number Present on current main. The clauses haven't been
touched in a long time so it's been present across recent releases.

I have reproduced this locally on main:

  • Yes
  • No

To Reproduce

  1. Send GET /authenticate/callback with no query parameters
    (curl http://localhost:4000/authenticate/callback is enough).
  2. Observe 500 in the response and an ActionClauseError pointing at
    lib/lightning_web/controllers/oidc_controller.ex:32.

Expected behaviour

Never 500. Two reasonable shapes for the fallback, and they could be
combined:

  • Non-browser / no-cookie / no-meaningful-headers request → plain
    400 Bad Request. Most of the traffic looks like this and a clean 400 is
    the right answer.
  • Browser-shaped request (User-Agent + cookies + Lightning-domain
    referer) → redirect to the log-in page with a flash explaining the
    callback couldn't be completed, similar to the "could not find user
    account" path already in the first clause of new/2.

That said, it's hard to imagine any IdP redirecting back to the callback
URL with literally no params, state, or cookies — so a plain 400 across
the board may well be enough, and is simpler. Open to either.

Additional context

  • Route is get "/authenticate/callback", OidcController, :new — the OAuth
    credential-creation callback (used when connecting external services via
    OAuth). The user-login OIDC callback is at
    /authenticate/:provider/callback, which is unaffected because the
    router enforces :provider is present.
  • A catch-all def new(conn, _params) clause handling the "no expected
    fields" case would resolve this; the discussion point is the exact
    response (400 vs. browser-aware redirect-with-flash).

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/lightning_web/controllers/oidc_controller.ex at OidcController.new/2 and inspect the route for GET /authenticate/callback. Reproduce the bare request with curl, then decide the fallback response described in the issue and verify malformed callback requests no longer return 500.

Written by the indexing model from the issue text.

Assessment

Tech stack
elixir
Domain
authentication, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.