letro/letro-authentication-service
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Remove the complete handler, make it go through the consent page

Browse Source
This commit is contained in:
Quentin Gliech
2025-04-11 14:08:28 +02:00
parent 59e5068855
commit e0dacf0761
7 changed files with 142 additions and 384 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
crates/handlers/src/lib.rs
View File

@@ -405,13 +405,10 @@ where
mas_router::OAuth2AuthorizationEndpoint::route(),
get(self::oauth2::authorization::get),
)
.route(
mas_router::ContinueAuthorizationGrant::route(),
get(self::oauth2::authorization::complete::get),
)
.route(
mas_router::Consent::route(),
get(self::oauth2::consent::get).post(self::oauth2::consent::post),
get(self::oauth2::authorization::consent::get)
.post(self::oauth2::authorization::consent::post),
)
.route(
mas_router::CompatLoginSsoComplete::route(),

2
crates/handlers/src/oauth2/authorization/callback.rs
View File

@@ -101,7 +101,7 @@ impl CallbackDestination {
})
}
pub async fn go<T: Serialize + Send + Sync>(
pub fn go<T: Serialize + Send + Sync>(
self,
templates: &Templates,
locale: &DataLocale,

271
crates/handlers/src/oauth2/authorization/complete.rs
View File

@@ -1,271 +0,0 @@
// Copyright 2024 New Vector Ltd.
// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
//
// SPDX-License-Identifier: AGPL-3.0-only
// Please see LICENSE in the repository root for full details.
use axum::{
extract::{Path, State},
response::{Html, IntoResponse, Response},
};
use axum_extra::TypedHeader;
use hyper::StatusCode;
use mas_axum_utils::{SessionInfoExt, cookies::CookieJar, csrf::CsrfExt, sentry::SentryEventID};
use mas_data_model::{AuthorizationGrant, BrowserSession, Client};
use mas_keystore::Keystore;
use mas_policy::{EvaluationResult, Policy};
use mas_router::{PostAuthAction, UrlBuilder};
use mas_storage::{
BoxClock, BoxRepository, BoxRng, Clock, RepositoryAccess,
oauth2::{OAuth2AuthorizationGrantRepository, OAuth2ClientRepository, OAuth2SessionRepository},
user::BrowserSessionRepository,
};
use mas_templates::{PolicyViolationContext, TemplateContext, Templates};
use oauth2_types::requests::AuthorizationResponse;
use thiserror::Error;
use tracing::warn;
use ulid::Ulid;
use super::callback::CallbackDestination;
use crate::{
BoundActivityTracker, PreferredLanguage, impl_from_error_for_route, oauth2::generate_id_token,
};
#[derive(Debug, Error)]
pub enum RouteError {
#[error(transparent)]
Internal(Box<dyn std::error::Error + Send + Sync + 'static>),
#[error("authorization grant was not found")]
NotFound,
#[error("authorization grant is not in a pending state")]
NotPending,
#[error("failed to load client")]
NoSuchClient,
}
impl IntoResponse for RouteError {
fn into_response(self) -> axum::response::Response {
let event = sentry::capture_error(&self);
// TODO: better error pages
let response = match self {
RouteError::NotFound => {
(StatusCode::NOT_FOUND, "authorization grant was not found").into_response()
}
RouteError::NotPending => (
StatusCode::BAD_REQUEST,
"authorization grant not in a pending state",
)
.into_response(),
RouteError::Internal(_) | Self::NoSuchClient => {
(StatusCode::INTERNAL_SERVER_ERROR, self.to_string()).into_response()
}
};
(SentryEventID::from(event), response).into_response()
}
}
impl_from_error_for_route!(mas_storage::RepositoryError);
impl_from_error_for_route!(mas_templates::TemplateError);
impl_from_error_for_route!(mas_policy::LoadError);
impl_from_error_for_route!(mas_policy::EvaluationError);
impl_from_error_for_route!(super::callback::IntoCallbackDestinationError);
impl_from_error_for_route!(super::callback::CallbackDestinationError);
#[tracing::instrument(
name = "handlers.oauth2.authorization_complete.get",
fields(grant.id = %grant_id),
skip_all,
err,
)]
pub(crate) async fn get(
mut rng: BoxRng,
clock: BoxClock,
PreferredLanguage(locale): PreferredLanguage,
State(templates): State<Templates>,
State(url_builder): State<UrlBuilder>,
State(key_store): State<Keystore>,
policy: Policy,
activity_tracker: BoundActivityTracker,
user_agent: Option<TypedHeader<headers::UserAgent>>,
mut repo: BoxRepository,
cookie_jar: CookieJar,
Path(grant_id): Path<Ulid>,
) -> Result<Response, RouteError> {
let (session_info, cookie_jar) = cookie_jar.session_info();
let maybe_session = session_info.load_active_session(&mut repo).await?;
let user_agent = user_agent.map(|TypedHeader(ua)| ua.to_string());
let grant = repo
.oauth2_authorization_grant()
.lookup(grant_id)
.await?
.ok_or(RouteError::NotFound)?;
let callback_destination = CallbackDestination::try_from(&grant)?;
let continue_grant = PostAuthAction::continue_grant(grant.id);
let Some(session) = maybe_session else {
// If there is no session, redirect to the login screen, redirecting here after
// logout
return Ok((
cookie_jar,
url_builder.redirect(&mas_router::Login::and_then(continue_grant)),
)
.into_response());
};
activity_tracker
.record_browser_session(&clock, &session)
.await;
let client = repo
.oauth2_client()
.lookup(grant.client_id)
.await?
.ok_or(RouteError::NoSuchClient)?;
match complete(
&mut rng,
&clock,
&activity_tracker,
user_agent,
repo,
key_store,
policy,
&url_builder,
grant,
&client,
&session,
)
.await
{
Ok(params) => {
let res = callback_destination.go(&templates, &locale, params).await?;
Ok((cookie_jar, res).into_response())
}
Err(GrantCompletionError::PolicyViolation(grant, res)) => {
warn!(violation = ?res, "Authorization grant for client {} denied by policy", client.id);
let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng);
let ctx = PolicyViolationContext::for_authorization_grant(grant, client)
.with_session(session)
.with_csrf(csrf_token.form_value())
.with_language(locale);
let content = templates.render_policy_violation(&ctx)?;
Ok((cookie_jar, Html(content)).into_response())
}
Err(GrantCompletionError::NotPending) => Err(RouteError::NotPending),
Err(GrantCompletionError::Internal(e)) => Err(RouteError::Internal(e)),
}
}
#[derive(Debug, Error)]
pub enum GrantCompletionError {
#[error(transparent)]
Internal(Box<dyn std::error::Error + Send + Sync + 'static>),
#[error("authorization grant is not in a pending state")]
NotPending,
#[error("denied by the policy")]
PolicyViolation(AuthorizationGrant, EvaluationResult),
}
impl_from_error_for_route!(GrantCompletionError: mas_storage::RepositoryError);
impl_from_error_for_route!(GrantCompletionError: super::callback::IntoCallbackDestinationError);
impl_from_error_for_route!(GrantCompletionError: mas_policy::LoadError);
impl_from_error_for_route!(GrantCompletionError: mas_policy::EvaluationError);
impl_from_error_for_route!(GrantCompletionError: super::super::IdTokenSignatureError);
pub(crate) async fn complete(
rng: &mut (impl rand::RngCore + rand::CryptoRng + Send),
clock: &impl Clock,
activity_tracker: &BoundActivityTracker,
user_agent: Option<String>,
mut repo: BoxRepository,
key_store: Keystore,
mut policy: Policy,
url_builder: &UrlBuilder,
grant: AuthorizationGrant,
client: &Client,
browser_session: &BrowserSession,
) -> Result<AuthorizationResponse, GrantCompletionError> {
// Verify that the grant is in a pending stage
if !grant.stage.is_pending() {
return Err(GrantCompletionError::NotPending);
}
// Run through the policy
let res = policy
.evaluate_authorization_grant(mas_policy::AuthorizationGrantInput {
user: Some(&browser_session.user),
client,
scope: &grant.scope,
grant_type: mas_policy::GrantType::AuthorizationCode,
requester: mas_policy::Requester {
ip_address: activity_tracker.ip(),
user_agent,
},
})
.await?;
if !res.valid() {
return Err(GrantCompletionError::PolicyViolation(grant, res));
}
// All good, let's start the session
let session = repo
.oauth2_session()
.add_from_browser_session(rng, clock, client, browser_session, grant.scope.clone())
.await?;
let grant = repo
.oauth2_authorization_grant()
.fulfill(clock, &session, grant)
.await?;
// Yep! Let's complete the auth now
let mut params = AuthorizationResponse::default();
// Did they request an ID token?
if grant.response_type_id_token {
// Fetch the last authentication
let last_authentication = repo
.browser_session()
.get_last_authentication(browser_session)
.await?;
params.id_token = Some(generate_id_token(
rng,
clock,
url_builder,
&key_store,
client,
Some(&grant),
browser_session,
None,
last_authentication.as_ref(),
)?);
}
// Did they request an auth code?
if let Some(code) = grant.code {
params.code = Some(code.code);
}
repo.save().await?;
activity_tracker
.record_oauth2_session(clock, &session)
.await;
Ok(params)
}

116
crates/handlers/src/oauth2/consent.rs → crates/handlers/src/oauth2/authorization/consent.rs
View File

@@ -16,6 +16,7 @@ use mas_axum_utils::{
sentry::SentryEventID,
};
use mas_data_model::AuthorizationGrantStage;
use mas_keystore::Keystore;
use mas_policy::Policy;
use mas_router::{PostAuthAction, UrlBuilder};
use mas_storage::{
@@ -23,11 +24,14 @@ use mas_storage::{
oauth2::{OAuth2AuthorizationGrantRepository, OAuth2ClientRepository},
};
use mas_templates::{ConsentContext, PolicyViolationContext, TemplateContext, Templates};
use oauth2_types::requests::AuthorizationResponse;
use thiserror::Error;
use ulid::Ulid;
use super::callback::CallbackDestination;
use crate::{
BoundActivityTracker, PreferredLanguage, impl_from_error_for_route,
oauth2::generate_id_token,
session::{SessionOrFallback, load_session_or_fallback},
};
@@ -45,9 +49,6 @@ pub enum RouteError {
#[error("Authorization grant already used")]
GrantNotPending,
#[error("Policy violation")]
PolicyViolation,
#[error("Failed to load client")]
NoSuchClient,
}
@@ -57,20 +58,24 @@ impl_from_error_for_route!(mas_storage::RepositoryError);
impl_from_error_for_route!(mas_policy::LoadError);
impl_from_error_for_route!(mas_policy::EvaluationError);
impl_from_error_for_route!(crate::session::SessionLoadError);
impl_from_error_for_route!(crate::oauth2::IdTokenSignatureError);
impl_from_error_for_route!(super::callback::IntoCallbackDestinationError);
impl_from_error_for_route!(super::callback::CallbackDestinationError);
impl IntoResponse for RouteError {
fn into_response(self) -> axum::response::Response {
let event_id = sentry::capture_error(&self);
(
SentryEventID::from(event_id),
StatusCode::INTERNAL_SERVER_ERROR,
SentryEventID::from(event_id),
self.to_string(),
)
.into_response()
}
}
#[tracing::instrument(
name = "handlers.oauth2.consent.get",
name = "handlers.oauth2.authorization.consent.get",
fields(grant.id = %grant_id),
skip_all,
err,
@@ -142,17 +147,7 @@ pub(crate) async fn get(
},
})
.await?;
if res.valid() {
let ctx = ConsentContext::new(grant, client)
.with_session(session)
.with_csrf(csrf_token.form_value())
.with_language(locale);
let content = templates.render_consent(&ctx)?;
Ok((cookie_jar, Html(content)).into_response())
} else {
if !res.valid() {
let ctx = PolicyViolationContext::for_authorization_grant(grant, client)
.with_session(session)
.with_csrf(csrf_token.form_value())
@@ -160,12 +155,21 @@ pub(crate) async fn get(
let content = templates.render_policy_violation(&ctx)?;
Ok((cookie_jar, Html(content)).into_response())
return Ok((cookie_jar, Html(content)).into_response());
}
let ctx = ConsentContext::new(grant, client)
.with_session(session)
.with_csrf(csrf_token.form_value())
.with_language(locale);
let content = templates.render_consent(&ctx)?;
Ok((cookie_jar, Html(content)).into_response())
}
#[tracing::instrument(
name = "handlers.oauth2.consent.post",
name = "handlers.oauth2.authorization.consent.post",
fields(grant.id = %grant_id),
skip_all,
err,
@@ -175,6 +179,7 @@ pub(crate) async fn post(
clock: BoxClock,
PreferredLanguage(locale): PreferredLanguage,
State(templates): State<Templates>,
State(key_store): State<Keystore>,
mut policy: Policy,
mut repo: BoxRepository,
activity_tracker: BoundActivityTracker,
@@ -199,6 +204,8 @@ pub(crate) async fn post(
SessionOrFallback::Fallback { response } => return Ok(response),
};
let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng);
let user_agent = user_agent.map(|ua| ua.to_string());
let grant = repo
@@ -206,15 +213,16 @@ pub(crate) async fn post(
.lookup(grant_id)
.await?
.ok_or(RouteError::GrantNotFound)?;
let next = PostAuthAction::continue_grant(grant_id);
let callback_destination = CallbackDestination::try_from(&grant)?;
let Some(session) = maybe_session else {
let Some(browser_session) = maybe_session else {
let next = PostAuthAction::continue_grant(grant_id);
let login = mas_router::Login::and_then(next);
return Ok((cookie_jar, url_builder.redirect(&login)).into_response());
};
activity_tracker
.record_browser_session(&clock, &session)
.record_browser_session(&clock, &browser_session)
.await;
let client = repo
@@ -225,7 +233,7 @@ pub(crate) async fn post(
let res = policy
.evaluate_authorization_grant(mas_policy::AuthorizationGrantInput {
user: Some(&session.user),
user: Some(&browser_session.user),
client: &client,
scope: &grant.scope,
grant_type: mas_policy::GrantType::AuthorizationCode,
@@ -237,10 +245,70 @@ pub(crate) async fn post(
.await?;
if !res.valid() {
return Err(RouteError::PolicyViolation);
let ctx = PolicyViolationContext::for_authorization_grant(grant, client)
.with_session(browser_session)
.with_csrf(csrf_token.form_value())
.with_language(locale);
let content = templates.render_policy_violation(&ctx)?;
return Ok((cookie_jar, Html(content)).into_response());
}
// All good, let's start the session
let session = repo
.oauth2_session()
.add_from_browser_session(
&mut rng,
&clock,
&client,
&browser_session,
grant.scope.clone(),
)
.await?;
let grant = repo
.oauth2_authorization_grant()
.fulfill(&clock, &session, grant)
.await?;
let mut params = AuthorizationResponse::default();
// Did they request an ID token?
if grant.response_type_id_token {
// Fetch the last authentication
let last_authentication = repo
.browser_session()
.get_last_authentication(&browser_session)
.await?;
params.id_token = Some(generate_id_token(
&mut rng,
&clock,
&url_builder,
&key_store,
&client,
Some(&grant),
&browser_session,
None,
last_authentication.as_ref(),
)?);
}
// Did they request an auth code?
if let Some(code) = grant.code {
params.code = Some(code.code);
}
repo.save().await?;
Ok((cookie_jar, next.go_next(&url_builder)).into_response())
activity_tracker
.record_oauth2_session(&clock, &session)
.await;
Ok((
cookie_jar,
callback_destination.go(&templates, &locale, params)?,
)
.into_response())
}

110
crates/handlers/src/oauth2/authorization/mod.rs
View File

@@ -31,7 +31,7 @@ use self::callback::CallbackDestination;
use crate::{BoundActivityTracker, PreferredLanguage, impl_from_error_for_route};
mod callback;
pub mod complete;
pub(crate) mod consent;
#[derive(Debug, Error)]
pub enum RouteError {
@@ -172,80 +172,66 @@ pub(crate) async fn get(
// Check if the request/request_uri/registration params are used. If so, reply
// with the right error since we don't support them.
if params.auth.request.is_some() {
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::RequestNotSupported),
)
.await?);
return Ok(callback_destination.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::RequestNotSupported),
)?);
}
if params.auth.request_uri.is_some() {
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::RequestUriNotSupported),
)
.await?);
return Ok(callback_destination.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::RequestUriNotSupported),
)?);
}
// Check if the client asked for a `token` response type, and bail out if it's
// the case, since we don't support them
if response_type.has_token() {
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::UnsupportedResponseType),
)
.await?);
return Ok(callback_destination.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::UnsupportedResponseType),
)?);
}
// If the client asked for a `id_token` response type, we must check if it can
// use the `implicit` grant type
if response_type.has_id_token() && !client.grant_types.contains(&GrantType::Implicit) {
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::UnauthorizedClient),
)
.await?);
return Ok(callback_destination.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::UnauthorizedClient),
)?);
}
if params.auth.registration.is_some() {
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::RegistrationNotSupported),
)
.await?);
return Ok(callback_destination.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::RegistrationNotSupported),
)?);
}
// Fail early if prompt=none; we never let it go through
if prompt.contains(&Prompt::None) {
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::LoginRequired),
)
.await?);
return Ok(callback_destination.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::LoginRequired),
)?);
}
let code: Option<AuthorizationCode> = if response_type.has_code() {
// Check if it is allowed to use this grant type
if !client.grant_types.contains(&GrantType::AuthorizationCode) {
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::UnauthorizedClient),
)
.await?);
return Ok(callback_destination.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::UnauthorizedClient),
)?);
}
// 32 random alphanumeric characters, about 190bit of entropy
@@ -265,13 +251,11 @@ pub(crate) async fn get(
// If the request had PKCE params but no code asked, it should get back with an
// error
if params.pkce.is_some() {
return Ok(callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::InvalidRequest),
)
.await?);
return Ok(callback_destination.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::InvalidRequest),
)?);
}
None
@@ -336,13 +320,11 @@ pub(crate) async fn get(
Ok(r) => r,
Err(err) => {
tracing::error!(%err);
callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::ServerError),
)
.await?
callback_destination.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::ServerError),
)?
}
};

