Server-side sync_sso rejects valid payloads with 422 "Login Error" when the base64 contains a `+` (triggered by `~` in synced fields)

Contribute Bug
1

Hi everyone~

I found a pretty obscure bug around b64encoded with ‘+’ values causing issues with the sync_sso endpoint only. I’m confident that it’s not an integration issue. I did use Claude Fable to diagnose the bug & generate the report, hope that’s ok. I’ve reviewed everything, reproduced it myself and agree with the root cause and fix. Let me know if you need anything else. It’s not super critical for me anymore since I found a workaround but it is a weird one others may hit.

Thanks for all your hard work!
Brandon

Summary

POST /admin/users/sync_sso double-decodes the sso parameter. If the base64-encoded payload contains a + character, the endpoint corrupts it into a space and rejects the request with the generic 422 Login Error — even though the payload and signature are correct.

For ASCII (URL-encoded) payload content, base64 only produces a + when a ~ (0x7E)
character sits at the right byte alignment — and only for clients whose URL-encoding
leaves ~ unescaped per RFC 3986 (e.g. Python ≥ 3.7, JavaScript’s encodeURIComponent).
So in practice this manifests as: sync works for everyone, except users with a ~ in
their bio/username/etc., and only sometimes, depending on how the rest of their profile
shifts the alignment. It looks like a per-user HMAC mystery and is nearly impossible to
diagnose from the outside. This probably impacts my site’s forums more than others, as
Koreans & Korean learners use ‘~’ frequently to end bios.

For browser logins & syncs, this issue does not manifest (see below). It’s only an issue
for server side sync_sso calls.

Root cause

In Admin::UsersController#sync_sso (users_controller.rb#L500):

sso = DiscourseConnect.parse("sso=#{params[:sso]}&sig=#{params[:sig]}", server_session:)

params[:sso] has already been form-decoded by Rack/Rails, but it is interpolated back
into query-string syntax without re-escaping. DiscourseConnect.parse then runs
Rack::Utils.parse_query over it (discourse_connect_base.rb#L93) —
a second decode of a value that was only encoded once. parse_query converts the
literal + to a space, and the base64 character check below raises PayloadParseError,
surfaced as 422 “Login Error”. (Before that check was added in #26140, the same
corruption failed at the signature comparison instead — same outcome.)

+ is the only base64-alphabet character that form-decoding mutates, so the double
decode is a silent no-op for most payloads — which is why this has survived unnoticed.
The browser login flow (session/sso_login) is unaffected: it parses the raw query
string, i.e. decodes exactly once.

The interpolation dates to fb750af8e29a20bbbf4b70b0f3f4e4a06f602b69 (Oct 2014,
“trivial update to allow api endpoint for sync_sso”), which replaced the original —
and correct — DiscourseSingleSignOn.parse(request.query_string).

This is not an integrator encoding mistake: the official sync_sso guide
shows the sso value sent with a single standard form-encoding, and the official
discourse_api gem does the same — so a to-the-letter client hits this whenever the
base64 happens to contain a +.

Steps to reproduce (deterministic)

  1. Build a valid signed sync_sso payload for any user, including a field whose value
    contains ~~~ (three consecutive tildes guarantee one lands at byte position
    ≡ 2 mod 3, forcing a + into the base64).

    Note for reproducing from Ruby: payloads built with Discourse’s own
    SingleSignOn class (or the discourse_api gem) can never contain a + in their
    base64 — Rack::Utils.build_query escapes ~ as %7E, and no byte in its output
    alphabet maps to a base64 +. To reproduce, build the inner query string by hand
    with a literal ~~~ in a field value (unknown keys are ignored), e.g.
    Base64.strict_encode64("external_id=1&email=user@example.com&username=someuser&filler=~~~"),
    sign it, and POST. This is also why the existing specs have never caught the bug:
    Discourse’s own tooling structurally cannot generate a triggering payload.

  2. POST /admin/users/sync_sso with sso/sig as correctly-encoded form params
    (+ sent as %2B on the wire).

  3. Response: 422 {"failed":"FAILED","message":"Login Error"}.

  4. Same payload with the tildes replaced by xxx (base64 now +-free): 200.

Verified against a production Discourse instance: identical payload structure, one +
in the base64 → 422; zero → 200; and the 422 payload succeeds if the + is pre-escaped
as %2B (i.e. pre-compensating for the extra decode).

The existing regression spec also round-trips the payload through
Rack::Utils.parse_query(sso.payload) before posting — pre-decoding it and cancelling
out the controller’s extra decode — so even a +-bearing payload would slip past it.

Suggested fix

Re-escape the param before rebuilding the query string, mirroring what the
SSO-provider side already does with CGI.escape(payload) in the same class
(discourse_connect_base.rb#L168):

sso = DiscourseConnect.parse(
  "sso=#{CGI.escape(params[:sso])}&sig=#{params[:sig]}", server_session:
)

Workaround for integrators (until fixed)

Pre-escape + as %2B in the sso value for sync_sso calls only (not for the
browser login redirect). Note this workaround must be removed if/when the endpoint
is fixed, as it would then over-encode.