🐛 Fix incorrect handling request access with deleted profiles

* 📎 Add minor improvements to team tests

* 🐛 Fix incorrect handling request access with deleted profiles

* 🐛 Fix redirect loop on empty route

Happens when the current profile is deleted from team

* 🐛 Fix urls on request access emails

* 📎 Revert url changes on emails

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
 

backend/resources/app/email/request-file-access-yourpenpot-view/en.html

@@ -207,7 +207,7 @@
                           <td align="center" bgcolor="#6911d4" role="presentation"
                             style="border:none;border-radius:8px;cursor:auto;mso-padding-alt:10px 25px;background:#6911d4;"
                             valign="middle">
-                            <a href="{{ public-uri }}/#/view/{{file-id}}?page-id={{page-id}}&section=interactions&index=0&share=true"
+                            <a href="{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}&section=interactions&index=0&share=true"
                               style="display:inline-block;background:#6911d4;color:#FFFFFF;font-family:Source Sans Pro, sans-serif;font-size:16px;font-weight:normal;line-height:120%;margin:0;text-decoration:none;text-transform:none;padding:10px 25px;mso-padding-alt:0px;border-radius:8px;"
                               target="_blank"> SEND A VIEW-ONLY LINK </a>
                           </td>

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}}&section=interactions&index=0&share=true
+{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}&section=interactions&index=0&share=true
 
 
 

backend/resources/app/email/request-file-access-yourpenpot/en.html

@@ -230,7 +230,7 @@
                           <td align="center" bgcolor="#6911d4" role="presentation"
                             style="border:none;border-radius:8px;cursor:auto;mso-padding-alt:10px 25px;background:#6911d4;"
                             valign="middle">
-                            <a href="{{ public-uri }}/#/view/{{file-id}}?page-id={{page-id}}&section=interactions&index=0&share=true"
+                            <a href="{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}&section=interactions&index=0&share=true"
                                style="display:inline-block;background:#6911d4;color:#FFFFFF;font-family:Source Sans Pro, sans-serif;font-size:16px;font-weight:normal;line-height:120%;margin:0;text-decoration:none;text-transform:none;padding:10px 25px;mso-padding-alt:0px;border-radius:8px;"
                                target="_blank"> SEND A VIEW-ONLY LINK </a>
                           </td>

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}}&section=interactions&index=0&share=true
+{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}&section=interactions&index=0&share=true
 
 
 

backend/resources/app/email/request-file-access/en.html

@@ -214,7 +214,7 @@
                           <td align="center" bgcolor="#6911d4" role="presentation"
                             style="border:none;border-radius:8px;cursor:auto;mso-padding-alt:10px 25px;background:#6911d4;"
                             valign="middle">
-                            <a href="{{ public-uri }}/#/dashboard/team/{{team-id}}/members?invite-email={{requested-by-email|urlescape }}"
+                            <a href="{{ public-uri }}/#/dashboard/members?team-id={{team-id}}&invite-email={{requested-by-email|urlescape }}"
                               style="display:inline-block;background:#6911d4;color:#FFFFFF;font-family:Source Sans Pro, sans-serif;font-size:16px;font-weight:normal;line-height:120%;margin:0;text-decoration:none;text-transform:none;padding:10px 25px;mso-padding-alt:0px;border-radius:8px;"
                               target="_blank"> GIVE ACCESS TO “{{team-name|abbreviate:25}}” TEAM </a>
                           </td>
@@ -247,7 +247,7 @@
                           <td align="center" bgcolor="#6911d4" role="presentation"
                             style="border:none;border-radius:8px;cursor:auto;mso-padding-alt:10px 25px;background:#6911d4;"
                             valign="middle">
-                            <a href="{{ public-uri }}/#/view/{{file-id}}?page-id={{page-id}}&section=interactions&index=0&share=true"
+                            <a href="{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}&section=interactions&index=0&share=true"
                                style="display:inline-block;background:#6911d4;color:#FFFFFF;font-family:Source Sans Pro, sans-serif;font-size:16px;font-weight:normal;line-height:120%;margin:0;text-decoration:none;text-transform:none;padding:10px 25px;mso-padding-alt:0px;border-radius:8px;"
                                target="_blank"> SEND A VIEW-ONLY LINK </a>
                           </td>

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}}&section=interactions&index=0&share=true
+{{ public-uri }}/#/view?file-id={{file-id}}&page-id={{page-id}}&section=interactions&index=0&share=true
-
 
 
 If you do not wish to grant access at this time, you can simply disregard this email.

backend/resources/app/email/request-team-access/en.html

@@ -205,7 +205,7 @@
                           <td align="center" bgcolor="#6911d4" role="presentation"
                             style="border:none;border-radius:8px;cursor:auto;mso-padding-alt:10px 25px;background:#6911d4;"
                             valign="middle">
-                            <a href="{{ public-uri }}/#/dashboard/team/{{team-id}}/members?invite-email={{requested-by-email|urlescape}}"
+                            <a href="{{ public-uri }}/#/dashboard/members?team-id={{team-id}}&invite-email={{requested-by-email|urlescape}}"
                               style="display:inline-block;background:#6911d4;color:#FFFFFF;font-family:Source Sans Pro, sans-serif;font-size:16px;font-weight:normal;line-height:120%;margin:0;text-decoration:none;text-transform:none;padding:10px 25px;mso-padding-alt:0px;border-radius:8px;"
                               target="_blank"> GIVE ACCESS TO “{{team-name|abbreviate:25}}” TEAM </a>
                           </td>

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.

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,13 +168,10 @@
 
         itoken))))
 
-(defn- add-user-to-team
+(defn- add-member-to-team
-  [conn profile team role email]
+  [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)}
@@ -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
+(def ^:private sql:valid-access-request-profiles
-  "SELECT p.email
+  "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
+(def ^:private xf:map-email (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
+  (let [emails           (set emails)
-                               (get-valid-requests-email conn (:id team)))
+
+        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)
+    (->> join-requests
-         (run! (partial add-user-to-team conn profile team role)))
+         (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}}))))
-
-

backend/test/backend_tests/rpc_team_test.clj

@@ -40,15 +40,14 @@
       (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!
+            invitations (th/db-query :team-invitation
-                        th/*pool*
+                                     {:team-id (:team-id data)
-                        ["select count(*) as num from team_invitation where team_id = ? and email_to = ?"
+                                      :email-to "foo@bar.com"})]
-                         (:team-id data) "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!
+            requests   (th/db-query :team-access-request
-                        th/*pool*
+                                    {:team-id (:id team)
-                        ["select count(*) as num from team_access_request where team_id = ? and requester_id = ?"
+                                     :requester-id (:id requester)})]
-                         (:id team) (: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/db-update! :team-access-request
-       th/*pool*
+                     {:valid-until (dt/in-past "1h")}
-       ["update team_access_request set valid_until = ? where team_id = ? and requester_id = ?"
+                     {:team-id (:id team)
-        (dt/in-past "1h") (:id team) (:id requester)])
+                      :requester-id (:id requester)})
 
       (t/is (th/success? (th/command! data)))
       (t/is (= 1 (:call-count @mock))))))

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)]

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)
+                         (let [team-id (dtm/get-last-team-id)]
-                                           (:default-team-id profile))]
+                           (if (contains? teams team-id)
                              (st/emit! (rt/nav :dashboard-recent
-                                             (assoc query-params :team-id team-id))))
+                                               (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})))))))))