From 292d1cc0481484a44ef0bd42bcc728b40faf5ff9 Mon Sep 17 00:00:00 2001 From: mdecimus Date: Thu, 3 Oct 2024 15:07:38 +0200 Subject: [PATCH] Include nonce parameter in id_tokens --- crates/common/src/auth/oauth/oidc.rs | 14 ++++++++-- crates/jmap/src/auth/oauth/auth.rs | 3 +- crates/jmap/src/auth/oauth/token.rs | 41 ++++++++++++++++++---------- tests/src/jmap/auth_oauth.rs | 15 ++++++---- tests/src/jmap/mod.rs | 12 ++++---- 5 files changed, 57 insertions(+), 28 deletions(-) diff --git a/crates/common/src/auth/oauth/oidc.rs b/crates/common/src/auth/oauth/oidc.rs index 16a9192b..8722f6dd 100644 --- a/crates/common/src/auth/oauth/oidc.rs +++ b/crates/common/src/auth/oauth/oidc.rs @@ -78,12 +78,20 @@ pub struct Userinfo { pub updated_at: Option, } +#[derive(Debug, Default, Eq, PartialEq, Deserialize, Serialize)] +pub struct Nonce { + #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default)] + pub nonce: Option, +} + impl Server { pub fn issue_id_token( &self, subject: impl Into, issuer: impl Into, audience: impl Into, + nonce: Option>, ) -> trc::Result { let now = now() as i64; @@ -93,7 +101,7 @@ impl Server { key_id: Some("default".into()), ..Default::default() }), - ClaimsSet::<()> { + ClaimsSet:: { registered: RegisteredClaims { issuer: Some(issuer.into()), subject: Some(subject.into()), @@ -103,7 +111,9 @@ impl Server { expiry: Some((now + self.core.oauth.oidc_expiry_id_token as i64).into()), ..Default::default() }, - private: (), + private: Nonce { + nonce: nonce.map(Into::into), + }, }, ) .into_encoded(&self.core.oauth.oidc_signing_secret) diff --git a/crates/jmap/src/auth/oauth/auth.rs b/crates/jmap/src/auth/oauth/auth.rs index 0b4ce726..08acb948 100644 --- a/crates/jmap/src/auth/oauth/auth.rs +++ b/crates/jmap/src/auth/oauth/auth.rs @@ -262,7 +262,8 @@ impl OAuthApiHandler for Server { user_code, expires_in: self.core.oauth.oauth_expiry_user_code, interval: 5, - }).no_cache() + }) + .no_cache() .into_http_response()) } diff --git a/crates/jmap/src/auth/oauth/token.rs b/crates/jmap/src/auth/oauth/token.rs index 026916a3..e629eab3 100644 --- a/crates/jmap/src/auth/oauth/token.rs +++ b/crates/jmap/src/auth/oauth/token.rs @@ -41,6 +41,7 @@ pub trait TokenHandler: Sync + Send { account_id: u32, client_id: &str, issuer: String, + nonce: Option, with_refresh_token: bool, ) -> impl Future> + Send; } @@ -63,10 +64,11 @@ impl TokenHandler for Server { .await; if grant_type.eq_ignore_ascii_case("authorization_code") { - response = if let (Some(code), Some(client_id), Some(redirect_uri)) = ( + response = if let (Some(code), Some(client_id), Some(redirect_uri), nonce) = ( params.get("code"), params.get("client_id"), params.get("redirect_uri"), + params.get("nonce"), ) { // Obtain code match self @@ -100,15 +102,21 @@ impl TokenHandler for Server { .await?; // Issue token - self.issue_token(oauth.account_id, &oauth.client_id, issuer, true) - .await - .map(TokenResponse::Granted) - .map_err(|err| { - trc::AuthEvent::Error - .into_err() - .details(err) - .caused_by(trc::location!()) - })? + self.issue_token( + oauth.account_id, + &oauth.client_id, + issuer, + nonce.map(Into::into), + true, + ) + .await + .map(TokenResponse::Granted) + .map_err(|err| { + trc::AuthEvent::Error + .into_err() + .details(err) + .caused_by(trc::location!()) + })? } } else { TokenResponse::error(ErrorType::InvalidGrant) @@ -122,9 +130,11 @@ impl TokenHandler for Server { } else if grant_type.eq_ignore_ascii_case("urn:ietf:params:oauth:grant-type:device_code") { response = TokenResponse::error(ErrorType::ExpiredToken); - if let (Some(device_code), Some(client_id)) = - (params.get("device_code"), params.get("client_id")) - { + if let (Some(device_code), Some(client_id), nonce) = ( + params.get("device_code"), + params.get("client_id"), + params.get("nonce"), + ) { // Obtain code if let Some(auth_code) = self .core @@ -157,6 +167,7 @@ impl TokenHandler for Server { oauth.account_id, &oauth.client_id, issuer, + nonce.map(Into::into), true, ) .await @@ -190,6 +201,7 @@ impl TokenHandler for Server { token_info.account_id, &token_info.client_id, issuer, + None, token_info.expires_in <= self.core.oauth.oauth_expiry_refresh_token_renew, ) @@ -251,6 +263,7 @@ impl TokenHandler for Server { account_id: u32, client_id: &str, issuer: String, + nonce: Option, with_refresh_token: bool, ) -> trc::Result { Ok(OAuthResponse { @@ -276,7 +289,7 @@ impl TokenHandler for Server { } else { None }, - id_token: match self.issue_id_token(account_id.to_string(), issuer, client_id) { + id_token: match self.issue_id_token(account_id.to_string(), issuer, client_id, nonce) { Ok(id_token) => Some(id_token), Err(err) => { trc::error!(err); diff --git a/tests/src/jmap/auth_oauth.rs b/tests/src/jmap/auth_oauth.rs index 7535b373..85558391 100644 --- a/tests/src/jmap/auth_oauth.rs +++ b/tests/src/jmap/auth_oauth.rs @@ -11,6 +11,7 @@ use biscuit::{jwk::JWKSet, SingleOrMultiple, JWT}; use bytes::Bytes; use common::auth::oauth::{ introspect::OAuthIntrospect, + oidc::Nonce, registration::{ClientRegistrationRequest, ClientRegistrationResponse}, }; use imap_proto::ResponseType; @@ -135,6 +136,7 @@ pub async fn test(params: &mut JMAPTest) { // Obtain token token_params.insert("redirect_uri".to_string(), "https://localhost".to_string()); + token_params.insert("nonce".to_string(), "abc1234".to_string()); let (token, refresh_token, id_token) = unwrap_oidc_token_response(post(&metadata.token_endpoint, &token_params).await); @@ -154,16 +156,19 @@ pub async fn test(params: &mut JMAPTest) { .is_empty()); // Verify ID token using the JWK set - let id_token = JWT::<(), biscuit::Empty>::new_encoded(&id_token) + let id_token = JWT::::new_encoded(&id_token) .decode_with_jwks(&jwk_set, None) .unwrap(); - let claims = &id_token.payload().unwrap().registered; - assert_eq!(claims.issuer, Some(oidc_metadata.issuer)); - assert_eq!(claims.subject, Some(john_int_id.to_string())); + let claims = id_token.payload().unwrap(); + let registered_claims = &claims.registered; + let private_claims = &claims.private; + assert_eq!(registered_claims.issuer, Some(oidc_metadata.issuer)); + assert_eq!(registered_claims.subject, Some(john_int_id.to_string())); assert_eq!( - claims.audience, + registered_claims.audience, Some(SingleOrMultiple::Single(client_id.to_string())) ); + assert_eq!(private_claims.nonce, Some("abc1234".to_string())); // Introspect token let access_introspect: OAuthIntrospect = post_with_auth::( diff --git a/tests/src/jmap/mod.rs b/tests/src/jmap/mod.rs index e14acb11..58f4ba52 100644 --- a/tests/src/jmap/mod.rs +++ b/tests/src/jmap/mod.rs @@ -370,8 +370,8 @@ pub async fn jmap_tests() { ) .await; - webhooks::test(&mut params).await; - /*email_query::test(&mut params, delete).await; + /*webhooks::test(&mut params).await; + email_query::test(&mut params, delete).await; email_get::test(&mut params).await; email_set::test(&mut params).await; email_parse::test(&mut params).await; @@ -384,9 +384,9 @@ pub async fn jmap_tests() { mailbox::test(&mut params).await; delivery::test(&mut params).await; auth_acl::test(&mut params).await; - auth_limits::test(&mut params).await; + auth_limits::test(&mut params).await;*/ auth_oauth::test(&mut params).await; - event_source::test(&mut params).await; + /*event_source::test(&mut params).await; push_subscription::test(&mut params).await; sieve_script::test(&mut params).await; vacation_response::test(&mut params).await; @@ -394,10 +394,10 @@ pub async fn jmap_tests() { websocket::test(&mut params).await; quota::test(&mut params).await; crypto::test(&mut params).await; - blob::test(&mut params).await;*/ + blob::test(&mut params).await; permissions::test(¶ms).await; purge::test(&mut params).await; - enterprise::test(&mut params).await; + enterprise::test(&mut params).await;*/ if delete { params.temp_dir.delete();