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

Always ask for consent, never for reauth

Browse Source 
Now that we have deduplicated clients, we're in this weird situation
where authorization grants just… go through.

This is because 4 years ago, I designed it to support prompt=consent and
prompt=none, but that never ended up being used/mentioned in the MSCs.

We also had support for max_age, but that required reauthing, which
doesn't work well with upstream providers.

So this removes support for prompt=consent|none and max_age, and makes
sure we always go through the consent page.

Lots of code deleted, yay!
This commit is contained in:
Quentin Gliech
2025-04-10 19:57:45 +02:00
parent 8f42fa028b
commit 73a4007c18
19 changed files with 88 additions and 638 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
crates/data-model/src/oauth2/authorization_grant.rs
View File

@@ -4,9 +4,7 @@
// SPDX-License-Identifier: AGPL-3.0-only
// Please see LICENSE in the repository root for full details.
use std::num::NonZeroU32;
use chrono::{DateTime, Duration, Utc};
use chrono::{DateTime, Utc};
use mas_iana::oauth::PkceCodeChallengeMethod;
use oauth2_types::{
pkce::{CodeChallengeError, CodeChallengeMethodExt},
@@ -158,11 +156,9 @@ pub struct AuthorizationGrant {
pub scope: Scope,
pub state: Option<String>,
pub nonce: Option<String>,
pub max_age: Option<NonZeroU32>,
pub response_mode: ResponseMode,
pub response_type_id_token: bool,
pub created_at: DateTime<Utc>,
pub requires_consent: bool,
pub login_hint: Option<String>,
}
@@ -174,18 +170,7 @@ impl std::ops::Deref for AuthorizationGrant {
}
}
const DEFAULT_MAX_AGE: Duration = Duration::microseconds(3600 * 24 * 365 * 1000 * 1000);
impl AuthorizationGrant {
#[must_use]
pub fn max_auth_time(&self) -> DateTime<Utc> {
let max_age = self
.max_age
.and_then(|x| Duration::try_seconds(x.get().into()))
.unwrap_or(DEFAULT_MAX_AGE);
self.created_at - max_age
}
#[must_use]
pub fn parse_login_hint(&self, homeserver: &str) -> LoginHint {
let Some(login_hint) = &self.login_hint else {
@@ -274,11 +259,9 @@ impl AuthorizationGrant {
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,
login_hint: Some(String::from("mxid:@example-user:example.com")),
}
}

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

@@ -11,7 +11,7 @@ use axum::{
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, Device};
use mas_data_model::{AuthorizationGrant, BrowserSession, Client};
use mas_keystore::Keystore;
use mas_policy::{EvaluationResult, Policy};
use mas_router::{PostAuthAction, UrlBuilder};
@@ -149,15 +149,6 @@ pub(crate) async fn get(
let res = callback_destination.go(&templates, &locale, params).await?;
Ok((cookie_jar, res).into_response())
}
Err(GrantCompletionError::RequiresReauth) => Ok((
cookie_jar,
url_builder.redirect(&mas_router::Reauth::and_then(continue_grant)),
)
.into_response()),
Err(GrantCompletionError::RequiresConsent) => {
let next = mas_router::Consent(grant_id);
Ok((cookie_jar, url_builder.redirect(&next)).into_response())
}
Err(GrantCompletionError::PolicyViolation(grant, res)) => {
warn!(violation = ?res, "Authorization grant for client {} denied by policy", client.id);
@@ -184,12 +175,6 @@ pub enum GrantCompletionError {
#[error("authorization grant is not in a pending state")]
NotPending,
#[error("user needs to reauthenticate")]
RequiresReauth,
#[error("client lacks consent")]
RequiresConsent,
#[error("denied by the policy")]
PolicyViolation(AuthorizationGrant, EvaluationResult),
}
@@ -218,18 +203,6 @@ pub(crate) async fn complete(
return Err(GrantCompletionError::NotPending);
}
// Check if the authentication is fresh enough
let authentication = repo
.browser_session()
.get_last_authentication(browser_session)
.await?;
let authentication = authentication.filter(|auth| auth.created_at > grant.max_auth_time());
let Some(valid_authentication) = authentication else {
repo.save().await?;
return Err(GrantCompletionError::RequiresReauth);
};
// Run through the policy
let res = policy
.evaluate_authorization_grant(mas_policy::AuthorizationGrantInput {
@@ -248,23 +221,6 @@ pub(crate) async fn complete(
return Err(GrantCompletionError::PolicyViolation(grant, res));
}
let current_consent = repo
.oauth2_client()
.get_consent_for_user(client, &browser_session.user)
.await?;
let lacks_consent = grant
.scope
.difference(&current_consent)
.filter(|scope| Device::from_scope_token(scope).is_none())
.any(|_| true);
// Check if the client lacks consent *or* if consent was explicitly asked
if lacks_consent || grant.requires_consent {
repo.save().await?;
return Err(GrantCompletionError::RequiresConsent);
}
// All good, let's start the session
let session = repo
.oauth2_session()
@@ -281,6 +237,12 @@ pub(crate) async fn complete(
// 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,
@@ -290,7 +252,7 @@ pub(crate) async fn complete(
Some(&grant),
browser_session,
None,
Some(&valid_authentication),
last_authentication.as_ref(),
)?);
}

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

@@ -6,20 +6,17 @@
use axum::{
extract::{Form, State},
response::{Html, IntoResponse, Response},
response::{IntoResponse, Response},
};
use axum_extra::TypedHeader;
use hyper::StatusCode;
use mas_axum_utils::{SessionInfoExt, cookies::CookieJar, csrf::CsrfExt, sentry::SentryEventID};
use mas_axum_utils::{SessionInfoExt, cookies::CookieJar, sentry::SentryEventID};
use mas_data_model::{AuthorizationCode, Pkce};
use mas_keystore::Keystore;
use mas_policy::Policy;
use mas_router::{PostAuthAction, UrlBuilder};
use mas_storage::{
BoxClock, BoxRepository, BoxRng,
oauth2::{OAuth2AuthorizationGrantRepository, OAuth2ClientRepository},
};
use mas_templates::{PolicyViolationContext, TemplateContext, Templates};
use mas_templates::Templates;
use oauth2_types::{
errors::{ClientError, ClientErrorCode},
pkce,
@@ -29,9 +26,8 @@ use oauth2_types::{
use rand::{Rng, distributions::Alphanumeric};
use serde::Deserialize;
use thiserror::Error;
use tracing::warn;
use self::{callback::CallbackDestination, complete::GrantCompletionError};
use self::callback::CallbackDestination;
use crate::{BoundActivityTracker, PreferredLanguage, impl_from_error_for_route};
mod callback;
@@ -134,10 +130,7 @@ pub(crate) async fn get(
clock: BoxClock,
PreferredLanguage(locale): PreferredLanguage,
State(templates): State<Templates>,
State(key_store): State<Keystore>,
State(url_builder): State<UrlBuilder>,
policy: Policy,
user_agent: Option<TypedHeader<headers::UserAgent>>,
activity_tracker: BoundActivityTracker,
mut repo: BoxRepository,
cookie_jar: CookieJar,
@@ -166,9 +159,6 @@ pub(crate) async fn get(
// Get the session info from the cookie
let (session_info, cookie_jar) = cookie_jar.session_info();
let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng);
let user_agent = user_agent.map(|TypedHeader(ua)| ua.to_string());
// One day, we will have try blocks
let res: Result<Response, RouteError> = ({
@@ -235,8 +225,8 @@ pub(crate) async fn get(
.await?);
}
// Fail early if prompt=none and there is no active session
if prompt.contains(&Prompt::None) && maybe_session.is_none() {
// Fail early if prompt=none; we never let it go through
if prompt.contains(&Prompt::None) {
return Ok(callback_destination
.go(
&templates,
@@ -287,8 +277,6 @@ pub(crate) async fn get(
None
};
let requires_consent = prompt.contains(&Prompt::Consent);
let grant = repo
.oauth2_authorization_grant()
.add(
@@ -300,151 +288,43 @@ pub(crate) async fn get(
code,
params.auth.state.clone(),
params.auth.nonce,
params.auth.max_age,
response_mode,
response_type.has_id_token(),
requires_consent,
params.auth.login_hint,
)
.await?;
let continue_grant = PostAuthAction::continue_grant(grant.id);
let res = match maybe_session {
// Cases where there is no active session, redirect to the relevant page
None if prompt.contains(&Prompt::None) => {
// This case should already be handled earlier
unreachable!();
}
None if prompt.contains(&Prompt::Create) => {
// Client asked for a registration, show the registration prompt
repo.save().await?;
url_builder.redirect(&mas_router::Register::and_then(continue_grant))
url_builder
.redirect(&mas_router::Register::and_then(continue_grant))
.into_response()
}
None => {
// Other cases where we don't have a session, ask for a login
repo.save().await?;
url_builder.redirect(&mas_router::Login::and_then(continue_grant))
url_builder
.redirect(&mas_router::Login::and_then(continue_grant))
.into_response()
}
// Special case when we already have a session but prompt=login|select_account
Some(session)
if prompt.contains(&Prompt::Login)
|| prompt.contains(&Prompt::SelectAccount) =>
{
// TODO: better pages here
Some(user_session) => {
// TODO: better support for prompt=create when we have a session
repo.save().await?;
activity_tracker.record_browser_session(&clock, &session).await;
url_builder.redirect(&mas_router::Reauth::and_then(continue_grant))
activity_tracker
.record_browser_session(&clock, &user_session)
.await;
url_builder
.redirect(&mas_router::Consent(grant.id))
.into_response()
}
// Else, we immediately try to complete the authorization grant
Some(user_session) if prompt.contains(&Prompt::None) => {
activity_tracker.record_browser_session(&clock, &user_session).await;
// With prompt=none, we should get back to the client immediately
match self::complete::complete(
&mut rng,
&clock,
&activity_tracker,
user_agent,
repo,
key_store,
policy,
&url_builder,
grant,
&client,
&user_session,
)
.await
{
Ok(params) => callback_destination.go(&templates, &locale, params).await?,
Err(GrantCompletionError::RequiresConsent) => {
callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::ConsentRequired),
)
.await?
}
Err(GrantCompletionError::RequiresReauth) => {
callback_destination
.go(
&templates,
&locale,
ClientError::from(ClientErrorCode::InteractionRequired),
)
.await?
}
Err(GrantCompletionError::PolicyViolation(_grant, _res)) => {
callback_destination
.go(&templates, &locale, ClientError::from(ClientErrorCode::AccessDenied))
.await?
}
Err(GrantCompletionError::Internal(e)) => {
return Err(RouteError::Internal(e))
}
Err(e @ GrantCompletionError::NotPending) => {
// This should never happen
return Err(RouteError::Internal(Box::new(e)));
}
}
}
Some(user_session) => {
activity_tracker.record_browser_session(&clock, &user_session).await;
let grant_id = grant.id;
// Else, we show the relevant reauth/consent page if necessary
match self::complete::complete(
&mut rng,
&clock,
&activity_tracker,
user_agent,
repo,
key_store,
policy,
&url_builder,
grant,
&client,
&user_session,
)
.await
{
Ok(params) => callback_destination.go(&templates, &locale, params).await?,
Err(GrantCompletionError::RequiresConsent) => {
url_builder.redirect(&mas_router::Consent(grant_id)).into_response()
}
Err(GrantCompletionError::PolicyViolation(grant, res)) => {
warn!(violation = ?res, "Authorization grant for client {} denied by policy", client.id);
let ctx = PolicyViolationContext::for_authorization_grant(grant, client)
.with_session(user_session)
.with_csrf(csrf_token.form_value())
.with_language(locale);
let content = templates.render_policy_violation(&ctx)?;
Html(content).into_response()
}
Err(GrantCompletionError::RequiresReauth) => {
url_builder.redirect(&mas_router::Reauth::and_then(continue_grant))
.into_response()
}
Err(GrantCompletionError::Internal(e)) => {
return Err(RouteError::Internal(e))
}
Err(e @ GrantCompletionError::NotPending) => {
// This should never happen
return Err(RouteError::Internal(Box::new(e)));
}
}
}
};
Ok(res)

24
crates/handlers/src/oauth2/consent.rs
View File

@@ -15,7 +15,7 @@ use mas_axum_utils::{
csrf::{CsrfExt, ProtectedForm},
sentry::SentryEventID,
};
use mas_data_model::{AuthorizationGrantStage, Device};
use mas_data_model::AuthorizationGrantStage;
use mas_policy::Policy;
use mas_router::{PostAuthAction, UrlBuilder};
use mas_storage::{
@@ -240,28 +240,6 @@ pub(crate) async fn post(
return Err(RouteError::PolicyViolation);
}
// Do not consent for the "urn:matrix:org.matrix.msc2967.client:device:*" scope
let scope_without_device = grant
.scope
.iter()
.filter(|s| Device::from_scope_token(s).is_none())
.cloned()
.collect();
repo.oauth2_client()
.give_consent_for_user(
&mut rng,
&clock,
&client,
&session.user,
&scope_without_device,
)
.await?;
repo.oauth2_authorization_grant()
.give_consent(grant)
.await?;
repo.save().await?;
Ok((cookie_jar, next.go_next(&url_builder)).into_response())

2
crates/handlers/src/oauth2/discovery.rs
View File

@@ -132,7 +132,7 @@ pub(crate) async fn get(
let request_uri_parameter_supported = Some(false);
let prompt_values_supported = Some({
let mut v = vec![Prompt::None, Prompt::Login];
let mut v = vec![Prompt::Login];
// Advertise for prompt=create if password registration is enabled
// TODO: we may want to be able to forward that to upstream providers if they
// support it

4
crates/handlers/src/oauth2/token.rs
View File

@@ -978,10 +978,8 @@ mod tests {
}),
Some("state".to_owned()),
Some("nonce".to_owned()),
None,
ResponseMode::Query,
false,
false,
None,
)
.await
@@ -1079,10 +1077,8 @@ mod tests {
}),
Some("state".to_owned()),
Some("nonce".to_owned()),
None,
ResponseMode::Query,
false,
false,
None,
)
.await

29
crates/storage-pg/.sqlx/query-854cc8cd3c1fc3dbbdf4ce81b561aafadb0f4e98caeaba01597c6f62875ae691.json generated
View File

@@ -1,29 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "\n INSERT INTO oauth2_authorization_grants (\n oauth2_authorization_grant_id,\n oauth2_client_id,\n redirect_uri,\n scope,\n state,\n nonce,\n max_age,\n response_mode,\n code_challenge,\n code_challenge_method,\n response_type_code,\n response_type_id_token,\n authorization_code,\n requires_consent,\n login_hint,\n created_at\n )\n VALUES\n ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16)\n ",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Uuid",
"Uuid",
"Text",
"Text",
"Text",
"Text",
"Int4",
"Text",
"Text",
"Text",
"Bool",
"Bool",
"Text",
"Bool",
"Text",
"Timestamptz"
]
},
"nullable": []
},
"hash": "854cc8cd3c1fc3dbbdf4ce81b561aafadb0f4e98caeaba01597c6f62875ae691"
}

30
crates/storage-pg/.sqlx/query-1d9c478c7a5e3a672610376a290b9a1afaaa6fa2fb137f7307002f058b206dbd.json → crates/storage-pg/.sqlx/query-890516adeeaf2d6a4fe83a69e42e18b8817d1bb511e08ee761a031fa12d27251.json generated
View File

@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "\n SELECT oauth2_authorization_grant_id\n , created_at\n , cancelled_at\n , fulfilled_at\n , exchanged_at\n , scope\n , state\n , redirect_uri\n , response_mode\n , nonce\n , max_age\n , oauth2_client_id\n , authorization_code\n , response_type_code\n , response_type_id_token\n , code_challenge\n , code_challenge_method\n , requires_consent\n , login_hint\n , oauth2_session_id\n FROM\n oauth2_authorization_grants\n\n WHERE authorization_code = $1\n ",
"query": "\n SELECT oauth2_authorization_grant_id\n , created_at\n , cancelled_at\n , fulfilled_at\n , exchanged_at\n , scope\n , state\n , redirect_uri\n , response_mode\n , nonce\n , oauth2_client_id\n , authorization_code\n , response_type_code\n , response_type_id_token\n , code_challenge\n , code_challenge_method\n , login_hint\n , oauth2_session_id\n FROM\n oauth2_authorization_grants\n\n WHERE authorization_code = $1\n ",
"describe": {
"columns": [
{
@@ -55,51 +55,41 @@
},
{
"ordinal": 10,
"name": "max_age",
"type_info": "Int4"
},
{
"ordinal": 11,
"name": "oauth2_client_id",
"type_info": "Uuid"
},
{
"ordinal": 12,
"ordinal": 11,
"name": "authorization_code",
"type_info": "Text"
},
{
"ordinal": 13,
"ordinal": 12,
"name": "response_type_code",
"type_info": "Bool"
},
{
"ordinal": 14,
"ordinal": 13,
"name": "response_type_id_token",
"type_info": "Bool"
},
{
"ordinal": 15,
"ordinal": 14,
"name": "code_challenge",
"type_info": "Text"
},
{
"ordinal": 16,
"ordinal": 15,
"name": "code_challenge_method",
"type_info": "Text"
},
{
"ordinal": 17,
"name": "requires_consent",
"type_info": "Bool"
},
{
"ordinal": 18,
"ordinal": 16,
"name": "login_hint",
"type_info": "Text"
},
{
"ordinal": 19,
"ordinal": 17,
"name": "oauth2_session_id",
"type_info": "Uuid"
}
@@ -120,17 +110,15 @@
false,
false,
true,
true,
false,
true,
false,
false,
true,
true,
false,
true,
true
]
},
"hash": "1d9c478c7a5e3a672610376a290b9a1afaaa6fa2fb137f7307002f058b206dbd"
"hash": "890516adeeaf2d6a4fe83a69e42e18b8817d1bb511e08ee761a031fa12d27251"
}

23
crates/storage-pg/.sqlx/query-8b7297c263336d70c2b647212b16f7ae39bc5cb1572e3a2dcfcd67f196a1fa39.json generated
View File

@@ -1,23 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "\n SELECT scope_token\n FROM oauth2_consents\n WHERE user_id = $1 AND oauth2_client_id = $2\n ",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "scope_token",
"type_info": "Text"
}
],
"parameters": {
"Left": [
"Uuid",
"Uuid"
]
},
"nullable": [
false
]
},
"hash": "8b7297c263336d70c2b647212b16f7ae39bc5cb1572e3a2dcfcd67f196a1fa39"
}

27
crates/storage-pg/.sqlx/query-96208afd8b81e00f2700b0ded40c1eb529d321d0aa543be70f86ef96d0d8ff28.json generated Normal file
View File

@@ -0,0 +1,27 @@
{
"db_name": "PostgreSQL",
"query": "\n INSERT INTO oauth2_authorization_grants (\n oauth2_authorization_grant_id,\n oauth2_client_id,\n redirect_uri,\n scope,\n state,\n nonce,\n response_mode,\n code_challenge,\n code_challenge_method,\n response_type_code,\n response_type_id_token,\n authorization_code,\n login_hint,\n created_at\n )\n VALUES\n ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)\n ",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Uuid",
"Uuid",
"Text",
"Text",
"Text",
"Text",
"Text",
"Text",
"Text",
"Bool",
"Bool",
"Text",
"Text",
"Timestamptz"
]
},
"nullable": []
},
"hash": "96208afd8b81e00f2700b0ded40c1eb529d321d0aa543be70f86ef96d0d8ff28"
}

