From 637ff3005aa2d7f6ae746b73148e920d4a2f2054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Valderrama?= Date: Thu, 14 May 2026 19:32:20 +0200 Subject: [PATCH] :sparkles: Add nitrate advanced permissions for move teams --- backend/src/app/rpc/commands/nitrate.clj | 55 +++++++++++++--- .../app/common/types/nitrate_permissions.cljc | 23 +++++-- .../types/nitrate_permissions_test.cljc | 66 +++++++++++++++++++ frontend/src/app/main/data/nitrate.cljs | 35 +++++++--- frontend/src/app/main/ui/dashboard/team.cljs | 5 +- .../src/app/main/ui/dashboard/team_form.cljs | 2 +- frontend/translations/en.po | 3 + frontend/translations/es.po | 3 + 8 files changed, 167 insertions(+), 25 deletions(-) diff --git a/backend/src/app/rpc/commands/nitrate.clj b/backend/src/app/rpc/commands/nitrate.clj index 8cf5a9bba2..55535f54a6 100644 --- a/backend/src/app/rpc/commands/nitrate.clj +++ b/backend/src/app/rpc/commands/nitrate.clj @@ -282,6 +282,20 @@ (assert-is-owner cfg profile-id team-id) (assert-not-default-team cfg team-id) (assert-membership cfg profile-id organization-id) + ;; Check moveTeams permission on the source organization + (when (contains? cf/flags :nitrate) + (let [org-perms (nitrate/call cfg :get-org-permissions + {:organization-id organization-id})] + (if (nil? org-perms) + (ex/raise :type :validation + :code :not-allowed + :hint "Unable to verify organization permissions") + (when-not (nitrate-perms/allowed? :move-team + {:org-perms org-perms + :profile-id profile-id}) + (ex/raise :type :validation + :code :not-allowed + :hint "You are not allowed to move teams that are part of this organization. If you need more information, contact the owner."))))) ;; Api call to nitrate (nitrate/call cfg :remove-team-from-org {:team-id team-id :organization-id organization-id}) @@ -308,18 +322,43 @@ (assert-membership cfg profile-id organization-id) (when (contains? cf/flags :nitrate) - (let [org-perms (nitrate/call cfg :get-org-permissions - {:organization-id organization-id})] - (if (nil? org-perms) + (let [team-with-org (nitrate/call cfg :get-team-org {:team-id team-id}) + source-org-id (get-in team-with-org [:organization :id]) + source-org-perms (when source-org-id + (nitrate/call cfg :get-org-permissions + {:organization-id source-org-id})) + target-org-perms (nitrate/call cfg :get-org-permissions + {:organization-id organization-id}) + target-org-same-owner? (and (some? source-org-perms) + (some? target-org-perms) + (= (:owner-id source-org-perms) + (:owner-id target-org-perms)))] + (when (nil? target-org-perms) (ex/raise :type :validation :code :not-allowed - :hint "Unable to verify organization permissions") - (when-not (nitrate-perms/allowed? :create-team - {:org-perms org-perms - :profile-id profile-id}) + :hint "Unable to verify organization permissions")) + + ;; Team already belongs to an organization: check move-teams on source org. + (when (some? source-org-id) + (when (nil? source-org-perms) (ex/raise :type :validation :code :not-allowed - :hint "You are not allowed to add teams in this organization"))))) + :hint "Unable to verify organization permissions")) + (when-not (nitrate-perms/allowed? :move-team + {:org-perms source-org-perms + :profile-id profile-id + :target-org-same-owner? target-org-same-owner?}) + (ex/raise :type :validation + :code :not-allowed + :hint "You are not allowed to move teams that are part of this organization. If you need more information, contact the owner."))) + + ;; Always check target create-teams permission (new/add and move flows). + (when-not (nitrate-perms/allowed? :create-team + {:org-perms target-org-perms + :profile-id profile-id}) + (ex/raise :type :validation + :code :not-allowed + :hint "You are not allowed to add teams in this organization")))) (let [team-members (db/query cfg :team-profile-rel {:team-id team-id})] ;; Add teammates to the org if needed diff --git a/common/src/app/common/types/nitrate_permissions.cljc b/common/src/app/common/types/nitrate_permissions.cljc index 2b16bcc1d0..ab1fa7ca75 100644 --- a/common/src/app/common/types/nitrate_permissions.cljc +++ b/common/src/app/common/types/nitrate_permissions.cljc @@ -8,7 +8,8 @@ (def ^:private defaults {:create-teams "any" - :delete-teams "onlyOwners"}) + :delete-teams "onlyOwners" + :move-teams "always"}) (defn- can-create-team? [{:keys [is-org-owner? permission-value]}] @@ -24,11 +25,24 @@ (boolean (:is-owner team-perms)) :else false)) +(defn- can-move-team? + [{:keys [permission-value target-org-same-owner?]}] + (cond + (= permission-value "never") + false + (= permission-value "always") + true + (= permission-value "myOrganizations") + (true? target-org-same-owner?) + :else false)) + (def ^:private action-rules {:create-team {:permission-key :create-teams :check-fn can-create-team?} :delete-team {:permission-key :delete-teams - :check-fn can-delete-team?}}) + :check-fn can-delete-team?} + :move-team {:permission-key :move-teams + :check-fn can-move-team?}}) (defn- normalize-org-permissions [org-perms] @@ -40,7 +54,7 @@ (defn allowed? "Returns true only for explicitly allowed actions (fail-closed)." - [action {:keys [org-perms profile-id team-perms allow-org-owner-delete?]}] + [action {:keys [org-perms profile-id team-perms allow-org-owner-delete? target-org-same-owner?]}] (let [{:keys [permission-key check-fn] :as rule} (get action-rules action) permissions (normalize-org-permissions org-perms) @@ -51,4 +65,5 @@ :else (boolean (check-fn {:is-org-owner? is-org-owner? :permission-value permission-value :team-perms team-perms - :allow-org-owner-delete? allow-org-owner-delete?}))))) + :allow-org-owner-delete? allow-org-owner-delete? + :target-org-same-owner? target-org-same-owner?}))))) diff --git a/common/test/common_tests/types/nitrate_permissions_test.cljc b/common/test/common_tests/types/nitrate_permissions_test.cljc index 44b7e9ec84..01957122dd 100644 --- a/common/test/common_tests/types/nitrate_permissions_test.cljc +++ b/common/test/common_tests/types/nitrate_permissions_test.cljc @@ -72,3 +72,69 @@ {:org-perms only-me-org :profile-id :member :team-perms {:is-owner true :is-admin true}}))))) + +(t/deftest move-team-always-allows-any-org-owner-or-all-users + (let [always-org (assoc org-perms :permissions {:create-teams "any" + :delete-teams "onlyOwners" + :move-teams "always"})] + ;; Org owner should always be allowed + (t/is (true? (nitrate-perms/allowed? :move-team + {:org-perms always-org + :profile-id :owner + :team-perms {}}))) + ;; Regular member should be allowed when move-teams is "always" + (t/is (true? (nitrate-perms/allowed? :move-team + {:org-perms always-org + :profile-id :member + :team-perms {}}))))) + +(t/deftest move-team-myorganizations-allows-only-within-same-owner + (let [my-orgs (assoc org-perms :permissions {:create-teams "any" + :delete-teams "onlyOwners" + :move-teams "myOrganizations"})] + ;; Org owner must also stay within same-owner organizations + (t/is (false? (nitrate-perms/allowed? :move-team + {:org-perms my-orgs + :profile-id :owner + :team-perms {} + :target-org-same-owner? false}))) + (t/is (true? (nitrate-perms/allowed? :move-team + {:org-perms my-orgs + :profile-id :owner + :team-perms {} + :target-org-same-owner? true}))) + ;; Regular member should be allowed only if target has same owner + (t/is (true? (nitrate-perms/allowed? :move-team + {:org-perms my-orgs + :profile-id :member + :team-perms {} + :target-org-same-owner? true}))) + (t/is (false? (nitrate-perms/allowed? :move-team + {:org-perms my-orgs + :profile-id :member + :team-perms {} + :target-org-same-owner? false}))))) + +(t/deftest move-team-never-denies-all + (let [never-org (assoc org-perms :permissions {:create-teams "any" + :delete-teams "onlyOwners" + :move-teams "never"})] + ;; Even org owner should be denied + (t/is (false? (nitrate-perms/allowed? :move-team + {:org-perms never-org + :profile-id :owner + :team-perms {}}))) + ;; Regular member should be denied + (t/is (false? (nitrate-perms/allowed? :move-team + {:org-perms never-org + :profile-id :member + :team-perms {}}))))) + +(t/deftest move-team-defaults-to-always + (let [default-org (assoc org-perms :permissions {:create-teams "any" + :delete-teams "onlyOwners"})] + ;; Should default to "always" when not specified + (t/is (true? (nitrate-perms/allowed? :move-team + {:org-perms default-org + :profile-id :member + :team-perms {}}))))) diff --git a/frontend/src/app/main/data/nitrate.cljs b/frontend/src/app/main/data/nitrate.cljs index ad33dfb8eb..c4c065b398 100644 --- a/frontend/src/app/main/data/nitrate.cljs +++ b/frontend/src/app/main/data/nitrate.cljs @@ -225,25 +225,40 @@ (->> (rp/cmd! :get-teams) (rx/mapcat (fn [teams] - (let [all-orgs (map dt/team->organization - (filter #(and (:is-default %) (:organization %)) teams)) - orgs (filter (fn [org] - (let [perm (get-in org [:permissions :create-teams]) - is-own? (= profile-id (:owner-id org))] - (or (= perm "any") is-own?))) all-orgs) - team (first (filter #(= (:id %) team-id) teams)) + (let [all-orgs (map dt/team->organization + (filter #(and (:is-default %) (:organization %)) teams)) + team (first (filter #(= (:id %) team-id) teams)) + source-org (:organization team) + current-org-id (:id source-org) + move-perm (dm/get-in source-org [:permissions :move-teams]) + source-owner-id (:owner-id source-org) + can-create? (fn [org] + (let [perm (dm/get-in org [:permissions :create-teams]) + is-own? (= profile-id (:owner-id org))] + (or (= perm "any") is-own?))) + orgs-by-move (case move-perm + "never" + [] + + "myOrganizations" + (filter #(= source-owner-id (:owner-id %)) all-orgs) + + ;; Default to always-allowed behavior. + all-orgs) + orgs (filter can-create? orgs-by-move) + selectable-orgs (remove #(= current-org-id (:id %)) orgs) on-confirm (fn [organization-id] (st/emit! (add-team-to-org {:team-id team-id :organization-id organization-id})))] (rx/of (dt/teams-fetched teams) - (if (empty? orgs) + (if (empty? selectable-orgs) (modal/show :no-permission-modal {:type :no-orgs-change}) (let [has-filtered? (< (count orgs) (count all-orgs)) extra-props (when has-filtered? {:info-message-key "dashboard.select-org-modal.permission-info"})] (modal/show :select-organization-modal - (merge {:organizations orgs - :current-organization-id (dm/get-in team [:organization :id]) + (merge {:organizations selectable-orgs + :current-organization-id current-org-id :on-confirm on-confirm :title-key "dashboard.change-org-modal.title" :choose-key "dashboard.change-org-modal.choose" diff --git a/frontend/src/app/main/ui/dashboard/team.cljs b/frontend/src/app/main/ui/dashboard/team.cljs index 0b627d31db..dd528c2fbb 100644 --- a/frontend/src/app/main/ui/dashboard/team.cljs +++ b/frontend/src/app/main/ui/dashboard/team.cljs @@ -1401,8 +1401,9 @@ is-owner? (= profile-id (:owner-id org))] (or (= perm "any") is-owner?)))))) - can-change-organization? (mf/with-memo [organizations] - (> (count organizations) 1)) + ;; Keep parity with UX requirement: hide only when user belongs to one org. + can-change-organization? (mf/with-memo [all-organizations] + (> (count all-organizations) 1)) can-add-to-organization? (mf/with-memo [organizations all-organizations] (and (pos? (count all-organizations)) diff --git a/frontend/src/app/main/ui/dashboard/team_form.cljs b/frontend/src/app/main/ui/dashboard/team_form.cljs index c0bc45e29b..18210f8c32 100644 --- a/frontend/src/app/main/ui/dashboard/team_form.cljs +++ b/frontend/src/app/main/ui/dashboard/team_form.cljs @@ -155,7 +155,7 @@ :no-orgs-create [(tr "dashboard.select-org-modal.title") (tr "dashboard.no-org-allows-create-team.message")] :no-orgs-change [(tr "dashboard.change-org-modal.title") - (tr "dashboard.no-org-allows-create-team.message")])] + (tr "dashboard.no-permission-move-team.message" organization-name)])] [:div {:class (stl/css :modal-overlay)} [:div {:class (stl/css :modal-container)} [:div {:class (stl/css :modal-header)} diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 9e3f47885f..4b411d7775 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -9503,3 +9503,6 @@ msgstr "Here you find all your organizations where you are allowed to create or msgid "dashboard.no-permission-delete-team.message" msgstr "You are not allowed to delete teams that are part of %s organization. If you need more information, contact the owner." + +msgid "dashboard.no-permission-move-team.message" +msgstr "You are not allowed to move teams that are part of %s organization. If you need more information, contact the owner." diff --git a/frontend/translations/es.po b/frontend/translations/es.po index ee3d0d5f90..7627fb6150 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -9194,3 +9194,6 @@ msgstr "Aquí encontrarás todas las organizaciones en las que tienes permiso pa msgid "dashboard.no-permission-delete-team.message" msgstr "No tienes permiso para eliminar equipos que pertenecen a la organización %s. Si necesitas más información, contacta con el propietario." + +msgid "dashboard.no-permission-move-team.message" +msgstr "No tienes permiso para mover equipos que son parte de la organización %s. Si necesitas más información, contacta con el propietario."