CFM-496_REST_API_v2_DELETE_CO_Group_returns_400 (#360)

* Fix groups readonly delete. Improve error information for priviledged users. Fix api user key title caclulation bug. * Fix external identity source title configuration * Revert detailed RecordNotFound errors for CO-privileged API users; restrict detail to platform API users
COmanage · Apr 6, 2026 · 9e1c138 · 9e1c138
1 parent 0d712e2
commit 9e1c138
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 16 deletions.
diff --git a/app/src/Controller/ApiUsersController.php b/app/src/Controller/ApiUsersController.php
@@ -56,9 +56,11 @@ public function generate(string $id) {
       $this->Flash->error($e->getMessage());
     }
 
-    [$title, , ] = StringUtilities::entityAndActionToTitle(null,
-                                                           'api.key',
-                                                           $this->request->getParam('action'));
+    [$title, , ] = StringUtilities::entityAndActionToTitle(
+      null,
+      null,
+      'api.key.' . $this->request->getParam('action'),
+    );
     $this->set('vv_title', $title);
 
     // Render the view.

diff --git a/app/src/Controller/ApiV2Controller.php b/app/src/Controller/ApiV2Controller.php
@@ -200,13 +200,12 @@ public function delete($id) {
     try {
       $obj = $table->findById($id)->firstOrFail();
 
-// XXX document AR-CO-1 when we implement hard delete/changelog
-//     note similar logic in StandardController
-      $table->deleteOrFail($obj);
-
       if(method_exists($obj, "isReadOnly") && $obj->isReadOnly()) {
         throw new BadRequestException(__d('error', 'edit.readonly'));
       }
+// XXX document AR-CO-1 when we implement hard delete/changelog
+//     note similar logic in StandardController
+      $table->deleteOrFail($obj);
 
       // Trigger provisioning, letting errors bubble up (AR-GMR-5)
       if(method_exists($table, "requestProvisioning")) {

diff --git a/app/src/Controller/Component/RegistryAuthComponent.php b/app/src/Controller/Component/RegistryAuthComponent.php
@@ -240,12 +240,16 @@ public function beforeFilter(EventInterface $event) {
         throw new ForbiddenException(__d('error', 'perm'));
       }
       catch(RecordNotFoundException $e) {
-        // Requested record does not exist. For platform API users, we can return
-        // a RecordNotFoundException, otherwise we recast to generate permission denied.
+        // The requested record does not exist. Because CO IDs are derived from existing
+        // records (via primary links / CO resolution helpers), we cannot reliably
+        // determine whether a privileged CO-scoped API user is actually related to
+        // a record that failed with RecordNotFoundException. To avoid leaking
+        // cross-CO information, we only allow platform API users to see the original
+        // exception and return a generic API failure for all other API users.
         $this->llog('debug', "User authorization failed: " . $e->getMessage());
-        
+
         $ApiUsers = TableRegistry::getTableLocator()->get('ApiUsers');
-        
+
         if($ApiUsers->getUserPrivilege($this->authenticatedUser) === true) {
           throw $e;
         } else {

diff --git a/app/src/Controller/ExternalIdentitySourcesController.php b/app/src/Controller/ExternalIdentitySourcesController.php
@@ -187,9 +187,11 @@ public function search(string $id) {
 
     $this->set('vv_search_attrs', $this->ExternalIdentitySources->searchableAttributes((int)$id));
 
-    [$title, , ] = StringUtilities::entityAndActionToTitle(null,
-                                                           $this->getName(),
-                                                           $this->request->getParam('action'));
+    [$title, , ] = StringUtilities::entityAndActionToTitle(
+      null,
+      null,
+      $this->getName() . '.search',
+    );
     $this->set('vv_title', $title);
   }
 

diff --git a/app/src/Controller/StandardController.php b/app/src/Controller/StandardController.php
@@ -252,6 +252,13 @@ public function delete($id) {
     try {
       $obj = $table->findById($id)->firstOrFail();
 
+      // Align with REST API: do not allow delete of read-only records
+      if (method_exists($obj, "isReadOnly") && $obj->isReadOnly()) {
+        $this->Flash->error(__d('error', 'edit.readonly'));
+        // Redirect to view, as we do for read-only edits
+        return $this->redirect(['action' => 'view', $obj->id]);
+      }
+
       // By default, a delete is a soft delete. The exceptions is when
       // deleting a CO (AR-CO-1). In v4, we permitted a controller level
       // flag to be set, but the only controller this really applies to

diff --git a/app/src/Model/Table/ApiUsersTable.php b/app/src/Model/Table/ApiUsersTable.php
@@ -179,11 +179,11 @@ public function generateKey(int $id) {
    * 
    * @since  COmanage Registry v5.0.0
    * @param  string $username API Username
-   * @return mixed            true if $username is a platform API user, an integer (the CO ID) if the user is a privileged API user within that CO, or false otherwise
+   * @return bool|int         true if $username is a platform API user, an integer (the CO ID) if the user is a privileged API user within that CO, or false otherwise
    * @throws InvalidArgumentException
    */
 
-  public function getUserPrivilege(string $username) {
+  public function getUserPrivilege(string $username): bool|int {
     $apiUser = $this->find()->where(['username' => $username])->contain('Cos')->first();
 
     if(empty($apiUser)) {