From 3dfa4a63e6d34ff100a8c05b866b7070fbd1bd2f Mon Sep 17 00:00:00 2001 From: defaultdino Date: Mon, 20 Jul 2026 16:03:45 +0200 Subject: [PATCH] handle inactive browser session in separate branch. new migration for index, edit old. cover all match cases for authorization grant lookup --- crates/handlers/src/oauth2/token.rs | 9 ++++++--- .../20260403000001_authz_grant_user_session_id.sql | 5 ++--- ...2_idx_oauth2_authorization_grants_user_session_id.sql | 9 +++++++++ crates/storage-pg/src/oauth2/authorization_grant.rs | 5 ++++- 4 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 crates/storage-pg/migrations/20260720134722_idx_oauth2_authorization_grants_user_session_id.sql diff --git a/crates/handlers/src/oauth2/token.rs b/crates/handlers/src/oauth2/token.rs index 60d4c8187..3f2a40dbd 100644 --- a/crates/handlers/src/oauth2/token.rs +++ b/crates/handlers/src/oauth2/token.rs @@ -18,8 +18,8 @@ use mas_axum_utils::{ record_error, }; use mas_data_model::{ - AuthorizationGrantStage, BoxClock, BoxRng, BrowserSession, Client, Clock, Device, - DeviceCodeGrantState, SiteConfig, TokenType, + AuthorizationGrantStage, BoxClock, BoxRng, Client, Clock, Device, DeviceCodeGrantState, + SiteConfig, TokenType, }; use mas_i18n::DataLocale; use mas_keystore::{Encrypter, Keystore}; @@ -516,9 +516,12 @@ async fn authorization_code_grant( .browser_session() .lookup(browser_session_id) .await? - .filter(BrowserSession::active) // this treats inactive sessions as 'no such browser session' errors .ok_or(RouteError::NoSuchBrowserSession(browser_session_id))?; + if !browser_session.active() { + return Err(RouteError::InvalidGrant(authz_grant.id)); + } + 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)?; diff --git a/crates/storage-pg/migrations/20260403000001_authz_grant_user_session_id.sql b/crates/storage-pg/migrations/20260403000001_authz_grant_user_session_id.sql index e618010a5..010cfc7af 100644 --- a/crates/storage-pg/migrations/20260403000001_authz_grant_user_session_id.sql +++ b/crates/storage-pg/migrations/20260403000001_authz_grant_user_session_id.sql @@ -6,6 +6,5 @@ ALTER TABLE "oauth2_authorization_grants" ADD COLUMN "user_session_id" UUID CONSTRAINT "oauth2_authorization_grants_user_session_id_fkey" - REFERENCES "user_sessions" ("user_session_id"); - -CREATE INDEX "oauth2_authorization_grants_user_session_id_idx" ON "oauth2_authorization_grants" (user_session_id); + REFERENCES "user_sessions" ("user_session_id") + ON DELETE SET NULL; \ No newline at end of file diff --git a/crates/storage-pg/migrations/20260720134722_idx_oauth2_authorization_grants_user_session_id.sql b/crates/storage-pg/migrations/20260720134722_idx_oauth2_authorization_grants_user_session_id.sql new file mode 100644 index 000000000..75c3a0dc4 --- /dev/null +++ b/crates/storage-pg/migrations/20260720134722_idx_oauth2_authorization_grants_user_session_id.sql @@ -0,0 +1,9 @@ +-- no-transaction + +-- 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. + +CREATE INDEX CONCURRENTLY IF NOT EXISTS + "oauth2_authorization_grants_user_session_id_idx" + ON "oauth2_authorization_grants" (user_session_id); \ No newline at end of file diff --git a/crates/storage-pg/src/oauth2/authorization_grant.rs b/crates/storage-pg/src/oauth2/authorization_grant.rs index 8a9ccfcf9..5cc416def 100644 --- a/crates/storage-pg/src/oauth2/authorization_grant.rs +++ b/crates/storage-pg/src/oauth2/authorization_grant.rs @@ -88,7 +88,7 @@ impl TryFrom for AuthorizationGrant { fulfilled_at, } } - (Some(fulfilled_at), None, None, None, None) => { + (Some(fulfilled_at), None, None, None, Some(_)) => { tracing::warn!( grant.id = %id, "Grant was fulfilled without a user_session_id, it will be treated as cancelled" @@ -97,6 +97,9 @@ impl TryFrom for AuthorizationGrant { cancelled_at: fulfilled_at, } } + (Some(fulfilled_at), None, None, None, None) => AuthorizationGrantStage::Cancelled { + cancelled_at: fulfilled_at, + }, ( Some(fulfilled_at), Some(exchanged_at),