MetalBear hardens OAuth, session scopes, and identity against the reference PDS

MetalBear

Twelve days, dozens of version bumps, and most of them touching src/oauth, src/session, src/identity, or the account registry. The through-line is a field-by-field audit against the reference PDS's TypeScript source that kept finding the same shape of bug: a check that existed in the reference and had no equivalent here.

the login_hint impersonation gap

GET /oauth/authorize minted an authorization code for whichever account login_hint named, with no check that the requesting browser actually controlled that account. login_hint isn't a secret -- it's the same handle anyone can resolve through com.atproto.identity.resolveHandle -- so this was an unauthenticated path to a valid OAuth token for any account on a MetalBear host, live since the endpoint was first added. The fix introduced a device session: a 30-day, HttpOnly, cookie-bound proof that a browser presented an account's password at least once, verified by POST /oauth/signin and checked explicitly against account credentials only, never an app password (an app password that could open one would be a scoped credential escalating itself).

That device session promptly broke in a second way: the frontend's /login never actually called /oauth/signin, so clicking Approve on the consent screen had nothing to check against and looped forever. And because MetalBear is multi-account, the first fix for that only supported one device session per browser -- signing into account B silently signed the browser out of account A. mb_device now carries up to five session tokens joined by ., oauth_authorize matches login_hint against all of them, and GET /oauth/session reports every signed-in subject so the consent page can offer an account picker instead of forcing a re-login. An account-management console (/account -> Connected apps, Active devices) followed, backed by new com.metalbear.oauth.* procedures that list and revoke device sessions and OAuth grants by a non-secret id derived from a SHA-256 hash, scoped so one account can never touch another's session by guessing an id.

the atproto scope oscillation

oauth_scope.c landed with a real parser for AT Protocol's static and dynamic scope grammar (atproto, transition:*, repo:<collection>?action=<action>, blob:*), but the requirement that a normal grant include the atproto scope then flip-flopped across five commits in three days: required, removed as "overly restrictive," removed again, reinstated, and finally corrected. The bug hiding under all of it was a one-character length check -- len == 6 && strncmp(p, "atproto", 6) == 0 -- comparing only six bytes of a seven-byte string, so no scope string containing atproto could ever match and every PAR request was rejected regardless of what it asked for. Fixing the length to 7 fixed the false rejections, but left the same six-byte strncmp in place, which meant a seven-character near-miss like atprotz now matched as a full-access grant. The final fix compared all seven bytes.

A quieter but more serious gap sat next to this in authenticate_request: its read-scope gate only ran when a collection had already been extracted from the request, and that extraction only happened for the three write NSIDs (createRecord/putRecord/deleteRecord). Every other authenticated read -- getSession, checkAccountStatus, getAccountInviteCodes -- had collection == NULL, so the collection && ... guard never fired and any non-empty, non-full scope reached every authenticated read regardless of how narrow it was. Fixed by recovering a collection generically and denying by default when none can be identified, rather than falling open.

narrowing OAuth against the reference

