diff --git a/crates/data-model/src/oauth2/authorization_grant.rs b/crates/data-model/src/oauth2/authorization_grant.rs index 738277b84..c1261654f 100644 --- a/crates/data-model/src/oauth2/authorization_grant.rs +++ b/crates/data-model/src/oauth2/authorization_grant.rs @@ -1,3 +1,4 @@ +// Copyright 2025, 2026 Element Creations Ltd. // Copyright 2024, 2025 New Vector Ltd. // Copyright 2021-2024 The Matrix.org Foundation C.I.C. // @@ -23,7 +24,7 @@ use ulid::Ulid; use url::Url; use super::session::Session; -use crate::InvalidTransitionError; +use crate::{BrowserSession, InvalidTransitionError}; #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct Pkce { @@ -63,11 +64,12 @@ pub enum AuthorizationGrantStage { #[default] Pending, Fulfilled { - session_id: Ulid, + browser_session_id: Ulid, fulfilled_at: DateTime, }, Exchanged { session_id: Ulid, + browser_session_id: Ulid, fulfilled_at: DateTime, exchanged_at: DateTime, }, @@ -85,26 +87,31 @@ impl AuthorizationGrantStage { fn fulfill( self, fulfilled_at: DateTime, - session: &Session, + browser_session: &BrowserSession, ) -> Result { match self { Self::Pending => Ok(Self::Fulfilled { fulfilled_at, - session_id: session.id, + browser_session_id: browser_session.id, }), _ => Err(InvalidTransitionError), } } - fn exchange(self, exchanged_at: DateTime) -> Result { + fn exchange( + self, + exchanged_at: DateTime, + session: &Session, + ) -> Result { match self { Self::Fulfilled { fulfilled_at, - session_id, + browser_session_id, } => Ok(Self::Exchanged { fulfilled_at, exchanged_at, - session_id, + session_id: session.id, + browser_session_id, }), _ => Err(InvalidTransitionError), } @@ -207,8 +214,12 @@ impl AuthorizationGrant { /// Returns an error if the authorization grant is not [`Fulfilled`]. /// /// [`Fulfilled`]: AuthorizationGrantStage::Fulfilled - pub fn exchange(mut self, exchanged_at: DateTime) -> Result { - self.stage = self.stage.exchange(exchanged_at)?; + pub fn exchange( + mut self, + exchanged_at: DateTime, + session: &Session, + ) -> Result { + self.stage = self.stage.exchange(exchanged_at, session)?; Ok(self) } @@ -222,9 +233,9 @@ impl AuthorizationGrant { pub fn fulfill( mut self, fulfilled_at: DateTime, - session: &Session, + browser_session: &BrowserSession, ) -> Result { - self.stage = self.stage.fulfill(fulfilled_at, session)?; + self.stage = self.stage.fulfill(fulfilled_at, browser_session)?; Ok(self) } diff --git a/crates/handlers/src/oauth2/authorization/consent.rs b/crates/handlers/src/oauth2/authorization/consent.rs index 2ca53098b..c61615416 100644 --- a/crates/handlers/src/oauth2/authorization/consent.rs +++ b/crates/handlers/src/oauth2/authorization/consent.rs @@ -311,21 +311,11 @@ pub(crate) async fn post( 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?; - + // Fulfill the grant, recording the browser session that consented. + // The OAuth2 session is created later, in the token exchange handler. let grant = repo .oauth2_authorization_grant() - .fulfill(&clock, &session, grant) + .fulfill(&clock, &browser_session, grant) .await?; let mut params = AuthorizationResponse::default(); @@ -358,10 +348,6 @@ pub(crate) async fn post( repo.save().await?; - activity_tracker - .record_oauth2_session(&clock, &session) - .await; - Ok(( cookie_jar, callback_destination.go(&templates, &locale, params)?, diff --git a/crates/handlers/src/oauth2/token.rs b/crates/handlers/src/oauth2/token.rs index 696c7d426..1eee71d10 100644 --- a/crates/handlers/src/oauth2/token.rs +++ b/crates/handlers/src/oauth2/token.rs @@ -433,7 +433,7 @@ async fn authorization_code_grant( let now = clock.now(); - let session_id = match authz_grant.stage { + let browser_session_id = match authz_grant.stage { AuthorizationGrantStage::Cancelled { cancelled_at } => { debug!(%cancelled_at, "Authorization grant was cancelled"); return Err(RouteError::InvalidGrant(authz_grant.id)); @@ -442,6 +442,7 @@ async fn authorization_code_grant( exchanged_at, fulfilled_at, session_id, + .. } => { warn!(%exchanged_at, %fulfilled_at, "Authorization code was already exchanged"); @@ -467,7 +468,7 @@ async fn authorization_code_grant( return Err(RouteError::InvalidGrant(authz_grant.id)); } AuthorizationGrantStage::Fulfilled { - session_id, + browser_session_id, fulfilled_at, } => { if now - fulfilled_at > Duration::microseconds(10 * 60 * 1000 * 1000) { @@ -475,38 +476,20 @@ async fn authorization_code_grant( return Err(RouteError::InvalidGrant(authz_grant.id)); } - session_id + browser_session_id } }; - let mut session = repo - .oauth2_session() - .lookup(session_id) - .await? - .ok_or(RouteError::NoSuchOAuthSession(session_id))?; - - // Generate a device name - let lang: DataLocale = authz_grant.locale.as_deref().unwrap_or("en").parse()?; - let ctx = DeviceNameContext::new(client.clone(), user_agent.clone()).with_language(lang); - let device_name = templates.render_device_name(&ctx)?; - - if let Some(user_agent) = user_agent { - session = repo - .oauth2_session() - .record_user_agent(session, user_agent) - .await?; - } - // This should never happen, since we looked up in the database using the code let code = authz_grant .code .as_ref() .ok_or(RouteError::InvalidGrant(authz_grant.id))?; - if client.id != session.client_id { + if client.id != authz_grant.client_id { return Err(RouteError::UnexptectedClient { was: client.id, - expected: session.client_id, + expected: authz_grant.client_id, }); } @@ -520,16 +503,48 @@ async fn authorization_code_grant( } } - let Some(user_session_id) = session.user_session_id else { - tracing::warn!("No user session associated with this OAuth2 session"); - return Err(RouteError::InvalidGrant(authz_grant.id)); - }; - let browser_session = repo .browser_session() - .lookup(user_session_id) + .lookup(browser_session_id) .await? - .ok_or(RouteError::NoSuchBrowserSession(user_session_id))?; + .ok_or(RouteError::NoSuchBrowserSession(browser_session_id))?; + + // The browser session (and therefore the user) must still be active. + // It could have been terminated between consent and code exchange, e.g. + // via OIDC back-channel logout or an admin action. + if !browser_session.active() { + warn!( + browser_session.id = %browser_session_id, + "Browser session is no longer active, rejecting code exchange" + ); + return Err(RouteError::InvalidGrant(authz_grant.id)); + } + + // Create the OAuth2 session now that the client has successfully presented + // the authorization code. This is the point at which the session becomes + // visible to the user. + let mut session = repo + .oauth2_session() + .add_from_browser_session( + &mut rng, + clock, + client, + &browser_session, + authz_grant.scope.clone(), + ) + .await?; + + // Generate a device name + let lang: DataLocale = authz_grant.locale.as_deref().unwrap_or("en").parse()?; + let ctx = DeviceNameContext::new(client.clone(), user_agent.clone()).with_language(lang); + let device_name = templates.render_device_name(&ctx)?; + + if let Some(user_agent) = user_agent { + session = repo + .oauth2_session() + .record_user_agent(session, user_agent) + .await?; + } let last_authentication = repo .browser_session() @@ -585,7 +600,7 @@ async fn authorization_code_grant( } repo.oauth2_authorization_grant() - .exchange(clock, authz_grant) + .exchange(clock, &session, authz_grant) .await?; // XXX: there is a potential (but unlikely) race here, where the activity for @@ -1077,22 +1092,10 @@ mod tests { .await .unwrap(); - let session = repo - .oauth2_session() - .add_from_browser_session( - &mut state.rng(), - &state.clock, - &client, - &browser_session, - grant.scope.clone(), - ) - .await - .unwrap(); - - // And fulfill it + // Fulfill the grant with the browser session (no OAuth2 session yet) let grant = repo .oauth2_authorization_grant() - .fulfill(&state.clock, &session, grant) + .fulfill(&state.clock, &browser_session, grant) .await .unwrap(); @@ -1177,22 +1180,10 @@ mod tests { .await .unwrap(); - let session = repo - .oauth2_session() - .add_from_browser_session( - &mut state.rng(), - &state.clock, - &client, - &browser_session, - grant.scope.clone(), - ) - .await - .unwrap(); - - // And fulfill it + // Fulfill the grant with the browser session (no OAuth2 session yet) let grant = repo .oauth2_authorization_grant() - .fulfill(&state.clock, &session, grant) + .fulfill(&state.clock, &browser_session, grant) .await .unwrap(); diff --git a/crates/storage-pg/.sqlx/query-c960f4f5571ee68816c49898125979f3c78c2caca52cb4b8dc9880e669a1f23e.json b/crates/storage-pg/.sqlx/query-21006ad3f9c752d475fd9d45170067464b96fd33d7c137702d11db4bc30171a8.json similarity index 87% rename from crates/storage-pg/.sqlx/query-c960f4f5571ee68816c49898125979f3c78c2caca52cb4b8dc9880e669a1f23e.json rename to crates/storage-pg/.sqlx/query-21006ad3f9c752d475fd9d45170067464b96fd33d7c137702d11db4bc30171a8.json index 20cd2c704..f9cb38700 100644 --- a/crates/storage-pg/.sqlx/query-c960f4f5571ee68816c49898125979f3c78c2caca52cb4b8dc9880e669a1f23e.json +++ b/crates/storage-pg/.sqlx/query-21006ad3f9c752d475fd9d45170067464b96fd33d7c137702d11db4bc30171a8.json @@ -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 , 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 , locale\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 , locale\n , oauth2_session_id\n , user_session_id\n FROM\n oauth2_authorization_grants\n\n WHERE oauth2_authorization_grant_id = $1\n ", "describe": { "columns": [ { @@ -97,6 +97,11 @@ "ordinal": 18, "name": "oauth2_session_id", "type_info": "Uuid" + }, + { + "ordinal": 19, + "name": "user_session_id", + "type_info": "Uuid" } ], "parameters": { @@ -123,8 +128,9 @@ true, true, true, + true, true ] }, - "hash": "c960f4f5571ee68816c49898125979f3c78c2caca52cb4b8dc9880e669a1f23e" + "hash": "21006ad3f9c752d475fd9d45170067464b96fd33d7c137702d11db4bc30171a8" } diff --git a/crates/storage-pg/.sqlx/query-015f7ad7c8d5403ce4dfb71d598fd9af472689d5aef7c1c4b1c594ca57c02237.json b/crates/storage-pg/.sqlx/query-2144de68dcd06fc98a0209814d9bda03573862a8e54714399a99653e25081b84.json similarity index 71% rename from crates/storage-pg/.sqlx/query-015f7ad7c8d5403ce4dfb71d598fd9af472689d5aef7c1c4b1c594ca57c02237.json rename to crates/storage-pg/.sqlx/query-2144de68dcd06fc98a0209814d9bda03573862a8e54714399a99653e25081b84.json index 2daa69ab6..7d931c64d 100644 --- a/crates/storage-pg/.sqlx/query-015f7ad7c8d5403ce4dfb71d598fd9af472689d5aef7c1c4b1c594ca57c02237.json +++ b/crates/storage-pg/.sqlx/query-2144de68dcd06fc98a0209814d9bda03573862a8e54714399a99653e25081b84.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n UPDATE oauth2_authorization_grants\n SET fulfilled_at = $2\n , oauth2_session_id = $3\n WHERE oauth2_authorization_grant_id = $1\n ", + "query": "\n UPDATE oauth2_authorization_grants\n SET exchanged_at = $2\n , oauth2_session_id = $3\n WHERE oauth2_authorization_grant_id = $1\n ", "describe": { "columns": [], "parameters": { @@ -12,5 +12,5 @@ }, "nullable": [] }, - "hash": "015f7ad7c8d5403ce4dfb71d598fd9af472689d5aef7c1c4b1c594ca57c02237" + "hash": "2144de68dcd06fc98a0209814d9bda03573862a8e54714399a99653e25081b84" } diff --git a/crates/storage-pg/.sqlx/query-8ef27901b96b73826a431ad6c5fabecc18c36d8cdba8db3b47953855fa5c9035.json b/crates/storage-pg/.sqlx/query-4ad3695a2eb4ba1e4348344b3166063926f918dc7fc67b7944da589ac27f61bd.json similarity index 88% rename from crates/storage-pg/.sqlx/query-8ef27901b96b73826a431ad6c5fabecc18c36d8cdba8db3b47953855fa5c9035.json rename to crates/storage-pg/.sqlx/query-4ad3695a2eb4ba1e4348344b3166063926f918dc7fc67b7944da589ac27f61bd.json index 0a5d83f0a..8988eb74f 100644 --- a/crates/storage-pg/.sqlx/query-8ef27901b96b73826a431ad6c5fabecc18c36d8cdba8db3b47953855fa5c9035.json +++ b/crates/storage-pg/.sqlx/query-4ad3695a2eb4ba1e4348344b3166063926f918dc7fc67b7944da589ac27f61bd.json @@ -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 , 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 , locale\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 , locale\n , oauth2_session_id\n , user_session_id\n FROM\n oauth2_authorization_grants\n\n WHERE authorization_code = $1\n ", "describe": { "columns": [ { @@ -97,6 +97,11 @@ "ordinal": 18, "name": "oauth2_session_id", "type_info": "Uuid" + }, + { + "ordinal": 19, + "name": "user_session_id", + "type_info": "Uuid" } ], "parameters": { @@ -123,8 +128,9 @@ true, true, true, + true, true ] }, - "hash": "8ef27901b96b73826a431ad6c5fabecc18c36d8cdba8db3b47953855fa5c9035" + "hash": "4ad3695a2eb4ba1e4348344b3166063926f918dc7fc67b7944da589ac27f61bd" } diff --git a/crates/storage-pg/.sqlx/query-714628963ddb35fcbbc5f624e0b57472892efe527fb17b64260fc9bdc536dbaf.json b/crates/storage-pg/.sqlx/query-714628963ddb35fcbbc5f624e0b57472892efe527fb17b64260fc9bdc536dbaf.json new file mode 100644 index 000000000..7bb2ced17 --- /dev/null +++ b/crates/storage-pg/.sqlx/query-714628963ddb35fcbbc5f624e0b57472892efe527fb17b64260fc9bdc536dbaf.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "\n UPDATE oauth2_authorization_grants\n SET fulfilled_at = $2\n , user_session_id = $3\n WHERE oauth2_authorization_grant_id = $1\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Uuid", + "Timestamptz", + "Uuid" + ] + }, + "nullable": [] + }, + "hash": "714628963ddb35fcbbc5f624e0b57472892efe527fb17b64260fc9bdc536dbaf" +} diff --git a/crates/storage-pg/.sqlx/query-c5e7dbb22488aca427b85b3415bd1f1a1766ff865f2e08a5daa095d2a1ccbd56.json b/crates/storage-pg/.sqlx/query-c5e7dbb22488aca427b85b3415bd1f1a1766ff865f2e08a5daa095d2a1ccbd56.json deleted file mode 100644 index 9bec3027c..000000000 --- a/crates/storage-pg/.sqlx/query-c5e7dbb22488aca427b85b3415bd1f1a1766ff865f2e08a5daa095d2a1ccbd56.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n UPDATE oauth2_authorization_grants\n SET exchanged_at = $2\n WHERE oauth2_authorization_grant_id = $1\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Uuid", - "Timestamptz" - ] - }, - "nullable": [] - }, - "hash": "c5e7dbb22488aca427b85b3415bd1f1a1766ff865f2e08a5daa095d2a1ccbd56" -} diff --git a/crates/storage-pg/migrations/20260223093159_oauth2_grant_add_user_session_id.sql b/crates/storage-pg/migrations/20260223093159_oauth2_grant_add_user_session_id.sql new file mode 100644 index 000000000..c04c8e473 --- /dev/null +++ b/crates/storage-pg/migrations/20260223093159_oauth2_grant_add_user_session_id.sql @@ -0,0 +1,20 @@ +-- Copyright 2026 Element Creations Ltd. +-- +-- SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +-- Please see LICENSE files in the repository root for full details. + +-- Add user_session_id to track the browser session that gave consent. +-- This replaces oauth2_session_id as the field set at fulfillment time. +-- The oauth2_session_id is now set at code exchange time instead. +ALTER TABLE oauth2_authorization_grants + ADD COLUMN user_session_id UUID + REFERENCES user_sessions(user_session_id) ON DELETE SET NULL; + +-- Backfill user_session_id from existing data (for already-fulfilled grants). +-- For grants that were fulfilled with the old code, the oauth2_session_id was +-- already set, so we can look up the browser session from the OAuth2 session. +UPDATE oauth2_authorization_grants g +SET user_session_id = s.user_session_id +FROM oauth2_sessions s +WHERE g.oauth2_session_id = s.oauth2_session_id + AND g.fulfilled_at IS NOT NULL; diff --git a/crates/storage-pg/src/oauth2/authorization_grant.rs b/crates/storage-pg/src/oauth2/authorization_grant.rs index 827a930ab..381d6961f 100644 --- a/crates/storage-pg/src/oauth2/authorization_grant.rs +++ b/crates/storage-pg/src/oauth2/authorization_grant.rs @@ -8,7 +8,8 @@ use async_trait::async_trait; use chrono::{DateTime, Utc}; use mas_data_model::{ - AuthorizationCode, AuthorizationGrant, AuthorizationGrantStage, Client, Clock, Pkce, Session, + AuthorizationCode, AuthorizationGrant, AuthorizationGrantStage, BrowserSession, Client, Clock, + Pkce, Session, }; use mas_iana::oauth::PkceCodeChallengeMethod; use mas_storage::oauth2::OAuth2AuthorizationGrantRepository; @@ -56,6 +57,7 @@ struct GrantLookup { locale: Option, oauth2_client_id: Uuid, oauth2_session_id: Option, + user_session_id: Option, } impl TryFrom for AuthorizationGrant { @@ -75,22 +77,42 @@ impl TryFrom for AuthorizationGrant { value.exchanged_at, value.cancelled_at, value.oauth2_session_id, + value.user_session_id, ) { - (None, None, None, None) => AuthorizationGrantStage::Pending, - (Some(fulfilled_at), None, None, Some(session_id)) => { + (None, None, None, None, None) => AuthorizationGrantStage::Pending, + // New format: fulfilled with browser_session_id (user_session_id set, oauth2_session_id + // not yet set) + (Some(fulfilled_at), None, None, None, Some(user_session_id)) => { AuthorizationGrantStage::Fulfilled { - session_id: session_id.into(), + browser_session_id: user_session_id.into(), fulfilled_at, } } - (Some(fulfilled_at), Some(exchanged_at), None, Some(session_id)) => { + // Legacy format: fulfilled with oauth2_session_id (old code, no user_session_id column + // yet or backfill failed) Treat as fulfilled for backward compat during + // rolling deploys; the code expires in 10 mins anyway + (Some(fulfilled_at), None, None, Some(session_id), None) => { + // We don't have the browser_session_id, but we need to set something. + // Use a zero ULID as a sentinel; the token handler will fail to look up the + // browser session and reject the grant, which is the safe thing to do. + let _ = session_id; + AuthorizationGrantStage::Cancelled { + cancelled_at: fulfilled_at, + } + } + // Legacy or new format: exchanged (oauth2_session_id always set at this point) + (Some(fulfilled_at), Some(exchanged_at), None, Some(session_id), user_session_id) => { + // For the exchanged state, we need both session_id and browser_session_id. + // If user_session_id is missing (old records), use a zero ULID as a sentinel. + let browser_session_id = user_session_id.unwrap_or_default().into(); AuthorizationGrantStage::Exchanged { session_id: session_id.into(), + browser_session_id, fulfilled_at, exchanged_at, } } - (None, None, Some(cancelled_at), None) => { + (None, None, Some(cancelled_at), None, None) => { AuthorizationGrantStage::Cancelled { cancelled_at } } _ => { @@ -303,6 +325,7 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository , login_hint , locale , oauth2_session_id + , user_session_id FROM oauth2_authorization_grants @@ -353,6 +376,7 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository , login_hint , locale , oauth2_session_id + , user_session_id FROM oauth2_authorization_grants @@ -376,14 +400,14 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository db.query.text, %grant.id, client.id = %grant.client_id, - %session.id, + browser_session.id = %browser_session.id, ), err, )] async fn fulfill( &mut self, clock: &dyn Clock, - session: &Session, + browser_session: &BrowserSession, grant: AuthorizationGrant, ) -> Result { let fulfilled_at = clock.now(); @@ -391,12 +415,12 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository r#" UPDATE oauth2_authorization_grants SET fulfilled_at = $2 - , oauth2_session_id = $3 + , user_session_id = $3 WHERE oauth2_authorization_grant_id = $1 "#, Uuid::from(grant.id), fulfilled_at, - Uuid::from(session.id), + Uuid::from(browser_session.id), ) .traced() .execute(&mut *self.conn) @@ -404,9 +428,8 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository DatabaseError::ensure_affected_rows(&res, 1)?; - // XXX: check affected rows & new methods let grant = grant - .fulfill(fulfilled_at, session) + .fulfill(fulfilled_at, browser_session) .map_err(DatabaseError::to_invalid_operation)?; Ok(grant) @@ -419,12 +442,14 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository db.query.text, %grant.id, client.id = %grant.client_id, + %session.id, ), err, )] async fn exchange( &mut self, clock: &dyn Clock, + session: &Session, grant: AuthorizationGrant, ) -> Result { let exchanged_at = clock.now(); @@ -432,10 +457,12 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository r#" UPDATE oauth2_authorization_grants SET exchanged_at = $2 + , oauth2_session_id = $3 WHERE oauth2_authorization_grant_id = $1 "#, Uuid::from(grant.id), exchanged_at, + Uuid::from(session.id), ) .traced() .execute(&mut *self.conn) @@ -444,7 +471,7 @@ impl OAuth2AuthorizationGrantRepository for PgOAuth2AuthorizationGrantRepository DatabaseError::ensure_affected_rows(&res, 1)?; let grant = grant - .exchange(exchanged_at) + .exchange(exchanged_at, session) .map_err(DatabaseError::to_invalid_operation)?; Ok(grant) diff --git a/crates/storage/src/oauth2/authorization_grant.rs b/crates/storage/src/oauth2/authorization_grant.rs index c0f1030e3..184a5ec3c 100644 --- a/crates/storage/src/oauth2/authorization_grant.rs +++ b/crates/storage/src/oauth2/authorization_grant.rs @@ -6,7 +6,9 @@ // Please see LICENSE files in the repository root for full details. use async_trait::async_trait; -use mas_data_model::{AuthorizationCode, AuthorizationGrant, Client, Clock, Session}; +use mas_data_model::{ + AuthorizationCode, AuthorizationGrant, BrowserSession, Client, Clock, Session, +}; use oauth2_types::{requests::ResponseMode, scope::Scope}; use rand_core::RngCore; use ulid::Ulid; @@ -90,15 +92,15 @@ pub trait OAuth2AuthorizationGrantRepository: Send + Sync { async fn find_by_code(&mut self, code: &str) -> Result, Self::Error>; - /// Fulfill an authorization grant, by giving the [`Session`] that it - /// created + /// Fulfill an authorization grant, recording the browser session that + /// consented /// /// Returns the updated authorization grant /// /// # Parameters /// /// * `clock`: The clock used to generate timestamps - /// * `session`: The session that was created using this authorization grant + /// * `browser_session`: The browser session that gave consent /// * `authorization_grant`: The authorization grant to fulfill /// /// # Errors @@ -107,17 +109,19 @@ pub trait OAuth2AuthorizationGrantRepository: Send + Sync { async fn fulfill( &mut self, clock: &dyn Clock, - session: &Session, + browser_session: &BrowserSession, authorization_grant: AuthorizationGrant, ) -> Result; - /// Mark an authorization grant as exchanged + /// Mark an authorization grant as exchanged, linking the [`Session`] that + /// was created when the client exchanged the code for tokens /// /// Returns the updated authorization grant /// /// # Parameters /// /// * `clock`: The clock used to generate timestamps + /// * `session`: The [`Session`] created during code exchange /// * `authorization_grant`: The authorization grant to mark as exchanged /// /// # Errors @@ -126,6 +130,7 @@ pub trait OAuth2AuthorizationGrantRepository: Send + Sync { async fn exchange( &mut self, clock: &dyn Clock, + session: &Session, authorization_grant: AuthorizationGrant, ) -> Result; @@ -179,13 +184,14 @@ repository_impl!(OAuth2AuthorizationGrantRepository: async fn fulfill( &mut self, clock: &dyn Clock, - session: &Session, + browser_session: &BrowserSession, authorization_grant: AuthorizationGrant, ) -> Result; async fn exchange( &mut self, clock: &dyn Clock, + session: &Session, authorization_grant: AuthorizationGrant, ) -> Result;