diff --git a/crates/data-model/src/oauth2/authorization_grant.rs b/crates/data-model/src/oauth2/authorization_grant.rs index 5638ca10..a9fd9304 100644 --- a/crates/data-model/src/oauth2/authorization_grant.rs +++ b/crates/data-model/src/oauth2/authorization_grant.rs @@ -19,6 +19,11 @@ use mas_iana::oauth::PkceCodeChallengeMethod; use oauth2_types::{ pkce::{CodeChallengeError, CodeChallengeMethodExt}, requests::ResponseMode, + scope::{Scope, OPENID, PROFILE}, +}; +use rand::{ + distributions::{Alphanumeric, DistString}, + RngCore, }; use serde::Serialize; use ulid::Ulid; @@ -146,7 +151,7 @@ pub struct AuthorizationGrant { pub code: Option, pub client_id: Ulid, pub redirect_uri: Url, - pub scope: oauth2_types::scope::Scope, + pub scope: Scope, pub state: Option, pub nonce: Option, pub max_age: Option, @@ -190,4 +195,25 @@ impl AuthorizationGrant { self.stage = self.stage.cancel(canceld_at)?; Ok(self) } + + pub fn sample(now: DateTime, rng: &mut impl RngCore) -> Self { + Self { + id: Ulid::from_datetime_with_source(now.into(), rng), + stage: AuthorizationGrantStage::Pending, + code: Some(AuthorizationCode { + code: Alphanumeric.sample_string(rng, 10), + pkce: None, + }), + client_id: Ulid::from_datetime_with_source(now.into(), rng), + redirect_uri: Url::parse("http://localhost:8080").unwrap(), + scope: Scope::from_iter([OPENID, PROFILE]), + state: Some(Alphanumeric.sample_string(rng, 10)), + nonce: Some(Alphanumeric.sample_string(rng, 10)), + max_age: None, + response_mode: ResponseMode::Query, + response_type_id_token: false, + created_at: now, + requires_consent: false, + } + } } diff --git a/crates/data-model/src/oauth2/client.rs b/crates/data-model/src/oauth2/client.rs index 288e7a7a..7025f396 100644 --- a/crates/data-model/src/oauth2/client.rs +++ b/crates/data-model/src/oauth2/client.rs @@ -12,12 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +use chrono::{DateTime, Utc}; use mas_iana::{ jose::JsonWebSignatureAlg, oauth::{OAuthAuthorizationEndpointResponseType, OAuthClientAuthenticationMethod}, }; use mas_jose::jwk::PublicJsonWebKeySet; use oauth2_types::requests::GrantType; +use rand::RngCore; use serde::Serialize; use thiserror::Error; use ulid::Ulid; @@ -120,4 +122,56 @@ impl Client { _ => Err(InvalidRedirectUriError::NotAllowed), } } + + pub fn samples(now: DateTime, rng: &mut impl RngCore) -> Vec { + vec![ + // A client with all the URIs set + Self { + id: Ulid::from_datetime_with_source(now.into(), rng), + client_id: "client1".to_owned(), + encrypted_client_secret: None, + redirect_uris: vec![ + Url::parse("https://client1.example.com/redirect").unwrap(), + Url::parse("https://client1.example.com/redirect2").unwrap(), + ], + response_types: vec![OAuthAuthorizationEndpointResponseType::Code], + grant_types: vec![GrantType::AuthorizationCode, GrantType::RefreshToken], + contacts: vec!["foo@client1.example.com".to_owned()], + client_name: Some("Client 1".to_owned()), + client_uri: Some(Url::parse("https://client1.example.com").unwrap()), + logo_uri: Some(Url::parse("https://client1.example.com/logo.png").unwrap()), + tos_uri: Some(Url::parse("https://client1.example.com/tos").unwrap()), + policy_uri: Some(Url::parse("https://client1.example.com/policy").unwrap()), + initiate_login_uri: Some( + Url::parse("https://client1.example.com/initiate-login").unwrap(), + ), + token_endpoint_auth_method: Some(OAuthClientAuthenticationMethod::None), + token_endpoint_auth_signing_alg: None, + id_token_signed_response_alg: None, + userinfo_signed_response_alg: None, + jwks: None, + }, + // Another client without any URIs set + Self { + id: Ulid::from_datetime_with_source(now.into(), rng), + client_id: "client2".to_owned(), + encrypted_client_secret: None, + redirect_uris: vec![Url::parse("https://client2.example.com/redirect").unwrap()], + response_types: vec![OAuthAuthorizationEndpointResponseType::Code], + grant_types: vec![GrantType::AuthorizationCode, GrantType::RefreshToken], + contacts: vec!["foo@client2.example.com".to_owned()], + client_name: None, + client_uri: None, + logo_uri: None, + tos_uri: None, + policy_uri: None, + initiate_login_uri: None, + token_endpoint_auth_method: None, + token_endpoint_auth_signing_alg: None, + id_token_signed_response_alg: None, + userinfo_signed_response_alg: None, + jwks: None, + }, + ] + } } diff --git a/crates/handlers/src/oauth2/authorization/complete.rs b/crates/handlers/src/oauth2/authorization/complete.rs index b51b2d5f..5529db61 100644 --- a/crates/handlers/src/oauth2/authorization/complete.rs +++ b/crates/handlers/src/oauth2/authorization/complete.rs @@ -21,7 +21,7 @@ use axum::{ use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; use mas_axum_utils::SessionInfoExt; -use mas_data_model::{AuthorizationGrant, BrowserSession}; +use mas_data_model::{AuthorizationGrant, BrowserSession, Client}; use mas_keystore::Encrypter; use mas_policy::PolicyFactory; use mas_router::{PostAuthAction, Route}; @@ -47,6 +47,9 @@ pub enum RouteError { #[error("authorization grant is not in a pending state")] NotPending, + + #[error("failed to load client")] + NoSuchClient, } impl IntoResponse for RouteError { @@ -62,8 +65,8 @@ impl IntoResponse for RouteError { "authorization grant not in a pending state", ) .into_response(), - RouteError::Internal(e) => { - (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response() + RouteError::Internal(_) | Self::NoSuchClient => { + (StatusCode::INTERNAL_SERVER_ERROR, self.to_string()).into_response() } } } @@ -112,7 +115,13 @@ pub(crate) async fn get( return Ok((cookie_jar, mas_router::Login::and_then(continue_grant).go()).into_response()); }; - match complete(rng, clock, grant, session, &policy_factory, repo).await { + let client = repo + .oauth2_client() + .lookup(grant.client_id) + .await? + .ok_or(RouteError::NoSuchClient)?; + + match complete(rng, clock, grant, client, session, &policy_factory, repo).await { Ok(params) => { let res = callback_destination.go(&templates, params).await?; Ok((cookie_jar, res).into_response()) @@ -128,7 +137,6 @@ pub(crate) async fn get( } Err(GrantCompletionError::NotPending) => Err(RouteError::NotPending), Err(GrantCompletionError::Internal(e)) => Err(RouteError::Internal(e)), - Err(e) => Err(RouteError::Internal(e.into())), } } @@ -148,9 +156,6 @@ pub enum GrantCompletionError { #[error("denied by the policy")] PolicyViolation, - - #[error("failed to load client")] - NoSuchClient, } impl_from_error_for_route!(GrantCompletionError: mas_storage::RepositoryError); @@ -163,6 +168,7 @@ pub(crate) async fn complete( mut rng: BoxRng, clock: BoxClock, grant: AuthorizationGrant, + client: Client, browser_session: BrowserSession, policy_factory: &PolicyFactory, mut repo: BoxRepository, @@ -181,19 +187,13 @@ pub(crate) async fn complete( // Run through the policy let mut policy = policy_factory.instantiate().await?; let res = policy - .evaluate_authorization_grant(&grant, &browser_session.user) + .evaluate_authorization_grant(&grant, &client, &browser_session.user) .await?; if !res.valid() { return Err(GrantCompletionError::PolicyViolation); } - let client = repo - .oauth2_client() - .lookup(grant.client_id) - .await? - .ok_or(GrantCompletionError::NoSuchClient)?; - let current_consent = repo .oauth2_client() .get_consent_for_user(&client, &browser_session.user) diff --git a/crates/handlers/src/oauth2/authorization/mod.rs b/crates/handlers/src/oauth2/authorization/mod.rs index 2e56f927..657fcaf4 100644 --- a/crates/handlers/src/oauth2/authorization/mod.rs +++ b/crates/handlers/src/oauth2/authorization/mod.rs @@ -341,6 +341,7 @@ pub(crate) async fn get( rng, clock, grant, + client, user_session, &policy_factory, repo, @@ -372,10 +373,7 @@ pub(crate) async fn get( Err(GrantCompletionError::Internal(e)) => { return Err(RouteError::Internal(e)) } - Err( - e @ (GrantCompletionError::NotPending - | GrantCompletionError::NoSuchClient), - ) => { + Err(e @ GrantCompletionError::NotPending) => { // This should never happen return Err(RouteError::Internal(Box::new(e))); } @@ -388,6 +386,7 @@ pub(crate) async fn get( rng, clock, grant, + client, user_session, &policy_factory, repo, @@ -413,10 +412,7 @@ pub(crate) async fn get( Err(GrantCompletionError::Internal(e)) => { return Err(RouteError::Internal(e)) } - Err( - e @ (GrantCompletionError::NotPending - | GrantCompletionError::NoSuchClient), - ) => { + Err(e @ GrantCompletionError::NotPending) => { // This should never happen return Err(RouteError::Internal(Box::new(e))); } diff --git a/crates/handlers/src/oauth2/consent.rs b/crates/handlers/src/oauth2/consent.rs index bbf98cf7..4d7ddb31 100644 --- a/crates/handlers/src/oauth2/consent.rs +++ b/crates/handlers/src/oauth2/consent.rs @@ -97,6 +97,12 @@ pub(crate) async fn get( .await? .ok_or(RouteError::GrantNotFound)?; + let client = repo + .oauth2_client() + .lookup(grant.client_id) + .await? + .ok_or(RouteError::NoSuchClient)?; + if !matches!(grant.stage, AuthorizationGrantStage::Pending) { return Err(RouteError::GrantNotPending); } @@ -106,11 +112,11 @@ pub(crate) async fn get( let mut policy = policy_factory.instantiate().await?; let res = policy - .evaluate_authorization_grant(&grant, &session.user) + .evaluate_authorization_grant(&grant, &client, &session.user) .await?; if res.valid() { - let ctx = ConsentContext::new(grant) + let ctx = ConsentContext::new(grant, client) .with_session(session) .with_csrf(csrf_token.form_value()); @@ -118,7 +124,7 @@ pub(crate) async fn get( Ok((cookie_jar, Html(content)).into_response()) } else { - let ctx = PolicyViolationContext::new(grant) + let ctx = PolicyViolationContext::new(grant, client) .with_session(session) .with_csrf(csrf_token.form_value()); @@ -167,21 +173,21 @@ pub(crate) async fn post( return Ok((cookie_jar, login.go()).into_response()); }; - let mut policy = policy_factory.instantiate().await?; - let res = policy - .evaluate_authorization_grant(&grant, &session.user) - .await?; - - if !res.valid() { - return Err(RouteError::PolicyViolation); - } - let client = repo .oauth2_client() .lookup(grant.client_id) .await? .ok_or(RouteError::NoSuchClient)?; + let mut policy = policy_factory.instantiate().await?; + let res = policy + .evaluate_authorization_grant(&grant, &client, &session.user) + .await?; + + if !res.valid() { + return Err(RouteError::PolicyViolation); + } + // Do not consent for the "urn:matrix:org.matrix.msc2967.client:device:*" scope let scope_without_device = grant .scope diff --git a/crates/policy/src/lib.rs b/crates/policy/src/lib.rs index d895339a..665afe5f 100644 --- a/crates/policy/src/lib.rs +++ b/crates/policy/src/lib.rs @@ -17,7 +17,7 @@ #![warn(clippy::pedantic)] #![allow(clippy::missing_errors_doc)] -use mas_data_model::{AuthorizationGrant, User}; +use mas_data_model::{AuthorizationGrant, Client, User}; use oauth2_types::registration::VerifiedClientMetadata; use opa_wasm::Runtime; use serde::Deserialize; @@ -246,6 +246,7 @@ impl Policy { skip_all, fields( data.authorization_grant.id = %authorization_grant.id, + data.client.id = %client.id, data.user.id = %user.id, ), err, @@ -253,12 +254,14 @@ impl Policy { pub async fn evaluate_authorization_grant( &mut self, authorization_grant: &AuthorizationGrant, + client: &Client, user: &User, ) -> Result { let authorization_grant = serde_json::to_value(authorization_grant)?; let user = serde_json::to_value(user)?; let input = serde_json::json!({ "authorization_grant": authorization_grant, + "client": client, "user": user, }); diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index 90110a30..feed6765 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -18,8 +18,8 @@ use chrono::Utc; use mas_data_model::{ - AuthorizationGrant, BrowserSession, CompatSsoLogin, CompatSsoLoginState, UpstreamOAuthLink, - UpstreamOAuthProvider, User, UserEmail, UserEmailVerification, + AuthorizationGrant, BrowserSession, Client, CompatSsoLogin, CompatSsoLoginState, + UpstreamOAuthLink, UpstreamOAuthProvider, User, UserEmail, UserEmailVerification, }; use mas_router::{PostAuthAction, Route}; use rand::Rng; @@ -395,25 +395,42 @@ impl RegisterContext { #[derive(Serialize)] pub struct ConsentContext { grant: AuthorizationGrant, + client: Client, action: PostAuthAction, } impl TemplateContext for ConsentContext { - fn sample(_now: chrono::DateTime, _rng: &mut impl Rng) -> Vec + fn sample(now: chrono::DateTime, rng: &mut impl Rng) -> Vec where Self: Sized, { - // TODO - vec![] + Client::samples(now, rng) + .into_iter() + .map(|client| { + let mut grant = AuthorizationGrant::sample(now, rng); + let action = PostAuthAction::continue_grant(grant.id); + // XXX + grant.client_id = client.id; + Self { + grant, + client, + action, + } + }) + .collect() } } impl ConsentContext { /// Constructs a context for the client consent page #[must_use] - pub fn new(grant: AuthorizationGrant) -> Self { + pub fn new(grant: AuthorizationGrant, client: Client) -> Self { let action = PostAuthAction::continue_grant(grant.id); - Self { grant, action } + Self { + grant, + client, + action, + } } } @@ -421,25 +438,42 @@ impl ConsentContext { #[derive(Serialize)] pub struct PolicyViolationContext { grant: AuthorizationGrant, + client: Client, action: PostAuthAction, } impl TemplateContext for PolicyViolationContext { - fn sample(_now: chrono::DateTime, _rng: &mut impl Rng) -> Vec + fn sample(now: chrono::DateTime, rng: &mut impl Rng) -> Vec where Self: Sized, { - // TODO - vec![] + Client::samples(now, rng) + .into_iter() + .map(|client| { + let mut grant = AuthorizationGrant::sample(now, rng); + let action = PostAuthAction::continue_grant(grant.id); + // XXX + grant.client_id = client.id; + Self { + grant, + client, + action, + } + }) + .collect() } } impl PolicyViolationContext { /// Constructs a context for the policy violation page #[must_use] - pub const fn new(grant: AuthorizationGrant) -> Self { + pub const fn new(grant: AuthorizationGrant, client: Client) -> Self { let action = PostAuthAction::continue_grant(grant.id); - Self { grant, action } + Self { + grant, + client, + action, + } } } diff --git a/templates/pages/consent.html b/templates/pages/consent.html index 5f376535..f90daa30 100644 --- a/templates/pages/consent.html +++ b/templates/pages/consent.html @@ -23,17 +23,17 @@ limitations under the License.
- {% if grant.client.logo_uri %} - + {% if client.logo_uri %} + {% endif %}
-

{{ grant.client.client_name | default(value=grant.client.client_id) }}

+

{{ client.client_name | default(value=client.client_id) }}

at {{ grant.redirect_uri }}

wants to access your Matrix account

-

This will allow {{ grant.client.client_name | default(value=grant.client.client_id) }} to:

+

This will allow {{ client.client_name | default(value=client.client_id) }} to:

    @@ -49,19 +49,19 @@ limitations under the License. {% endfor %}

-

Make sure that you trust {{ grant.client.client_name }}

+

Make sure that you trust {{ client.client_name }}

You may be sharing sensitive information with this site or app. - {% if grant.client.policy_uri or grant.client.tos_uri %} - Find out how {{ grant.client.client_name }} will handle your data by reviewing it's - {% if grant.client.policy_uri %} - privacy policy{% if not grant.client.tos_uri %}.{% endif %} + {% if client.policy_uri or client.tos_uri %} + Find out how {{ client.client_name }} will handle your data by reviewing it's + {% if client.policy_uri %} + privacy policy{% if not client.tos_uri %}.{% endif %} {% endif %} - {% if grant.client.policy_uri and grant.client.tos_uri%} + {% if client.policy_uri and client.tos_uri%} and {% endif %} - {% if grant.client.tos_uri %} - terms of service. + {% if client.tos_uri %} + terms of service. {% endif %} {% endif %}

diff --git a/templates/pages/policy_violation.html b/templates/pages/policy_violation.html index 90a42717..81b76b46 100644 --- a/templates/pages/policy_violation.html +++ b/templates/pages/policy_violation.html @@ -24,11 +24,11 @@ limitations under the License.

This might be because of the client which authored the request, the currently logged in user, or the request itself.

- {% if grant.client.logo_uri %} - + {% if client.logo_uri %} + {% endif %}
-

{{ grant.client.client_name | default(value=grant.client.client_id) }}

+

{{ client.client_name | default(value=client.client_id) }}