🐛 Fix missing membership check in create-team (#11166)

backend/src/app/nitrate.clj

@@ -669,3 +669,17 @@
                 :context {:team-id (:id team)
                           :organization-id (:organization-id params)}))
     team))
+
+(defn assert-membership
+  "Verifies that the user is a member of the organization.
+  Raises an exception if the organization doesn't exist or the user is not a member."
+  [cfg profile-id organization-id]
+  (let [membership (call cfg :get-organization-membership {:profile-id profile-id
+                                                           :organization-id organization-id})]
+    (when-not (:organization-id membership)
+      (ex/raise :type :validation
+                :code :organization-does-not-exist))
+
+    (when-not (:is-member membership)
+      (ex/raise :type :validation
+                :code :user-doesnt-belong-organization))))

backend/src/app/rpc/commands/nitrate.clj

@@ -41,17 +41,6 @@
       (ex/raise :type :validation
                 :code :cant-move-default-team))))
 
-(defn assert-membership [cfg profile-id organization-id]
-  (let [membership (nitrate/call cfg :get-organization-membership {:profile-id profile-id
-                                                                   :organization-id organization-id})]
-    (when-not (:organization-id membership)
-      (ex/raise :type :validation
-                :code :organization-does-not-exist))
-
-    (when-not (:is-member membership)
-      (ex/raise :type :validation
-                :code :user-doesnt-belong-organization))))
-
 
 (def schema:connectivity
   [:map {:title "nitrate-connectivity"}
@@ -335,7 +324,7 @@
     (when-not skip-validation
       (assert-valid-teams cfg profile-id id default-team-id teams-to-delete teams-to-leave))
 
-    (assert-membership cfg profile-id id)
+    (nitrate/assert-membership cfg profile-id id)
 
     ;; delete only eligible teams (non-protected and without files)
     (doseq [id deletable-team-ids]
@@ -421,7 +410,7 @@
 
   (assert-is-owner cfg profile-id team-id)
   (assert-not-default-team cfg team-id)
-  (assert-membership cfg profile-id organization-id)
+  (nitrate/assert-membership cfg profile-id organization-id)
   ;; Check moveTeams permission on the source organization
   (when (contains? cf/flags :admin-console)
     (let [organization-perms (nitrate/call cfg :get-organization-permissions
@@ -491,7 +480,7 @@
 
   (assert-is-owner cfg profile-id team-id)
   (assert-not-default-team cfg team-id)
-  (assert-membership cfg profile-id organization-id)
+  (nitrate/assert-membership cfg profile-id organization-id)
 
   (when (contains? cf/flags :admin-console)
     (let [organization-member-ids-before (into #{} (nitrate/call cfg :get-organization-members {:organization-id organization-id}))
@@ -575,7 +564,7 @@
    ::db/transaction true}
   [{:keys [::db/conn] :as cfg} {:keys [::rpc/profile-id organization-id emails]}]
   (or (when (contains? cf/flags :admin-console)
-        (assert-membership cfg profile-id organization-id)
+        (nitrate/assert-membership cfg profile-id organization-id)
         (let [emails-array   (db/create-array conn "text" emails)
               profiles       (db/exec! conn [sql:get-profiles-by-emails emails-array])
               email->id      (into {} (map (fn [p] [(:email p) (:id p)])) profiles)
@@ -603,7 +592,7 @@
       (when-not (or (:is-admin perms) (:is-owner perms))
         (ex/raise :type :validation
                   :code :insufficient-permissions))
-      (assert-membership cfg profile-id organization-id)
+      (nitrate/assert-membership cfg profile-id organization-id)
       (let [organization-members     (nitrate/call cfg :get-organization-members {:organization-id organization-id})
             organization-member-ids  (into #{} organization-members)
             team-members    (db/query cfg :team-profile-rel {:team-id team-id})
@@ -631,7 +620,7 @@
       (let [team-members    (db/query cfg :team-profile-rel {:team-id team-id})
             team-member-ids (into #{} (map :profile-id team-members))]
         ;; Validate requester membership in all organizations before fetching members.
-        (run! #(assert-membership cfg profile-id %) organization-ids)
+        (run! #(nitrate/assert-membership cfg profile-id %) organization-ids)
 
         (into {}
               (map (fn [organization-id]
@@ -664,7 +653,7 @@
       (when-not (or (:is-admin perms) (:is-owner perms))
         (ex/raise :type :validation
                   :code :insufficient-permissions))
-      (assert-membership cfg profile-id organization-id)
+      (nitrate/assert-membership cfg profile-id organization-id)
       (let [{:keys [allows-anybody external-emails]} (get-external-invitation-info cfg team-id organization-id)]
         {:has-external-invitations (boolean (seq external-emails))
          :allows-anybody allows-anybody}))

backend/src/app/rpc/commands/teams.clj

@@ -538,6 +538,9 @@
   ;; When creating inside an organization, verify the user has permission to do so.
   ;; Fail closed: if organization permissions cannot be fetched, deny the operation.
   (when (and organization-id (contains? cf/flags :admin-console))
+    ;; Verify caller is a member of the organization
+    (nitrate/assert-membership cfg profile-id organization-id)
+
     (let [organization-perms (nitrate/call cfg :get-organization-permissions
                                            {:organization-id organization-id})]
       (if (nil? organization-perms)

backend/test/backend_tests/rpc_team_test.clj

@@ -1157,3 +1157,53 @@
                 :name "My Valid Team"}
           out  (th/command! data)]
       (t/is (th/success? out)))))
+
+(t/deftest create-team-in-organization-regression
+  (with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}]
+    (let [owner           (th/create-profile* 401 {:is-active true})
+          non-member      (th/create-profile* 402 {:is-active true})
+          organization-id (uuid/random)
+          params          {::th/type :create-team
+                           ::rpc/profile-id (:id owner)
+                           :name "Test Team"
+                           :organization-id organization-id}
+
+          nitrate-call-fn
+          (fn [_cfg method p]
+            (case method
+              :get-organization-membership
+              (if (= (:profile-id p) (:id non-member))
+                {:organization-id organization-id :is-member false}
+                {:organization-id organization-id :is-member true})
+
+              :get-organization-permissions
+              {:owner-id (:id owner)
+               :permissions {:create-teams "any"}}
+
+              :set-team-organization
+              (let [team-id (:team-id p)]
+                {:id team-id
+                 :name "Test Team"
+                 :organization-id organization-id
+                 :default-project-id (uuid/random)})
+
+              nil))]
+
+      ;; Non-member should be denied with :user-doesnt-belong-organization
+      (with-redefs [cf/flags (conj cf/flags :admin-console)
+                    nitrate/call nitrate-call-fn]
+        (let [out (th/command! (assoc params ::rpc/profile-id (:id non-member)))]
+          (t/is (not (th/success? out)))
+          (let [edata (-> out :error ex-data)]
+            (t/is (= :validation (:type edata)))
+            (t/is (= :user-doesnt-belong-organization (:code edata))))))
+
+      ;; Authorized member should succeed
+      (th/reset-mock! audit-mock)
+      (with-redefs [cf/flags (conj cf/flags :admin-console)
+                    nitrate/call nitrate-call-fn]
+        (let [out (th/command! params)]
+          (t/is (th/success? out))
+          (let [team (:result out)]
+            (t/is (uuid? (:id team)))
+            (t/is (= "Test Team" (:name team)))))))))