18
crates/storage-pg/.sqlx/query-9a6c197ff4ad80217262d48f8792ce7e16bc5df0677c7cd4ecb4fdbc5ee86395.json generated
View File

@@ -1,18 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "\n INSERT INTO oauth2_consents\n (oauth2_consent_id, user_id, oauth2_client_id, scope_token, created_at)\n SELECT id, $2, $3, scope_token, $5 FROM UNNEST($1::uuid[], $4::text[]) u(id, scope_token)\n ON CONFLICT (user_id, oauth2_client_id, scope_token) DO UPDATE SET refreshed_at = $5\n ",
"describe": {
"columns": [],
"parameters": {
"Left": [
"UuidArray",
"Uuid",
"Uuid",
"TextArray",
"Timestamptz"
]
},
"nullable": []
},
"hash": "9a6c197ff4ad80217262d48f8792ce7e16bc5df0677c7cd4ecb4fdbc5ee86395"
}

30
crates/storage-pg/.sqlx/query-e0d3be7e741581430e3e4719c7e19596837234c94a398570bdac42652c2c4652.json → crates/storage-pg/.sqlx/query-bf6d1e3e3145438c988b1a47fc13f0168a63e278d8f8c947cb3ab65173f92ae4.json generated
View File

@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "\n SELECT oauth2_authorization_grant_id\n , created_at\n , cancelled_at\n , fulfilled_at\n , exchanged_at\n , scope\n , state\n , redirect_uri\n , response_mode\n , nonce\n , max_age\n , oauth2_client_id\n , authorization_code\n , response_type_code\n , response_type_id_token\n , code_challenge\n , code_challenge_method\n , requires_consent\n , login_hint\n , oauth2_session_id\n FROM\n oauth2_authorization_grants\n\n WHERE oauth2_authorization_grant_id = $1\n ",
"query": "\n SELECT oauth2_authorization_grant_id\n , created_at\n , cancelled_at\n , fulfilled_at\n , exchanged_at\n , scope\n , state\n , redirect_uri\n , response_mode\n , nonce\n , oauth2_client_id\n , authorization_code\n , response_type_code\n , response_type_id_token\n , code_challenge\n , code_challenge_method\n , login_hint\n , oauth2_session_id\n FROM\n oauth2_authorization_grants\n\n WHERE oauth2_authorization_grant_id = $1\n ",
"describe": {
"columns": [
{
@@ -55,51 +55,41 @@
},
{
"ordinal": 10,
"name": "max_age",
"type_info": "Int4"
},
{
"ordinal": 11,
"name": "oauth2_client_id",
"type_info": "Uuid"
},
{
"ordinal": 12,
"ordinal": 11,
"name": "authorization_code",
"type_info": "Text"
},
{
"ordinal": 13,
"ordinal": 12,
"name": "response_type_code",
"type_info": "Bool"
},
{
"ordinal": 14,
"ordinal": 13,
"name": "response_type_id_token",
"type_info": "Bool"
},
{
"ordinal": 15,
"ordinal": 14,
"name": "code_challenge",
"type_info": "Text"
},
{
"ordinal": 16,
"ordinal": 15,
"name": "code_challenge_method",
"type_info": "Text"
},
{
"ordinal": 17,
"name": "requires_consent",
"type_info": "Bool"
},
{
"ordinal": 18,
"ordinal": 16,
"name": "login_hint",
"type_info": "Text"
},
{
"ordinal": 19,
"ordinal": 17,
"name": "oauth2_session_id",
"type_info": "Uuid"
}
@@ -120,17 +110,15 @@
false,
false,
true,
true,
false,
true,
false,
false,
true,
true,
false,
true,
true
]
},
"hash": "e0d3be7e741581430e3e4719c7e19596837234c94a398570bdac42652c2c4652"
"hash": "bf6d1e3e3145438c988b1a47fc13f0168a63e278d8f8c947cb3ab65173f92ae4"
}

