From 479e0099319153be0b61f267249ef599f444d4c3 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 6 Dec 2022 18:05:32 +0100 Subject: [PATCH] data-model: simplify the compat sessions --- crates/data-model/src/compat.rs | 111 +++--------------- crates/data-model/src/traits.rs | 8 -- crates/graphql/src/model/compat_sessions.rs | 9 +- crates/graphql/src/model/users.rs | 2 +- crates/handlers/src/compat/login.rs | 10 +- .../handlers/src/compat/login_sso_redirect.rs | 2 +- crates/storage/src/compat.rs | 107 ++++++++--------- crates/storage/src/lib.rs | 4 - crates/templates/src/context.rs | 22 ++-- 9 files changed, 85 insertions(+), 190 deletions(-) diff --git a/crates/data-model/src/compat.rs b/crates/data-model/src/compat.rs index 05136edd..d6f772db 100644 --- a/crates/data-model/src/compat.rs +++ b/crates/data-model/src/compat.rs @@ -20,9 +20,10 @@ use rand::{ }; use serde::Serialize; use thiserror::Error; +use ulid::Ulid; use url::Url; -use crate::{StorageBackend, StorageBackendMarker, User}; +use crate::User; static DEVICE_ID_LENGTH: usize = 10; @@ -81,123 +82,49 @@ impl TryFrom for Device { } } -#[derive(Debug, Clone, PartialEq, Serialize)] -#[serde(bound = "T: StorageBackend")] -pub struct CompatSession { - #[serde(skip_serializing)] - pub data: T::CompatSessionData, +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct CompatSession { + pub id: Ulid, pub user: User, pub device: Device, pub created_at: DateTime, pub finished_at: Option>, } -impl From> for CompatSession<()> { - fn from(t: CompatSession) -> Self { - Self { - data: (), - user: t.user, - device: t.device, - created_at: t.created_at, - finished_at: t.finished_at, - } - } -} - -#[derive(Debug, Clone, PartialEq)] -pub struct CompatAccessToken { - pub data: T::CompatAccessTokenData, +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CompatAccessToken { + pub id: Ulid, pub token: String, pub created_at: DateTime, pub expires_at: Option>, } -impl From> for CompatAccessToken<()> { - fn from(t: CompatAccessToken) -> Self { - Self { - data: (), - token: t.token, - created_at: t.created_at, - expires_at: t.expires_at, - } - } -} - -#[derive(Debug, Clone, PartialEq)] -pub struct CompatRefreshToken { - pub data: T::CompatRefreshTokenData, +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CompatRefreshToken { + pub id: Ulid, pub token: String, pub created_at: DateTime, } -impl From> for CompatRefreshToken<()> { - fn from(t: CompatRefreshToken) -> Self { - Self { - data: (), - token: t.token, - created_at: t.created_at, - } - } -} - -#[derive(Debug, Clone, PartialEq, Serialize)] -#[serde(bound = "T: StorageBackend")] -pub enum CompatSsoLoginState { +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub enum CompatSsoLoginState { Pending, Fulfilled { fulfilled_at: DateTime, - session: CompatSession, + session: CompatSession, }, Exchanged { fulfilled_at: DateTime, exchanged_at: DateTime, - session: CompatSession, + session: CompatSession, }, } -impl From> for CompatSsoLoginState<()> { - fn from(t: CompatSsoLoginState) -> Self { - match t { - CompatSsoLoginState::Pending => Self::Pending, - CompatSsoLoginState::Fulfilled { - fulfilled_at, - session, - } => Self::Fulfilled { - fulfilled_at, - session: session.into(), - }, - CompatSsoLoginState::Exchanged { - fulfilled_at, - exchanged_at, - session, - } => Self::Exchanged { - fulfilled_at, - exchanged_at, - session: session.into(), - }, - } - } -} - -#[derive(Debug, Clone, PartialEq, Serialize)] -#[serde(bound = "T: StorageBackend")] -pub struct CompatSsoLogin { - #[serde(skip_serializing)] - pub data: T::CompatSsoLoginData, +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct CompatSsoLogin { + pub id: Ulid, pub redirect_uri: Url, pub login_token: String, pub created_at: DateTime, - pub state: CompatSsoLoginState, -} - -impl From> for CompatSsoLogin<()> { - fn from(t: CompatSsoLogin) -> Self { - Self { - data: (), - redirect_uri: t.redirect_uri, - login_token: t.login_token, - created_at: t.created_at, - state: t.state.into(), - } - } + pub state: CompatSsoLoginState, } diff --git a/crates/data-model/src/traits.rs b/crates/data-model/src/traits.rs index 49d1d1fe..921894b9 100644 --- a/crates/data-model/src/traits.rs +++ b/crates/data-model/src/traits.rs @@ -33,18 +33,10 @@ pub trait StorageBackend { type ClientData: Data; type SessionData: Data; type AuthorizationGrantData: Data; - type CompatAccessTokenData: Data; - type CompatRefreshTokenData: Data; - type CompatSessionData: Data; - type CompatSsoLoginData: Data; } impl StorageBackend for () { type AuthorizationGrantData = (); type ClientData = (); - type CompatAccessTokenData = (); - type CompatRefreshTokenData = (); - type CompatSessionData = (); - type CompatSsoLoginData = (); type SessionData = (); } diff --git a/crates/graphql/src/model/compat_sessions.rs b/crates/graphql/src/model/compat_sessions.rs index d4461434..5b272b18 100644 --- a/crates/graphql/src/model/compat_sessions.rs +++ b/crates/graphql/src/model/compat_sessions.rs @@ -15,7 +15,6 @@ use async_graphql::{Description, Object, ID}; use chrono::{DateTime, Utc}; use mas_data_model::CompatSsoLoginState; -use mas_storage::PostgresqlBackend; use url::Url; use super::{NodeType, User}; @@ -23,13 +22,13 @@ use super::{NodeType, User}; /// A compat session represents a client session which used the legacy Matrix /// login API. #[derive(Description)] -pub struct CompatSession(pub mas_data_model::CompatSession); +pub struct CompatSession(pub mas_data_model::CompatSession); #[Object(use_type_description)] impl CompatSession { /// ID of the object. pub async fn id(&self) -> ID { - NodeType::CompatSession.id(self.0.data) + NodeType::CompatSession.id(self.0.id) } /// The user authorized for this session. @@ -56,13 +55,13 @@ impl CompatSession { /// A compat SSO login represents a login done through the legacy Matrix login /// API, via the `m.login.sso` login method. #[derive(Description)] -pub struct CompatSsoLogin(pub mas_data_model::CompatSsoLogin); +pub struct CompatSsoLogin(pub mas_data_model::CompatSsoLogin); #[Object(use_type_description)] impl CompatSsoLogin { /// ID of the object. pub async fn id(&self) -> ID { - NodeType::CompatSsoLogin.id(self.0.data) + NodeType::CompatSsoLogin.id(self.0.id) } /// When the object was created. diff --git a/crates/graphql/src/model/users.rs b/crates/graphql/src/model/users.rs index 6a15b0da..8ec4b003 100644 --- a/crates/graphql/src/model/users.rs +++ b/crates/graphql/src/model/users.rs @@ -94,7 +94,7 @@ impl User { let mut connection = Connection::new(has_previous_page, has_next_page); connection.edges.extend(edges.into_iter().map(|u| { Edge::new( - OpaqueCursor(NodeCursor(NodeType::CompatSsoLogin, u.data)), + OpaqueCursor(NodeCursor(NodeType::CompatSsoLogin, u.id)), CompatSsoLogin(u), ) })); diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index 2c481182..d9386cdd 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -22,7 +22,7 @@ use mas_storage::{ get_compat_sso_login_by_token, mark_compat_sso_login_as_exchanged, CompatSsoLoginLookupError, }, - Clock, LookupError, PostgresqlBackend, + Clock, LookupError, }; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, skip_serializing_none, DurationMilliSeconds}; @@ -267,14 +267,14 @@ async fn token_login( txn: &mut Transaction<'_, Postgres>, clock: &Clock, token: &str, -) -> Result, RouteError> { +) -> Result { let login = get_compat_sso_login_by_token(&mut *txn, token).await?; let now = clock.now(); match login.state { CompatSsoLoginState::Pending => { tracing::error!( - compat_sso_login.id = %login.data, + compat_sso_login.id = %login.id, "Exchanged a token for a login that was not fullfilled yet" ); return Err(RouteError::InvalidLoginToken); @@ -291,7 +291,7 @@ async fn token_login( if now > exchanged_at + Duration::seconds(30) { // TODO: log that session out tracing::error!( - compat_sso_login.id = %login.data, + compat_sso_login.id = %login.id, "Login token exchanged a second time more than 30s after" ); } @@ -312,7 +312,7 @@ async fn user_password_login( txn: &mut Transaction<'_, Postgres>, username: String, password: String, -) -> Result, RouteError> { +) -> Result { let (clock, mut rng) = crate::rng_and_clock()?; let device = Device::generate(&mut rng); diff --git a/crates/handlers/src/compat/login_sso_redirect.rs b/crates/handlers/src/compat/login_sso_redirect.rs index 9a146cd4..55c57da7 100644 --- a/crates/handlers/src/compat/login_sso_redirect.rs +++ b/crates/handlers/src/compat/login_sso_redirect.rs @@ -87,5 +87,5 @@ pub async fn get( let mut conn = pool.acquire().await?; let login = insert_compat_sso_login(&mut conn, &mut rng, &clock, token, redirect_url).await?; - Ok(url_builder.absolute_redirect(&CompatLoginSsoComplete::new(login.data, params.action))) + Ok(url_builder.absolute_redirect(&CompatLoginSsoComplete::new(login.id, params.action))) } diff --git a/crates/storage/src/compat.rs b/crates/storage/src/compat.rs index 0aa13262..d6c31cc8 100644 --- a/crates/storage/src/compat.rs +++ b/crates/storage/src/compat.rs @@ -31,7 +31,7 @@ use uuid::Uuid; use crate::{ pagination::{process_page, QueryBuilderExt}, user::lookup_user_by_username, - Clock, DatabaseInconsistencyError, LookupError, PostgresqlBackend, + Clock, DatabaseInconsistencyError, LookupError, }; struct CompatAccessTokenLookup { @@ -73,13 +73,7 @@ pub async fn lookup_active_compat_access_token( executor: impl PgExecutor<'_>, clock: &Clock, token: &str, -) -> Result< - ( - CompatAccessToken, - CompatSession, - ), - CompatAccessTokenLookupError, -> { +) -> Result<(CompatAccessToken, CompatSession), CompatAccessTokenLookupError> { let res = sqlx::query_as!( CompatAccessTokenLookup, r#" @@ -123,7 +117,7 @@ pub async fn lookup_active_compat_access_token( } let token = CompatAccessToken { - data: res.compat_access_token_id.into(), + id: res.compat_access_token_id.into(), token: res.compat_access_token, created_at: res.compat_access_token_created_at, expires_at: res.compat_access_token_expires_at, @@ -156,7 +150,7 @@ pub async fn lookup_active_compat_access_token( let device = Device::try_from(res.compat_session_device_id).unwrap(); let session = CompatSession { - data: res.compat_session_id.into(), + id: res.compat_session_id.into(), user, device, created_at: res.compat_session_created_at, @@ -204,14 +198,7 @@ impl LookupError for CompatRefreshTokenLookupError { pub async fn lookup_active_compat_refresh_token( executor: impl PgExecutor<'_>, token: &str, -) -> Result< - ( - CompatRefreshToken, - CompatAccessToken, - CompatSession, - ), - CompatRefreshTokenLookupError, -> { +) -> Result<(CompatRefreshToken, CompatAccessToken, CompatSession), CompatRefreshTokenLookupError> { let res = sqlx::query_as!( CompatRefreshTokenLookup, r#" @@ -255,13 +242,13 @@ pub async fn lookup_active_compat_refresh_token( .await?; let refresh_token = CompatRefreshToken { - data: res.compat_refresh_token_id.into(), + id: res.compat_refresh_token_id.into(), token: res.compat_refresh_token, created_at: res.compat_refresh_token_created_at, }; let access_token = CompatAccessToken { - data: res.compat_access_token_id.into(), + id: res.compat_access_token_id.into(), token: res.compat_access_token, created_at: res.compat_access_token_created_at, expires_at: res.compat_access_token_expires_at, @@ -294,7 +281,7 @@ pub async fn lookup_active_compat_refresh_token( let device = Device::try_from(res.compat_session_device_id).unwrap(); let session = CompatSession { - data: res.compat_session_id.into(), + id: res.compat_session_id.into(), user, device, created_at: res.compat_session_created_at, @@ -321,7 +308,7 @@ pub async fn compat_login( username: &str, password: &str, device: Device, -) -> Result, anyhow::Error> { +) -> Result { let mut txn = conn.begin().await.context("could not start transaction")?; // First, lookup the user @@ -375,7 +362,7 @@ pub async fn compat_login( .context("could not insert compat session")?; let session = CompatSession { - data: id, + id, user, device, created_at, @@ -389,7 +376,7 @@ pub async fn compat_login( #[tracing::instrument( skip_all, fields( - compat_session.id = %session.data, + compat_session.id = %session.id, compat_session.device.id = session.device.as_str(), compat_access_token.id, user.id = %session.user.id, @@ -400,10 +387,10 @@ pub async fn add_compat_access_token( executor: impl PgExecutor<'_>, mut rng: impl Rng + Send, clock: &Clock, - session: &CompatSession, + session: &CompatSession, token: String, expires_after: Option, -) -> Result, anyhow::Error> { +) -> Result { let created_at = clock.now(); let id = Ulid::from_datetime_with_source(created_at.into(), &mut rng); tracing::Span::current().record("compat_access_token.id", tracing::field::display(id)); @@ -417,7 +404,7 @@ pub async fn add_compat_access_token( VALUES ($1, $2, $3, $4, $5) "#, Uuid::from(id), - Uuid::from(session.data), + Uuid::from(session.id), token, created_at, expires_at, @@ -428,7 +415,7 @@ pub async fn add_compat_access_token( .context("could not insert compat access token")?; Ok(CompatAccessToken { - data: id, + id, token, created_at, expires_at, @@ -438,14 +425,14 @@ pub async fn add_compat_access_token( #[tracing::instrument( skip_all, fields( - compat_access_token.id = %access_token.data, + compat_access_token.id = %access_token.id, ), err(Display), )] pub async fn expire_compat_access_token( executor: impl PgExecutor<'_>, clock: &Clock, - access_token: CompatAccessToken, + access_token: CompatAccessToken, ) -> Result<(), anyhow::Error> { let expires_at = clock.now(); let res = sqlx::query!( @@ -454,7 +441,7 @@ pub async fn expire_compat_access_token( SET expires_at = $2 WHERE compat_access_token_id = $1 "#, - Uuid::from(access_token.data), + Uuid::from(access_token.id), expires_at, ) .execute(executor) @@ -473,9 +460,9 @@ pub async fn expire_compat_access_token( #[tracing::instrument( skip_all, fields( - compat_session.id = %session.data, + compat_session.id = %session.id, compat_session.device.id = session.device.as_str(), - compat_access_token.id = %access_token.data, + compat_access_token.id = %access_token.id, compat_refresh_token.id, user.id = %session.user.id, ), @@ -485,10 +472,10 @@ pub async fn add_compat_refresh_token( executor: impl PgExecutor<'_>, mut rng: impl Rng + Send, clock: &Clock, - session: &CompatSession, - access_token: &CompatAccessToken, + session: &CompatSession, + access_token: &CompatAccessToken, token: String, -) -> Result, anyhow::Error> { +) -> Result { let created_at = clock.now(); let id = Ulid::from_datetime_with_source(created_at.into(), &mut rng); tracing::Span::current().record("compat_refresh_token.id", tracing::field::display(id)); @@ -501,8 +488,8 @@ pub async fn add_compat_refresh_token( VALUES ($1, $2, $3, $4, $5) "#, Uuid::from(id), - Uuid::from(session.data), - Uuid::from(access_token.data), + Uuid::from(session.id), + Uuid::from(access_token.id), token, created_at, ) @@ -512,7 +499,7 @@ pub async fn add_compat_refresh_token( .context("could not insert compat refresh token")?; Ok(CompatRefreshToken { - data: id, + id, token, created_at, }) @@ -558,14 +545,14 @@ pub async fn compat_logout( #[tracing::instrument( skip_all, fields( - compat_refresh_token.id = %refresh_token.data, + compat_refresh_token.id = %refresh_token.id, ), err(Display), )] pub async fn consume_compat_refresh_token( executor: impl PgExecutor<'_>, clock: &Clock, - refresh_token: CompatRefreshToken, + refresh_token: CompatRefreshToken, ) -> Result<(), anyhow::Error> { let consumed_at = clock.now(); let res = sqlx::query!( @@ -574,7 +561,7 @@ pub async fn consume_compat_refresh_token( SET consumed_at = $2 WHERE compat_refresh_token_id = $1 "#, - Uuid::from(refresh_token.data), + Uuid::from(refresh_token.id), consumed_at, ) .execute(executor) @@ -604,7 +591,7 @@ pub async fn insert_compat_sso_login( clock: &Clock, login_token: String, redirect_uri: Url, -) -> Result, anyhow::Error> { +) -> Result { let created_at = clock.now(); let id = Ulid::from_datetime_with_source(created_at.into(), &mut rng); tracing::Span::current().record("compat_sso_login.id", tracing::field::display(id)); @@ -626,7 +613,7 @@ pub async fn insert_compat_sso_login( .context("could not insert compat SSO login")?; Ok(CompatSsoLogin { - data: id, + id, login_token, redirect_uri, created_at, @@ -654,7 +641,7 @@ struct CompatSsoLoginLookup { user_email_confirmed_at: Option>, } -impl TryFrom for CompatSsoLogin { +impl TryFrom for CompatSsoLogin { type Error = DatabaseInconsistencyError; fn try_from(res: CompatSsoLoginLookup) -> Result { @@ -702,7 +689,7 @@ impl TryFrom for CompatSsoLogin { (Some(id), Some(device_id), Some(created_at), finished_at, Some(user)) => { let device = Device::try_from(device_id).map_err(|_| DatabaseInconsistencyError)?; Some(CompatSession { - data: id.into(), + id: id.into(), user, device, created_at, @@ -734,7 +721,7 @@ impl TryFrom for CompatSsoLogin { }; Ok(CompatSsoLogin { - data: res.compat_sso_login_id.into(), + id: res.compat_sso_login_id.into(), login_token: res.compat_sso_login_token, redirect_uri, created_at: res.compat_sso_login_created_at, @@ -766,7 +753,7 @@ impl LookupError for CompatSsoLoginLookupError { pub async fn get_compat_sso_login_by_id( executor: impl PgExecutor<'_>, id: Ulid, -) -> Result, CompatSsoLoginLookupError> { +) -> Result { let res = sqlx::query_as!( CompatSsoLoginLookup, r#" @@ -820,7 +807,7 @@ pub async fn get_paginated_user_compat_sso_logins( after: Option, first: Option, last: Option, -) -> Result<(bool, bool, Vec>), anyhow::Error> { +) -> Result<(bool, bool, Vec), anyhow::Error> { // TODO: this queries too much (like user info) which we probably don't need // because we already have them let mut query = QueryBuilder::new( @@ -877,7 +864,7 @@ pub async fn get_paginated_user_compat_sso_logins( pub async fn get_compat_sso_login_by_token( executor: impl PgExecutor<'_>, token: &str, -) -> Result, CompatSsoLoginLookupError> { +) -> Result { let res = sqlx::query_as!( CompatSsoLoginLookup, r#" @@ -920,7 +907,7 @@ pub async fn get_compat_sso_login_by_token( skip_all, fields( %user.id, - compat_sso_login.id = %login.data, + compat_sso_login.id = %login.id, compat_sso_login.redirect_uri = %login.redirect_uri, compat_session.id, compat_session.device.id = device.as_str(), @@ -932,9 +919,9 @@ pub async fn fullfill_compat_sso_login( mut rng: impl Rng + Send, clock: &Clock, user: User, - mut login: CompatSsoLogin, + mut login: CompatSsoLogin, device: Device, -) -> Result, anyhow::Error> { +) -> Result { if !matches!(login.state, CompatSsoLoginState::Pending) { bail!("sso login in wrong state"); }; @@ -961,7 +948,7 @@ pub async fn fullfill_compat_sso_login( .context("could not insert compat session")?; let session = CompatSession { - data: id, + id, user, device, created_at, @@ -978,8 +965,8 @@ pub async fn fullfill_compat_sso_login( WHERE compat_sso_login_id = $1 "#, - Uuid::from(login.data), - Uuid::from(session.data), + Uuid::from(login.id), + Uuid::from(session.id), fulfilled_at, ) .execute(&mut txn) @@ -1002,7 +989,7 @@ pub async fn fullfill_compat_sso_login( #[tracing::instrument( skip_all, fields( - compat_sso_login.id = %login.data, + compat_sso_login.id = %login.id, compat_sso_login.redirect_uri = %login.redirect_uri, ), err(Display), @@ -1010,8 +997,8 @@ pub async fn fullfill_compat_sso_login( pub async fn mark_compat_sso_login_as_exchanged( executor: impl PgExecutor<'_>, clock: &Clock, - mut login: CompatSsoLogin, -) -> Result, anyhow::Error> { + mut login: CompatSsoLogin, +) -> Result { let (fulfilled_at, session) = match login.state { CompatSsoLoginState::Fulfilled { fulfilled_at, @@ -1029,7 +1016,7 @@ pub async fn mark_compat_sso_login_as_exchanged( WHERE compat_sso_login_id = $1 "#, - Uuid::from(login.data), + Uuid::from(login.id), exchanged_at, ) .execute(executor) diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 9cc4057a..ebe208dd 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -107,10 +107,6 @@ pub struct PostgresqlBackend; impl StorageBackend for PostgresqlBackend { type AuthorizationGrantData = Ulid; type ClientData = Ulid; - type CompatAccessTokenData = Ulid; - type CompatRefreshTokenData = Ulid; - type CompatSessionData = Ulid; - type CompatSsoLoginData = Ulid; type SessionData = Ulid; } diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index 61d070f5..bcde9836 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -256,7 +256,7 @@ pub enum PostAuthContextInner { /// TODO: add the login context in there ContinueCompatSsoLogin { /// The compat SSO login request - login: Box>, + login: Box, }, /// Change the account password @@ -512,7 +512,7 @@ impl ReauthContext { /// Context used by the `sso.html` template #[derive(Serialize)] pub struct CompatSsoContext { - login: CompatSsoLogin<()>, + login: CompatSsoLogin, action: PostAuthAction, } @@ -521,17 +521,16 @@ impl TemplateContext for CompatSsoContext { where Self: Sized, { + let id = Ulid::from_datetime_with_source(now.into(), rng); vec![CompatSsoContext { login: CompatSsoLogin { - data: (), + id, redirect_uri: Url::parse("https://app.element.io/").unwrap(), login_token: "abcdefghijklmnopqrstuvwxyz012345".into(), created_at: now, state: CompatSsoLoginState::Pending, }, - action: PostAuthAction::ContinueCompatSsoLogin { - data: Ulid::from_datetime_with_source(now.into(), rng), - }, + action: PostAuthAction::ContinueCompatSsoLogin { data: id }, }] } } @@ -539,14 +538,9 @@ impl TemplateContext for CompatSsoContext { impl CompatSsoContext { /// Constructs a context for the legacy SSO login page #[must_use] - pub fn new(login: T, action: PostAuthAction) -> Self - where - T: Into>, - { - Self { - login: login.into(), - action, - } + pub fn new(login: CompatSsoLogin, action: PostAuthAction) -> Self +where { + Self { login, action } } }