diff --git a/backend/src/app/rpc/commands/teams.clj b/backend/src/app/rpc/commands/teams.clj index f211b2ff70..45b029e987 100644 --- a/backend/src/app/rpc/commands/teams.clj +++ b/backend/src/app/rpc/commands/teams.clj @@ -780,29 +780,29 @@ team (if (contains? cf/flags :nitrate) (nitrate/add-org-info-to-team cfg team params) team) - perms (get team :permissions)] + perms (get team :permissions) + org (:organization team) + in-org? (and (contains? cf/flags :nitrate) org) + can-delete? + (if in-org? + (nitrate-perms/allowed? :delete-team + {:org-perms {:owner-id (dm/get-in team [:organization :owner-id]) + :permissions (dm/get-in team [:organization :permissions])} + :profile-id profile-id + :team-perms perms + ;; `onlyMe` is for a future org-level flow. + :allow-org-owner-delete? false}) + (boolean (:is-owner perms)))] + + (when-not can-delete? + (ex/raise :type :validation + :code :only-owner-can-delete-team)) (when (:is-default team) (ex/raise :type :validation :code :non-deletable-team :hint "impossible to delete default team")) - ;; Check delete permissions based on organization settings. - ;; For non-org teams or when nitrate is disabled, only owners can delete. - (if (and (:organization team) (contains? cf/flags :nitrate)) - (let [org-perms {:owner-id (dm/get-in team [:organization :owner-id]) - :permissions (dm/get-in team [:organization :permissions])}] - (when-not (nitrate-perms/allowed? :delete-team - {:org-perms org-perms - :profile-id profile-id - :team-perms perms}) - (ex/raise :type :validation - :code :not-allowed - :hint "You are not allowed to delete teams in this organization"))) - (when-not (:is-owner perms) - (ex/raise :type :validation - :code :only-owner-can-delete-team))) - (let [delay (ldel/get-deletion-delay team) team (db/update! conn :team {:deleted-at (ct/in-future delay)} diff --git a/common/src/app/common/types/nitrate_permissions.cljc b/common/src/app/common/types/nitrate_permissions.cljc index bcf8c675eb..2b16bcc1d0 100644 --- a/common/src/app/common/types/nitrate_permissions.cljc +++ b/common/src/app/common/types/nitrate_permissions.cljc @@ -8,15 +8,27 @@ (def ^:private defaults {:create-teams "any" - :delete-teams "ownersAndAdmins"}) + :delete-teams "onlyOwners"}) + +(defn- can-create-team? + [{:keys [is-org-owner? permission-value]}] + (or is-org-owner? + (= permission-value "any"))) + +(defn- can-delete-team? + [{:keys [is-org-owner? permission-value team-perms allow-org-owner-delete?]}] + (cond + (= permission-value "onlyMe") + (and allow-org-owner-delete? is-org-owner?) + (= permission-value "onlyOwners") + (boolean (:is-owner team-perms)) + :else false)) (def ^:private action-rules - {:create-team {:permission-key :create-teams - :allowed-values #{"any"} - :requires-admin? false} - :delete-team {:permission-key :delete-teams - :allowed-values #{"ownersAndAdmins"} - :requires-admin? true}}) + {:create-team {:permission-key :create-teams + :check-fn can-create-team?} + :delete-team {:permission-key :delete-teams + :check-fn can-delete-team?}}) (defn- normalize-org-permissions [org-perms] @@ -28,14 +40,15 @@ (defn allowed? "Returns true only for explicitly allowed actions (fail-closed)." - [action {:keys [org-perms profile-id team-perms]}] - (let [{:keys [permission-key allowed-values requires-admin?] :as rule} + [action {:keys [org-perms profile-id team-perms allow-org-owner-delete?]}] + (let [{:keys [permission-key check-fn] :as rule} (get action-rules action) permissions (normalize-org-permissions org-perms) - is-owner? (owner? org-perms profile-id) - is-admin? (boolean (:is-admin team-perms))] + is-org-owner? (owner? org-perms profile-id) + permission-value (get permissions permission-key)] (cond - (nil? rule) false - is-owner? true - (and requires-admin? (not is-admin?)) false - :else (contains? allowed-values (get permissions permission-key))))) + (nil? rule) false + :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?}))))) diff --git a/common/src/app/common/types/organization.cljc b/common/src/app/common/types/organization.cljc index fcf244f444..aa6455d32c 100644 --- a/common/src/app/common/types/organization.cljc +++ b/common/src/app/common/types/organization.cljc @@ -19,7 +19,7 @@ [:permissions {:optional true} [:maybe [:map [:create-teams {:optional true} [:maybe [:enum "any" "onlyMe"]]] - [:delete-teams {:optional true} [:maybe [:enum "ownersAndAdmins" "onlyOwners"]]]]]]]) + [:delete-teams {:optional true} [:maybe [:enum "onlyMe" "onlyOwners"]]]]]]]) (def schema:team-with-organization diff --git a/common/test/common_tests/types/nitrate_permissions_test.cljc b/common/test/common_tests/types/nitrate_permissions_test.cljc index be0ca77d12..44b7e9ec84 100644 --- a/common/test/common_tests/types/nitrate_permissions_test.cljc +++ b/common/test/common_tests/types/nitrate_permissions_test.cljc @@ -12,7 +12,7 @@ (def org-perms {:owner-id :owner :permissions {:create-teams "any" - :delete-teams "ownersAndAdmins"}}) + :delete-teams "onlyOwners"}}) (t/deftest unknown-action-is-denied (t/is (false? (nitrate-perms/allowed? :unknown @@ -20,15 +20,15 @@ :profile-id :member :team-perms {:is-admin true}})))) -(t/deftest owner-is-always-allowed +(t/deftest org-owner-is-allowed-for-create (t/is (true? (nitrate-perms/allowed? :create-team {:org-perms org-perms :profile-id :owner :team-perms {:is-admin false}}))) - (t/is (true? (nitrate-perms/allowed? :delete-team - {:org-perms org-perms - :profile-id :owner - :team-perms {:is-admin false}})))) + (t/is (false? (nitrate-perms/allowed? :delete-team + {:org-perms org-perms + :profile-id :owner + :team-perms {:is-admin false}})))) (t/deftest create-team-permission-rules (t/is (true? (nitrate-perms/allowed? :create-team @@ -37,21 +37,38 @@ :team-perms {:is-admin false}}))) (t/is (false? (nitrate-perms/allowed? :create-team {:org-perms (assoc org-perms :permissions {:create-teams "none" - :delete-teams "ownersAndAdmins"}) + :delete-teams "onlyOwners"}) :profile-id :member :team-perms {:is-admin false}})))) -(t/deftest delete-team-requires-admin-and-policy - (t/is (false? (nitrate-perms/allowed? :delete-team - {:org-perms org-perms - :profile-id :member - :team-perms {:is-admin false}}))) +(t/deftest delete-team-onlyowners-allows-only-team-owners (t/is (true? (nitrate-perms/allowed? :delete-team {:org-perms org-perms :profile-id :member - :team-perms {:is-admin true}}))) + :team-perms {:is-owner true :is-admin true}}))) + (t/is (false? (nitrate-perms/allowed? :delete-team + {:org-perms org-perms + :profile-id :member + :team-perms {:is-admin true}}))) (t/is (false? (nitrate-perms/allowed? :delete-team {:org-perms (assoc org-perms :permissions {:create-teams "any" - :delete-teams "onlyOwners"}) + :delete-teams "invalid-value"}) :profile-id :member :team-perms {:is-admin true}})))) + +(t/deftest delete-team-onlyme-is-gated-for-future-org-flow + (let [only-me-org (assoc org-perms :permissions {:create-teams "any" + :delete-teams "onlyMe"})] + (t/is (false? (nitrate-perms/allowed? :delete-team + {:org-perms only-me-org + :profile-id :owner + :team-perms {:is-owner false :is-admin false}}))) + (t/is (true? (nitrate-perms/allowed? :delete-team + {:org-perms only-me-org + :allow-org-owner-delete? true + :profile-id :owner + :team-perms {:is-owner false :is-admin false}}))) + (t/is (false? (nitrate-perms/allowed? :delete-team + {:org-perms only-me-org + :profile-id :member + :team-perms {:is-owner true :is-admin true}}))))) diff --git a/frontend/src/app/main/ui/dashboard/sidebar.cljs b/frontend/src/app/main/ui/dashboard/sidebar.cljs index ca52a687e9..de21ab9310 100644 --- a/frontend/src/app/main/ui/dashboard/sidebar.cljs +++ b/frontend/src/app/main/ui/dashboard/sidebar.cljs @@ -467,9 +467,6 @@ :owner-cant-leave-team (rx/of (ntf/error (tr "errors.team-leave.owner-cant-leave"))) - :not-allowed - (rx/of (modal/show :no-permission-modal {:type :delete-team})) - (rx/throw error)))) leave-fn @@ -577,20 +574,11 @@ :class (stl/css :team-options-item)} (tr "dashboard.leave-team")]) - (let [is-owner? (get-in team [:permissions :is-owner]) - is-admin? (get-in team [:permissions :is-admin]) - organization (:organization team) - is-org-team? (some? organization) - in-org? (and (contains? cf/flags :nitrate) is-org-team?) - show-delete? (if in-org? - (or is-owner? is-admin?) - is-owner?)] - - (when show-delete? - [:> dropdown-menu-item* {:on-click on-delete-clicked - :class (stl/css :team-options-item :warning) - :data-testid "delete-team"} - (tr "dashboard.delete-team")]))])) + (when (get-in team [:permissions :is-owner]) + [:> dropdown-menu-item* {:on-click on-delete-clicked + :class (stl/css :team-options-item :warning) + :data-testid "delete-team"} + (tr "dashboard.delete-team")])])) (mf/defc org-options-dropdown* {::mf/private true}