14
crates/storage-pg/.sqlx/query-d83421d4a16f4ad084dd0db5abb56d3688851c36a48a50aa6104e8291e73630d.json generated
View File

@@ -1,14 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "\n UPDATE oauth2_authorization_grants AS og\n SET\n requires_consent = 'f'\n WHERE\n og.oauth2_authorization_grant_id = $1\n ",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Uuid"
]
},
"nullable": []
},
"hash": "d83421d4a16f4ad084dd0db5abb56d3688851c36a48a50aa6104e8291e73630d"
}

9
crates/storage-pg/migrations/20250410174306_oauth2_authorization_default_requires_consent.sql Normal file
View File

@@ -0,0 +1,9 @@
-- Copyright 2025 New Vector Ltd.
--
-- SPDX-License-Identifier: AGPL-3.0-only
-- Please see LICENSE in the repository root for full details.
-- We stopped reading/writing to this column, but it's not nullable.
-- So we need to add a default value, and drop it in the next release
ALTER TABLE oauth2_authorization_grants
ALTER COLUMN requires_consent SET DEFAULT false;

74
crates/storage-pg/src/oauth2/authorization_grant.rs
View File

@@ -4,8 +4,6 @@
// SPDX-License-Identifier: AGPL-3.0-only
// Please see LICENSE in the repository root for full details.
use std::num::NonZeroU32;
use async_trait::async_trait;
use chrono::{DateTime, Utc};
use mas_data_model::{
@@ -48,13 +46,11 @@ struct GrantLookup {
nonce: Option<String>,
redirect_uri: String,
response_mode: String,
max_age: Option<i32>,
response_type_code: bool,
response_type_id_token: bool,
authorization_code: Option<String>,
code_challenge: Option<String>,
code_challenge_method: Option<String>,
requires_consent: bool,
login_hint: Option<String>,
oauth2_client_id: Uuid,
oauth2_session_id: Option<Uuid>,
@@ -153,25 +149,6 @@ impl TryFrom<GrantLookup> for AuthorizationGrant {
.source(e)
})?;
let max_age = value
.max_age
.map(u32::try_from)
.transpose()
.map_err(|e| {
DatabaseInconsistencyError::on("oauth2_authorization_grants")
.column("max_age")
.row(id)
.source(e)
})?
.map(NonZeroU32::try_from)
.transpose()
.map_err(|e| {
DatabaseInconsistencyError::on("oauth2_authorization_grants")
.column("max_age")
.row(id)
.source(e)
})?;
Ok(AuthorizationGrant {
id,
stage,
@@ -180,12 +157,10 @@ impl TryFrom<GrantLookup> for AuthorizationGrant {
scope,
state: value.state,
nonce: value.nonce,
max_age,
response_mode,
redirect_uri,
created_at: value.created_at,
response_type_id_token: value.response_type_id_token,
requires_consent: value.requires_consent,
login_hint: value.login_hint,
})
}
@@ -216,10 +191,8 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository
code: Option<AuthorizationCode>,
state: Option<String>,
nonce: Option<String>,
max_age: Option<NonZeroU32>,
response_mode: ResponseMode,
response_type_id_token: bool,
requires_consent: bool,
login_hint: Option<String>,
) -> Result<AuthorizationGrant, Self::Error> {
let code_challenge = code
@@ -230,8 +203,6 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository
.as_ref()
.and_then(|c| c.pkce.as_ref())
.map(|p| p.challenge_method.to_string());
// TODO: this conversion is a bit ugly
let max_age_i32 = max_age.map(|x| i32::try_from(u32::from(x)).unwrap_or(i32::MAX));
let code_str = code.as_ref().map(|c| &c.code);
let created_at = clock.now();
@@ -247,19 +218,17 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository
scope,
state,
nonce,
max_age,
response_mode,
code_challenge,
code_challenge_method,
response_type_code,
response_type_id_token,
authorization_code,
requires_consent,
login_hint,
created_at
)
VALUES
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16)
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)
"#,
Uuid::from(id),
Uuid::from(client.id),
@@ -267,14 +236,12 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository
scope.to_string(),
state,
nonce,
max_age_i32,
response_mode.to_string(),
code_challenge,
code_challenge_method,
code.is_some(),
response_type_id_token,
code_str,
requires_consent,
login_hint,
created_at,
)
@@ -291,11 +258,9 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository
scope,
state,
nonce,
max_age,
response_mode,
created_at,
response_type_id_token,
requires_consent,
login_hint,
})
}
@@ -323,14 +288,12 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository
, redirect_uri
, response_mode
, nonce
, max_age
, oauth2_client_id
, authorization_code
, response_type_code
, response_type_id_token
, code_challenge
, code_challenge_method
, requires_consent
, login_hint
, oauth2_session_id
FROM
@@ -374,14 +337,12 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository
, redirect_uri
, response_mode
, nonce
, max_age
, oauth2_client_id
, authorization_code
, response_type_code
, response_type_id_token
, code_challenge
, code_challenge_method
, requires_consent
, login_hint
, oauth2_session_id
FROM
@@ -480,37 +441,4 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository
Ok(grant)
}
#[tracing::instrument(
name = "db.oauth2_authorization_grant.give_consent",
skip_all,
fields(
db.query.text,
%grant.id,
client.id = %grant.client_id,
),
err,
)]
async fn give_consent(
&mut self,
mut grant: AuthorizationGrant,
) -> Result<AuthorizationGrant, Self::Error> {
sqlx::query!(
r#"
UPDATE oauth2_authorization_grants AS og
SET
requires_consent = 'f'
WHERE
og.oauth2_authorization_grant_id = $1
"#,
Uuid::from(grant.id),
)
.traced()
.execute(&mut *self.conn)
.await?;
grant.requires_consent = false;
Ok(grant)
}
}

