Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support compatibility sessions that do not have devices #3801

Merged
merged 5 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/cli/src/commands/manage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion crates/data-model/src/compat/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions crates/handlers/src/compat/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>")]
Expand Down Expand Up @@ -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);
Expand All @@ -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());
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions crates/handlers/src/graphql/model/compat_sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
20 changes: 11 additions & 9 deletions crates/handlers/src/oauth2/introspection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -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;
17 changes: 10 additions & 7 deletions crates/storage-pg/src/app_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions crates/storage-pg/src/compat/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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());

Expand Down
40 changes: 24 additions & 16 deletions crates/storage-pg/src/compat/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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,
Expand Down Expand Up @@ -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>,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
)]
Expand Down
2 changes: 1 addition & 1 deletion crates/storage-pg/src/compat/sso_login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion crates/tasks/src/matrix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,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);
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/SessionDetail/SessionDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type Props = {
title: string | ReactNode;
lastActive?: Date;
signedIn?: Date;
deviceId?: string;
deviceId?: string | null;
ipAddress?: string;
scopes?: string[];
details?: Detail[];
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/gql/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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<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'];
Expand Down Expand Up @@ -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' };

Expand Down
Loading