From 7e44ae62a272c8ed45d8bb9e8ad04bf2172d793c Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 11:18:58 +0200 Subject: [PATCH 01/12] :bug: Fix profile deletion issue with 1 participant --- backend/src/app/rpc/commands/profile.clj | 22 +-- backend/src/app/tasks/orphan_teams_gc.clj | 1 - .../test/backend_tests/rpc_profile_test.clj | 157 +++++++++++++++++- 3 files changed, 167 insertions(+), 13 deletions(-) diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index fe33da10d..5bdf6943c 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -400,18 +400,18 @@ ;; --- HELPERS (def sql:owned-teams - "with owner_teams as ( - select tpr.team_id as id - from team_profile_rel as tpr - where tpr.is_owner is true - and tpr.profile_id = ? + "WITH owner_teams AS ( + SELECT tpr.team_id AS id + FROM team_profile_rel AS tpr + WHERE tpr.is_owner IS TRUE + AND tpr.profile_id = ? ) - select tpr.team_id as id, - count(tpr.profile_id) - 1 as participants - from team_profile_rel as tpr - where tpr.team_id in (select id from owner_teams) - and tpr.profile_id != ? - group by 1") + SELECT tpr.team_id AS id, + count(tpr.profile_id) AS participants + FROM team_profile_rel AS tpr + WHERE tpr.team_id IN (SELECT id from owner_teams) + AND tpr.profile_id != ? + GROUP BY 1") (defn- get-owned-teams-with-participants [conn profile-id] diff --git a/backend/src/app/tasks/orphan_teams_gc.clj b/backend/src/app/tasks/orphan_teams_gc.clj index c04123a83..bbc267e57 100644 --- a/backend/src/app/tasks/orphan_teams_gc.clj +++ b/backend/src/app/tasks/orphan_teams_gc.clj @@ -22,7 +22,6 @@ [_ cfg] (fn [params] (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (l/inf :hint "gc started" :rollback? (boolean (:rollback? params))) (let [total (delete-orphan-teams! cfg)] (l/inf :hint "task finished" :teams total diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index 00d2f2ac0..d52a0bac9 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -126,7 +126,7 @@ ;; (th/print-result! out) (t/is (nil? (:error out))))))) -(t/deftest profile-deletion-simple +(t/deftest profile-deletion-1 (let [prof (th/create-profile* 1) file (th/create-file* 1 {:profile-id (:id prof) :project-id (:default-project-id prof) @@ -177,6 +177,161 @@ (let [result (:result out)] (t/is (= uuid/zero (:id result))))))) + +(t/deftest profile-deletion-2 + (let [prof1 (th/create-profile* 1) + prof2 (th/create-profile* 2) + file1 (th/create-file* 1 {:profile-id (:id prof1) + :project-id (:default-project-id prof1) + :is-shared false}) + team1 (th/create-team* 1 {:profile-id (:id prof1)}) + + role1 (th/create-team-role* {:team-id (:id team1) + :profile-id (:id prof2) + + :role :editor})] + ;; Assert all roles for team + (let [roles (th/db-query :team-profile-rel {:team-id (:id team1)})] + (t/is (= 2 (count roles)))) + + ;; Request profile to be deleted + (let [params {::th/type :delete-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + + (let [error (:error out) + edata (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type edata) :validation)) + (t/is (= (:code edata) :owner-teams-with-people)))))) + +(t/deftest profile-deletion-3 + (let [prof1 (th/create-profile* 1) + prof2 (th/create-profile* 2) + prof3 (th/create-profile* 3) + file1 (th/create-file* 1 {:profile-id (:id prof1) + :project-id (:default-project-id prof1) + :is-shared false}) + team1 (th/create-team* 1 {:profile-id (:id prof1)}) + + role1 (th/create-team-role* {:team-id (:id team1) + :profile-id (:id prof2) + :role :editor}) + role2 (th/create-team-role* {:team-id (:id team1) + :profile-id (:id prof3) + :role :editor})] + + ;; Assert all roles for team + (let [roles (th/db-query :team-profile-rel {:team-id (:id team1)})] + (t/is (= 3 (count roles)))) + + ;; Request profile to be deleted (it should fail) + (let [params {::th/type :delete-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + + (let [error (:error out) + edata (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type edata) :validation)) + (t/is (= (:code edata) :owner-teams-with-people)))) + + ;; Leave team by role 1 + (let [params {::th/type :leave-team + ::rpc/profile-id (:id prof2) + :id (:id team1)} + out (th/command! params)] + + ;; (th/print-result! out) + (t/is (nil? (:result out))) + (t/is (nil? (:error out)))) + + ;; Request profile to be deleted (it should fail) + (let [params {::th/type :delete-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + (let [error (:error out) + edata (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type edata) :validation)) + (t/is (= (:code edata) :owner-teams-with-people)))) + + + ;; Leave team by role 0 (the default) and reassing owner to role 3 + ;; without reassinging it (should fail) + (let [params {::th/type :leave-team + ::rpc/profile-id (:id prof1) + ;; :reassign-to (:id prof3) + :id (:id team1)} + out (th/command! params)] + + ;; (th/print-result! out) + + (let [error (:error out) + edata (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type edata) :validation)) + (t/is (= (:code edata) :owner-cant-leave-team)))) + + ;; Leave team by role 0 (the default) and reassing owner to role 3 + (let [params {::th/type :leave-team + ::rpc/profile-id (:id prof1) + :reassign-to (:id prof3) + :id (:id team1)} + out (th/command! params)] + + ;; (th/print-result! out) + (t/is (nil? (:result out))) + (t/is (nil? (:error out)))) + + ;; Request profile to be deleted (it should fail) + (let [params {::th/type :delete-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + + (t/is (= {} (:result out))) + (t/is (nil? (:error out)))) + + ;; query files after profile soft deletion + (let [params {::th/type :get-project-files + ::rpc/profile-id (:id prof1) + :project-id (:default-project-id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) + (t/is (= 1 (count (:result out))))) + + ;; execute permanent deletion task + (let [result (th/run-task! :objects-gc {:min-age 0})] + (t/is (= 1 (:processed result)))) + + (let [row (th/db-get :team + {:id (:default-team-id prof1)} + {::db/remove-deleted false})] + (t/is (nil? (:deleted-at row)))) + + (let [result (th/run-task! :orphan-teams-gc {:min-age 0})] + (t/is (= 1 (:processed result)))) + + (let [row (th/db-get :team + {:id (:default-team-id prof1)} + {::db/remove-deleted false})] + (t/is (dt/instant? (:deleted-at row)))) + + ;; query profile after delete + (let [params {::th/type :get-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + (let [result (:result out)] + (t/is (= uuid/zero (:id result))))))) + + + (t/deftest registration-domain-whitelist (let [whitelist #{"gmail.com" "hey.com" "ya.ru"}] (t/testing "allowed email domain" From 2cddbc8a3de54ef0659c64e8874ed19d13d528dd Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 12:13:12 +0200 Subject: [PATCH 02/12] :bug: Fix error handling on account deletion process --- frontend/src/app/main/data/users.cljs | 6 ++---- frontend/src/app/main/ui/settings/delete_account.cljs | 11 ++++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index 8a540317f..e82fee08f 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -537,10 +537,8 @@ on-success identity}} (meta params)] (->> (rp/cmd! :delete-profile {}) (rx/tap on-success) - (rx/delay-at-least 300) - (rx/catch (constantly (rx/of 1))) - (rx/map logged-out) - (rx/catch on-error)))))) + (rx/catch on-error) + (rx/delay-at-least 300)))))) ;; --- EVENT: request-profile-recovery diff --git a/frontend/src/app/main/ui/settings/delete_account.cljs b/frontend/src/app/main/ui/settings/delete_account.cljs index 384907dfe..3dd81a34b 100644 --- a/frontend/src/app/main/ui/settings/delete_account.cljs +++ b/frontend/src/app/main/ui/settings/delete_account.cljs @@ -18,11 +18,12 @@ [rumext.v2 :as mf])) (defn on-error - [{:keys [code] :as error}] - (if (= :owner-teams-with-people code) - (let [msg (tr "notifications.profile-deletion-not-allowed")] - (rx/of (msg/error msg))) - (rx/throw error))) + [cause] + (let [code (-> cause ex-data :code)] + (if (= :owner-teams-with-people code) + (let [msg (tr "notifications.profile-deletion-not-allowed")] + (rx/of (msg/error msg))) + (rx/throw cause)))) (mf/defc delete-account-modal {::mf/register modal/components From 272edec3c647990794a3fbb6b8dd2c61ea073577 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 13:52:23 +0200 Subject: [PATCH 03/12] :bug: Add missing logged-out event after account deletion --- frontend/src/app/main/data/users.cljs | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index e82fee08f..d84da5e52 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -538,6 +538,7 @@ (->> (rp/cmd! :delete-profile {}) (rx/tap on-success) (rx/catch on-error) + (rx/map logged-out) (rx/delay-at-least 300)))))) ;; --- EVENT: request-profile-recovery From 67489c0bb9b42dd77ba453d7b7cc6a5cc076d43d Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 11:18:58 +0200 Subject: [PATCH 04/12] :bug: Fix profile deletion issue with 1 participant --- backend/src/app/rpc/commands/profile.clj | 22 +-- backend/src/app/tasks/orphan_teams_gc.clj | 1 - .../test/backend_tests/rpc_profile_test.clj | 154 +++++++++++++++++- 3 files changed, 164 insertions(+), 13 deletions(-) diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 4064f0dd6..24afb711c 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -399,18 +399,18 @@ ;; --- HELPERS (def sql:owned-teams - "with owner_teams as ( - select tpr.team_id as id - from team_profile_rel as tpr - where tpr.is_owner is true - and tpr.profile_id = ? + "WITH owner_teams AS ( + SELECT tpr.team_id AS id + FROM team_profile_rel AS tpr + WHERE tpr.is_owner IS TRUE + AND tpr.profile_id = ? ) - select tpr.team_id as id, - count(tpr.profile_id) - 1 as participants - from team_profile_rel as tpr - where tpr.team_id in (select id from owner_teams) - and tpr.profile_id != ? - group by 1") + SELECT tpr.team_id AS id, + count(tpr.profile_id) AS participants + FROM team_profile_rel AS tpr + WHERE tpr.team_id IN (SELECT id from owner_teams) + AND tpr.profile_id != ? + GROUP BY 1") (defn- get-owned-teams-with-participants [conn profile-id] diff --git a/backend/src/app/tasks/orphan_teams_gc.clj b/backend/src/app/tasks/orphan_teams_gc.clj index 5bdb360c0..0e3de973a 100644 --- a/backend/src/app/tasks/orphan_teams_gc.clj +++ b/backend/src/app/tasks/orphan_teams_gc.clj @@ -54,7 +54,6 @@ [_ cfg] (fn [{:keys [props] :as task}] (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (l/inf :hint "gc started" :rollback? (boolean (:rollback? props))) (let [total (delete-orphan-teams cfg)] (l/inf :hint "task finished" :teams total diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index 1777fecb1..82168ecaf 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -127,7 +127,7 @@ ;; (th/print-result! out) (t/is (nil? (:error out))))))) -(t/deftest profile-deletion-simple +(t/deftest profile-deletion-1 (let [prof (th/create-profile* 1) file (th/create-file* 1 {:profile-id (:id prof) :project-id (:default-project-id prof) @@ -178,6 +178,158 @@ (let [result (:result out)] (t/is (= uuid/zero (:id result))))))) +(t/deftest profile-deletion-2 + (let [prof1 (th/create-profile* 1) + prof2 (th/create-profile* 2) + file1 (th/create-file* 1 {:profile-id (:id prof1) + :project-id (:default-project-id prof1) + :is-shared false}) + team1 (th/create-team* 1 {:profile-id (:id prof1)}) + + role1 (th/create-team-role* {:team-id (:id team1) + :profile-id (:id prof2) + + :role :editor})] + ;; Assert all roles for team + (let [roles (th/db-query :team-profile-rel {:team-id (:id team1)})] + (t/is (= 2 (count roles)))) + + ;; Request profile to be deleted + (let [params {::th/type :delete-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + + (let [error (:error out) + edata (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type edata) :validation)) + (t/is (= (:code edata) :owner-teams-with-people)))))) + +(t/deftest profile-deletion-3 + (let [prof1 (th/create-profile* 1) + prof2 (th/create-profile* 2) + prof3 (th/create-profile* 3) + file1 (th/create-file* 1 {:profile-id (:id prof1) + :project-id (:default-project-id prof1) + :is-shared false}) + team1 (th/create-team* 1 {:profile-id (:id prof1)}) + + role1 (th/create-team-role* {:team-id (:id team1) + :profile-id (:id prof2) + :role :editor}) + role2 (th/create-team-role* {:team-id (:id team1) + :profile-id (:id prof3) + :role :editor})] + + ;; Assert all roles for team + (let [roles (th/db-query :team-profile-rel {:team-id (:id team1)})] + (t/is (= 3 (count roles)))) + + ;; Request profile to be deleted (it should fail) + (let [params {::th/type :delete-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + + (let [error (:error out) + edata (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type edata) :validation)) + (t/is (= (:code edata) :owner-teams-with-people)))) + + ;; Leave team by role 1 + (let [params {::th/type :leave-team + ::rpc/profile-id (:id prof2) + :id (:id team1)} + out (th/command! params)] + + ;; (th/print-result! out) + (t/is (nil? (:result out))) + (t/is (nil? (:error out)))) + + ;; Request profile to be deleted (it should fail) + (let [params {::th/type :delete-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + (let [error (:error out) + edata (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type edata) :validation)) + (t/is (= (:code edata) :owner-teams-with-people)))) + + + ;; Leave team by role 0 (the default) and reassing owner to role 3 + ;; without reassinging it (should fail) + (let [params {::th/type :leave-team + ::rpc/profile-id (:id prof1) + ;; :reassign-to (:id prof3) + :id (:id team1)} + out (th/command! params)] + + ;; (th/print-result! out) + + (let [error (:error out) + edata (ex-data error)] + (t/is (th/ex-info? error)) + (t/is (= (:type edata) :validation)) + (t/is (= (:code edata) :owner-cant-leave-team)))) + + ;; Leave team by role 0 (the default) and reassing owner to role 3 + (let [params {::th/type :leave-team + ::rpc/profile-id (:id prof1) + :reassign-to (:id prof3) + :id (:id team1)} + out (th/command! params)] + + ;; (th/print-result! out) + (t/is (nil? (:result out))) + (t/is (nil? (:error out)))) + + ;; Request profile to be deleted (it should fail) + (let [params {::th/type :delete-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + + (t/is (= {} (:result out))) + (t/is (nil? (:error out)))) + + ;; query files after profile soft deletion + (let [params {::th/type :get-project-files + ::rpc/profile-id (:id prof1) + :project-id (:default-project-id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + (t/is (nil? (:error out))) + (t/is (= 1 (count (:result out))))) + + ;; execute permanent deletion task + (let [result (th/run-task! :objects-gc {:min-age 0})] + (t/is (= 1 (:processed result)))) + + (let [row (th/db-get :team + {:id (:default-team-id prof1)} + {::db/remove-deleted false})] + (t/is (nil? (:deleted-at row)))) + + (let [result (th/run-task! :orphan-teams-gc {:min-age 0})] + (t/is (= 1 (:processed result)))) + + (let [row (th/db-get :team + {:id (:default-team-id prof1)} + {::db/remove-deleted false})] + (t/is (dt/instant? (:deleted-at row)))) + + ;; query profile after delete + (let [params {::th/type :get-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + (let [result (:result out)] + (t/is (= uuid/zero (:id result))))))) + (t/deftest email-blacklist-1 (t/is (false? (email.blacklist/enabled? th/*system*))) (t/is (true? (email.blacklist/enabled? (assoc th/*system* :app.email/blacklist [])))) From 56476acc1946af31e2b980576008c095fb037e1d Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 12:13:12 +0200 Subject: [PATCH 05/12] :bug: Fix error handling on account deletion process --- frontend/src/app/main/data/users.cljs | 6 ++---- frontend/src/app/main/ui/settings/delete_account.cljs | 11 ++++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index e4f8c4d07..bc2b7dbcd 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -569,10 +569,8 @@ on-success identity}} (meta params)] (->> (rp/cmd! :delete-profile {}) (rx/tap on-success) - (rx/delay-at-least 300) - (rx/catch (constantly (rx/of 1))) - (rx/map logged-out) - (rx/catch on-error)))))) + (rx/catch on-error) + (rx/delay-at-least 300)))))) ;; --- EVENT: request-profile-recovery diff --git a/frontend/src/app/main/ui/settings/delete_account.cljs b/frontend/src/app/main/ui/settings/delete_account.cljs index d7e941cea..d4ed25f78 100644 --- a/frontend/src/app/main/ui/settings/delete_account.cljs +++ b/frontend/src/app/main/ui/settings/delete_account.cljs @@ -18,11 +18,12 @@ [rumext.v2 :as mf])) (defn on-error - [{:keys [code] :as error}] - (if (= :owner-teams-with-people code) - (let [msg (tr "notifications.profile-deletion-not-allowed")] - (rx/of (msg/error msg))) - (rx/throw error))) + [cause] + (let [code (-> cause ex-data :code)] + (if (= :owner-teams-with-people code) + (let [msg (tr "notifications.profile-deletion-not-allowed")] + (rx/of (msg/error msg))) + (rx/throw cause)))) (mf/defc delete-account-modal {::mf/register modal/components From f9af7f0f09f2659d67272a225d14a5cd257ff4d4 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 13:42:05 +0200 Subject: [PATCH 06/12] :bug: Make profile deletion follow the delete-object flow This removes the need of the specific task for cleaning orphan teams. --- backend/src/app/main.clj | 7 -- backend/src/app/rpc/commands/profile.clj | 26 ++++---- backend/src/app/tasks/delete_object.clj | 27 +++++++- backend/src/app/tasks/objects_gc.clj | 11 +--- backend/src/app/tasks/orphan_teams_gc.clj | 65 ------------------- .../test/backend_tests/rpc_profile_test.clj | 65 ++++++++++++------- 6 files changed, 83 insertions(+), 118 deletions(-) delete mode 100644 backend/src/app/tasks/orphan_teams_gc.clj diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 5a959d39b..f9797521d 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -343,7 +343,6 @@ ::wrk/tasks {:sendmail (ig/ref ::email/handler) :objects-gc (ig/ref :app.tasks.objects-gc/handler) - :orphan-teams-gc (ig/ref :app.tasks.orphan-teams-gc/handler) :file-gc (ig/ref :app.tasks.file-gc/handler) :file-xlog-gc (ig/ref :app.tasks.file-xlog-gc/handler) :tasks-gc (ig/ref :app.tasks.tasks-gc/handler) @@ -388,9 +387,6 @@ {::db/pool (ig/ref ::db/pool) ::sto/storage (ig/ref ::sto/storage)} - :app.tasks.orphan-teams-gc/handler - {::db/pool (ig/ref ::db/pool)} - :app.tasks.delete-object/handler {::db/pool (ig/ref ::db/pool)} @@ -479,9 +475,6 @@ {:cron #app/cron "0 0 0 * * ?" ;; daily :task :objects-gc} - {:cron #app/cron "0 0 0 * * ?" ;; daily - :task :orphan-teams-gc} - {:cron #app/cron "0 0 0 * * ?" ;; daily :task :storage-gc-deleted} diff --git a/backend/src/app/rpc/commands/profile.clj b/backend/src/app/rpc/commands/profile.clj index 24afb711c..985a8b211 100644 --- a/backend/src/app/rpc/commands/profile.clj +++ b/backend/src/app/rpc/commands/profile.clj @@ -28,7 +28,7 @@ [app.tokens :as tokens] [app.util.services :as sv] [app.util.time :as dt] - [app.worker :as-alias wrk] + [app.worker :as wrk] [cuerdas.core :as str] [promesa.exec :as px])) @@ -366,13 +366,13 @@ ;; --- MUTATION: Delete Profile -(declare ^:private get-owned-teams-with-participants) +(declare ^:private get-owned-teams) (sv/defmethod ::delete-profile {::doc/added "1.0"} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id] :as params}] (db/with-atomic [conn pool] - (let [teams (get-owned-teams-with-participants conn profile-id) + (let [teams (get-owned-teams conn profile-id) deleted-at (dt/now)] ;; If we found owned teams with participants, we don't allow @@ -384,15 +384,18 @@ :hint "The user need to transfer ownership of owned teams." :context {:teams (mapv :id teams)})) - (doseq [{:keys [id]} teams] - (db/update! conn :team - {:deleted-at deleted-at} - {:id id})) - + ;; Mark profile deleted immediatelly (db/update! conn :profile {:deleted-at deleted-at} {:id profile-id}) + ;; Schedule cascade deletion to a worker + (wrk/submit! {::db/conn conn + ::wrk/task :delete-object + ::wrk/params {:object :profile + :deleted-at deleted-at + :id profile-id}}) + (rph/with-transform {} (session/delete-fn cfg))))) @@ -406,15 +409,14 @@ AND tpr.profile_id = ? ) SELECT tpr.team_id AS id, - count(tpr.profile_id) AS participants + count(tpr.profile_id) - 1 AS participants FROM team_profile_rel AS tpr WHERE tpr.team_id IN (SELECT id from owner_teams) - AND tpr.profile_id != ? GROUP BY 1") -(defn- get-owned-teams-with-participants +(defn get-owned-teams [conn profile-id] - (db/exec! conn [sql:owned-teams profile-id profile-id])) + (db/exec! conn [sql:owned-teams profile-id])) (def ^:private sql:profile-existence "select exists (select * from profile diff --git a/backend/src/app/tasks/delete_object.clj b/backend/src/app/tasks/delete_object.clj index 06bbd36f7..9c48d2309 100644 --- a/backend/src/app/tasks/delete_object.clj +++ b/backend/src/app/tasks/delete_object.clj @@ -10,6 +10,8 @@ [app.common.logging :as l] [app.db :as db] [app.rpc.commands.files :as files] + [app.rpc.commands.profile :as profile] + [app.util.time :as dt] [clojure.spec.alpha :as s] [integrant.core :as ig])) @@ -21,7 +23,9 @@ (defmethod delete-object :file [{:keys [::db/conn] :as cfg} {:keys [id deleted-at]}] (when-let [file (db/get* conn :file {:id id} {::db/remove-deleted false})] - (l/trc :hint "marking for deletion" :rel "file" :id (str id)) + (l/trc :hint "marking for deletion" :rel "file" :id (str id) + :deleted-at (dt/format-instant deleted-at)) + (db/update! conn :file {:deleted-at deleted-at} {:id id} @@ -53,7 +57,9 @@ (defmethod delete-object :project [{:keys [::db/conn] :as cfg} {:keys [id deleted-at]}] - (l/trc :hint "marking for deletion" :rel "project" :id (str id)) + (l/trc :hint "marking for deletion" :rel "project" :id (str id) + :deleted-at (dt/format-instant deleted-at)) + (db/update! conn :project {:deleted-at deleted-at} {:id id} @@ -68,7 +74,8 @@ (defmethod delete-object :team [{:keys [::db/conn] :as cfg} {:keys [id deleted-at]}] - (l/trc :hint "marking for deletion" :rel "team" :id (str id)) + (l/trc :hint "marking for deletion" :rel "team" :id (str id) + :deleted-at (dt/format-instant deleted-at)) (db/update! conn :team {:deleted-at deleted-at} {:id id} @@ -87,6 +94,20 @@ :object :project :deleted-at deleted-at))))) +(defmethod delete-object :profile + [{:keys [::db/conn] :as cfg} {:keys [id deleted-at]}] + (l/trc :hint "marking for deletion" :rel "profile" :id (str id) + :deleted-at (dt/format-instant deleted-at)) + + (db/update! conn :profile + {:deleted-at deleted-at} + {:id id} + {::db/return-keys false}) + + (doseq [team (profile/get-owned-teams conn id)] + (delete-object cfg (assoc team + :object :team + :deleted-at deleted-at)))) (defmethod delete-object :default [_cfg props] diff --git a/backend/src/app/tasks/objects_gc.clj b/backend/src/app/tasks/objects_gc.clj index 16f745836..0cfd66a61 100644 --- a/backend/src/app/tasks/objects_gc.clj +++ b/backend/src/app/tasks/objects_gc.clj @@ -35,11 +35,6 @@ ;; Mark as deleted the storage object (some->> photo-id (sto/touch-object! storage)) - ;; And finally, permanently delete the profile. The - ;; relevant objects will be deleted using DELETE - ;; CASCADE database triggers. This may leave orphan - ;; teams, but there is a special task for deleting - ;; orphaned teams. (db/delete! conn :profile {:id id}) (inc total)) @@ -269,15 +264,15 @@ 0))) (def ^:private deletion-proc-vars - [#'delete-file-media-objects! + [#'delete-profiles! + #'delete-file-media-objects! #'delete-file-data-fragments! #'delete-file-object-thumbnails! #'delete-file-thumbnails! #'delete-files! #'delete-projects! #'delete-fonts! - #'delete-teams! - #'delete-profiles!]) + #'delete-teams!]) (defn- execute-proc! "A generic function that executes the specified proc iterativelly diff --git a/backend/src/app/tasks/orphan_teams_gc.clj b/backend/src/app/tasks/orphan_teams_gc.clj deleted file mode 100644 index 0e3de973a..000000000 --- a/backend/src/app/tasks/orphan_teams_gc.clj +++ /dev/null @@ -1,65 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) KALEIDOS INC - -(ns app.tasks.orphan-teams-gc - "A maintenance task that performs orphan teams GC." - (:require - [app.common.logging :as l] - [app.db :as db] - [app.util.time :as dt] - [app.worker :as wrk] - [clojure.spec.alpha :as s] - [integrant.core :as ig])) - -(def ^:private sql:get-orphan-teams - "SELECT t.id - FROM team AS t - LEFT JOIN team_profile_rel AS tpr - ON (t.id = tpr.team_id) - WHERE tpr.profile_id IS NULL - AND t.deleted_at IS NULL - ORDER BY t.created_at ASC - FOR UPDATE OF t - SKIP LOCKED") - -(defn- delete-orphan-teams - "Find all orphan teams (with no members) and mark them for - deletion (soft delete)." - [{:keys [::db/conn] :as cfg}] - (let [deleted-at (dt/now)] - (->> (db/cursor conn sql:get-orphan-teams) - (map :id) - (reduce (fn [total team-id] - (l/trc :hint "mark orphan team for deletion" :id (str team-id)) - - (db/update! conn :team - {:deleted-at deleted-at} - {:id team-id}) - - (wrk/submit! (-> cfg - (assoc ::wrk/task :delete-object) - (assoc ::wrk/params {:object :team - :deleted-at deleted-at - :id team-id}))) - (inc total)) - 0)))) - -(defmethod ig/pre-init-spec ::handler [_] - (s/keys :req [::db/pool])) - -(defmethod ig/init-key ::handler - [_ cfg] - (fn [{:keys [props] :as task}] - (db/tx-run! cfg (fn [{:keys [::db/conn] :as cfg}] - (let [total (delete-orphan-teams cfg)] - (l/inf :hint "task finished" - :teams total - :rollback? (boolean (:rollback? props))) - - (when (:rollback? props) - (db/rollback! conn)) - - {:processed total}))))) diff --git a/backend/test/backend_tests/rpc_profile_test.clj b/backend/test/backend_tests/rpc_profile_test.clj index 82168ecaf..839494a17 100644 --- a/backend/test/backend_tests/rpc_profile_test.clj +++ b/backend/test/backend_tests/rpc_profile_test.clj @@ -153,23 +153,22 @@ (t/is (nil? (:error out))) (t/is (= 1 (count (:result out))))) - ;; execute permanent deletion task - (let [result (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 1 (:processed result)))) - - (let [row (th/db-get :team - {:id (:default-team-id prof)} - {::db/remove-deleted false})] - (t/is (nil? (:deleted-at row)))) - - (let [result (th/run-task! :orphan-teams-gc {:min-age 0})] - (t/is (= 1 (:processed result)))) + (th/run-pending-tasks!) (let [row (th/db-get :team {:id (:default-team-id prof)} {::db/remove-deleted false})] (t/is (dt/instant? (:deleted-at row)))) + ;; execute permanent deletion task + (let [result (th/run-task! :objects-gc {:min-age 0})] + (t/is (= 4 (:processed result)))) + + (let [row (th/db-get :team + {:id (:default-team-id prof)} + {::db/remove-deleted false})] + (t/is (nil? row))) + ;; query profile after delete (let [params {::th/type :get-profile ::rpc/profile-id (:id prof)} @@ -259,7 +258,6 @@ (t/is (= (:type edata) :validation)) (t/is (= (:code edata) :owner-teams-with-people)))) - ;; Leave team by role 0 (the default) and reassing owner to role 3 ;; without reassinging it (should fail) (let [params {::th/type :leave-team @@ -287,7 +285,7 @@ (t/is (nil? (:result out))) (t/is (nil? (:error out)))) - ;; Request profile to be deleted (it should fail) + ;; Request profile to be deleted (let [params {::th/type :delete-profile ::rpc/profile-id (:id prof1)} out (th/command! params)] @@ -305,22 +303,16 @@ (t/is (nil? (:error out))) (t/is (= 1 (count (:result out))))) + (th/run-pending-tasks!) + ;; execute permanent deletion task (let [result (th/run-task! :objects-gc {:min-age 0})] - (t/is (= 1 (:processed result)))) + (t/is (= 4 (:processed result)))) (let [row (th/db-get :team {:id (:default-team-id prof1)} {::db/remove-deleted false})] - (t/is (nil? (:deleted-at row)))) - - (let [result (th/run-task! :orphan-teams-gc {:min-age 0})] - (t/is (= 1 (:processed result)))) - - (let [row (th/db-get :team - {:id (:default-team-id prof1)} - {::db/remove-deleted false})] - (t/is (dt/instant? (:deleted-at row)))) + (t/is (nil? row))) ;; query profile after delete (let [params {::th/type :get-profile @@ -330,6 +322,33 @@ (let [result (:result out)] (t/is (= uuid/zero (:id result))))))) + +(t/deftest profile-deletion-4 + (let [prof1 (th/create-profile* 1) + file1 (th/create-file* 1 {:profile-id (:id prof1) + :project-id (:default-project-id prof1) + :is-shared false}) + team1 (th/create-team* 1 {:profile-id (:id prof1)}) + team2 (th/create-team* 2 {:profile-id (:id prof1)})] + + ;; Request profile to be deleted + (let [params {::th/type :delete-profile + ::rpc/profile-id (:id prof1)} + out (th/command! params)] + ;; (th/print-result! out) + (t/is (= {} (:result out))) + (t/is (nil? (:error out)))) + + (th/run-pending-tasks!) + + (let [rows (th/db-exec! ["select id,name,deleted_at from team where deleted_at is not null"])] + (t/is (= 3 (count rows)))) + + ;; execute permanent deletion task + (let [result (th/run-task! :objects-gc {:min-age 0})] + (t/is (= 8 (:processed result)))))) + + (t/deftest email-blacklist-1 (t/is (false? (email.blacklist/enabled? th/*system*))) (t/is (true? (email.blacklist/enabled? (assoc th/*system* :app.email/blacklist [])))) From ba721def26595453d929c2604cccbc922ad5a004 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 13:46:52 +0200 Subject: [PATCH 07/12] :sparkles: Add srepl helpers for profile deletion handling --- backend/src/app/srepl/main.clj | 39 +++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/backend/src/app/srepl/main.clj b/backend/src/app/srepl/main.clj index aed63f1cd..1c4c5430a 100644 --- a/backend/src/app/srepl/main.clj +++ b/backend/src/app/srepl/main.clj @@ -502,8 +502,6 @@ :restored) - - (defn- restore-project* [{:keys [::db/conn] :as cfg} project-id] @@ -535,6 +533,24 @@ :restored) +(defn- restore-profile* + [{:keys [::db/conn] :as cfg} profile-id] + (db/update! conn :profile + {:deleted-at nil} + {:id profile-id}) + + (doseq [{:keys [id]} (profile/get-owned-teams conn profile-id)] + (restore-team* cfg id)) + + :restored) + + +(defn restore-deleted-profile! + "Mark a team and all related objects as not deleted" + [profile-id] + (let [profile-id (h/parse-uuid profile-id)] + (db/tx-run! main/system restore-profile* profile-id))) + (defn restore-deleted-team! "Mark a team and all related objects as not deleted" [team-id] @@ -562,6 +578,15 @@ (assoc ::wrk/params {:object :team :deleted-at (dt/now) :id team-id}))))) +(defn delete-profile! + "Mark a profile for deletion" + [profile-id] + (let [profile-id (h/parse-uuid profile-id)] + (wrk/invoke! (-> main/system + (assoc ::wrk/task :delete-object) + (assoc ::wrk/params {:object :profile + :deleted-at (dt/now) + :id profile-id}))))) (defn delete-project! "Mark a project for deletion" [project-id] @@ -582,6 +607,15 @@ :deleted-at (dt/now) :id file-id}))))) +(defn process-deleted-profiles-cascade + [] + (->> (db/exec! main/system ["select id, deleted_at from profile where deleted_at is not null"]) + (run! (fn [{:keys [id deleted-at]}] + (wrk/invoke! (-> main/system + (assoc ::wrk/task :delete-object) + (assoc ::wrk/params {:object :profile + :deleted-at deleted-at + :id id}))))))) (defn process-deleted-teams-cascade [] @@ -593,7 +627,6 @@ :deleted-at deleted-at :id id}))))))) - (defn process-deleted-projects-cascade [] (->> (db/exec! main/system ["select id, deleted_at from project where deleted_at is not null"]) From 17015c53538fa073a4d411fff5882aa3c3795df9 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 13:52:23 +0200 Subject: [PATCH 08/12] :bug: Add missing logged-out event after account deletion --- frontend/src/app/main/data/users.cljs | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index bc2b7dbcd..d3e2592d2 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -570,6 +570,7 @@ (->> (rp/cmd! :delete-profile {}) (rx/tap on-success) (rx/catch on-error) + (rx/map logged-out) (rx/delay-at-least 300)))))) ;; --- EVENT: request-profile-recovery From 7facd69039d21edb577f70fafc9d57f88c0ab2cf Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 16:10:04 +0200 Subject: [PATCH 09/12] :bug: Set correct order of execution for logged-out event --- frontend/src/app/main/data/users.cljs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index d3e2592d2..8ba0f7547 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -569,8 +569,8 @@ on-success identity}} (meta params)] (->> (rp/cmd! :delete-profile {}) (rx/tap on-success) - (rx/catch on-error) (rx/map logged-out) + (rx/catch on-error) (rx/delay-at-least 300)))))) ;; --- EVENT: request-profile-recovery From f364666d48896cec4d8d7090383d47f6756764ef Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 16:10:26 +0200 Subject: [PATCH 10/12] :bug: Fix error handling on verify-token page --- .../src/app/main/ui/auth/verify_token.cljs | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/frontend/src/app/main/ui/auth/verify_token.cljs b/frontend/src/app/main/ui/auth/verify_token.cljs index a914bd46a..c2925b429 100644 --- a/frontend/src/app/main/ui/auth/verify_token.cljs +++ b/frontend/src/app/main/ui/auth/verify_token.cljs @@ -70,27 +70,28 @@ (rx/subs! (fn [tdata] (handle-token tdata)) - (fn [{:keys [type code] :as error}] - (cond - (or (= :validation type) - (= :invalid-token code) - (= :token-expired (:reason error))) - (reset! bad-token true) + (fn [cause] + (let [{:keys [type code] :as error} (ex-data cause)] + (cond + (or (= :validation type) + (= :invalid-token code) + (= :token-expired (:reason error))) + (reset! bad-token true) - (= :email-already-exists code) - (let [msg (tr "errors.email-already-exists")] - (ts/schedule 100 #(st/emit! (msg/error msg))) - (st/emit! (rt/nav :auth-login))) + (= :email-already-exists code) + (let [msg (tr "errors.email-already-exists")] + (ts/schedule 100 #(st/emit! (msg/error msg))) + (st/emit! (rt/nav :auth-login))) - (= :email-already-validated code) - (let [msg (tr "errors.email-already-validated")] - (ts/schedule 100 #(st/emit! (msg/warn msg))) - (st/emit! (rt/nav :auth-login))) + (= :email-already-validated code) + (let [msg (tr "errors.email-already-validated")] + (ts/schedule 100 #(st/emit! (msg/warn msg))) + (st/emit! (rt/nav :auth-login))) - :else - (let [msg (tr "errors.generic")] - (ts/schedule 100 #(st/emit! (msg/error msg))) - (st/emit! (rt/nav :auth-login)))))))) + :else + (let [msg (tr "errors.generic")] + (ts/schedule 100 #(st/emit! (msg/error msg))) + (st/emit! (rt/nav :auth-login))))))))) (if @bad-token [:> static/invalid-token {}] From c45a105186df74cc214f92f7ab609d0fda8fe602 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 16:10:04 +0200 Subject: [PATCH 11/12] :bug: Set correct order of execution for logged-out event --- frontend/src/app/main/data/users.cljs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index d84da5e52..7879ed603 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -537,8 +537,8 @@ on-success identity}} (meta params)] (->> (rp/cmd! :delete-profile {}) (rx/tap on-success) - (rx/catch on-error) (rx/map logged-out) + (rx/catch on-error) (rx/delay-at-least 300)))))) ;; --- EVENT: request-profile-recovery From 56160cf64d1568098c0a35f3107ef82703b5525e Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 27 Jun 2024 16:10:26 +0200 Subject: [PATCH 12/12] :bug: Fix error handling on verify-token page --- .../src/app/main/ui/auth/verify_token.cljs | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/frontend/src/app/main/ui/auth/verify_token.cljs b/frontend/src/app/main/ui/auth/verify_token.cljs index a914bd46a..c2925b429 100644 --- a/frontend/src/app/main/ui/auth/verify_token.cljs +++ b/frontend/src/app/main/ui/auth/verify_token.cljs @@ -70,27 +70,28 @@ (rx/subs! (fn [tdata] (handle-token tdata)) - (fn [{:keys [type code] :as error}] - (cond - (or (= :validation type) - (= :invalid-token code) - (= :token-expired (:reason error))) - (reset! bad-token true) + (fn [cause] + (let [{:keys [type code] :as error} (ex-data cause)] + (cond + (or (= :validation type) + (= :invalid-token code) + (= :token-expired (:reason error))) + (reset! bad-token true) - (= :email-already-exists code) - (let [msg (tr "errors.email-already-exists")] - (ts/schedule 100 #(st/emit! (msg/error msg))) - (st/emit! (rt/nav :auth-login))) + (= :email-already-exists code) + (let [msg (tr "errors.email-already-exists")] + (ts/schedule 100 #(st/emit! (msg/error msg))) + (st/emit! (rt/nav :auth-login))) - (= :email-already-validated code) - (let [msg (tr "errors.email-already-validated")] - (ts/schedule 100 #(st/emit! (msg/warn msg))) - (st/emit! (rt/nav :auth-login))) + (= :email-already-validated code) + (let [msg (tr "errors.email-already-validated")] + (ts/schedule 100 #(st/emit! (msg/warn msg))) + (st/emit! (rt/nav :auth-login))) - :else - (let [msg (tr "errors.generic")] - (ts/schedule 100 #(st/emit! (msg/error msg))) - (st/emit! (rt/nav :auth-login)))))))) + :else + (let [msg (tr "errors.generic")] + (ts/schedule 100 #(st/emit! (msg/error msg))) + (st/emit! (rt/nav :auth-login))))))))) (if @bad-token [:> static/invalid-token {}]