From 4d7f1c5a0fa7beb65123391b19777cca9fcdf514 Mon Sep 17 00:00:00 2001
From: Quentin Gliech <quenting@element.io>
Date: Mon, 26 Jun 2023 14:34:46 +0200
Subject: [PATCH] Ensure we're deleting rows in related tables when deleting
 upstream providers

---
 crates/storage-pg/sqlx-data.json              | 24 +++++++++++++++++
 crates/storage-pg/src/upstream_oauth2/mod.rs  | 11 ++++++++
 .../src/upstream_oauth2/provider.rs           | 27 ++++++++++++++++++-
 3 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/crates/storage-pg/sqlx-data.json b/crates/storage-pg/sqlx-data.json
index 01ee45393..de4cc6926 100644
--- a/crates/storage-pg/sqlx-data.json
+++ b/crates/storage-pg/sqlx-data.json
@@ -1226,6 +1226,18 @@
     },
     "query": "\n                UPDATE upstream_oauth_links\n                SET user_id = $1\n                WHERE upstream_oauth_link_id = $2\n            "
   },
+  "82fec0e13755e7032457d94fe8d212c62f14c80a98b61d82965f1b93f841c014": {
+    "describe": {
+      "columns": [],
+      "nullable": [],
+      "parameters": {
+        "Left": [
+          "Uuid"
+        ]
+      }
+    },
+    "query": "\n                DELETE FROM upstream_oauth_authorization_sessions\n                WHERE upstream_oauth_provider_id = $1\n            "
+  },
   "836fb7567d84057fa7f1edaab834c21a158a5762fe220b6bfacd6576be6c613c": {
     "describe": {
       "columns": [
@@ -1380,6 +1392,18 @@
     },
     "query": "\n                SELECT oauth2_client_id\n                     , encrypted_client_secret\n                     , ARRAY(\n                           SELECT redirect_uri\n                           FROM oauth2_client_redirect_uris r\n                           WHERE r.oauth2_client_id = c.oauth2_client_id\n                       ) AS \"redirect_uris!\"\n                     , grant_type_authorization_code\n                     , grant_type_refresh_token\n                     , client_name\n                     , logo_uri\n                     , client_uri\n                     , policy_uri\n                     , tos_uri\n                     , jwks_uri\n                     , jwks\n                     , id_token_signed_response_alg\n                     , userinfo_signed_response_alg\n                     , token_endpoint_auth_method\n                     , token_endpoint_auth_signing_alg\n                     , initiate_login_uri\n                FROM oauth2_clients c\n\n                WHERE oauth2_client_id = ANY($1::uuid[])\n            "
   },
+  "8a32a39c43147dfd9dbd25ff04686c3cdbc52ea5689ce3454d15e8ed31756f38": {
+    "describe": {
+      "columns": [],
+      "nullable": [],
+      "parameters": {
+        "Left": [
+          "Uuid"
+        ]
+      }
+    },
+    "query": "\n                DELETE FROM upstream_oauth_links\n                WHERE upstream_oauth_provider_id = $1\n            "
+  },
   "8a79c7c392dd930628caadec80c9b2645501475ab4feacbac59ca1bc52b16c3f": {
     "describe": {
       "columns": [],
diff --git a/crates/storage-pg/src/upstream_oauth2/mod.rs b/crates/storage-pg/src/upstream_oauth2/mod.rs
index e729e06ae..fd699db57 100644
--- a/crates/storage-pg/src/upstream_oauth2/mod.rs
+++ b/crates/storage-pg/src/upstream_oauth2/mod.rs
@@ -80,6 +80,12 @@ mod tests {
         assert_eq!(provider.issuer, "https://example.com/");
         assert_eq!(provider.client_id, "client-id");
 
+        // It should be in the list of all providers
+        let providers = repo.upstream_oauth_provider().all().await.unwrap();
+        assert_eq!(providers.len(), 1);
+        assert_eq!(providers[0].issuer, "https://example.com/");
+        assert_eq!(providers[0].client_id, "client-id");
+
         // Start a session
         let session = repo
             .upstream_oauth_session()
@@ -181,6 +187,11 @@ mod tests {
         assert_eq!(links.edges.len(), 1);
         assert_eq!(links.edges[0].id, link.id);
         assert_eq!(links.edges[0].user_id, Some(user.id));
+
+        // Try deleting the provider
+        repo.upstream_oauth_provider().delete(provider).await.unwrap();
+        let providers = repo.upstream_oauth_provider().all().await.unwrap();
+        assert!(providers.is_empty());
     }
 
     /// Test that the pagination works as expected in the upstream OAuth
diff --git a/crates/storage-pg/src/upstream_oauth2/provider.rs b/crates/storage-pg/src/upstream_oauth2/provider.rs
index e3051ec13..eb187dbd6 100644
--- a/crates/storage-pg/src/upstream_oauth2/provider.rs
+++ b/crates/storage-pg/src/upstream_oauth2/provider.rs
@@ -298,7 +298,32 @@ impl<'c> UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'
         err,
     )]
     async fn delete_by_id(&mut self, id: Ulid) -> Result<(), Self::Error> {
+        // Delete the authorization sessions first, as they have a foreign key constraint
+        // on the links and the providers.
         sqlx::query!(
+            r#"
+                DELETE FROM upstream_oauth_authorization_sessions
+                WHERE upstream_oauth_provider_id = $1
+            "#,
+            Uuid::from(id),
+        )
+            .traced()
+            .execute(&mut *self.conn)
+            .await?;
+
+        // Delete the links next, as they have a foreign key constraint on the providers.
+        sqlx::query!(
+            r#"
+                DELETE FROM upstream_oauth_links
+                WHERE upstream_oauth_provider_id = $1
+            "#,
+            Uuid::from(id),
+        )
+            .traced()
+            .execute(&mut *self.conn)
+            .await?;
+
+        let res = sqlx::query!(
             r#"
                 DELETE FROM upstream_oauth_providers
                 WHERE upstream_oauth_provider_id = $1
@@ -309,7 +334,7 @@ impl<'c> UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<'
         .execute(&mut *self.conn)
         .await?;
 
-        Ok(())
+        DatabaseError::ensure_affected_rows(&res, 1)
     }
 
     #[tracing::instrument(