From 9c856d3c66d3ef6790e3744e409f0892004d521e Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 18 Oct 2026 14:50:04 -0700 Subject: [PATCH] Accept only S256 PKCE challenges The authorization endpoint took the plain method, and assumed it when a challenge came without a method (RFC 7636's default), for every client but stateless ones, though the metadata lists only S256. With plain the challenge is the verifier, so it protects nothing. Codes issued with plain before an upgrade still verify. --- crates/http/src/auth/oauth/auth.rs | 18 ++++++++++-------- tests/src/system/oidc.rs | 23 +++++++++++++++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/crates/http/src/auth/oauth/auth.rs b/crates/http/src/auth/oauth/auth.rs index 5b640e4..5d394eb 100644 --- a/crates/http/src/auth/oauth/auth.rs +++ b/crates/http/src/auth/oauth/auth.rs @@ -2,6 +2,8 @@ * SPDX-FileCopyrightText: 2020 Stalwart Labs LLC * * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL + * + * Modified by Coffey Labs in 2026 for INBUXA. */ use super::{DeviceAuthResponse, FormData, MAX_POST_LEN, OAuthCode, PkceCodeChallenge}; @@ -260,16 +262,16 @@ impl OAuthApiHandler for Server { } // Parse and validate PKCE challenge (RFC 7636). + // inbuxa: S256 only, as the metadata says. With `plain` (also + // RFC 7636's default when no method is named) the challenge is + // the verifier, so it protects nothing. let pkce_challenge = match code_challenge { - Some(challenge) => match code_challenge_method.as_deref().unwrap_or("plain") { - "S256" => PkceCodeChallenge::S256(challenge), - "plain" if stateless_client.is_none() => { - PkceCodeChallenge::Plain(challenge) - } + Some(challenge) => match code_challenge_method.as_deref() { + Some("S256") => PkceCodeChallenge::S256(challenge), _ => { - return Err(trc::AuthEvent::Error - .into_err() - .details("Unsupported PKCE code_challenge_method.")); + return Err(trc::AuthEvent::Error.into_err().details( + "Unsupported PKCE code_challenge_method: use S256.", + )); } }, None => { diff --git a/tests/src/system/oidc.rs b/tests/src/system/oidc.rs index 664f44b..1b00597 100644 --- a/tests/src/system/oidc.rs +++ b/tests/src/system/oidc.rs @@ -351,6 +351,29 @@ pub async fn test(test: &mut TestServer) { "unknown resource indicator must not be authorized" ); + // inbuxa: only S256 is accepted. `plain`, named or as RFC 7636's default + // when no method is given, sends the verifier itself as the challenge. + for method in [Some("plain"), None] { + let (status, _) = post_login_raw(&LoginRequest::AuthCode { + account_name: "user@example.org".to_string(), + account_secret: "this is a very strong password".to_string(), + mfa_token: None, + client_id: client_id.to_string(), + redirect_uri: "com.example.app:/cb".to_string().into(), + nonce: None, + scope: Some(PROFILE_SCOPE.to_string()), + code_challenge: Some(PKCE_VERIFIER.to_string()), + code_challenge_method: method.map(str::to_string), + state: None, + resource: vec![], + }) + .await; + assert_ne!( + status, 200, + "PKCE method {method:?} must not be authorized" + ); + } + // Authenticate with the correct password, PKCE (S256), scope and a valid resource indicator let response = http .post::( -- 2.54.0