Restrict team delete to owners, prep org-owner flow

This commit is contained in:
María Valderrama 2026-05-12 12:29:24 +02:00
parent 46c642cf6d
commit e3df1d6f1f
5 changed files with 82 additions and 64 deletions

View File

@ -780,29 +780,29 @@
team (if (contains? cf/flags :nitrate) team (if (contains? cf/flags :nitrate)
(nitrate/add-org-info-to-team cfg team params) (nitrate/add-org-info-to-team cfg team params)
team) 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) (when (:is-default team)
(ex/raise :type :validation (ex/raise :type :validation
:code :non-deletable-team :code :non-deletable-team
:hint "impossible to delete default 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) (let [delay (ldel/get-deletion-delay team)
team (db/update! conn :team team (db/update! conn :team
{:deleted-at (ct/in-future delay)} {:deleted-at (ct/in-future delay)}

View File

@ -8,15 +8,27 @@
(def ^:private defaults (def ^:private defaults
{:create-teams "any" {: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 (def ^:private action-rules
{:create-team {:permission-key :create-teams {:create-team {:permission-key :create-teams
:allowed-values #{"any"} :check-fn can-create-team?}
:requires-admin? false} :delete-team {:permission-key :delete-teams
:delete-team {:permission-key :delete-teams :check-fn can-delete-team?}})
:allowed-values #{"ownersAndAdmins"}
:requires-admin? true}})
(defn- normalize-org-permissions (defn- normalize-org-permissions
[org-perms] [org-perms]
@ -28,14 +40,15 @@
(defn allowed? (defn allowed?
"Returns true only for explicitly allowed actions (fail-closed)." "Returns true only for explicitly allowed actions (fail-closed)."
[action {:keys [org-perms profile-id team-perms]}] [action {:keys [org-perms profile-id team-perms allow-org-owner-delete?]}]
(let [{:keys [permission-key allowed-values requires-admin?] :as rule} (let [{:keys [permission-key check-fn] :as rule}
(get action-rules action) (get action-rules action)
permissions (normalize-org-permissions org-perms) permissions (normalize-org-permissions org-perms)
is-owner? (owner? org-perms profile-id) is-org-owner? (owner? org-perms profile-id)
is-admin? (boolean (:is-admin team-perms))] permission-value (get permissions permission-key)]
(cond (cond
(nil? rule) false (nil? rule) false
is-owner? true :else (boolean (check-fn {:is-org-owner? is-org-owner?
(and requires-admin? (not is-admin?)) false :permission-value permission-value
:else (contains? allowed-values (get permissions permission-key))))) :team-perms team-perms
:allow-org-owner-delete? allow-org-owner-delete?})))))

View File

@ -19,7 +19,7 @@
[:permissions {:optional true} [:permissions {:optional true}
[:maybe [:map [:maybe [:map
[:create-teams {:optional true} [:maybe [:enum "any" "onlyMe"]]] [: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 (def schema:team-with-organization

View File

@ -12,7 +12,7 @@
(def org-perms (def org-perms
{:owner-id :owner {:owner-id :owner
:permissions {:create-teams "any" :permissions {:create-teams "any"
:delete-teams "ownersAndAdmins"}}) :delete-teams "onlyOwners"}})
(t/deftest unknown-action-is-denied (t/deftest unknown-action-is-denied
(t/is (false? (nitrate-perms/allowed? :unknown (t/is (false? (nitrate-perms/allowed? :unknown
@ -20,15 +20,15 @@
:profile-id :member :profile-id :member
:team-perms {:is-admin true}})))) :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 (t/is (true? (nitrate-perms/allowed? :create-team
{:org-perms org-perms {:org-perms org-perms
:profile-id :owner :profile-id :owner
:team-perms {:is-admin false}}))) :team-perms {:is-admin false}})))
(t/is (true? (nitrate-perms/allowed? :delete-team (t/is (false? (nitrate-perms/allowed? :delete-team
{:org-perms org-perms {:org-perms org-perms
:profile-id :owner :profile-id :owner
:team-perms {:is-admin false}})))) :team-perms {:is-admin false}}))))
(t/deftest create-team-permission-rules (t/deftest create-team-permission-rules
(t/is (true? (nitrate-perms/allowed? :create-team (t/is (true? (nitrate-perms/allowed? :create-team
@ -37,21 +37,38 @@
:team-perms {:is-admin false}}))) :team-perms {:is-admin false}})))
(t/is (false? (nitrate-perms/allowed? :create-team (t/is (false? (nitrate-perms/allowed? :create-team
{:org-perms (assoc org-perms :permissions {:create-teams "none" {:org-perms (assoc org-perms :permissions {:create-teams "none"
:delete-teams "ownersAndAdmins"}) :delete-teams "onlyOwners"})
:profile-id :member :profile-id :member
:team-perms {:is-admin false}})))) :team-perms {:is-admin false}}))))
(t/deftest delete-team-requires-admin-and-policy (t/deftest delete-team-onlyowners-allows-only-team-owners
(t/is (false? (nitrate-perms/allowed? :delete-team
{:org-perms org-perms
:profile-id :member
:team-perms {:is-admin false}})))
(t/is (true? (nitrate-perms/allowed? :delete-team (t/is (true? (nitrate-perms/allowed? :delete-team
{:org-perms org-perms {:org-perms org-perms
:profile-id :member :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 (t/is (false? (nitrate-perms/allowed? :delete-team
{:org-perms (assoc org-perms :permissions {:create-teams "any" {:org-perms (assoc org-perms :permissions {:create-teams "any"
:delete-teams "onlyOwners"}) :delete-teams "invalid-value"})
:profile-id :member :profile-id :member
:team-perms {:is-admin true}})))) :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}})))))

View File

@ -467,9 +467,6 @@
:owner-cant-leave-team :owner-cant-leave-team
(rx/of (ntf/error (tr "errors.team-leave.owner-cant-leave"))) (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)))) (rx/throw error))))
leave-fn leave-fn
@ -577,20 +574,11 @@
:class (stl/css :team-options-item)} :class (stl/css :team-options-item)}
(tr "dashboard.leave-team")]) (tr "dashboard.leave-team")])
(let [is-owner? (get-in team [:permissions :is-owner]) (when (get-in team [:permissions :is-owner])
is-admin? (get-in team [:permissions :is-admin]) [:> dropdown-menu-item* {:on-click on-delete-clicked
organization (:organization team) :class (stl/css :team-options-item :warning)
is-org-team? (some? organization) :data-testid "delete-team"}
in-org? (and (contains? cf/flags :nitrate) is-org-team?) (tr "dashboard.delete-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")]))]))
(mf/defc org-options-dropdown* (mf/defc org-options-dropdown*
{::mf/private true} {::mf/private true}