100
crates/storage-pg/src/oauth2/client.rs
View File

@@ -6,20 +6,15 @@
use std::{
collections::{BTreeMap, BTreeSet},
str::FromStr,
string::ToString,
};
use async_trait::async_trait;
use mas_data_model::{Client, JwksOrJwksUri, User};
use mas_data_model::{Client, JwksOrJwksUri};
use mas_iana::{jose::JsonWebSignatureAlg, oauth::OAuthClientAuthenticationMethod};
use mas_jose::jwk::PublicJsonWebKeySet;
use mas_storage::{Clock, oauth2::OAuth2ClientRepository};
use oauth2_types::{
oidc::ApplicationType,
requests::GrantType,
scope::{Scope, ScopeToken},
};
use oauth2_types::{oidc::ApplicationType, requests::GrantType};
use opentelemetry_semantic_conventions::attribute::DB_QUERY_TEXT;
use rand::RngCore;
use sqlx::PgConnection;
@@ -698,97 +693,6 @@ impl OAuth2ClientRepository for PgOAuth2ClientRepository<'_> {
.collect()
}
#[tracing::instrument(
name = "db.oauth2_client.get_consent_for_user",
skip_all,
fields(
db.query.text,
%user.id,
%client.id,
),
err,
)]
async fn get_consent_for_user(
&mut self,
client: &Client,
user: &User,
) -> Result<Scope, Self::Error> {
let scope_tokens: Vec<String> = sqlx::query_scalar!(
r#"
SELECT scope_token
FROM oauth2_consents
WHERE user_id = $1 AND oauth2_client_id = $2
"#,
Uuid::from(user.id),
Uuid::from(client.id),
)
.fetch_all(&mut *self.conn)
.await?;
let scope: Result<Scope, _> = scope_tokens
.into_iter()
.map(|s| ScopeToken::from_str(&s))
.collect();
let scope = scope.map_err(|e| {
DatabaseInconsistencyError::on("oauth2_consents")
.column("scope_token")
.source(e)
})?;
Ok(scope)
}
#[tracing::instrument(
name = "db.oauth2_client.give_consent_for_user",
skip_all,
fields(
db.query.text,
%user.id,
%client.id,
%scope,
),
err,
)]
async fn give_consent_for_user(
&mut self,
rng: &mut (dyn RngCore + Send),
clock: &dyn Clock,
client: &Client,
user: &User,
scope: &Scope,
) -> Result<(), Self::Error> {
let now = clock.now();
let (tokens, ids): (Vec<String>, Vec<Uuid>) = scope
.iter()
.map(|token| {
(
token.to_string(),
Uuid::from(Ulid::from_datetime_with_source(now.into(), rng)),
)
})
.unzip();
sqlx::query!(
r#"
INSERT INTO oauth2_consents
(oauth2_consent_id, user_id, oauth2_client_id, scope_token, created_at)
SELECT id, $2, $3, scope_token, $5 FROM UNNEST($1::uuid[], $4::text[]) u(id, scope_token)
ON CONFLICT (user_id, oauth2_client_id, scope_token) DO UPDATE SET refreshed_at = $5
"#,
&ids,
Uuid::from(user.id),
Uuid::from(client.id),
&tokens,
now,
)
.traced()
.execute(&mut *self.conn)
.await?;
Ok(())
}
#[tracing::instrument(
name = "db.oauth2_client.delete_by_id",
skip_all,

