From c059dbb33760bfdf40a94612772f3fe9418ff250 Mon Sep 17 00:00:00 2001 From: lafleur Date: Mon, 11 Aug 2025 22:04:07 +0200 Subject: [PATCH] OIDC: embed user_id in consent --- src/api/client/oidc/authorize.rs | 40 +++++++++++++------------------ src/api/client/oidc/login.rs | 6 ++--- src/web/oidc.rs | 1 + src/web/oidc/authorize.rs | 5 +++- src/web/oidc/consent.rs | 2 ++ src/web/templates/consent.html.j2 | 5 +++- 6 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/api/client/oidc/authorize.rs b/src/api/client/oidc/authorize.rs index 5e767057..21d3bad6 100644 --- a/src/api/client/oidc/authorize.rs +++ b/src/api/client/oidc/authorize.rs @@ -1,7 +1,7 @@ use axum::extract::{Query, State}; use conduwuit::{Result, err}; use conduwuit_web::oidc::{ - AuthorizationQuery, OidcRequest, OidcResponse, oidc_consent_form, oidc_login_form, + oidc_consent_form, oidc_login_form, AuthorizationQuery, OidcRequest, OidcResponse, }; use oxide_auth::{ endpoint::{OwnerConsent, Solicitation}, @@ -40,25 +40,20 @@ pub(crate) async fn authorize( // Redirect to the login page if no token or token not known. let hostname = services.config.server_name.host(); - match oauth.authorization_header() { - | None => { - return Ok(oidc_login_form(hostname, &query)); - }, - | Some(token) => - if services.users.find_from_token(token).await.is_err() { - return Ok(oidc_login_form(hostname, &query)); - }, - } - // TODO register the device ID ? - tracing::debug!( - "submitting OIDC authorisation for token : {:#?}", - oauth.authorization_header().unwrap() - ); + let Some(token) = oauth.authorization_header() else { + return Ok(oidc_login_form(hostname, &query)); + }; + + tracing::debug!("submitting OIDC authorisation for token : {token:#?}"); + // Get the user id from the token and add it to the query. + let (owner_id, _) = services.oidc.get_user_for_token(token)?; + let mut query_with_user_id = query.clone(); + query_with_user_id.username = Some(owner_id.localpart().to_string()); services .oidc .endpoint() - .with_solicitor(oidc_consent_form(hostname, &query)) + .with_solicitor(oidc_consent_form(hostname, &query_with_user_id)) .authorization_flow() .execute(oauth) .map_err(|err| err!("authorization failed: {err:?}")) @@ -67,10 +62,10 @@ pub(crate) async fn authorize( /// Whether a user allows their device to access this homeserver's resources. #[derive(serde::Deserialize)] pub(crate) struct Allowance { - allow: Option, + allow: Option, } -/// # `POST /_matrix/client/unstable/org.matrix.msc2964/authorize?allow=[Option]` +/// # `POST /_matrix/client/unstable/org.matrix.msc2964/authorize?allow=[Option]` /// /// Authorize the device based on the user's consent. If the user allows /// it to access their data, the client may request a token at the @@ -80,16 +75,15 @@ pub(crate) async fn authorize_consent( State(services): State, oauth: OidcRequest, ) -> Result { - let allowed = allow.unwrap_or(false); - tracing::debug!("processing user's consent: {:?} - {:?}", allowed, oauth); + tracing::debug!("processing user's consent: {:?} - {:?}", allow, oauth); services .oidc .endpoint() .with_solicitor(FnSolicitor( - move |_: &mut _, solicitation: Solicitation<'_>| match allowed { - | false => OwnerConsent::Denied, - | true => OwnerConsent::Authorized(solicitation.pre_grant().client_id.clone()), + move |_: &mut _, _: Solicitation<'_>| match allow.clone() { + | None => OwnerConsent::Denied, + | Some(user_id) => OwnerConsent::Authorized(user_id), }, )) .authorization_flow() diff --git a/src/api/client/oidc/login.rs b/src/api/client/oidc/login.rs index 78c2b922..880e3967 100644 --- a/src/api/client/oidc/login.rs +++ b/src/api/client/oidc/login.rs @@ -1,7 +1,7 @@ use axum::extract::State; use conduwuit::{Result, err, utils::hash::verify_password}; use conduwuit_web::oidc::{ - AuthorizationQuery, LoginError, LoginQuery, OidcRequest, OidcResponse, oidc_consent_form, + LoginError, LoginQuery, OidcRequest, OidcResponse, oidc_consent_form, }; use ruma::user_id::UserId; @@ -38,14 +38,12 @@ pub(crate) async fn oidc_login( } let hostname = services.config.server_name.host(); - let authorization_query: AuthorizationQuery = query.into(); tracing::info!("logging in {user_id:?}"); - tracing::debug!("login {user_id} authorisation query : {authorization_query:#?}"); services .oidc .endpoint() - .with_solicitor(oidc_consent_form(hostname, &authorization_query)) + .with_solicitor(oidc_consent_form(hostname, &query.into())) .authorization_flow() .execute(request) .map_err(|err| err!(Request(Unknown("authorisation failed: {err:?}")))) diff --git a/src/web/oidc.rs b/src/web/oidc.rs index 84349d0b..0605a279 100644 --- a/src/web/oidc.rs +++ b/src/web/oidc.rs @@ -43,6 +43,7 @@ pub(crate) struct ConsentPageTemplate<'a> { nonce: &'a str, hostname: &'a str, route: &'a str, + user_id: &'a str, client_id: &'a str, client_secret: Option<&'a str>, redirect_uri: &'a str, diff --git a/src/web/oidc/authorize.rs b/src/web/oidc/authorize.rs index 9721bd3f..5f29885f 100644 --- a/src/web/oidc/authorize.rs +++ b/src/web/oidc/authorize.rs @@ -3,7 +3,7 @@ use url::Url; use super::LoginQuery; /// The set of parameters required for an OIDC authorization request. -#[derive(serde::Deserialize, Debug)] +#[derive(serde::Deserialize, Debug, Clone)] pub struct AuthorizationQuery { pub client_id: String, pub client_secret: Option, @@ -14,6 +14,7 @@ pub struct AuthorizationQuery { pub code_challenge_method: String, pub response_type: String, pub response_mode: Option, + pub username: Option, } impl From for AuthorizationQuery { @@ -28,6 +29,7 @@ impl From for AuthorizationQuery { code_challenge_method, response_type, response_mode, + username, .. } = value; @@ -41,6 +43,7 @@ impl From for AuthorizationQuery { code_challenge_method, response_type, response_mode: Some(response_mode), + username: Some(username), } } } diff --git a/src/web/oidc/consent.rs b/src/web/oidc/consent.rs index 5967ed8f..4b76f7aa 100644 --- a/src/web/oidc/consent.rs +++ b/src/web/oidc/consent.rs @@ -27,10 +27,12 @@ pub fn oidc_consent_form(hostname: &str, query: &AuthorizationQuery) -> OidcResp /// Render the html contents of the user consent page. fn consent_page(hostname: &str, query: &AuthorizationQuery, route: &str, nonce: &str) -> String { let response_mode = &query.response_mode.clone().unwrap_or("fragment".to_string()); + let user_id = query.username.clone().expect("user_id in authorization query"); let template = ConsentPageTemplate { nonce, hostname, route, + user_id: &encode(&user_id), client_id: &encode(query.client_id.as_str()), client_secret: query.client_secret.as_deref(), redirect_uri: &encode(query.redirect_uri.as_str()), diff --git a/src/web/templates/consent.html.j2 b/src/web/templates/consent.html.j2 index 054a9fb7..25843841 100644 --- a/src/web/templates/consent.html.j2 +++ b/src/web/templates/consent.html.j2 @@ -6,7 +6,10 @@ '{{ client_id }}' (at {{ redirect_uri }}) is requesting permission for '{{ scope }}'

- +