From cef1369fccdc60ee2cdd15b27f7e350fecbffd79 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Wed, 15 Jan 2025 18:32:22 +0000 Subject: [PATCH 1/5] Support compatibility sessions that do not have devices This is needed for full support of existing Synapse access tokens --- crates/cli/src/commands/manage.rs | 2 +- crates/data-model/src/compat/session.rs | 2 +- crates/handlers/src/compat/login.rs | 8 +++--- .../src/graphql/model/compat_sessions.rs | 5 ++-- crates/handlers/src/oauth2/introspection.rs | 20 +++++++------- ...f6b4d308302bb1fa3accb12932d1e5ce457e9.json | 2 +- ...35939_allow_deviceless_compat_sessions.sql | 7 +++++ crates/storage-pg/src/app_session.rs | 17 +++++++----- crates/storage-pg/src/compat/mod.rs | 6 ++--- crates/storage-pg/src/compat/session.rs | 26 +++++++++++-------- crates/storage-pg/src/compat/sso_login.rs | 2 +- crates/tasks/src/matrix.rs | 6 ++++- 12 files changed, 62 insertions(+), 41 deletions(-) create mode 100644 crates/storage-pg/migrations/20250114135939_allow_deviceless_compat_sessions.sql diff --git a/crates/cli/src/commands/manage.rs b/crates/cli/src/commands/manage.rs index da5a7f98e..4dd03e5e8 100644 --- a/crates/cli/src/commands/manage.rs +++ b/crates/cli/src/commands/manage.rs @@ -341,7 +341,7 @@ impl Options { info!( %compat_access_token.id, %compat_session.id, - %compat_session.device, + compat_session.device = compat_session.device.map(tracing::field::display), %user.id, %user.username, "Compatibility token issued: {}", compat_access_token.token diff --git a/crates/data-model/src/compat/session.rs b/crates/data-model/src/compat/session.rs index f3d311a5a..e07c0fb7d 100644 --- a/crates/data-model/src/compat/session.rs +++ b/crates/data-model/src/compat/session.rs @@ -71,7 +71,7 @@ pub struct CompatSession { pub id: Ulid, pub state: CompatSessionState, pub user_id: Ulid, - pub device: Device, + pub device: Option, pub user_session_id: Option, pub created_at: DateTime, pub is_synapse_admin: bool, diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index 4c92f6226..ff74bf8ef 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -130,7 +130,7 @@ pub enum Identifier { #[derive(Debug, Serialize, Deserialize)] pub struct ResponseBody { access_token: String, - device_id: Device, + device_id: Option, user_id: String, refresh_token: Option, #[serde_as(as = "Option>")] @@ -601,7 +601,7 @@ mod tests { let body: ResponseBody = response.json(); assert!(!body.access_token.is_empty()); - assert_eq!(body.device_id.as_str().len(), 10); + assert_eq!(body.device_id.as_ref().unwrap().as_str().len(), 10); assert_eq!(body.user_id, "@alice:example.com"); assert_eq!(body.refresh_token, None); assert_eq!(body.expires_in_ms, None); @@ -622,7 +622,7 @@ mod tests { let body: ResponseBody = response.json(); assert!(!body.access_token.is_empty()); - assert_eq!(body.device_id.as_str().len(), 10); + assert_eq!(body.device_id.as_ref().unwrap().as_str().len(), 10); assert_eq!(body.user_id, "@alice:example.com"); assert!(body.refresh_token.is_some()); assert!(body.expires_in_ms.is_some()); @@ -776,7 +776,7 @@ mod tests { let body: ResponseBody = response.json(); assert!(!body.access_token.is_empty()); - assert_eq!(body.device_id, device); + assert_eq!(body.device_id, Some(device)); assert_eq!(body.user_id, "@alice:example.com"); assert_eq!(body.refresh_token, None); assert_eq!(body.expires_in_ms, None); diff --git a/crates/handlers/src/graphql/model/compat_sessions.rs b/crates/handlers/src/graphql/model/compat_sessions.rs index ec67b7030..cc5269416 100644 --- a/crates/handlers/src/graphql/model/compat_sessions.rs +++ b/crates/handlers/src/graphql/model/compat_sessions.rs @@ -7,6 +7,7 @@ use anyhow::Context as _; use async_graphql::{Context, Description, Enum, Object, ID}; use chrono::{DateTime, Utc}; +use mas_data_model::Device; use mas_storage::{compat::CompatSessionRepository, user::UserRepository}; use url::Url; @@ -81,8 +82,8 @@ impl CompatSession { } /// The Matrix Device ID of this session. - async fn device_id(&self) -> &str { - self.session.device.as_str() + async fn device_id(&self) -> Option<&str> { + self.session.device.as_ref().map(Device::as_str) } /// When the object was created. diff --git a/crates/handlers/src/oauth2/introspection.rs b/crates/handlers/src/oauth2/introspection.rs index fcb0a205f..0b187278f 100644 --- a/crates/handlers/src/oauth2/introspection.rs +++ b/crates/handlers/src/oauth2/introspection.rs @@ -10,7 +10,7 @@ use mas_axum_utils::{ client_authorization::{ClientAuthorization, CredentialsVerificationError}, sentry::SentryEventID, }; -use mas_data_model::{TokenFormatError, TokenType}; +use mas_data_model::{Device, TokenFormatError, TokenType}; use mas_iana::oauth::{OAuthClientAuthenticationMethod, OAuthTokenTypeHint}; use mas_keystore::Encrypter; use mas_storage::{ @@ -364,11 +364,12 @@ pub(crate) async fn post( } // Grant the synapse admin scope if the session has the admin flag set. - let synapse_admin = session.is_synapse_admin.then_some(SYNAPSE_ADMIN_SCOPE); - let device_scope = session.device.to_scope_token(); - let scope = [API_SCOPE, device_scope] + let synapse_admin_scope_opt = session.is_synapse_admin.then_some(SYNAPSE_ADMIN_SCOPE); + let device_scope_opt = session.device.as_ref().map(Device::to_scope_token); + let scope = [API_SCOPE] .into_iter() - .chain(synapse_admin) + .chain(device_scope_opt) + .chain(synapse_admin_scope_opt) .collect(); activity_tracker @@ -423,11 +424,12 @@ pub(crate) async fn post( } // Grant the synapse admin scope if the session has the admin flag set. - let synapse_admin = session.is_synapse_admin.then_some(SYNAPSE_ADMIN_SCOPE); - let device_scope = session.device.to_scope_token(); - let scope = [API_SCOPE, device_scope] + let synapse_admin_scope_opt = session.is_synapse_admin.then_some(SYNAPSE_ADMIN_SCOPE); + let device_scope_opt = session.device.as_ref().map(Device::to_scope_token); + let scope = [API_SCOPE] .into_iter() - .chain(synapse_admin) + .chain(device_scope_opt) + .chain(synapse_admin_scope_opt) .collect(); activity_tracker diff --git a/crates/storage-pg/.sqlx/query-bb6f55a4cc10bec8ec0fc138485f6b4d308302bb1fa3accb12932d1e5ce457e9.json b/crates/storage-pg/.sqlx/query-bb6f55a4cc10bec8ec0fc138485f6b4d308302bb1fa3accb12932d1e5ce457e9.json index 2c81606b9..1360f4ba8 100644 --- a/crates/storage-pg/.sqlx/query-bb6f55a4cc10bec8ec0fc138485f6b4d308302bb1fa3accb12932d1e5ce457e9.json +++ b/crates/storage-pg/.sqlx/query-bb6f55a4cc10bec8ec0fc138485f6b4d308302bb1fa3accb12932d1e5ce457e9.json @@ -61,7 +61,7 @@ }, "nullable": [ false, - false, + true, false, true, false, diff --git a/crates/storage-pg/migrations/20250114135939_allow_deviceless_compat_sessions.sql b/crates/storage-pg/migrations/20250114135939_allow_deviceless_compat_sessions.sql new file mode 100644 index 000000000..8bf40f727 --- /dev/null +++ b/crates/storage-pg/migrations/20250114135939_allow_deviceless_compat_sessions.sql @@ -0,0 +1,7 @@ +-- Copyright 2025 New Vector Ltd. +-- +-- SPDX-License-Identifier: AGPL-3.0-only +-- Please see LICENSE in the repository root for full details. + +-- Drop the `NOT NULL` requirement on compat sessions, so we can import device-less access tokens from Synapse. +ALTER TABLE compat_sessions ALTER COLUMN device_id DROP NOT NULL; diff --git a/crates/storage-pg/src/app_session.rs b/crates/storage-pg/src/app_session.rs index 6905dbc1e..7a71036c9 100644 --- a/crates/storage-pg/src/app_session.rs +++ b/crates/storage-pg/src/app_session.rs @@ -117,16 +117,19 @@ impl TryFrom for AppSession { None, Some(user_id), None, - Some(device_id), + device_id_opt, Some(is_synapse_admin), ) => { let id = compat_session_id.into(); - let device = Device::try_from(device_id).map_err(|e| { - DatabaseInconsistencyError::on("compat_sessions") - .column("device_id") - .row(id) - .source(e) - })?; + let device = device_id_opt + .map(Device::try_from) + .transpose() + .map_err(|e| { + DatabaseInconsistencyError::on("compat_sessions") + .column("device_id") + .row(id) + .source(e) + })?; let state = match finished_at { None => CompatSessionState::Valid, diff --git a/crates/storage-pg/src/compat/mod.rs b/crates/storage-pg/src/compat/mod.rs index e58fa9071..93dd38447 100644 --- a/crates/storage-pg/src/compat/mod.rs +++ b/crates/storage-pg/src/compat/mod.rs @@ -83,7 +83,7 @@ mod tests { .await .unwrap(); assert_eq!(session.user_id, user.id); - assert_eq!(session.device.as_str(), device_str); + assert_eq!(session.device.as_ref().unwrap().as_str(), device_str); assert!(session.is_valid()); assert!(!session.is_finished()); @@ -117,7 +117,7 @@ mod tests { .expect("compat session not found"); assert_eq!(session_lookup.id, session.id); assert_eq!(session_lookup.user_id, user.id); - assert_eq!(session_lookup.device.as_str(), device_str); + assert_eq!(session.device.as_ref().unwrap().as_str(), device_str); assert!(session_lookup.is_valid()); assert!(!session_lookup.is_finished()); @@ -154,7 +154,7 @@ mod tests { let session_lookup = &list.edges[0].0; assert_eq!(session_lookup.id, session.id); assert_eq!(session_lookup.user_id, user.id); - assert_eq!(session_lookup.device.as_str(), device_str); + assert_eq!(session.device.as_ref().unwrap().as_str(), device_str); assert!(session_lookup.is_valid()); assert!(!session_lookup.is_finished()); diff --git a/crates/storage-pg/src/compat/session.rs b/crates/storage-pg/src/compat/session.rs index 12001cfe6..7e6f11f3c 100644 --- a/crates/storage-pg/src/compat/session.rs +++ b/crates/storage-pg/src/compat/session.rs @@ -47,7 +47,7 @@ impl<'c> PgCompatSessionRepository<'c> { struct CompatSessionLookup { compat_session_id: Uuid, - device_id: String, + device_id: Option, user_id: Uuid, user_session_id: Option, created_at: DateTime, @@ -63,12 +63,16 @@ impl TryFrom for CompatSession { fn try_from(value: CompatSessionLookup) -> Result { let id = value.compat_session_id.into(); - let device = Device::try_from(value.device_id).map_err(|e| { - DatabaseInconsistencyError::on("compat_sessions") - .column("device_id") - .row(id) - .source(e) - })?; + let device = value + .device_id + .map(Device::try_from) + .transpose() + .map_err(|e| { + DatabaseInconsistencyError::on("compat_sessions") + .column("device_id") + .row(id) + .source(e) + })?; let state = match value.finished_at { None => CompatSessionState::Valid, @@ -118,12 +122,12 @@ impl TryFrom for (CompatSession, Option Result { let id = value.compat_session_id.into(); - let device = Device::try_from(value.device_id).map_err(|e| { + let device = Some(Device::try_from(value.device_id).map_err(|e| { DatabaseInconsistencyError::on("compat_sessions") .column("device_id") .row(id) .source(e) - })?; + })?); let state = match value.finished_at { None => CompatSessionState::Valid, @@ -347,7 +351,7 @@ impl CompatSessionRepository for PgCompatSessionRepository<'_> { id, state: CompatSessionState::default(), user_id: user.id, - device, + device: Some(device), user_session_id: browser_session.map(|s| s.id), created_at, is_synapse_admin, @@ -364,7 +368,7 @@ impl CompatSessionRepository for PgCompatSessionRepository<'_> { db.query.text, %compat_session.id, user.id = %compat_session.user_id, - compat_session.device.id = compat_session.device.as_str(), + compat_session.device.id = compat_session.device.as_ref().map(mas_data_model::Device::as_str), ), err, )] diff --git a/crates/storage-pg/src/compat/sso_login.rs b/crates/storage-pg/src/compat/sso_login.rs index 5e6121a81..8b75ab130 100644 --- a/crates/storage-pg/src/compat/sso_login.rs +++ b/crates/storage-pg/src/compat/sso_login.rs @@ -293,7 +293,7 @@ impl CompatSsoLoginRepository for PgCompatSsoLoginRepository<'_> { db.query.text, %compat_sso_login.id, %compat_session.id, - compat_session.device.id = compat_session.device.as_str(), + compat_session.device.id = compat_session.device.as_ref().map(mas_data_model::Device::as_str), user.id = %compat_session.user_id, ), err, diff --git a/crates/tasks/src/matrix.rs b/crates/tasks/src/matrix.rs index 0f58773b3..3c558576c 100644 --- a/crates/tasks/src/matrix.rs +++ b/crates/tasks/src/matrix.rs @@ -208,7 +208,11 @@ impl RunnableJob for SyncDevicesJob { .map_err(JobError::retry)?; for (compat_session, _) in page.edges { - devices.insert(compat_session.device.as_str().to_owned()); + let Some(ref device) = compat_session.device else { + // Do not sync compat sessions without devices + continue; + }; + devices.insert(device.as_str().to_owned()); cursor = cursor.after(compat_session.id); } From 6ed745ac7ebac49c3370c555ffdcf13dd0b22390 Mon Sep 17 00:00:00 2001 From: reivilibre Date: Fri, 24 Jan 2025 11:57:14 +0000 Subject: [PATCH 2/5] Update crates/tasks/src/matrix.rs Co-authored-by: Quentin Gliech --- crates/tasks/src/matrix.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/tasks/src/matrix.rs b/crates/tasks/src/matrix.rs index 3c558576c..e941c2b90 100644 --- a/crates/tasks/src/matrix.rs +++ b/crates/tasks/src/matrix.rs @@ -208,11 +208,9 @@ impl RunnableJob for SyncDevicesJob { .map_err(JobError::retry)?; for (compat_session, _) in page.edges { - let Some(ref device) = compat_session.device else { - // Do not sync compat sessions without devices - continue; + if let Some(ref device) = compat_session.device { + devices.insert(device.as_str().to_owned()); }; - devices.insert(device.as_str().to_owned()); cursor = cursor.after(compat_session.id); } From c9fc01150970c2778b4bc7b3d67632fe32eed7a3 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Fri, 24 Jan 2025 15:07:35 +0000 Subject: [PATCH 3/5] fixup! Support compatibility sessions that do not have devices --- crates/storage-pg/src/compat/session.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/storage-pg/src/compat/session.rs b/crates/storage-pg/src/compat/session.rs index 7e6f11f3c..de5ea6b2f 100644 --- a/crates/storage-pg/src/compat/session.rs +++ b/crates/storage-pg/src/compat/session.rs @@ -100,7 +100,7 @@ impl TryFrom for CompatSession { #[enum_def] struct CompatSessionAndSsoLoginLookup { compat_session_id: Uuid, - device_id: String, + device_id: Option, user_id: Uuid, user_session_id: Option, created_at: DateTime, @@ -122,12 +122,16 @@ impl TryFrom for (CompatSession, Option Result { let id = value.compat_session_id.into(); - let device = Some(Device::try_from(value.device_id).map_err(|e| { - DatabaseInconsistencyError::on("compat_sessions") - .column("device_id") - .row(id) - .source(e) - })?); + let device = value + .device_id + .map(Device::try_from) + .transpose() + .map_err(|e| { + DatabaseInconsistencyError::on("compat_sessions") + .column("device_id") + .row(id) + .source(e) + })?; let state = match value.finished_at { None => CompatSessionState::Valid, From 88055532c310a49baa9010c2bfdbc5f3e1584166 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Fri, 24 Jan 2025 15:30:12 +0000 Subject: [PATCH 4/5] (generated) --- frontend/schema.graphql | 2 +- frontend/src/gql/graphql.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/schema.graphql b/frontend/schema.graphql index 3eb56573e..5b302c929 100644 --- a/frontend/schema.graphql +++ b/frontend/schema.graphql @@ -337,7 +337,7 @@ type CompatSession implements Node & CreationEvent { """ The Matrix Device ID of this session. """ - deviceId: String! + deviceId: String """ When the object was created. """ diff --git a/frontend/src/gql/graphql.ts b/frontend/src/gql/graphql.ts index 3980ea976..ef7cf7902 100644 --- a/frontend/src/gql/graphql.ts +++ b/frontend/src/gql/graphql.ts @@ -235,7 +235,7 @@ export type CompatSession = CreationEvent & Node & { /** When the object was created. */ createdAt: Scalars['DateTime']['output']; /** The Matrix Device ID of this session. */ - deviceId: Scalars['String']['output']; + deviceId?: Maybe; /** When the session ended. */ finishedAt?: Maybe; /** ID of the object. */ @@ -1515,7 +1515,7 @@ export type EndBrowserSessionMutation = { __typename?: 'Mutation', endBrowserSes export type OAuth2Client_DetailFragment = { __typename?: 'Oauth2Client', id: string, clientId: string, clientName?: string | null, clientUri?: string | null, logoUri?: string | null, tosUri?: string | null, policyUri?: string | null, redirectUris: Array } & { ' $fragmentName'?: 'OAuth2Client_DetailFragment' }; -export type CompatSession_SessionFragment = { __typename?: 'CompatSession', id: string, createdAt: string, deviceId: string, finishedAt?: string | null, lastActiveIp?: string | null, lastActiveAt?: string | null, userAgent?: { __typename?: 'UserAgent', name?: string | null, os?: string | null, model?: string | null, deviceType: DeviceType } | null, ssoLogin?: { __typename?: 'CompatSsoLogin', id: string, redirectUri: string } | null } & { ' $fragmentName'?: 'CompatSession_SessionFragment' }; +export type CompatSession_SessionFragment = { __typename?: 'CompatSession', id: string, createdAt: string, deviceId?: string | null, finishedAt?: string | null, lastActiveIp?: string | null, lastActiveAt?: string | null, userAgent?: { __typename?: 'UserAgent', name?: string | null, os?: string | null, model?: string | null, deviceType: DeviceType } | null, ssoLogin?: { __typename?: 'CompatSsoLogin', id: string, redirectUri: string } | null } & { ' $fragmentName'?: 'CompatSession_SessionFragment' }; export type EndCompatSessionMutationVariables = Exact<{ id: Scalars['ID']['input']; @@ -1547,7 +1547,7 @@ export type PasswordCreationDoubleInput_SiteConfigFragment = { __typename?: 'Sit export type BrowserSession_DetailFragment = { __typename?: 'BrowserSession', id: string, createdAt: string, finishedAt?: string | null, lastActiveIp?: string | null, lastActiveAt?: string | null, userAgent?: { __typename?: 'UserAgent', name?: string | null, model?: string | null, os?: string | null } | null, lastAuthentication?: { __typename?: 'Authentication', id: string, createdAt: string } | null, user: { __typename?: 'User', id: string, username: string } } & { ' $fragmentName'?: 'BrowserSession_DetailFragment' }; -export type CompatSession_DetailFragment = { __typename?: 'CompatSession', id: string, createdAt: string, deviceId: string, finishedAt?: string | null, lastActiveIp?: string | null, lastActiveAt?: string | null, userAgent?: { __typename?: 'UserAgent', name?: string | null, os?: string | null, model?: string | null } | null, ssoLogin?: { __typename?: 'CompatSsoLogin', id: string, redirectUri: string } | null } & { ' $fragmentName'?: 'CompatSession_DetailFragment' }; +export type CompatSession_DetailFragment = { __typename?: 'CompatSession', id: string, createdAt: string, deviceId?: string | null, finishedAt?: string | null, lastActiveIp?: string | null, lastActiveAt?: string | null, userAgent?: { __typename?: 'UserAgent', name?: string | null, os?: string | null, model?: string | null } | null, ssoLogin?: { __typename?: 'CompatSsoLogin', id: string, redirectUri: string } | null } & { ' $fragmentName'?: 'CompatSession_DetailFragment' }; export type OAuth2Session_DetailFragment = { __typename?: 'Oauth2Session', id: string, scope: string, createdAt: string, finishedAt?: string | null, lastActiveIp?: string | null, lastActiveAt?: string | null, client: { __typename?: 'Oauth2Client', id: string, clientId: string, clientName?: string | null, clientUri?: string | null, logoUri?: string | null } } & { ' $fragmentName'?: 'OAuth2Session_DetailFragment' }; From 8a28545401233f0a7ba2ea6155ceb8542f180f7c Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Fri, 24 Jan 2025 15:33:54 +0000 Subject: [PATCH 5/5] Client-side: add nullability to deviceId It is hidden if null already anyway --- frontend/src/components/SessionDetail/SessionDetails.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/SessionDetail/SessionDetails.tsx b/frontend/src/components/SessionDetail/SessionDetails.tsx index 3b1775fd0..67430360b 100644 --- a/frontend/src/components/SessionDetail/SessionDetails.tsx +++ b/frontend/src/components/SessionDetail/SessionDetails.tsx @@ -26,7 +26,7 @@ type Props = { title: string | ReactNode; lastActive?: Date; signedIn?: Date; - deviceId?: string; + deviceId?: string | null; ipAddress?: string; scopes?: string[]; details?: Detail[];