25
crates/storage-pg/src/oauth2/mod.rs
View File

@@ -135,10 +135,8 @@ mod tests {
}),
Some("state".to_owned()),
Some("nonce".to_owned()),
None,
ResponseMode::Query,
true,
false,
None,
)
.await
@@ -175,29 +173,6 @@ mod tests {
.await
.unwrap();
// Lookup the consent the user gave to the client
let consent = repo
.oauth2_client()
.get_consent_for_user(&client, &user)
.await
.unwrap();
assert!(consent.is_empty());
// Give consent to the client
let scope = Scope::from_iter([OPENID]);
repo.oauth2_client()
.give_consent_for_user(&mut rng, &clock, &client, &user, &scope)
.await
.unwrap();
// Lookup the consent the user gave to the client
let consent = repo
.oauth2_client()
.get_consent_for_user(&client, &user)
.await
.unwrap();
assert_eq!(scope, consent);
// Lookup a non-existing session
let session = repo.oauth2_session().lookup(Ulid::nil()).await.unwrap();
assert_eq!(session, None);

30
crates/storage/src/oauth2/authorization_grant.rs
View File

@@ -4,8 +4,6 @@
// SPDX-License-Identifier: AGPL-3.0-only
// Please see LICENSE in the repository root for full details.
use std::num::NonZeroU32;
use async_trait::async_trait;
use mas_data_model::{AuthorizationCode, AuthorizationGrant, Client, Session};
use oauth2_types::{requests::ResponseMode, scope::Scope};
@@ -37,12 +35,9 @@ pub trait OAuth2AuthorizationGrantRepository: Send + Sync {
/// `response_type` was requested
/// * `state`: The state the client sent, if set
/// * `nonce`: The nonce the client sent, if set
/// * `max_age`: The maximum age since the user last authenticated, if asked
/// by the client
/// * `response_mode`: The response mode the client requested
/// * `response_type_id_token`: Whether the `id_token` `response_type` was
/// requested
/// * `requires_consent`: Whether the client explicitly requested consent
/// * `login_hint`: The login_hint the client sent, if set
///
/// # Errors
@@ -59,10 +54,8 @@ pub trait OAuth2AuthorizationGrantRepository: Send + Sync {
code: Option<AuthorizationCode>,
state: Option<String>,
nonce: Option<String>,
max_age: Option<NonZeroU32>,
response_mode: ResponseMode,
response_type_id_token: bool,
requires_consent: bool,
login_hint: Option<String>,
) -> Result<AuthorizationGrant, Self::Error>;
@@ -131,22 +124,6 @@ pub trait OAuth2AuthorizationGrantRepository: Send + Sync {
clock: &dyn Clock,
authorization_grant: AuthorizationGrant,
) -> Result<AuthorizationGrant, Self::Error>;
/// Unset the `requires_consent` flag on an authorization grant
///
/// Returns the updated authorization grant
///
/// # Parameters
///
/// * `authorization_grant`: The authorization grant to update
///
/// # Errors
///
/// Returns [`Self::Error`] if the underlying repository fails
async fn give_consent(
&mut self,
authorization_grant: AuthorizationGrant,
) -> Result<AuthorizationGrant, Self::Error>;
}
repository_impl!(OAuth2AuthorizationGrantRepository:
@@ -160,10 +137,8 @@ repository_impl!(OAuth2AuthorizationGrantRepository:
code: Option<AuthorizationCode>,
state: Option<String>,
nonce: Option<String>,
max_age: Option<NonZeroU32>,
response_mode: ResponseMode,
response_type_id_token: bool,
requires_consent: bool,
login_hint: Option<String>,
) -> Result<AuthorizationGrant, Self::Error>;
@@ -184,9 +159,4 @@ repository_impl!(OAuth2AuthorizationGrantRepository:
clock: &dyn Clock,
authorization_grant: AuthorizationGrant,
) -> Result<AuthorizationGrant, Self::Error>;
async fn give_consent(
&mut self,
authorization_grant: AuthorizationGrant,
) -> Result<AuthorizationGrant, Self::Error>;
);

58
crates/storage/src/oauth2/client.rs
View File

@@ -7,10 +7,10 @@
use std::collections::{BTreeMap, BTreeSet};
use async_trait::async_trait;
use mas_data_model::{Client, User};
use mas_data_model::Client;
use mas_iana::{jose::JsonWebSignatureAlg, oauth::OAuthClientAuthenticationMethod};
use mas_jose::jwk::PublicJsonWebKeySet;
use oauth2_types::{oidc::ApplicationType, requests::GrantType, scope::Scope};
use oauth2_types::{oidc::ApplicationType, requests::GrantType};
use rand_core::RngCore;
use ulid::Ulid;
use url::Url;
@@ -171,45 +171,6 @@ pub trait OAuth2ClientRepository: Send + Sync {
/// Returns [`Self::Error`] if the underlying repository fails
async fn all_static(&mut self) -> Result<Vec<Client>, Self::Error>;
/// Get the list of scopes that the user has given consent for the given
/// client
///
/// # Parameters
///
/// * `client`: The client to get the consent for
/// * `user`: The user to get the consent for
///
/// # Errors
///
/// Returns [`Self::Error`] if the underlying repository fails
async fn get_consent_for_user(
&mut self,
client: &Client,
user: &User,
) -> Result<Scope, Self::Error>;
/// Give consent for a set of scopes for the given client and user
///
/// # Parameters
///
/// * `rng`: The random number generator to use
/// * `clock`: The clock used to generate timestamps
/// * `client`: The client to give the consent for
/// * `user`: The user to give the consent for
/// * `scope`: The scope to give consent for
///
/// # Errors
///
/// Returns [`Self::Error`] if the underlying repository fails
async fn give_consent_for_user(
&mut self,
rng: &mut (dyn RngCore + Send),
clock: &dyn Clock,
client: &Client,
user: &User,
scope: &Scope,
) -> Result<(), Self::Error>;
/// Delete a client
///
/// # Parameters
@@ -288,19 +249,4 @@ repository_impl!(OAuth2ClientRepository:
async fn delete(&mut self, client: Client) -> Result<(), Self::Error>;
async fn delete_by_id(&mut self, id: Ulid) -> Result<(), Self::Error>;
async fn get_consent_for_user(
&mut self,
client: &Client,
user: &User,
) -> Result<Scope, Self::Error>;
async fn give_consent_for_user(
&mut self,
rng: &mut (dyn RngCore + Send),
clock: &dyn Clock,
client: &Client,
user: &User,
scope: &Scope,
) -> Result<(), Self::Error>;
);