From dcd5cd23f4869cbe530f59e2ca07ff3788d400f5 Mon Sep 17 00:00:00 2001
From: Firstyear <william@blackhats.net.au>
Date: Wed, 5 Mar 2025 13:21:09 +1000
Subject: [PATCH] Handle form-post as a response mode (#3467)
Some oauth2 clients apparently ignore what we tell them
and request response modes we don't support.
First, we should deserialise these and error correctly.
Second, to maintain temporary compatibility, we remap
form-post to query. This will be removed in future.
---
proto/src/oauth2.rs | 6 +++++-
server/lib/src/idm/oauth2.rs | 41 +++++++++++++++++++++++++++---------
2 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/proto/src/oauth2.rs b/proto/src/oauth2.rs
index 026a3c053..0e7229768 100644
--- a/proto/src/oauth2.rs
+++ b/proto/src/oauth2.rs
@@ -7,7 +7,8 @@ use serde::{Deserialize, Serialize};
use serde_with::base64::{Base64, UrlSafe};
use serde_with::formats::SpaceSeparator;
use serde_with::{
- formats, serde_as, skip_serializing_none, NoneAsEmptyString, StringWithSeparator,
+ formats, rust::deserialize_ignore_any, serde_as, skip_serializing_none, NoneAsEmptyString,
+ StringWithSeparator,
};
use url::Url;
use uuid::Uuid;
@@ -353,6 +354,9 @@ pub enum ResponseType {
pub enum ResponseMode {
Query,
Fragment,
+ FormPost,
+ #[serde(other, deserialize_with = "deserialize_ignore_any")]
+ Invalid,
}
fn response_modes_supported_default() -> Vec<ResponseMode> {
diff --git a/server/lib/src/idm/oauth2.rs b/server/lib/src/idm/oauth2.rs
index a6cb8e618..9628e1ea4 100644
--- a/server/lib/src/idm/oauth2.rs
+++ b/server/lib/src/idm/oauth2.rs
@@ -122,6 +122,11 @@ impl std::fmt::Display for Oauth2Error {
}
// == internal state formats that we encrypt and send.
+#[derive(Serialize, Deserialize, Debug, PartialEq)]
+enum SupportedResponseMode {
+ Query,
+ Fragment,
+}
#[serde_as]
#[derive(Serialize, Deserialize, Debug, PartialEq)]
@@ -145,7 +150,7 @@ struct ConsentToken {
// We stash some details here for oidc.
pub nonce: Option<String>,
/// The format the response should be returned to the application in.
- pub response_mode: ResponseMode,
+ pub response_mode: SupportedResponseMode,
}
#[serde_as]
@@ -239,7 +244,7 @@ pub struct AuthorisePermitSuccess {
// The exchange code as a String
pub code: String,
/// The format the response should be returned to the application in.
- pub response_mode: ResponseMode,
+ response_mode: SupportedResponseMode,
}
impl AuthorisePermitSuccess {
@@ -252,7 +257,7 @@ impl AuthorisePermitSuccess {
redirect_uri.set_fragment(None);
match self.response_mode {
- ResponseMode::Query => {
+ SupportedResponseMode::Query => {
redirect_uri
.query_pairs_mut()
.append_pair("code", &self.code);
@@ -261,7 +266,7 @@ impl AuthorisePermitSuccess {
redirect_uri.query_pairs_mut().append_pair("state", state);
};
}
- ResponseMode::Fragment => {
+ SupportedResponseMode::Fragment => {
redirect_uri.set_query(None);
// Per [the RFC](https://www.rfc-editor.org/rfc/rfc6749#section-3.1.2), we can't set query pairs on fragment-containing redirects, only query ones.
@@ -285,7 +290,7 @@ pub struct AuthoriseReject {
// Where the client wants us to go back to.
pub redirect_uri: Url,
/// The format the response should be returned to the application in.
- pub response_mode: ResponseMode,
+ response_mode: SupportedResponseMode,
}
impl AuthoriseReject {
@@ -305,8 +310,8 @@ impl AuthoriseReject {
.finish();
match self.response_mode {
- ResponseMode::Query => redirect_uri.set_query(Some(&encoded)),
- ResponseMode::Fragment => redirect_uri.set_fragment(Some(&encoded)),
+ SupportedResponseMode::Query => redirect_uri.set_query(Some(&encoded)),
+ SupportedResponseMode::Fragment => redirect_uri.set_fragment(Some(&encoded)),
}
redirect_uri
@@ -1873,15 +1878,31 @@ impl IdmServerProxyReadTransaction<'_> {
admin_warn!("Unsupported OAuth2 response_type (should be 'code')");
return Err(Oauth2Error::UnsupportedResponseType);
}
+
let Some(response_mode) = auth_req.get_response_mode() else {
- admin_warn!(
+ warn!(
"Invalid response_mode {:?} for response_type {:?}",
- auth_req.response_mode,
- auth_req.response_type
+ auth_req.response_mode, auth_req.response_type
);
return Err(Oauth2Error::InvalidRequest);
};
+ let response_mode = match response_mode {
+ ResponseMode::Query => SupportedResponseMode::Query,
+ ResponseMode::Fragment => SupportedResponseMode::Fragment,
+ ResponseMode::FormPost => {
+ warn!(
+ "Invalid response mode form_post requested - many clients request this incorrectly but proceed with response_mode=query. Remapping to query."
+ );
+ warn!("This behaviour WILL BE REMOVED in a future release.");
+ SupportedResponseMode::Query
+ }
+ ResponseMode::Invalid => {
+ warn!("Invalid response mode requested, unable to proceed");
+ return Err(Oauth2Error::InvalidRequest);
+ }
+ };
+
/*
* 4.1.2.1. Error Response
*