diff --git a/crates/data-model/src/upstream_oauth2/session.rs b/crates/data-model/src/upstream_oauth2/session.rs index 23139c929..ef3623cfc 100644 --- a/crates/data-model/src/upstream_oauth2/session.rs +++ b/crates/data-model/src/upstream_oauth2/session.rs @@ -30,6 +30,12 @@ pub enum UpstreamOAuthAuthorizationSessionState { extra_callback_parameters: Option, userinfo: Option, }, + Unlinked { + completed_at: DateTime, + consumed_at: Option>, + unlinked_at: DateTime, + id_token: Option, + }, } impl UpstreamOAuthAuthorizationSessionState { @@ -57,7 +63,9 @@ impl UpstreamOAuthAuthorizationSessionState { extra_callback_parameters, userinfo, }), - Self::Completed { .. } | Self::Consumed { .. } => Err(InvalidTransitionError), + Self::Completed { .. } | Self::Consumed { .. } | Self::Unlinked { .. } => { + Err(InvalidTransitionError) + } } } @@ -85,7 +93,9 @@ impl UpstreamOAuthAuthorizationSessionState { extra_callback_parameters, userinfo, }), - Self::Pending | Self::Consumed { .. } => Err(InvalidTransitionError), + Self::Pending | Self::Consumed { .. } | Self::Unlinked { .. } => { + Err(InvalidTransitionError) + } } } @@ -98,7 +108,7 @@ impl UpstreamOAuthAuthorizationSessionState { #[must_use] pub fn link_id(&self) -> Option { match self { - Self::Pending => None, + Self::Pending | Self::Unlinked { .. } => None, Self::Completed { link_id, .. } | Self::Consumed { link_id, .. } => Some(*link_id), } } @@ -114,9 +124,9 @@ impl UpstreamOAuthAuthorizationSessionState { pub fn completed_at(&self) -> Option> { match self { Self::Pending => None, - Self::Completed { completed_at, .. } | Self::Consumed { completed_at, .. } => { - Some(*completed_at) - } + Self::Completed { completed_at, .. } + | Self::Consumed { completed_at, .. } + | Self::Unlinked { completed_at, .. } => Some(*completed_at), } } @@ -130,9 +140,9 @@ impl UpstreamOAuthAuthorizationSessionState { pub fn id_token(&self) -> Option<&str> { match self { Self::Pending => None, - Self::Completed { id_token, .. } | Self::Consumed { id_token, .. } => { - id_token.as_deref() - } + Self::Completed { id_token, .. } + | Self::Consumed { id_token, .. } + | Self::Unlinked { id_token, .. } => id_token.as_deref(), } } @@ -145,7 +155,7 @@ impl UpstreamOAuthAuthorizationSessionState { #[must_use] pub fn extra_callback_parameters(&self) -> Option<&serde_json::Value> { match self { - Self::Pending => None, + Self::Pending | Self::Unlinked { .. } => None, Self::Completed { extra_callback_parameters, .. @@ -160,7 +170,7 @@ impl UpstreamOAuthAuthorizationSessionState { #[must_use] pub fn userinfo(&self) -> Option<&serde_json::Value> { match self { - Self::Pending => None, + Self::Pending | Self::Unlinked { .. } => None, Self::Completed { userinfo, .. } | Self::Consumed { userinfo, .. } => userinfo.as_ref(), } } @@ -177,6 +187,22 @@ impl UpstreamOAuthAuthorizationSessionState { match self { Self::Pending | Self::Completed { .. } => None, Self::Consumed { consumed_at, .. } => Some(*consumed_at), + Self::Unlinked { consumed_at, .. } => *consumed_at, + } + } + + /// Get the time at which the upstream OAuth 2.0 authorization session was + /// unlinked. + /// + /// Returns `None` if the upstream OAuth 2.0 authorization session state is + /// not [`Unlinked`]. + /// + /// [`Unlinked`]: UpstreamOAuthAuthorizationSessionState::Unlinked + #[must_use] + pub fn unlinked_at(&self) -> Option> { + match self { + Self::Pending | Self::Completed { .. } | Self::Consumed { .. } => None, + Self::Unlinked { unlinked_at, .. } => Some(*unlinked_at), } } @@ -206,6 +232,15 @@ impl UpstreamOAuthAuthorizationSessionState { pub fn is_consumed(&self) -> bool { matches!(self, Self::Consumed { .. }) } + + /// Returns `true` if the upstream OAuth 2.0 authorization session state is + /// [`Unlinked`]. + /// + /// [`Unlinked`]: UpstreamOAuthAuthorizationSessionState::Unlinked + #[must_use] + pub fn is_unlinked(&self) -> bool { + matches!(self, Self::Unlinked { .. }) + } } #[derive(Debug, Clone, PartialEq, Eq, Serialize)] diff --git a/crates/storage-pg/.sqlx/query-ea30b3809fd7c1d4e9983909c0219f343953a89f2a43f6b8c4ab4fbea7645ccc.json b/crates/storage-pg/.sqlx/query-37a124678323380357fa9d1375fd125fb35476ac3008e5adbd04a761d5edcd42.json similarity index 82% rename from crates/storage-pg/.sqlx/query-ea30b3809fd7c1d4e9983909c0219f343953a89f2a43f6b8c4ab4fbea7645ccc.json rename to crates/storage-pg/.sqlx/query-37a124678323380357fa9d1375fd125fb35476ac3008e5adbd04a761d5edcd42.json index 3a49d2a87..eac08aed7 100644 --- a/crates/storage-pg/.sqlx/query-ea30b3809fd7c1d4e9983909c0219f343953a89f2a43f6b8c4ab4fbea7645ccc.json +++ b/crates/storage-pg/.sqlx/query-37a124678323380357fa9d1375fd125fb35476ac3008e5adbd04a761d5edcd42.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n upstream_oauth_authorization_session_id,\n upstream_oauth_provider_id,\n upstream_oauth_link_id,\n state,\n code_challenge_verifier,\n nonce,\n id_token,\n extra_callback_parameters,\n userinfo,\n created_at,\n completed_at,\n consumed_at\n FROM upstream_oauth_authorization_sessions\n WHERE upstream_oauth_authorization_session_id = $1\n ", + "query": "\n SELECT\n upstream_oauth_authorization_session_id,\n upstream_oauth_provider_id,\n upstream_oauth_link_id,\n state,\n code_challenge_verifier,\n nonce,\n id_token,\n extra_callback_parameters,\n userinfo,\n created_at,\n completed_at,\n consumed_at,\n unlinked_at\n FROM upstream_oauth_authorization_sessions\n WHERE upstream_oauth_authorization_session_id = $1\n ", "describe": { "columns": [ { @@ -62,6 +62,11 @@ "ordinal": 11, "name": "consumed_at", "type_info": "Timestamptz" + }, + { + "ordinal": 12, + "name": "unlinked_at", + "type_info": "Timestamptz" } ], "parameters": { @@ -81,8 +86,9 @@ true, false, true, + true, true ] }, - "hash": "ea30b3809fd7c1d4e9983909c0219f343953a89f2a43f6b8c4ab4fbea7645ccc" + "hash": "37a124678323380357fa9d1375fd125fb35476ac3008e5adbd04a761d5edcd42" } diff --git a/crates/storage-pg/.sqlx/query-3ed73cfce8ef6a1108f454e18b1668f64b76975dba07e67d04ed7a52e2e8107f.json b/crates/storage-pg/.sqlx/query-3ed73cfce8ef6a1108f454e18b1668f64b76975dba07e67d04ed7a52e2e8107f.json new file mode 100644 index 000000000..49d451a18 --- /dev/null +++ b/crates/storage-pg/.sqlx/query-3ed73cfce8ef6a1108f454e18b1668f64b76975dba07e67d04ed7a52e2e8107f.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "\n UPDATE upstream_oauth_authorization_sessions SET\n upstream_oauth_link_id = NULL,\n unlinked_at = $2\n WHERE upstream_oauth_link_id = $1\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Uuid", + "Timestamptz" + ] + }, + "nullable": [] + }, + "hash": "3ed73cfce8ef6a1108f454e18b1668f64b76975dba07e67d04ed7a52e2e8107f" +} diff --git a/crates/storage-pg/.sqlx/query-cc60ad934d347fb4546205d1fe07e9d2f127cb15b1bb650d1ea3805a4c55b196.json b/crates/storage-pg/.sqlx/query-cc60ad934d347fb4546205d1fe07e9d2f127cb15b1bb650d1ea3805a4c55b196.json new file mode 100644 index 000000000..00e04bffa --- /dev/null +++ b/crates/storage-pg/.sqlx/query-cc60ad934d347fb4546205d1fe07e9d2f127cb15b1bb650d1ea3805a4c55b196.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "\n DELETE FROM upstream_oauth_links\n WHERE upstream_oauth_link_id = $1\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Uuid" + ] + }, + "nullable": [] + }, + "hash": "cc60ad934d347fb4546205d1fe07e9d2f127cb15b1bb650d1ea3805a4c55b196" +} diff --git a/crates/storage-pg/.sqlx/query-fcd8b4b9e003d1540357c6bf1ff9c715560d011d4c01112703a9c046170c84f1.json b/crates/storage-pg/.sqlx/query-fcd8b4b9e003d1540357c6bf1ff9c715560d011d4c01112703a9c046170c84f1.json index ef1ac0372..f5503fa0e 100644 --- a/crates/storage-pg/.sqlx/query-fcd8b4b9e003d1540357c6bf1ff9c715560d011d4c01112703a9c046170c84f1.json +++ b/crates/storage-pg/.sqlx/query-fcd8b4b9e003d1540357c6bf1ff9c715560d011d4c01112703a9c046170c84f1.json @@ -23,7 +23,7 @@ "Left": [] }, "nullable": [ - true, + false, true, null ] diff --git a/crates/storage-pg/migrations/20250317151803_upstream_oauth_session_unlinked_at.sql b/crates/storage-pg/migrations/20250317151803_upstream_oauth_session_unlinked_at.sql new file mode 100644 index 000000000..a3ad0dc86 --- /dev/null +++ b/crates/storage-pg/migrations/20250317151803_upstream_oauth_session_unlinked_at.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. + +ALTER TABLE upstream_oauth_authorization_sessions + ADD COLUMN unlinked_at TIMESTAMP WITH TIME ZONE; diff --git a/crates/storage-pg/src/upstream_oauth2/link.rs b/crates/storage-pg/src/upstream_oauth2/link.rs index accbcb8c1..ea9cda163 100644 --- a/crates/storage-pg/src/upstream_oauth2/link.rs +++ b/crates/storage-pg/src/upstream_oauth2/link.rs @@ -374,4 +374,53 @@ impl UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'_> { .try_into() .map_err(DatabaseError::to_invalid_operation) } + + #[tracing::instrument( + name = "db.upstream_oauth_link.remove", + skip_all, + fields( + db.query.text, + upstream_oauth_link.id, + upstream_oauth_link.provider_id, + %upstream_oauth_link.subject, + ), + err, + )] + async fn remove( + &mut self, + clock: &dyn Clock, + upstream_oauth_link: UpstreamOAuthLink, + ) -> Result<(), Self::Error> { + // Unlink the authorization sessions first, as they have a foreign key + // constraint on the links. + sqlx::query!( + r#" + UPDATE upstream_oauth_authorization_sessions SET + upstream_oauth_link_id = NULL, + unlinked_at = $2 + WHERE upstream_oauth_link_id = $1 + "#, + Uuid::from(upstream_oauth_link.id), + clock.now() + ) + .traced() + .execute(&mut *self.conn) + .await?; + + // Then delete the link itself + let res = sqlx::query!( + r#" + DELETE FROM upstream_oauth_links + WHERE upstream_oauth_link_id = $1 + "#, + Uuid::from(upstream_oauth_link.id), + ) + .traced() + .execute(&mut *self.conn) + .await?; + + DatabaseError::ensure_affected_rows(&res, 1)?; + + Ok(()) + } } diff --git a/crates/storage-pg/src/upstream_oauth2/session.rs b/crates/storage-pg/src/upstream_oauth2/session.rs index fe1510e85..d9cad86a7 100644 --- a/crates/storage-pg/src/upstream_oauth2/session.rs +++ b/crates/storage-pg/src/upstream_oauth2/session.rs @@ -45,6 +45,7 @@ struct SessionLookup { completed_at: Option>, consumed_at: Option>, extra_callback_parameters: Option, + unlinked_at: Option>, } impl TryFrom for UpstreamOAuthAuthorizationSession { @@ -59,8 +60,11 @@ impl TryFrom for UpstreamOAuthAuthorizationSession { value.userinfo, value.completed_at, value.consumed_at, + value.unlinked_at, ) { - (None, None, None, None, None, None) => UpstreamOAuthAuthorizationSessionState::Pending, + (None, None, None, None, None, None, None) => { + UpstreamOAuthAuthorizationSessionState::Pending + } ( Some(link_id), id_token, @@ -68,6 +72,7 @@ impl TryFrom for UpstreamOAuthAuthorizationSession { userinfo, Some(completed_at), None, + None, ) => UpstreamOAuthAuthorizationSessionState::Completed { completed_at, link_id: link_id.into(), @@ -82,6 +87,7 @@ impl TryFrom for UpstreamOAuthAuthorizationSession { userinfo, Some(completed_at), Some(consumed_at), + None, ) => UpstreamOAuthAuthorizationSessionState::Consumed { completed_at, link_id: link_id.into(), @@ -90,6 +96,14 @@ impl TryFrom for UpstreamOAuthAuthorizationSession { userinfo, consumed_at, }, + (_, id_token, _, _, Some(completed_at), consumed_at, Some(unlinked_at)) => { + UpstreamOAuthAuthorizationSessionState::Unlinked { + completed_at, + id_token, + consumed_at, + unlinked_at, + } + } _ => { return Err(DatabaseInconsistencyError::on( "upstream_oauth_authorization_sessions", @@ -142,7 +156,8 @@ impl UpstreamOAuthSessionRepository for PgUpstreamOAuthSessionRepository<'_> { userinfo, created_at, completed_at, - consumed_at + consumed_at, + unlinked_at FROM upstream_oauth_authorization_sessions WHERE upstream_oauth_authorization_session_id = $1 "#, diff --git a/crates/storage/src/upstream_oauth2/link.rs b/crates/storage/src/upstream_oauth2/link.rs index e75959b9a..cca070d86 100644 --- a/crates/storage/src/upstream_oauth2/link.rs +++ b/crates/storage/src/upstream_oauth2/link.rs @@ -200,6 +200,22 @@ pub trait UpstreamOAuthLinkRepository: Send + Sync { /// /// Returns [`Self::Error`] if the underlying repository fails async fn count(&mut self, filter: UpstreamOAuthLinkFilter<'_>) -> Result; + + /// Delete a [`UpstreamOAuthLink`] + /// + /// # Parameters + /// + /// * `clock`: The clock used to generate timestamps + /// * `upstream_oauth_link`: The [`UpstreamOAuthLink`] to delete + /// + /// # Errors + /// + /// Returns [`Self::Error`] if the underlying repository fails + async fn remove( + &mut self, + clock: &dyn Clock, + upstream_oauth_link: UpstreamOAuthLink, + ) -> Result<(), Self::Error>; } repository_impl!(UpstreamOAuthLinkRepository: @@ -233,4 +249,6 @@ repository_impl!(UpstreamOAuthLinkRepository: ) -> Result, Self::Error>; async fn count(&mut self, filter: UpstreamOAuthLinkFilter<'_>) -> Result; + + async fn remove(&mut self, clock: &dyn Clock, upstream_oauth_link: UpstreamOAuthLink) -> Result<(), Self::Error>; );