fix(oidc): enforce allowed_grant_types + token_endpoint_auth_method at /oauth2/token #290
No reviewers
Labels
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
psa-systems/bunyip!290
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/BUNYIP-254-token-endpoint-enforcement"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
BUNYIP-254: the schema in migration
20260417000040_create_oidc_clients.sqlcarriesallowed_grant_types TEXT[]andtoken_endpoint_auth_method TEXT CHECK (... IN ('none', 'client_secret_basic', 'private_key_jwt')), but the runtime ignored both columns. The audit named this high-severity: a client registered code-only (e.g. lets-chat per20260618032217_register_lets_chat_oidc_client.sql) could still use therefresh_tokengrant, andauthenticate_clientreturned Ok for anyclient_type == "public"regardless of the registered auth method, so a confidential client mis-registered as public bypassed all secret verification.Load + load.
OAuthClientgainsallowed_grant_types: Vec<String>andtoken_endpoint_auth_method: String.load_client's SELECT pulls both columns./oauth2/tokenenforcement. Afterauthenticate_clientsucceeds, the runtime assertsclient.allowed_grant_types.contains(&body.grant_type)and rejects withunauthorized_clientper RFC 6749 §5.2 when not. The check fires BEFORE the per-grant dispatch so the rejection path is uniform acrossauthorization_codeandrefresh_token.authenticate_clientrewrite. Now branches ontoken_endpoint_auth_method:client_secret_basic: confidential client required, Argon2id verify of the Basic-auth secret (existing path, just gated by the auth method).none: public client required, NOclient_secretmay be presented. PKCE handles replay protection separately at/oauth2/authorize.private_key_jwt: the schema accepts this value, but the runtime does not implement JWT client assertion verification. Refuse withinvalid_clientrather than fall through (which would otherwise either accept any caller or 500). Tracked as a follow-up.invalid_client.Mis-registered pairs (public client +
client_secret_basic, confidential client +none) are now refused at the runtime even when the CHECK constraint allows them, so a misconfigured registration can't open a quiet bypass.#BUNYIP-254
BUNYIP-254: the schema in migration `20260417000040_create_oidc_clients.sql` carries `allowed_grant_types TEXT[]` and `token_endpoint_auth_method TEXT CHECK (... IN ('none', 'client_secret_basic', 'private_key_jwt'))`, but the runtime ignored both columns. The audit named this high-severity: a client registered code-only (e.g. lets-chat per `20260618032217_register_lets_chat_oidc_client.sql`) could still use the `refresh_token` grant, and `authenticate_client` returned Ok for any `client_type == "public"` regardless of the registered auth method, so a confidential client mis-registered as public bypassed all secret verification. Load + load. `OAuthClient` gains `allowed_grant_types: Vec<String>` and `token_endpoint_auth_method: String`. `load_client`'s SELECT pulls both columns. `/oauth2/token` enforcement. After `authenticate_client` succeeds, the runtime asserts `client.allowed_grant_types.contains(&body.grant_type)` and rejects with `unauthorized_client` per RFC 6749 §5.2 when not. The check fires BEFORE the per-grant dispatch so the rejection path is uniform across `authorization_code` and `refresh_token`. `authenticate_client` rewrite. Now branches on `token_endpoint_auth_method`: - `client_secret_basic`: confidential client required, Argon2id verify of the Basic-auth secret (existing path, just gated by the auth method). - `none`: public client required, NO `client_secret` may be presented. PKCE handles replay protection separately at `/oauth2/authorize`. - `private_key_jwt`: the schema accepts this value, but the runtime does not implement JWT client assertion verification. Refuse with `invalid_client` rather than fall through (which would otherwise either accept any caller or 500). Tracked as a follow-up. - Any other value (the CHECK constraint refuses these, but defense in depth): refuse with a descriptive `invalid_client`. Mis-registered pairs (public client + `client_secret_basic`, confidential client + `none`) are now refused at the runtime even when the CHECK constraint allows them, so a misconfigured registration can't open a quiet bypass. #BUNYIP-254