The reference PDS refuses OAuth credentials outright for a specific set of account-management endpoints, independent of how broad the grant's scope is. MetalBear's OAuth branch only ever checked scope breadth via mb_scope_set_is_full_access -- so a full-access OAuth grant could already call deactivateAccount and requestAccountDelete, endpoints the reference reserves for a session JWT alone. A five-commit pass closed this out: a narrow METALBEAR_ACCESS_TAKENDOWN scope for createSession's allowTakendown case, restricted further so a taken-down account's service-auth token can only ever target createAccount (migrating to another PDS, the one thing it's for); requestPlcOperationSignature/signPlcOperation and requestAccountDelete/requestEmailUpdate/getAccountInviteCodes moved into full_access_route, since neither checked their own scope and relied entirely on a top-level gate that admitted app passwords; and finally oauth_forbidden_route, checked unconditionally before any scope parsing, blocking OAuth/DPoP credentials from six endpoints regardless of grant. requestEmailUpdate was the sharpest of these -- an app password minting an email-change token is a direct path to account takeover, not a scope the credential was ever meant to authorize. A related fix in the same window rejects any createSession password over 512 bytes before it reaches scrypt, matching the reference's OLD_PASSWORD_MAX_LENGTH guard against a proportional-cost hashing DoS.

PLC operations that silently did nothing

getRecommendedDidCredentials advised clients to use the account's own atproto verification key as a PLC rotationKeys entry, instead of the server's actual PLC rotation key (server->plc_rotation) -- two different keys for two different purposes, conflated. A client trusting that advice for an identity migration would submit an operation requestPlcOperationSignature's co-sign step could never recognize, since it looks for its own rotation key already present in the pending operation. The same handler had a second, independent bug: it checked rotationKeys entries against server->service_did (a did:web string), which no real PLC operation would ever contain, so every legitimate submission was refused with "Rotation keys do not include server's rotation key" -- and even a correctly-keyed operation that passed would have been silently discarded, since the handler validated structure and returned success without ever calling wf_plc_submit_operation_raw. Confirmed live against plc.directory once both were fixed. A related gap surfaced days later: neither admin.updateAccountHandle nor self-service identity.updateHandle touched the DID document on a rename, so a did:plc account's alsoKnownAs kept naming its old handle indefinitely even after DNS and local state agreed on the new one. Both routes now call wf_plc_build_handle_update and submit before any local mutation, so a PLC failure blocks the rename entirely instead of leaving the DID document and local state disagreeing.

client authentication and metadata parity

A field-by-field diff of .well-known/oauth-authorization-server against the reference's build-metadata.ts turned up MetalBear advertising private_key_jwt as a supported token_endpoint_auth_method when nothing in the codebase parsed or verified a client_assertion JWT -- a confidential client that trusted the metadata would find no verifier on the other end. Pulled the claim the next day, then implemented it for real the day after: /oauth/token now fetches a client's metadata document, resolves its signing JWKS (inline or via jwks_uri, restricted to https and loopback http against SSRF), and verifies the assertion through wf_oauth_verify_client_assertion per RFC 7523 before trusting the asserted client_id. The same diff pass added a missing resource_documentation field and, separately, brought PAR and token handling in line with real-world clients: bsky.app sends client_id twice (once from parameters, once from auth.payload), which needed duplicate form keys tolerated, and dpop_jkt needed to be optional at PAR with extraction from the DPoP proof header instead -- matching the reference's allowMissingDpopProof.

passkeys, and DPoP made mandatory

WebAuthn/passkey login landed as a parallel path to password sign-in, ending at the same device-session helper oauth_signin already used. It's built on a purpose-written CBOR parser in src/oauth/webauthn.c rather than wolfram's wf_cbor_parse, whose DAG-CBOR canonical map-key ordering is the wrong fit for a browser-produced attestationObject; attestation is always none, since the security property is the device session already proving password ownership at registration, not authenticator attestation. A same-day follow-up dropped residentKey: "required" to "preferred" -- MetalBear never does discoverable/usernameless login, so the stricter setting only excluded compatible authenticators for no benefit MetalBear actually used.

The range closes on the sharpest finding of the twelve days: /oauth/token minted access and refresh tokens by trusting whatever dpop_jkt string a client put in the request body, never verifying an actual DPoP proof at token exchange -- and dpop_jkt was optional at PAR too, under a claimed "loopback clients" exemption that doesn't exist in the AT Protocol OAuth profile, which mandates DPoP for every client with no exceptions. A client could omit DPoP through the whole flow and receive a token with no cnf.jkt at all; paired with a companion bug in wolfram that silently bound such a token to whatever key a later DPoP proof happened to carry, a leaked unbound token was exactly as reusable as a stolen plain bearer token. Fixed in three independently-sufficient layers: PAR now requires dpop_jkt present rather than merely well-shaped when given; /oauth/token now cryptographically verifies a real DPoP proof on every grant and feeds that verified value -- never the request body's own claim -- into code exchange and refresh; and metalbear_oauth_exchange_code/refresh reject a NULL or empty dpop_jkt outright, so no future caller can reintroduce the gap by skipping verification.

Underneath all of this, account.c, key_rotation.c, and account_registry.c were migrated to C++17 with RAII for their sqlite3 handles, and a run of registry bugs got fixed along the way -- six timestamp writers using SQLite's bare datetime('now') shape instead of RFC 3339 (with a one-time corrective pass for rows already written wrong), and find_by_did/find_by_email omitting the email column their own read_entry expected at a fixed index. 0.7.0 to 0.38.2.