diff --git a/CHANGES.md b/CHANGES.md index a9550c037d..e235a8f009 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -67,6 +67,7 @@ is a number of cores) - Fix problem with grid layout crashing [Taiga #10127](https://tree.taiga.io/project/penpot/issue/10127) - Fix rename locked boards [Taiga #10174](https://tree.taiga.io/project/penpot/issue/10174) - Fix update-libraries dialog disappear when clicking outside [Taiga #10238](https://tree.taiga.io/project/penpot/issue/10238) +- Fix incorrect handling of team access requests with deleted/recreated users ## 2.4.3 diff --git a/backend/resources/app/email/request-file-access-yourpenpot-view/en.html b/backend/resources/app/email/request-file-access-yourpenpot-view/en.html index 11f8b8dea1..e80d46488a 100644 --- a/backend/resources/app/email/request-file-access-yourpenpot-view/en.html +++ b/backend/resources/app/email/request-file-access-yourpenpot-view/en.html @@ -207,7 +207,7 @@ - SEND A VIEW-ONLY LINK @@ -251,4 +251,4 @@ - \ No newline at end of file + diff --git a/backend/resources/app/email/request-file-access-yourpenpot-view/en.txt b/backend/resources/app/email/request-file-access-yourpenpot-view/en.txt index 67eb6cedf7..397f3c821d 100644 --- a/backend/resources/app/email/request-file-access-yourpenpot-view/en.txt +++ b/backend/resources/app/email/request-file-access-yourpenpot-view/en.txt @@ -6,7 +6,7 @@ Since this file is in your Penpot team, you can provide access by sending a view To proceed, please click the link below to generate and send the view-only link: -{{ public-uri }}/#/view/{{file-id}}?page-id={{page-id}}§ion=interactions&index=0&share=true +{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}§ion=interactions&index=0&share=true diff --git a/backend/resources/app/email/request-file-access-yourpenpot/en.html b/backend/resources/app/email/request-file-access-yourpenpot/en.html index 72d8e42821..596e59bf32 100644 --- a/backend/resources/app/email/request-file-access-yourpenpot/en.html +++ b/backend/resources/app/email/request-file-access-yourpenpot/en.html @@ -230,9 +230,9 @@ - SEND A VIEW-ONLY LINK + SEND A VIEW-ONLY LINK @@ -274,4 +274,4 @@ - \ No newline at end of file + diff --git a/backend/resources/app/email/request-file-access-yourpenpot/en.txt b/backend/resources/app/email/request-file-access-yourpenpot/en.txt index 140cb04453..81f5d5c72e 100644 --- a/backend/resources/app/email/request-file-access-yourpenpot/en.txt +++ b/backend/resources/app/email/request-file-access-yourpenpot/en.txt @@ -19,7 +19,7 @@ Alternatively, you can create and share a view-only link to the file. This will Click the link below to generate and send the link: -{{ public-uri }}/#/view/{{file-id}}?page-id={{page-id}}§ion=interactions&index=0&share=true +{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}§ion=interactions&index=0&share=true diff --git a/backend/resources/app/email/request-file-access/en.html b/backend/resources/app/email/request-file-access/en.html index 4ea4acfcc9..bda16c658d 100644 --- a/backend/resources/app/email/request-file-access/en.html +++ b/backend/resources/app/email/request-file-access/en.html @@ -214,7 +214,7 @@ - GIVE ACCESS TO “{{team-name|abbreviate:25}}” TEAM @@ -247,9 +247,9 @@ - SEND A VIEW-ONLY LINK + SEND A VIEW-ONLY LINK @@ -292,4 +292,4 @@ - \ No newline at end of file + diff --git a/backend/resources/app/email/request-file-access/en.txt b/backend/resources/app/email/request-file-access/en.txt index d327e47803..d12e44ee7a 100644 --- a/backend/resources/app/email/request-file-access/en.txt +++ b/backend/resources/app/email/request-file-access/en.txt @@ -13,7 +13,7 @@ This will automatically include {{requested-by|abbreviate:25}} in the team, so t Click the link below to provide team access: -{{ public-uri }}/#/dashboard/team/{{team-id}}/members?invite-email={{requested-by-email|urlescape}} +{{ public-uri }}/#/dashboard/members?team-id{{team-id}}&invite-email={{requested-by-email|urlescape}} @@ -23,8 +23,7 @@ Alternatively, you can create and share a view-only link to the file. This will Click the link below to generate and send the link: -{{ public-uri }}/#/view/{{file-id}}?page-id={{page-id}}§ion=interactions&index=0&share=true - +{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}§ion=interactions&index=0&share=true If you do not wish to grant access at this time, you can simply disregard this email. diff --git a/backend/resources/app/email/request-team-access/en.html b/backend/resources/app/email/request-team-access/en.html index 2d9e266488..b8045fa1e3 100644 --- a/backend/resources/app/email/request-team-access/en.html +++ b/backend/resources/app/email/request-team-access/en.html @@ -205,7 +205,7 @@ - GIVE ACCESS TO “{{team-name|abbreviate:25}}” TEAM @@ -249,4 +249,4 @@ - \ No newline at end of file + diff --git a/backend/resources/app/email/request-team-access/en.txt b/backend/resources/app/email/request-team-access/en.txt index 225bc1e26f..c05d2b8693 100644 --- a/backend/resources/app/email/request-team-access/en.txt +++ b/backend/resources/app/email/request-team-access/en.txt @@ -4,7 +4,7 @@ Hello! To provide access, please click the link below: -{{ public-uri }}/#/dashboard/team/{{team-id}}/members?invite-email={{requested-by-email|urlescape}} +{{ public-uri }}/#/dashboard/members?team-id={{team-id}}&invite-email={{requested-by-email|urlescape}} If you do not wish to grant access at this time, you can simply disregard this email. diff --git a/backend/src/app/rpc/commands/teams_invitations.clj b/backend/src/app/rpc/commands/teams_invitations.clj index f8a0dfbcf5..12669d9bb2 100644 --- a/backend/src/app/rpc/commands/teams_invitations.clj +++ b/backend/src/app/rpc/commands/teams_invitations.clj @@ -6,6 +6,7 @@ (ns app.rpc.commands.teams-invitations (:require + [app.common.data :as d] [app.common.data.macros :as dm] [app.common.exceptions :as ex] [app.common.features :as cfeat] @@ -15,7 +16,6 @@ [app.common.uuid :as uuid] [app.config :as cf] [app.db :as db] - [app.db.sql :as sql] [app.email :as eml] [app.loggers.audit :as audit] [app.main :as-alias main] @@ -168,19 +168,16 @@ itoken)))) -(defn- add-user-to-team - [conn profile team role email] +(defn- add-member-to-team + [conn profile team role member] (let [team-id (:id team) - member (db/get* conn :profile - {:email (str/lower email)} - {::sql/columns [:id :email]}) params (merge {:team-id team-id :profile-id (:id member)} (get types.team/permissions-for-role role))] - ;; Do not allow blocked users to join teams. + ;; Do not allow blocked users to join teams. (when (:is-blocked member) (ex/raise :type :restriction :code :profile-blocked)) @@ -205,29 +202,33 @@ (eml/send! {::eml/conn conn ::eml/factory eml/join-team :public-uri (cf/get :public-uri) - :to email + :to (:email member) :invited-by (:fullname profile) :team (:name team) :team-id (:id team)}))) -(def sql:valid-requests-email - "SELECT p.email +(def ^:private sql:valid-access-request-profiles + "SELECT p.id, p.email, p.is_blocked FROM team_access_request AS tr JOIN profile AS p ON (tr.requester_id = p.id) WHERE tr.team_id = ? - AND tr.auto_join_until > now()") + AND tr.auto_join_until > now() + AND (p.deleted_at IS NULL OR + p.deleted_at > now())") -(defn- get-valid-requests-email +(defn- get-valid-access-request-profiles [conn team-id] - (db/exec! conn [sql:valid-requests-email team-id])) + (db/exec! conn [sql:valid-access-request-profiles team-id])) -(def ^:private xf:map-email - (map :email)) +(def ^:private xf:map-email (map :email)) (defn- create-team-invitations [{:keys [::db/conn] :as cfg} {:keys [profile team role emails] :as params}] - (let [join-requests (into #{} xf:map-email - (get-valid-requests-email conn (:id team))) + (let [emails (set emails) + + join-requests (->> (get-valid-access-request-profiles conn (:id team)) + (d/index-by :email)) + team-members (into #{} xf:map-email (teams/get-team-members conn (:id team))) @@ -245,8 +246,10 @@ ;; For requested invitations, do not send invitation emails, add ;; the user directly to the team - (->> (filter join-requests emails) - (run! (partial add-user-to-team conn profile team role))) + (->> join-requests + (filter #(contains? emails (key %))) + (map val) + (run! (partial add-member-to-team conn profile team role))) invitations)) @@ -572,5 +575,3 @@ (with-meta {:request request} {::audit/props {:request 1}})))) - - diff --git a/backend/test/backend_tests/rpc_team_test.clj b/backend/test/backend_tests/rpc_team_test.clj index dd614151e8..66fa5d74ce 100644 --- a/backend/test/backend_tests/rpc_team_test.clj +++ b/backend/test/backend_tests/rpc_team_test.clj @@ -37,18 +37,17 @@ :role :editor}] ;; invite external user without complaints - (let [data (assoc data :emails ["foo@bar.com"]) - out (th/command! data) + (let [data (assoc data :emails ["foo@bar.com"]) + out (th/command! data) ;; retrieve the value from the database and check its content - invitation (db/exec-one! - th/*pool* - ["select count(*) as num from team_invitation where team_id = ? and email_to = ?" - (:team-id data) "foo@bar.com"])] + invitations (th/db-query :team-invitation + {:team-id (:team-id data) + :email-to "foo@bar.com"})] ;; (th/print-result! out) (t/is (th/success? out)) (t/is (= 1 (:call-count (deref mock)))) - (t/is (= 1 (:num invitation)))) + (t/is (= 1 (count invitations)))) ;; invite internal user without complaints (th/reset-mock! mock) @@ -102,6 +101,105 @@ (t/is (= :validation (:type edata))) (t/is (= :member-is-muted (:code edata)))))))) +(t/deftest create-team-invitations-with-request-access + (with-mocks [mock {:target 'app.email/send! :return nil}] + (let [profile1 (th/create-profile* 1 {:is-active true}) + requester (th/create-profile* 2 {:is-active true :email "requester@example.com"}) + + team (th/create-team* 1 {:profile-id (:id profile1)}) + proj (th/create-project* 1 {:profile-id (:id profile1) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id profile1) + :project-id (:id proj)})] + (let [data {::th/type :create-team-access-request + ::rpc/profile-id (:id requester) + :file-id (:id file)} + out (th/command! data)] + (t/is (th/success? out)) + (t/is (= 1 (:call-count @mock)))) + + (th/reset-mock! mock) + + (let [data {::th/type :create-team-invitations + ::rpc/profile-id (:id profile1) + :team-id (:id team) + :role :editor + :emails ["requester@example.com"]} + out (th/command! data)] + (t/is (th/success? out)) + (t/is (= 1 (:call-count @mock))) + + ;; Check that request is properly removed + (let [requests (th/db-query :team-access-request + {:requester-id (:id requester)})] + (t/is (= 0 (count requests)))) + + (let [rows (th/db-query :team-profile-rel {:team-id (:id team)})] + (t/is (= 2 (count rows)))))))) + + +(t/deftest create-team-invitations-with-request-access-2 + (with-mocks [mock {:target 'app.email/send! :return nil}] + (let [profile1 (th/create-profile* 1 {:is-active true}) + requester (th/create-profile* 2 {:is-active true + :email "requester@example.com"}) + + team (th/create-team* 1 {:profile-id (:id profile1)}) + proj (th/create-project* 1 {:profile-id (:id profile1) + :team-id (:id team)}) + file (th/create-file* 1 {:profile-id (:id profile1) + :project-id (:id proj)})] + + ;; Create the first access request + (let [data {::th/type :create-team-access-request + ::rpc/profile-id (:id requester) + :file-id (:id file)} + out (th/command! data)] + (t/is (th/success? out)) + (t/is (= 1 (:call-count @mock)))) + + (th/reset-mock! mock) + + ;; Proceed to delete the requester user + (th/db-update! :profile + {:deleted-at (dt/in-past "1h")} + {:id (:id requester)}) + + ;; Create a new profile with the same email + (let [requester' (th/create-profile* 3 {:is-active true :email "requester@example.com"})] + + ;; Create a request access with new requester + (let [data {::th/type :create-team-access-request + ::rpc/profile-id (:id requester') + :file-id (:id file)} + out (th/command! data)] + (t/is (th/success? out)) + (t/is (= 1 (:call-count @mock)))) + + (th/reset-mock! mock) + + ;; Create an invitation for the requester email + (let [data {::th/type :create-team-invitations + ::rpc/profile-id (:id profile1) + :team-id (:id team) + :role :editor + :emails ["requester@example.com"]} + out (th/command! data)] + (t/is (th/success? out)) + (t/is (= 1 (:call-count @mock)))) + + ;; Check that request is properly removed + (let [requests (th/db-query :team-access-request + {:requester-id (:id requester')})] + (t/is (= 0 (count requests)))) + + (let [[r1 r2 :as rows] (th/db-query :team-profile-rel + {:team-id (:id team)} + {:order-by [:created-at]})] + (t/is (= 2 (count rows))) + (t/is (= (:profile-id r1) (:id profile1))) + (t/is (= (:profile-id r2) (:id requester')))))))) + (t/deftest invitation-tokens (with-mocks [mock {:target 'app.email/send! :return nil}] @@ -486,14 +584,12 @@ ;; request success (let [out (th/command! data) ;; retrieve the value from the database and check its content - request (db/exec-one! - th/*pool* - ["select count(*) as num from team_access_request where team_id = ? and requester_id = ?" - (:id team) (:id requester)])] - + requests (th/db-query :team-access-request + {:team-id (:id team) + :requester-id (:id requester)})] (t/is (th/success? out)) (t/is (= 1 (:call-count @mock))) - (t/is (= 1 (:num request)))) + (t/is (= 1 (count requests)))) ;; request again fails (th/reset-mock! mock) @@ -509,10 +605,10 @@ ;; request again when is expired success (th/reset-mock! mock) - (db/exec-one! - th/*pool* - ["update team_access_request set valid_until = ? where team_id = ? and requester_id = ?" - (dt/in-past "1h") (:id team) (:id requester)]) + (th/db-update! :team-access-request + {:valid-until (dt/in-past "1h")} + {:team-id (:id team) + :requester-id (:id requester)}) (t/is (th/success? (th/command! data))) (t/is (= 1 (:call-count @mock)))))) diff --git a/frontend/src/app/main/data/profile.cljs b/frontend/src/app/main/data/profile.cljs index 1427e23174..c69a3fa455 100644 --- a/frontend/src/app/main/data/profile.cljs +++ b/frontend/src/app/main/data/profile.cljs @@ -72,6 +72,7 @@ (def profile-fetched? (ptk/type? ::profile-fetched)) +;; FIXME: make it as general purpose handler, not only on profile (defn- on-fetch-profile-exception [cause] (let [data (ex-data cause)] diff --git a/frontend/src/app/main/ui/routes.cljs b/frontend/src/app/main/ui/routes.cljs index 03fa26bb87..6e04498d8d 100644 --- a/frontend/src/app/main/ui/routes.cljs +++ b/frontend/src/app/main/ui/routes.cljs @@ -109,7 +109,11 @@ ;; avoids some race conditions that causes unexpected redirects ;; on invitations workflows (and probably other cases). (->> (rp/cmd! :get-profile) - (rx/subs! (fn [{:keys [id] :as profile}] + (rx/mapcat (fn [profile] + (->> (rp/cmd! :get-teams {}) + (rx/map (fn [teams] + (assoc profile ::teams (into #{} (map :id) teams))))))) + (rx/subs! (fn [{:keys [id ::teams] :as profile}] (cond (= id uuid/zero) (do @@ -117,10 +121,12 @@ (st/emit! (rt/nav :auth-login))) empty-path? - (let [team-id (or (dtm/get-last-team-id) - (:default-team-id profile))] - (st/emit! (rt/nav :dashboard-recent - (assoc query-params :team-id team-id)))) + (let [team-id (dtm/get-last-team-id)] + (if (contains? teams team-id) + (st/emit! (rt/nav :dashboard-recent + (assoc query-params :team-id team-id))) + (st/emit! (rt/nav :dashboard-recent + (assoc query-params :team-id (:default-team-id profile)))))) :else (st/emit! (rt/assign-exception {:type :not-found})))))))))