Support compatibility sessions that do not have devices (#3801)

Co-authored-by: Quentin Gliech <[email protected]>
element-hq · Jan 27, 2025 · 0c26dd8 · 0c26dd8
1 parent 747159c
commit 0c26dd8
Show file tree

Hide file tree

Showing 15 changed files with 74 additions and 51 deletions.
diff --git a/crates/cli/src/commands/manage.rs b/crates/cli/src/commands/manage.rs
@@ -310,7 +310,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
@@ -71,7 +71,7 @@ pub struct CompatSession {
     pub id: Ulid,
     pub state: CompatSessionState,
     pub user_id: Ulid,
-    pub device: Device,
+    pub device: Option<Device>,
     pub user_session_id: Option<Ulid>,
     pub created_at: DateTime<Utc>,
     pub is_synapse_admin: bool,

diff --git 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<Device>,
     user_id: String,
     refresh_token: Option<String>,
     #[serde_as(as = "Option<DurationMilliSeconds<i64>>")]
@@ -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
@@ -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
@@ -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/...rage-pg/.sqlx/query-bb6f55a4cc10bec8ec0fc138485f6b4d308302bb1fa3accb12932d1e5ce457e9.json b/...rage-pg/.sqlx/query-bb6f55a4cc10bec8ec0fc138485f6b4d308302bb1fa3accb12932d1e5ce457e9.json
diff --git a/crates/storage-pg/migrations/20250114135939_allow_deviceless_compat_sessions.sql 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
@@ -117,16 +117,19 @@ impl TryFrom<AppSessionLookup> 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
@@ -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
@@ -47,7 +47,7 @@ impl<'c> PgCompatSessionRepository<'c> {
 
 struct CompatSessionLookup {
     compat_session_id: Uuid,
-    device_id: String,
+    device_id: Option<String>,
     user_id: Uuid,
     user_session_id: Option<Uuid>,
     created_at: DateTime<Utc>,
@@ -63,12 +63,16 @@ impl TryFrom<CompatSessionLookup> for CompatSession {
 
     fn try_from(value: CompatSessionLookup) -> Result<Self, Self::Error> {
         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,
@@ -96,7 +100,7 @@ impl TryFrom<CompatSessionLookup> for CompatSession {
 #[enum_def]
 struct CompatSessionAndSsoLoginLookup {
     compat_session_id: Uuid,
-    device_id: String,
+    device_id: Option<String>,
     user_id: Uuid,
     user_session_id: Option<Uuid>,
     created_at: DateTime<Utc>,
@@ -118,12 +122,16 @@ impl TryFrom<CompatSessionAndSsoLoginLookup> for (CompatSession, Option<CompatSs
 
     fn try_from(value: CompatSessionAndSsoLoginLookup) -> Result<Self, Self::Error> {
         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,
@@ -347,7 +355,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 +372,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
@@ -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
@@ -207,7 +207,9 @@ impl RunnableJob for SyncDevicesJob {
                 .map_err(JobError::retry)?;
 
             for (compat_session, _) in page.edges {
-                devices.insert(compat_session.device.as_str().to_owned());
+                if let Some(ref device) = compat_session.device {
+                    devices.insert(device.as_str().to_owned());
+                };
                 cursor = cursor.after(compat_session.id);
             }
 

diff --git 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/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[];

diff --git 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<Scalars['String']['output']>;
   /** When the session ended. */
   finishedAt?: Maybe<Scalars['DateTime']['output']>;
   /** ID of the object. */
@@ -1579,7 +1579,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<string> } & { ' $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'];
@@ -1611,7 +1611,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' };