Skip to content

OAuth callback answers a bare 500 when the vendor's token endpoint cannot be reached #344

Description

@beardthelion

What happens

Finishing an OAuth consent ends on a bare 500 with no Location header when the vendor's token
endpoint cannot be reached at the moment the person is redirected back. They have already approved
the connection at the vendor and are dropped on a blank server error, with no way to tell whether it
worked.

The handler documents the opposite. server/src/plugins/routes.ts says of the callback:

Every failure ends the same way: back at Settings with a word about what happened, and nothing
written.

Every enumerated failure does that. This one does not.

Why

redeemAuthorizationCode in server/src/plugins/oauth.ts converts a refusal into null by asking
!response.ok, and that question needs a response to ask. There is none when the connection is
refused, the name does not resolve, TLS will not agree, or the 15 second AbortSignal.timeout
fires. fetch rejects, and nothing between there and the browser catches it: the call site is
unwrapped, and server/src installs no app.onError, so Hono's default handler answers 500 with no
Location.

The adjacent cases are handled, which is what makes this one easy to miss. A non-OK status returns
null, and a 200 carrying something that is not JSON was fixed in #242 with a .catch(() => null)
on the body read. Only the transport was left.

registerDynamicClient in the same file has the same gap against the same documented contract, one
step earlier in the flow. Its throw escapes ensureOAuthClient and reaches
POST /api/plugins/servers/:id/connect, which catches only CatalogueEntryUnknownError and
rethrows the rest, so an unreachable registration endpoint answers 500 where the route already has a
502 written for a vendor that will not register the deployment.

Reproducing

Drive the callback with a token endpoint that cannot be reached. Against the real route with a
transport that rejects:

connection failure -> 500, no Location
timeout            -> 500, no Location
html 200           -> 302 .../settings/connected-accounts?connected=failed

The third line is the neighbouring case that already works, for contrast.

It also reproduces against a real socket: point the token endpoint at a closed port for a refused
connection, at an unresolvable name for a DNS failure, or at a server that accepts and never answers
for the timeout.

Scope

This is the ordinary shape of a vendor outage, and it lands on the one step where somebody has most
reason to believe the connection succeeded. Nothing is written on the failing path, so there is no
bad state left behind, and no secret reaches the browser: the response body is empty.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions