diff --git a/.serena/memories/backend/auth-permissions-product-domains.md b/.serena/memories/backend/auth-permissions-product-domains.md index fc9dc51bbf..9d7c5d6725 100644 --- a/.serena/memories/backend/auth-permissions-product-domains.md +++ b/.serena/memories/backend/auth-permissions-product-domains.md @@ -27,6 +27,7 @@ - Team/project commands mix DB changes, email, message bus notifications, media/storage cleanup, feature flags, quotas, and audit metadata. Keep mutations transactional when the existing command does so. - Invitation flows validate muted/bounced emails before sending and use tokenized invitation state. Accepting an invitation is tied to the invited member identity, not just possession of a token. +- The whole invitation family is team-admin only: creating, updating and deleting invitations, minting the invitation token (`get-team-invitation-token`) and listing the pending ones (`get-team-invitations`, which carries the invited email and the offered role). The token of an invitation that grants the `owner` role is restricted to the team owner, because an admin cannot create nor promote an owner invitation (`cant-promote-to-owner`). This family is the exception to the rule above: it answers `:validation` / `:insufficient-permissions` instead of `:not-found`, so the five commands speak one language. - Logical deletion is used for many product objects; prefer existing logical-deletion helpers over hard deletes unless the command already performs permanent cleanup. - Bounced/spam-complaint emails can mute/block a profile for login/registration and email sending. Devenv Mailpit is the normal local path for registration/email-flow testing. diff --git a/.serena/memories/frontend/dashboard-viewer-subtleties.md b/.serena/memories/frontend/dashboard-viewer-subtleties.md index dfdff4af18..779d32e1ee 100644 --- a/.serena/memories/frontend/dashboard-viewer-subtleties.md +++ b/.serena/memories/frontend/dashboard-viewer-subtleties.md @@ -6,6 +6,7 @@ - Project fetch replaces each project map completely instead of merging, so fields such as `deleted-at` can disappear cleanly. - Dashboard file/project mutations are often optimistic local updates with fire-and-forget RPC watchers. Bulk permanent delete/restore paths use SSE progress and progress notifications. - File creation/duplication strips file `:data` before putting file summaries into dashboard state. +- The pending invitations are a team-admin only list (backend `:is-admin` already includes the owner). The dashboard hides the invitations entry from the header tabs and the team-options dropdown for non-admins, `invitations-section-content` keys the section on `:is-admin`, and `fetch-invitations` answers `insufficient-permissions` by emitting `invitations-fetched` with `[]`, so a demoted admin does not keep the cached emails and the header does not paint the invite button from a stale list. The organization `send-invitations` policy only gates inviting and resending, not the list. ## Viewer diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index 255111c6f9..1611589578 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -85,6 +85,17 @@ (def check-read-permissions! (perms/make-check-fn has-read-permissions?)) +(defn check-invitation-permissions + "Returns the team permissions of `profile-id`, raising the error of the + invitation family when they are not the admin ones that every invitation + command requires. Accepts a connection, a pool or a cfg." + [conn profile-id team-id] + (let [perms (get-permissions conn profile-id team-id)] + (when-not (:is-admin perms) + (ex/raise :type :validation + :code :insufficient-permissions)) + perms)) + (defn decode-row [{:keys [features subscription] :as row}] (when row @@ -489,8 +500,9 @@ {::doc/added "1.17" ::sm/params schema:get-team-invitations} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id team-id]}] + (check-invitation-permissions cfg profile-id team-id) + (dm/with-open [conn (db/open pool)] - (check-read-permissions! cfg profile-id team-id) (get-team-invitations conn team-id))) diff --git a/backend/src/app/rpc/commands/teams_invitations.clj b/backend/src/app/rpc/commands/teams_invitations.clj index 66ac06493d..e33ac6475f 100644 --- a/backend/src/app/rpc/commands/teams_invitations.clj +++ b/backend/src/app/rpc/commands/teams_invitations.clj @@ -485,7 +485,7 @@ ::doc/module :teams ::sm/params schema:create-team-invitations} [cfg {:keys [::rpc/profile-id team-id role emails] :as params}] - (let [perms (teams/get-permissions cfg profile-id team-id) + (let [perms (teams/check-invitation-permissions cfg profile-id team-id) profile (db/get-by-id cfg :profile profile-id) team (db/get-by-id cfg :team team-id) team-with-org (when (contains? cf/flags :admin-console) @@ -502,10 +502,6 @@ (count emails) (count (:invitations params)))] - (when-not (:is-admin perms) - (ex/raise :type :validation - :code :insufficient-permissions)) - (when (and (contains? cf/flags :admin-console) organization (not (cto/allowed? :send-invitations @@ -631,26 +627,43 @@ ::doc/module :teams ::sm/params schema:get-team-invitation-token} [{:keys [::db/pool] :as cfg} {:keys [::rpc/profile-id team-id email] :as params}] - (teams/check-edition-permissions! cfg profile-id team-id) - (let [email (profile/clean-email email) + ;; The token is a bearer credential that accepts the invitation with the + ;; role of the invitation row, so it takes the same permissions as the + ;; rest of the invitation commands. + (let [perms (teams/check-invitation-permissions cfg profile-id team-id) + email (profile/clean-email email) invit (-> (db/get pool :team-invitation {:team-id team-id :email-to email}) - (update :role keyword)) + (update :role keyword))] - member (profile/get-profile-by-email pool (:email-to invit)) - token (create-invitation-token cfg {:team-id (:team-id invit) - ;; The inviter is the creator of - ;; the invitation row, not the - ;; profile requesting the token. - :profile-id (or (:created-by invit) - profile-id) - :valid-until (:valid-until invit) - :role (:role invit) - :member-id (:id member) - :member-email (or (:email member) - (profile/clean-email (:email-to invit)))})] - {:token token})) + ;; An admin cannot create nor promote an owner invitation, so it + ;; cannot mint its token either: only the team owner can. + (when (and (= :owner (:role invit)) + (not (:is-owner perms))) + (ex/raise :type :validation + :code :cant-promote-to-owner + :hint "only the team owner can get an owner invitation token")) + + (let [member (profile/get-profile-by-email pool (:email-to invit)) + token (create-invitation-token cfg {:team-id (:team-id invit) + ;; The inviter is the creator of + ;; the invitation row, not the + ;; profile requesting the token. + :profile-id (or (:created-by invit) + profile-id) + :valid-until (:valid-until invit) + :role (:role invit) + :member-id (:id member) + :member-email (or (:email member) + (profile/clean-email (:email-to invit)))})] + ;; The event is named as an action and carries the granted role, so + ;; the minting of the token is traceable in the audit log. + (rph/with-meta + {:token token} + {::audit/name "create-team-invitation-token" + ::audit/props {:role (:role invit) + :invitation-id (:id invit)}})))) ;; --- Mutation: Update invitation role @@ -666,11 +679,7 @@ ::sm/params schema:update-team-invitation-role ::db/transaction true} [{:keys [::db/conn]} {:keys [::rpc/profile-id team-id email role] :as params}] - (let [perms (teams/get-permissions conn profile-id team-id)] - - (when-not (:is-admin perms) - (ex/raise :type :validation - :code :insufficient-permissions)) + (let [perms (teams/check-invitation-permissions conn profile-id team-id)] ;; Don't allow promote to owner to admin users. (when (and (not (:is-owner perms)) (= role :owner)) @@ -695,17 +704,13 @@ ::sm/params schema:delete-team-invition ::db/transaction true} [{:keys [::db/conn]} {:keys [::rpc/profile-id team-id email] :as params}] - (let [perms (teams/get-permissions conn profile-id team-id)] + (teams/check-invitation-permissions conn profile-id team-id) - (when-not (:is-admin perms) - (ex/raise :type :validation - :code :insufficient-permissions)) - - (let [invitation (db/delete! conn :team-invitation - {:team-id team-id - :email-to (profile/clean-email email)} - {::db/return-keys true})] - (rph/wrap nil {::audit/props {:invitation-id (:id invitation)}})))) + (let [invitation (db/delete! conn :team-invitation + {:team-id team-id + :email-to (profile/clean-email email)} + {::db/return-keys true})] + (rph/wrap nil {::audit/props {:invitation-id (:id invitation)}}))) ;; --- Mutation: Request Team Invitation diff --git a/backend/test/backend_tests/rpc_team_test.clj b/backend/test/backend_tests/rpc_team_test.clj index 10638cee1d..06318a1a05 100644 --- a/backend/test/backend_tests/rpc_team_test.clj +++ b/backend/test/backend_tests/rpc_team_test.clj @@ -15,6 +15,7 @@ [app.db :as db] [app.email.blacklist :as email.blacklist] [app.http :as http] + [app.loggers.audit :as audit] [app.nitrate :as nitrate] [app.rpc :as-alias rpc] [app.rpc.commands.teams :as teams] @@ -24,11 +25,23 @@ [backend-tests.helpers :as th] [clojure.test :as t] [datoteka.fs :as fs] - [mockery.core :refer [with-mocks]])) + [mockery.core :refer [with-mocks]] + [yetti.request])) (t/use-fixtures :once th/state-init) (t/use-fixtures :each th/database-reset) +;; Minimal request stub, so the audit event can be built the way the backend +;; builds it: with the client ip in the event context. +(def ^:private http-request + (reify + yetti.request/IRequest + (get-header [_ header] + (case header + "x-forwarded-for" "127.0.0.44" + "x-real-ip" "127.0.0.43" + nil)))) + (t/deftest create-team-invitations (with-mocks [mock {:target 'app.email/send! :return nil}] (let [profile1 (th/create-profile* 1 {:is-active true}) @@ -364,26 +377,229 @@ (t/is (= (:id profile2) (:member-id claims)))))))) -(t/deftest get-team-invitation-token-requires-edition-permissions - (let [profile1 (th/create-profile* 1 {:is-active true}) - profile2 (th/create-profile* 2 {:is-active true}) - team (th/create-team* 1 {:profile-id (:id profile1)}) +(t/deftest get-team-invitation-token-requires-admin-permissions + ;; The token is a bearer credential: whoever holds it accepts the + ;; invitation with the role of the invitation row. Minting it takes the + ;; same permissions as the rest of the invitation commands, so an editor + ;; (the lowest role above viewer) must not get it. + (let [owner (th/create-profile* 1 {:is-active true}) + viewer (th/create-profile* 2 {:is-active true}) + editor (th/create-profile* 3 {:is-active true}) + admin (th/create-profile* 4 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id owner)}) pool (:app.db/pool th/*system*)] (th/create-team-role* {:team-id (:id team) - :profile-id (:id profile2) + :profile-id (:id viewer) :role :viewer}) + (th/create-team-role* {:team-id (:id team) + :profile-id (:id editor) + :role :editor}) + (th/create-team-role* {:team-id (:id team) + :profile-id (:id admin) + :role :admin}) (db/insert! pool :team-invitation {:team-id (:id team) :email-to "victim@example.com" :role "editor" + :created-by (:id owner) :valid-until (ct/in-future "48h")}) - (let [data {::th/type :get-team-invitation-token - ::rpc/profile-id (:id profile2) - :team-id (:id team) - :email "victim@example.com"} - out (th/command! data)] - (t/is (not (th/success? out))) - (t/is (= :not-found (-> out :error ex-data :type)))))) + (let [request (fn [profile-id] + {::th/type :get-team-invitation-token + ::rpc/profile-id (:id profile-id) + :team-id (:id team) + :email "victim@example.com"})] + (t/testing "a viewer cannot get the token" + (let [out (th/command! (request viewer))] + (t/is (not (th/success? out))) + (t/is (= :validation (-> out :error ex-data :type))) + (t/is (th/ex-of-code? (:error out) :insufficient-permissions)))) + + (t/testing "an editor cannot get the token" + (let [out (th/command! (request editor))] + (t/is (not (th/success? out))) + (t/is (= :validation (-> out :error ex-data :type))) + (t/is (th/ex-of-code? (:error out) :insufficient-permissions)))) + + (t/testing "an admin can get the token" + (let [out (th/command! (request admin))] + (t/is (th/success? out)) + (let [claims (tokens/decode th/*system* (-> out :result :token))] + (t/is (= :team-invitation (:iss claims))) + (t/is (= :editor (:role claims))) + (t/is (= (:id team) (:team-id claims))) + (t/is (= "victim@example.com" (:member-email claims)))))) + + (t/testing "the team owner can get the token" + (let [out (th/command! (request owner))] + (t/is (th/success? out))))))) + + +(t/deftest get-team-invitation-token-owner-invitation-requires-team-owner + ;; An admin cannot create nor promote an owner invitation, so it cannot + ;; mint the token of one either: only the team owner can. + (let [owner (th/create-profile* 1 {:is-active true}) + admin (th/create-profile* 2 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id owner)}) + pool (:app.db/pool th/*system*)] + (th/create-team-role* {:team-id (:id team) + :profile-id (:id admin) + :role :admin}) + (db/insert! pool :team-invitation + {:team-id (:id team) + :email-to "new-owner@example.com" + :role "owner" + :created-by (:id owner) + :valid-until (ct/in-future "48h")}) + (let [request (fn [profile-id] + {::th/type :get-team-invitation-token + ::rpc/profile-id (:id profile-id) + :team-id (:id team) + :email "new-owner@example.com"})] + (t/testing "an admin cannot get the token of an owner invitation" + (let [out (th/command! (request admin))] + (t/is (not (th/success? out))) + (t/is (th/ex-of-code? (:error out) :cant-promote-to-owner)))) + + (t/testing "the team owner can get the token of an owner invitation" + (let [out (th/command! (request owner))] + (t/is (th/success? out)) + (let [claims (tokens/decode th/*system* (-> out :result :token))] + (t/is (= :owner (:role claims))) + (t/is (= "new-owner@example.com" (:member-email claims))))))))) + + +(t/deftest get-team-invitation-token-emits-audit-event + ;; Minting the token is an action, not a read: the event is named as such + ;; and carries the granted role, so the extraction of the token does not go + ;; unnoticed in the audit log. + (let [owner (th/create-profile* 1 {:is-active true}) + admin (th/create-profile* 2 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id owner)}) + pool (:app.db/pool th/*system*)] + (th/create-team-role* {:team-id (:id team) + :profile-id (:id admin) + :role :admin}) + (let [invitation (db/insert! pool :team-invitation + {:team-id (:id team) + :email-to "victim@example.com" + :role "editor" + :created-by (:id owner) + :valid-until (ct/in-future "48h")})] + + (let [params {::rpc/profile-id (:id admin) + :team-id (:id team) + :email "victim@example.com"} + result (th/command-raw! (assoc params ::th/type :get-team-invitation-token)) + [mdata _] (get-in th/*system* [:app.rpc/methods :get-team-invitation-token]) + ;; The cfg of a command carries its type, the same way + ;; `resolve-methods` does it in production. + event (audit/prepare-rpc-event (assoc th/*system* ::rpc/module "main" + ::rpc/type "command") + mdata + (with-meta params {:app.http/request http-request}) + result)] + (t/is (= "create-team-invitation-token" (:name event))) + (t/is (= "command" (:type event))) + (t/is (= (:id admin) (:profile-id event))) + (t/is (= (:id team) (get-in event [:props :team-id]))) + (t/is (= "victim@example.com" (get-in event [:props :email]))) + (t/is (= :editor (get-in event [:props :role]))) + (t/is (= (:id invitation) (get-in event [:props :invitation-id]))) + (t/is (not (contains? (:props event) :token))))))) + + +(t/deftest get-team-invitations-requires-admin-permissions + ;; The pending invitations carry the email of the invited person and the + ;; role offered to them: the same data the rest of the invitation + ;; commands already require admin for. + (let [owner (th/create-profile* 1 {:is-active true}) + viewer (th/create-profile* 2 {:is-active true}) + editor (th/create-profile* 3 {:is-active true}) + admin (th/create-profile* 4 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id owner)}) + pool (:app.db/pool th/*system*)] + (th/create-team-role* {:team-id (:id team) + :profile-id (:id viewer) + :role :viewer}) + (th/create-team-role* {:team-id (:id team) + :profile-id (:id editor) + :role :editor}) + (th/create-team-role* {:team-id (:id team) + :profile-id (:id admin) + :role :admin}) + (db/insert! pool :team-invitation + {:team-id (:id team) + :email-to "victim@example.com" + :role "editor" + :created-by (:id owner) + :valid-until (ct/in-future "48h")}) + (let [request (fn [profile-id] + {::th/type :get-team-invitations + ::rpc/profile-id (:id profile-id) + :team-id (:id team)})] + (t/testing "a viewer cannot list the pending invitations" + (let [out (th/command! (request viewer))] + (t/is (not (th/success? out))) + (t/is (= :validation (-> out :error ex-data :type))) + (t/is (th/ex-of-code? (:error out) :insufficient-permissions)))) + + (t/testing "an editor cannot list the pending invitations" + (let [out (th/command! (request editor))] + (t/is (not (th/success? out))) + (t/is (= :validation (-> out :error ex-data :type))) + (t/is (th/ex-of-code? (:error out) :insufficient-permissions)))) + + (t/testing "an admin can list the pending invitations" + (let [out (th/command! (request admin))] + (t/is (th/success? out)) + (t/is (= 1 (count (:result out)))) + (t/is (= "victim@example.com" (:email (first (:result out))))))) + + (t/testing "the team owner can list the pending invitations" + (let [out (th/command! (request owner))] + (t/is (th/success? out)) + (t/is (= 1 (count (:result out))))))))) + + +(t/deftest invitation-permissions-deny-non-members + ;; A profile that is not a member of the team has no permissions row at + ;; all, so the check gets nil instead of a map of flags. That is also + ;; the case of an organization owner that is not a member of the team: + ;; the team permissions lookup is membership only, so they are left out + ;; of the invitation commands too. + ;; + ;; NOTE: the audit wrapper is installed when the system boots, and only + ;; when the audit log, webhooks or telemetry flags are on, none of them + ;; is on in the test environment. So there is no way to observe here + ;; whether a denied call emits an event: that is a property of the + ;; wrapper, shared by every command. + (let [owner (th/create-profile* 1 {:is-active true}) + outsider (th/create-profile* 2 {:is-active true}) + team (th/create-team* 1 {:profile-id (:id owner)}) + pool (:app.db/pool th/*system*)] + (db/insert! pool :team-invitation + {:team-id (:id team) + :email-to "victim@example.com" + :role "editor" + :created-by (:id owner) + :valid-until (ct/in-future "48h")}) + + (t/testing "a non member cannot get the token" + (let [out (th/command! {::th/type :get-team-invitation-token + ::rpc/profile-id (:id outsider) + :team-id (:id team) + :email "victim@example.com"})] + (t/is (not (th/success? out))) + (t/is (= :validation (-> out :error ex-data :type))) + (t/is (th/ex-of-code? (:error out) :insufficient-permissions)))) + + (t/testing "a non member cannot list the pending invitations" + (let [out (th/command! {::th/type :get-team-invitations + ::rpc/profile-id (:id outsider) + :team-id (:id team)})] + (t/is (not (th/success? out))) + (t/is (= :validation (-> out :error ex-data :type))) + (t/is (th/ex-of-code? (:error out) :insufficient-permissions)))))) (t/deftest accept-invitation-tokens diff --git a/frontend/src/app/main/data/team.cljs b/frontend/src/app/main/data/team.cljs index 081f41102d..523431c577 100644 --- a/frontend/src/app/main/data/team.cljs +++ b/frontend/src/app/main/data/team.cljs @@ -261,7 +261,20 @@ (watch [_ state _] (let [team-id (:current-team-id state)] (->> (rp/cmd! :get-team-invitations {:team-id team-id}) - (rx/map (partial invitations-fetched team-id))))))) + (rx/map (partial invitations-fetched team-id)) + (rx/catch (fn [cause] + (let [{:keys [type code]} (ex-data cause)] + (if (and (= :validation type) + (= :insufficient-permissions code)) + ;; The pending invitations are an admin only + ;; list, so a denied refetch means the cached + ;; copy is no longer valid: drop it instead of + ;; leaving stale emails around. + (do + (log/warn :hint "fetch-invitations: no permission, clearing" + :team-id (str team-id)) + (rx/of (invitations-fetched team-id []))) + (rx/throw cause)))))))))) (defn- team-initialized [team-id] diff --git a/frontend/src/app/main/ui/dashboard/organization_team_switch_menus.cljs b/frontend/src/app/main/ui/dashboard/organization_team_switch_menus.cljs index 2124f5d673..521d98076c 100644 --- a/frontend/src/app/main/ui/dashboard/organization_team_switch_menus.cljs +++ b/frontend/src/app/main/ui/dashboard/organization_team_switch_menus.cljs @@ -180,10 +180,12 @@ :data-testid "team-members"} (tr "labels.members")] - [:> dropdown-menu-item* {:on-click on-invitations-click - :class (stl/css :options-item) - :data-testid "team-invitations"} - (tr "labels.invitations")] + ;; The pending invitations are an admin only list. + (when (:is-admin permissions) + [:> dropdown-menu-item* {:on-click on-invitations-click + :class (stl/css :options-item) + :data-testid "team-invitations"} + (tr "labels.invitations")]) (when (contains? cf/flags :webhooks) [:> dropdown-menu-item* {:on-click on-webhooks-click diff --git a/frontend/src/app/main/ui/dashboard/team.cljs b/frontend/src/app/main/ui/dashboard/team.cljs index b84ed95f0b..7de6e8ea04 100644 --- a/frontend/src/app/main/ui/dashboard/team.cljs +++ b/frontend/src/app/main/ui/dashboard/team.cljs @@ -96,6 +96,7 @@ invitations-section? (= section :dashboard-team-invitations) webhooks-section? (= section :dashboard-team-webhooks) permissions (:permissions team) + is-admin? (boolean (:is-admin permissions)) can-invite? (dnt/can-send-invitations? {:organization (:organization team) :profile-id (:id profile) @@ -131,15 +132,18 @@ [:ul {:class (stl/css :dashboard-header-options)} [:li {:class (when members-section? (stl/css :active))} [:a {:on-click on-nav-members} (tr "labels.members")]] - [:li {:class (when invitations-section? (stl/css :active))} - [:a {:on-click on-nav-invitations} (tr "labels.invitations")]] + (when is-admin? + [:li {:class (when invitations-section? (stl/css :active))} + [:a {:on-click on-nav-invitations} (tr "labels.invitations")]]) (when (contains? cfg/flags :webhooks) [:li {:class (when webhooks-section? (stl/css :active))} [:a {:on-click on-nav-webhooks} (tr "labels.webhooks")]]) [:li {:class (when settings-section? (stl/css :active))} [:a {:on-click on-nav-settings} (tr "labels.settings")]]]] [:div {:class (stl/css :dashboard-buttons)} - (when (and (or invitations-section? members-section?) (not-empty invitations)) + (when (and is-admin? + (or invitations-section? members-section?) + (not-empty invitations)) (let [invite-button (mf/html [:> button* {:class (stl/css :invite-button) :variant "secondary" @@ -669,7 +673,7 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (mf/defc invitation-role-selector* - [{:keys [can-invite role status on-change]}] + [{:keys [role status on-change]}] (let [show? (mf/use-state false) label (cond (= role :owner) (tr "labels.owner") @@ -690,7 +694,7 @@ (on-change role event))))] [:* - (if (and can-invite (= status :pending)) + (if (= status :pending) [:div {:class (stl/css :rol-selector :has-priv) :on-click on-show} [:span {:class (stl/css :rol-label)} label] @@ -713,9 +717,16 @@ :on-click on-change'} (tr "labels.viewer")]]]])) +(defn invitation-link-copyable? + "The link of an owner invitation can only be copied by the team owner: an + admin cannot create nor promote an owner invitation, so the backend does + not mint its token for them." + [is-owner? role] + (or (not= :owner role) is-owner?)) + (mf/defc invitation-actions* {::mf/private true} - [{:keys [invitation team-id]}] + [{:keys [invitation team-id is-owner?]}] (let [email (:email invitation) copied* (mf/use-state false) copied? (deref copied*) @@ -761,15 +772,16 @@ (-> (dtm/copy-invitation-link params) (with-meta {::ev/origin :team}))))))] - [:> icon-button* {:variant "ghost" - :aria-label (tr "labels.copy-invitation-link") - :on-click on-copy - :icon (if copied? "tick" "clipboard")}])) + (when (invitation-link-copyable? is-owner? (:role invitation)) + [:> icon-button* {:variant "ghost" + :aria-label (tr "labels.copy-invitation-link") + :on-click on-copy + :icon (if copied? "tick" "clipboard")}]))) (mf/defc invitation-row* {::mf/wrap [mf/memo] ::mf/private true} - [{:keys [invitation can-invite team-id selected on-select-change]}] + [{:keys [invitation team-id selected on-select-change is-owner?]}] (let [expired? (:expired invitation) email (:email invitation) @@ -787,12 +799,11 @@ on-change (mf/use-fn - (mf/deps can-invite on-select-change) + (mf/deps on-select-change) (fn [event] - (when can-invite - (let [email (-> (dom/get-current-target event) - (dom/get-data "attr"))] - (on-select-change email))))) + (let [email (-> (dom/get-current-target event) + (dom/get-data "attr"))] + (on-select-change email)))) on-change-role (mf/use-fn @@ -804,26 +815,23 @@ [:div {:class (stl/css :table-row :table-row-invitations)} [:div {:class (stl/css :table-field :field-email)} - (if can-invite - [:div {:class (stl/css :input-wrapper)} - [:label - [:span {:class (stl/css-case :input-checkbox true - :global/checked (is-selected? email))} - deprecated-icon/status-tick] + [:div {:class (stl/css :input-wrapper)} + [:label + [:span {:class (stl/css-case :input-checkbox true + :global/checked (is-selected? email))} + deprecated-icon/status-tick] - [:input {:type "checkbox" - :id (dm/str "email-" email) - :data-attr email - :value email - :checked (is-selected? email) - :on-change on-change}] - email]] - [:div email])] + [:input {:type "checkbox" + :id (dm/str "email-" email) + :data-attr email + :value email + :checked (is-selected? email) + :on-change on-change}] + email]]] [:div {:class (stl/css :table-field :field-roles)} [:> invitation-role-selector* - {:can-invite can-invite - :role role + {:role role :status status :on-change on-change-role}]] @@ -831,14 +839,14 @@ [:& badge-notification {:type type :content badge-content}]] [:div {:class (stl/css :table-field :field-actions)} - (when ^boolean can-invite - [:> invitation-actions* - {:invitation invitation - :team-id team-id}])]])) + [:> invitation-actions* + {:invitation invitation + :team-id team-id + :is-owner? is-owner?}]]])) (mf/defc empty-invitation-table* {::mf/private true} - [{:keys [can-invite team]}] + [{:keys [team]}] (let [route (mf/deref refs/route) invite-email (-> route :query-params :invite-email) @@ -850,15 +858,12 @@ :invite-email invite-email}))))] [:div {:class (stl/css :empty-invitations)} [:div (tr "labels.no-invitations")] - (if ^boolean can-invite - [:* - [:div (tr "labels.no-invitations-gather-people")] - [:> button* {:variant "primary" - :class (stl/css :btn-empty-invitations) - :on-click on-invite-member - :data-testid "invite-member"} - (tr "dashboard.invite-profile")]] - [:div {:class (stl/css :no-permission-text)} (tr "dashboard.invitations.no-permission")])])) + [:div (tr "labels.no-invitations-gather-people")] + [:> button* {:variant "primary" + :class (stl/css :btn-empty-invitations) + :on-click on-invite-member + :data-testid "invite-member"} + (tr "dashboard.invite-profile")]])) (mf/defc invitation-modal {::mf/register modal/components @@ -1057,6 +1062,17 @@ :on-click on-confirm'} accept]]]]])) +(defn invitations-section-content + "What the invitations section shows: the list of the pending invitations + for the team admins, or just the permission notice. The pending invitations + are an admin only list, and `:is-admin` already includes the team owner. + The organization invite policy does not gate the list, only the invite + actions." + [permissions] + (if (:is-admin permissions) + :list + :no-permission)) + (mf/defc invitation-section* {::mf/private true} [{:keys [team profile]}] @@ -1070,6 +1086,8 @@ :profile-id (:id profile) :team-permissions permissions}) + is-owner? (boolean (:is-owner permissions)) + selected (mf/use-state #{}) ;; Sort state: {:field :status/:role, :direction :asc/:desc} @@ -1080,12 +1098,11 @@ on-select-change (mf/use-fn - (mf/deps can-invite? selected) + (mf/deps selected) (fn [email] - (when can-invite? - (if (contains? @selected email) - (swap! selected disj email) - (swap! selected conj email))))) + (if (contains? @selected email) + (swap! selected disj email) + (swap! selected conj email)))) on-confirm-delete (mf/use-fn @@ -1195,57 +1212,63 @@ (reset! sort-state {:field nil :direction :asc})) [:div {:class (stl/css :invitations)} - (when (and (not can-invite?) - (seq @invitations)) + ;; The pending invitations are an admin only list: a member that is not + ;; a team admin does not get them, so there is nothing to show but the + ;; permission notice. + (case (invitations-section-content permissions) + :no-permission [:div {:class (stl/css :empty-invitations)} [:div {:class (stl/css :no-permission-text)} - (tr "dashboard.invitations.no-permission")]]) - (when (and can-invite? - (> (count @selected) 0)) + (tr "dashboard.invitations.no-permission")]] + [:* - [:div {:class (stl/css :invitations-actions)} - [:div - (tr "team.invitations-selected" (i18n/c (count @selected)))] - [:div - [:> button* {:variant "secondary" - :type "button" - :on-click on-resend} - (tr "labels.resend-invitation")]] - [:> icon-button* {:on-click on-delete - :variant "destructive" - :aria-label (tr "labels.delete-invitation") - :icon "delete"}]]]) - [:div {:class (stl/css :table-header)} - [:div {:class (stl/css :title-field-name)} (tr "labels.invitations")] - [:div {:class (stl/css :title-field-role)} (tr "labels.role") - [:> icon-button* {:variant "action" - :class (stl/css-case :sort-active (= (:field @sort-state) :role) - :sort-inactive (not= (:field @sort-state) :role)) - :aria-label (tr "dashboard.order-invitations-by-role") - :icon (if (= (:field @sort-state) :role) - (if (= (:direction @sort-state) :asc) "arrow-down" "arrow-up") - "arrow-down") - :on-click on-order-by-role}]] - [:div {:class (stl/css :title-field-status)} (tr "labels.status") - [:> icon-button* {:variant "action" - :class (stl/css-case :sort-active (= (:field @sort-state) :status) - :sort-inactive (not= (:field @sort-state) :status)) - :aria-label (tr "dashboard.order-invitations-by-status") - :icon (if (= (:field @sort-state) :status) - (if (= (:direction @sort-state) :asc) "arrow-down" "arrow-up") - "arrow-down") - :on-click on-order-by-status}]]] - (if (empty? @invitations) - [:> empty-invitation-table* {:can-invite can-invite? :team team}] - [:div {:class (stl/css :table-rows)} - (for [invitation @invitations] - [:> invitation-row* - {:key (:email invitation) - :invitation invitation - :can-invite can-invite? - :team-id team-id - :selected selected - :on-select-change on-select-change}])])])) + (when (> (count @selected) 0) + [:div {:class (stl/css :invitations-actions)} + [:div + (tr "team.invitations-selected" (i18n/c (count @selected)))] + ;; Resending goes through the invite command, so it needs the + ;; organization invite policy; deleting only needs team admin. + (when can-invite? + [:div + [:> button* {:variant "secondary" + :type "button" + :on-click on-resend} + (tr "labels.resend-invitation")]]) + [:> icon-button* {:on-click on-delete + :variant "destructive" + :aria-label (tr "labels.delete-invitation") + :icon "delete"}]]) + [:div {:class (stl/css :table-header)} + [:div {:class (stl/css :title-field-name)} (tr "labels.invitations")] + [:div {:class (stl/css :title-field-role)} (tr "labels.role") + [:> icon-button* {:variant "action" + :class (stl/css-case :sort-active (= (:field @sort-state) :role) + :sort-inactive (not= (:field @sort-state) :role)) + :aria-label (tr "dashboard.order-invitations-by-role") + :icon (if (= (:field @sort-state) :role) + (if (= (:direction @sort-state) :asc) "arrow-down" "arrow-up") + "arrow-down") + :on-click on-order-by-role}]] + [:div {:class (stl/css :title-field-status)} (tr "labels.status") + [:> icon-button* {:variant "action" + :class (stl/css-case :sort-active (= (:field @sort-state) :status) + :sort-inactive (not= (:field @sort-state) :status)) + :aria-label (tr "dashboard.order-invitations-by-status") + :icon (if (= (:field @sort-state) :status) + (if (= (:direction @sort-state) :asc) "arrow-down" "arrow-up") + "arrow-down") + :on-click on-order-by-status}]]] + (if (empty? @invitations) + [:> empty-invitation-table* {:team team}] + [:div {:class (stl/css :table-rows)} + (for [invitation @invitations] + [:> invitation-row* + {:key (:email invitation) + :invitation invitation + :is-owner? is-owner? + :team-id team-id + :selected selected + :on-select-change on-select-change}])])])])) (mf/defc team-invitations-page* [{:keys [team profile]}] diff --git a/frontend/test/frontend_tests/data/team_test.cljs b/frontend/test/frontend_tests/data/team_test.cljs new file mode 100644 index 0000000000..9bf84efc79 --- /dev/null +++ b/frontend/test/frontend_tests/data/team_test.cljs @@ -0,0 +1,81 @@ +;; 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/. + +(ns frontend-tests.data.team-test + (:require + [app.common.uuid :as uuid] + [app.main.data.team :as dtm] + [app.main.repo :as rp] + [app.main.ui.dashboard.team :as dtui] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] + [frontend-tests.helpers.mock :as mock] + [potok.v2.core :as ptk])) + +(defn- fetch-invitations-events + "Runs the watch of `fetch-invitations` with `rp/cmd!` answering with + `response` and resolves to the team id and the events it emits." + [response] + (let [team-id (uuid/next) + state {:current-team-id team-id} + emitted (atom [])] + (-> (mock/with-mocks* + {rp/cmd! (mock/stub (fn [_cmd _params] response))} + (await (async/observe (ptk/watch (dtm/fetch-invitations) state nil) + :on-next #(swap! emitted conj %)))) + (.then (fn [_] {:team-id team-id :events @emitted}))))) + +(t/deftest ^:async fetch-invitations-clears-the-list-on-permission-error + ;; The pending invitations are an admin only list: a member that cannot + ;; invite gets an insufficient permissions error. That must not become an + ;; error toast, and the cached list must be dropped so the stale emails do + ;; not stay around. + (let [{:keys [team-id events]} + (await (fetch-invitations-events + (rx/throw (ex-info "insufficient permissions" + {:type :validation + :code :insufficient-permissions})))) + event (first events) + state (ptk/update event {:teams {team-id {:id team-id + :invitations [{:email "stale@example.com" + :role :editor}]}}})] + (t/is (= 1 (count events))) + (t/is (= ::dtm/invitations-fetched (ptk/type event))) + (t/is (= [] (get-in state [:teams team-id :invitations]))))) + +(t/deftest ^:async fetch-invitations-propagates-other-errors + ;; Only the permission failure is swallowed: any other error is a real + ;; problem and must reach the error reporting. + (let [outcome (await (mock/with-mocks* + {rp/cmd! (mock/stub (fn [_cmd _params] + (rx/throw (ex-info "boom" {:type :server}))))} + (try + (await (async/observe (ptk/watch (dtm/fetch-invitations) + {:current-team-id (uuid/next)} + nil))) + :resolved + (catch js/Error _ :rejected))))] + (t/is (= :rejected outcome)))) + +(t/deftest ^:async fetch-invitations-emits-the-update-event + (let [{:keys [events]} (await (fetch-invitations-events + (rx/of [{:email "invited@example.com" :role :editor}])))] + (t/is (= 1 (count events))) + (t/is (= ::dtm/invitations-fetched (ptk/type (first events)))))) + +(t/deftest invitations-section-shows-the-list-only-to-the-team-admins + ;; The list is admin only, and the backend normalizes `:is-admin` to + ;; include the team owner. The organization invite policy does not gate it. + (t/is (= :list (dtui/invitations-section-content {:is-owner false :is-admin true}))) + (t/is (= :list (dtui/invitations-section-content {:is-owner true :is-admin true}))) + (t/is (= :no-permission (dtui/invitations-section-content {:is-owner false :is-admin false})))) + +(t/deftest owner-invitation-link-is-copyable-only-by-the-team-owner + ;; An admin cannot create nor promote an owner invitation, so the backend + ;; does not mint its token for them: the action is not offered either. + (t/is (true? (dtui/invitation-link-copyable? true :owner))) + (t/is (false? (dtui/invitation-link-copyable? false :owner))) + (t/is (true? (dtui/invitation-link-copyable? false :editor))) + (t/is (true? (dtui/invitation-link-copyable? false :viewer)))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index c4b052c69d..3f25052df3 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -24,6 +24,7 @@ [frontend-tests.data.repo-test] [frontend-tests.data.store-test] [frontend-tests.data.svg-upload-test] + [frontend-tests.data.team-test] [frontend-tests.data.uploads-test] [frontend-tests.data.viewer-test] [frontend-tests.data.workspace-colors-test] @@ -178,6 +179,7 @@ 'frontend-tests.data.exports-assets-test 'frontend-tests.data.exports-files-test 'frontend-tests.data.svg-upload-test + 'frontend-tests.data.team-test 'frontend-tests.data.uploads-test 'frontend-tests.data.viewer-test 'frontend-tests.data.workspace-colors-test