1
crates/handlers/src/oauth2/mod.rs
View File

@@ -23,7 +23,6 @@ use mas_storage::{Clock, RepositoryAccess};
use thiserror::Error;
pub mod authorization;
pub mod consent;
pub mod device;
pub mod discovery;
pub mod introspection;

19
crates/router/src/endpoints.rs
View File

@@ -60,9 +60,7 @@ impl PostAuthAction {
pub fn go_next(&self, url_builder: &UrlBuilder) -> axum::response::Redirect {
match self {
Self::ContinueAuthorizationGrant { id } => {
url_builder.redirect(&ContinueAuthorizationGrant(*id))
}
Self::ContinueAuthorizationGrant { id } => url_builder.redirect(&Consent(*id)),
Self::ContinueDeviceCodeGrant { id } => {
url_builder.redirect(&DeviceCodeConsent::new(*id))
}
@@ -521,21 +519,6 @@ impl SimpleRoute for AccountPasswordChange {
const PATH: &'static str = "/account/password/change";
}
/// `GET /authorize/{grant_id}`
#[derive(Debug, Clone)]
pub struct ContinueAuthorizationGrant(pub Ulid);
impl Route for ContinueAuthorizationGrant {
type Query = ();
fn route() -> &'static str {
"/authorize/{grant_id}"
}
fn path(&self) -> std::borrow::Cow<'static, str> {
format!("/authorize/{}", self.0).into()
}
}
/// `GET /consent/{grant_id}`
#[derive(Debug, Clone)]
pub struct Consent(pub Ulid);