Skip to content

redirect_uri asymmetry: query string included at /authorize but dropped at /token, breaking login for any app URL with query params #13

Open

Description

@melvincarvalho

Summary

redirect_uri is computed two different ways: the authorization request includes the URL's query string, the token exchange drops it. Any app loaded at a URL with a query parameter therefore fails at the token exchange with invalid_grant, and login can never complete.

The asymmetry

solid-oidc.js:464 — authorization request + client registration:

const sanitizedRedirect = redirectUrl.origin + redirectUrl.pathname + redirectUrl.search

solid-oidc.js:562 — token exchange:

redirect_uri: url.origin + url.pathname,

For http://localhost:5444/public/apps/tasks/?tracker=today-data.jsonld these produce:

Step Value sent
/authorize + client registration …/apps/tasks/?tracker=today-data.jsonld
/token …/apps/tasks/

RFC 6749 §4.1.3 requires the token-exchange redirect_uri to be identical to the one in the authorization request, so the AS rejects it.

Why this has never been seen before

The two expressions produce identical strings whenever the app URL has no query string, so the bug is invisible for every query-less app. It has been present since v0.0.1 — git log -S on both lines points at the initial release commit — and only fires the first time an app is loaded with a query parameter.

Reproduction

Against a jspod pod (oidc-provider):

  1. Load an xlogin app at a URL with a query param, e.g. http://localhost:5444/public/apps/tasks/?tracker=today-data.jsonld
  2. Sign in via the login pill and complete consent
  3. POST /idp/token → 400 Bad Request, and login silently never completes

Server-side, oidc-provider throws at lib/actions/grants/authorization_code.js:77:

if (code.redirectUri !== ctx.oidc.params.redirect_uri) {
  throw new InvalidGrant('authorization code redirect_uri mismatch');
}

The stored authorization code is left with no consumed field, confirming rejection happens before consumption.

Loading the same app at http://localhost:5444/public/apps/tasks/ (no query) succeeds.

Suggested fix

solid-oidc.js:562:

-      redirect_uri: url.origin + url.pathname,
+      redirect_uri: url.origin + url.pathname + url.search,

This is safe: lines 535–537 already strip code, iss and state from url before this point, so url.search reconstructs the original redirect URI exactly, and is '' when the app had no query string (preserving current behaviour for every existing caller).

Worth considering a shared helper so the value is derived in exactly one place — the same asymmetry class as #7.

Test coverage

A regression test that runs the full flow from a redirect URL carrying a query parameter would have caught this, and would catch the reverse drift too.

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