From 489ff6f2a2d7c16af941021a50ca4dfd1904c601 Mon Sep 17 00:00:00 2001 From: Daniel Flanagan Date: Thu, 12 Mar 2026 14:44:42 -0500 Subject: [PATCH] fix: add missing DCR fields, PKCE verifier validation, and Cargo.lock sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add policy_uri, tos_uri, software_id, software_version to DCR per RFC 7591 - Add code_verifier length (43-128) and charset validation per RFC 7636 ยง4.1 - Warn at startup if OIDC server enabled without identity providers - Include Cargo.lock update for ring dependency --- Cargo.lock | 1 + src/api/client/oidc.rs | 2 +- src/service/oauth/mod.rs | 5 ++++- src/service/oauth/oidc_server.rs | 22 ++++++++++++++++++++-- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7d742002..38e3d4b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5519,6 +5519,7 @@ dependencies = [ "rand 0.8.5", "regex", "reqwest 0.13.1", + "ring", "ruma", "rustls", "rustyline-async", diff --git a/src/api/client/oidc.rs b/src/api/client/oidc.rs index e995ef41..73eaac2a 100644 --- a/src/api/client/oidc.rs +++ b/src/api/client/oidc.rs @@ -58,7 +58,7 @@ pub(crate) async fn registration_route(State(services): State, Jso let reg = oidc.register_client(body)?; info!("OIDC client registered: {} ({})", reg.client_id, reg.client_name.as_deref().unwrap_or("unnamed")); - Ok((StatusCode::CREATED, Json(serde_json::json!({"client_id": reg.client_id, "client_id_issued_at": reg.registered_at, "redirect_uris": reg.redirect_uris, "client_name": reg.client_name, "client_uri": reg.client_uri, "logo_uri": reg.logo_uri, "contacts": reg.contacts, "token_endpoint_auth_method": reg.token_endpoint_auth_method, "grant_types": reg.grant_types, "response_types": reg.response_types, "application_type": reg.application_type})))) + Ok((StatusCode::CREATED, Json(serde_json::json!({"client_id": reg.client_id, "client_id_issued_at": reg.registered_at, "redirect_uris": reg.redirect_uris, "client_name": reg.client_name, "client_uri": reg.client_uri, "logo_uri": reg.logo_uri, "contacts": reg.contacts, "token_endpoint_auth_method": reg.token_endpoint_auth_method, "grant_types": reg.grant_types, "response_types": reg.response_types, "application_type": reg.application_type, "policy_uri": reg.policy_uri, "tos_uri": reg.tos_uri, "software_id": reg.software_id, "software_version": reg.software_version})))) } #[derive(Debug, Deserialize)] diff --git a/src/service/oauth/mod.rs b/src/service/oauth/mod.rs index 9871435a..bc95639e 100644 --- a/src/service/oauth/mod.rs +++ b/src/service/oauth/mod.rs @@ -15,7 +15,7 @@ use ruma::UserId; use serde::Serialize; use serde_json::Value as JsonValue; use tuwunel_core::{ - Err, Result, err, implement, info, + Err, Result, err, implement, info, warn, utils::{hash::sha256, result::LogErr, stream::ReadyExt}, }; use url::Url; @@ -46,6 +46,9 @@ impl crate::Service for Service { let oidc_server = if !args.server.config.identity_provider.is_empty() || args.server.config.well_known.client.is_some() { + if args.server.config.identity_provider.is_empty() { + warn!("OIDC server enabled (well_known.client is set) but no identity_provider configured; authorization flow will not work"); + } info!("Initializing OIDC server for next-gen auth (MSC2965)"); Some(Arc::new(OidcServer::build(args)?)) } else { diff --git a/src/service/oauth/oidc_server.rs b/src/service/oauth/oidc_server.rs index 16f4fe50..7faac23e 100644 --- a/src/service/oauth/oidc_server.rs +++ b/src/service/oauth/oidc_server.rs @@ -23,6 +23,8 @@ pub struct DcrRequest { pub logo_uri: Option, #[serde(default)] pub contacts: Vec, pub token_endpoint_auth_method: Option, pub grant_types: Option>, pub response_types: Option>, pub application_type: Option, + pub policy_uri: Option, pub tos_uri: Option, + pub software_id: Option, pub software_version: Option, } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -30,7 +32,8 @@ pub struct OidcClientRegistration { pub client_id: String, pub redirect_uris: Vec, pub client_name: Option, pub client_uri: Option, pub logo_uri: Option, pub contacts: Vec, pub token_endpoint_auth_method: String, pub grant_types: Vec, pub response_types: Vec, - pub application_type: Option, pub registered_at: u64, + pub application_type: Option, pub policy_uri: Option, pub tos_uri: Option, + pub software_id: Option, pub software_version: Option, pub registered_at: u64, } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -124,7 +127,8 @@ impl OidcServer { token_endpoint_auth_method: auth_method, grant_types: request.grant_types.unwrap_or_else(|| vec!["authorization_code".to_owned(), "refresh_token".to_owned()]), response_types: request.response_types.unwrap_or_else(|| vec!["code".to_owned()]), - application_type: request.application_type, + application_type: request.application_type, policy_uri: request.policy_uri, tos_uri: request.tos_uri, + software_id: request.software_id, software_version: request.software_version, registered_at: SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap_or_default().as_secs(), }; self.db.oidcclientid_registration.raw_put(&*client_id, Cbor(®istration)); @@ -172,6 +176,7 @@ impl OidcServer { if let Some(challenge) = &session.code_challenge { let Some(verifier) = code_verifier else { return Err!(Request(Forbidden("code_verifier required for PKCE"))); }; + Self::validate_code_verifier(verifier)?; let method = session.code_challenge_method.as_deref().unwrap_or("S256"); let computed = match method { | "S256" => { let hash = utils::hash::sha256::hash(verifier.as_bytes()); b64.encode(hash) }, @@ -184,6 +189,19 @@ impl OidcServer { Ok(session) } + /// Validate code_verifier per RFC 7636 Section 4.1: must be 43-128 + /// characters using only unreserved characters [A-Z] / [a-z] / [0-9] / + /// "-" / "." / "_" / "~". + fn validate_code_verifier(verifier: &str) -> Result { + if !(43..=128).contains(&verifier.len()) { + return Err!(Request(InvalidParam("code_verifier must be 43-128 characters"))); + } + if !verifier.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'.' || b == b'_' || b == b'~') { + return Err!(Request(InvalidParam("code_verifier contains invalid characters"))); + } + Ok(()) + } + pub fn sign_id_token(&self, claims: &IdTokenClaims) -> Result { let mut header = jwt::Header::new(jwt::Algorithm::ES256); header.kid = Some(self.key_id.clone());