Improve People Picker URL structure

app/config/routes.php

@@ -114,6 +114,9 @@ function (RouteBuilder $builder) {
     ['controller' => 'ApiV2', 'action' => 'generateApiKey', 'model' => 'api_users'])
     ->setPass(['id'])
     ->setPatterns(['id' => '[0-9]+']);
+  $builder->get(
+    '/people/pick',
+    ['controller' => 'ApiV2', 'action' => 'pick', 'model' => 'people']);
   // These establish the usual CRUD options on all models:
   $builder->delete(
     '/{model}/{id}', ['controller' => 'ApiV2', 'action' => 'delete'])

app/src/Controller/ApiV2Controller.php

@@ -136,6 +136,25 @@ public function beforeRender(\Cake\Event\EventInterface $event) {
 
     return parent::beforeRender($event);
   }
+
+  /**
+   * Calculate the CO ID associated with the request.
+   *
+   * @since  COmanage Registry v5.0.0
+   * @return int      CO ID, or null if no CO contextwas found
+   */
+
+  public function calculateRequestedCOID(): ?int {
+    if($this->request->getQuery('group_id') !== null) {
+      $groupId = $this->request->getQuery('group_id');
+      $Group = TableRegistry::getTableLocator()->get('Groups');
+
+      $groupRecord = $Group->get($groupId);
+      return $groupRecord->co_id;
+    }
+
+    return null;
+  }
 
   /**
    * Handle a delete action for a Standard object.
@@ -181,7 +200,42 @@ public function delete($id) {
       throw new BadRequestException($this->exceptionToError($e));
     }
   }
-
+
+  protected function dispatchIndex(string $mode = 'default') {
+    // There are use cases where we will pass co_id and another model_id as a query parameter. The co_id might be
+    // required for the primary link calculations while the foreign key for filtering. Since we are using the
+    // most constrained identifier to calculate the co_id, we then check if the two parameters match. If not,
+    // the request should fail, so as to prevent any security holes.
+    if($this->request->getQuery('co_id') !== null
+      && $this->getCOID() !== null
+      && (int)$this->getCOID() !== (int)$this->request->getQuery('co_id')) {
+      $this->llog('error', 'CO Id calculated from Group ID does not match CO Id query parameter');
+      // Mask this with a generic UnauthorizedException
+      throw new UnauthorizedException(__d('error', 'perm'));
+    }
+
+    // $modelsName = Models
+    $modelsName = $this->name;
+    // $table = the actual table object
+    $table = $this->$modelsName;
+
+    $reqParameters = [...$this->request->getQuery()];
+    $pickerMode = ($mode === 'picker');
+
+    // Construct the Query
+    $query = $this->getIndexQuery($pickerMode, $reqParameters);
+
+    if(method_exists($table, 'findIndexed')) {
+      $query = $table->findIndexed($query);
+    }
+    // This magically makes REST calls paginated... can use eg direction=,
+    // sort=, limit=, page=
+    $this->set($this->tableName, $this->paginate($query));
+
+    // Let the view render
+    $this->render('/Standard/api/v2/json/index');
+  }
+
   /**
    * Handle an edit action for a Standard object.
    *
@@ -300,33 +354,7 @@ public function generateApiKey(string $id) {
    */
 
   public function index() {
-    // $modelsName = Models
-    $modelsName = $this->name;
-    // $table = the actual table object
-    $table = $this->$modelsName;
-
-    $reqParameters = [];
-    $pickerMode = false;
-    if($this->request->is('ajax')) {
-      $reqParameters = [...$this->request->getQuery()];
-      if($this->request->getQuery('picker') !== null) {
-        $pickerMode = filter_var($this->request->getQuery('picker'), FILTER_VALIDATE_BOOLEAN);
-      }
-    }
-
-
-    // Construct the Query
-    $query = $this->getIndexQuery($pickerMode, $reqParameters);
-
-    if(method_exists($table, 'findIndexed')) {
-      $query = $table->findIndexed($query);
-    }
-      // This magically makes REST calls paginated... can use eg direction=,
-    // sort=, limit=, page= 
-    $this->set($this->tableName, $this->paginate($query));
-
-    // Let the view render
-    $this->render('/Standard/api/v2/json/index');
+    $this->dispatchIndex();
   }
 
   /**
@@ -354,4 +382,14 @@ public function view($id = null) {
     // Let the view render
     $this->render('/Standard/api/v2/json/index');
   }
+
+  /**
+   * Pick a set of Standard Objects.
+   *
+   * @since  COmanage Registry v5.0.0
+   */
+
+  public function pick() {
+    $this->dispatchIndex(mode: 'picker');
+  }
 }

app/src/Controller/AppController.php

@@ -346,7 +346,8 @@ public function getPrimaryLink(bool $lookup=false) {
           }
         }
 
-        if(empty($this->cur_pl->value) && !$this->$modelsName->allowEmptyPrimaryLink()) {
+        if(empty($this->cur_pl->value)
+           && !$this->$modelsName->allowEmptyPrimaryLink($this->request->getParam('action'))) {
           throw new \RuntimeException(__d('error', 'primary_link'));
         }
       }

app/src/Lib/Traits/IndexQueryTrait.php

@@ -201,7 +201,7 @@ public function getIndexQuery(bool $pickerMode = false, array $requestParams = [
       // Specific expressions per view
       $query = match($requestParams['for'] ?? '') {
         // GroupMembers Add view: We need to filter the active members
-        'GroupMembers' => $query->leftJoinWith('GroupMembers', fn($q) => $q->where(['GroupMembers.group_id' => (int)($requestParams['groupid'] ?? -1)]))
+        'GroupMembers' => $query->leftJoinWith('GroupMembers', fn($q) => $q->where(['GroupMembers.group_id' => (int)($requestParams['group_id'] ?? -1)]))
                                 ->where($this->getTableLocator()->get('GroupMembers')->checkValidity($query))
                                 ->where(fn(QueryExpression $exp, Query $query) => $exp->isNull('GroupMembers.' . StringUtilities::classNameToForeignKey($table->getAlias()))),
         // Just return the query

app/src/Model/Table/PeopleTable.php

@@ -148,6 +148,7 @@ public function initialize(array $config): void {
     $this->setRequiresCO(true);
     $this->setRedirectGoal('self');
     $this->setAllowLookupPrimaryLink(['provision']);
+    $this->setAllowUnkeyedPrimaryLink(['pick']);
 
 // XXX does some of this stuff really belong in the controller?
     $this->setEditContains([
@@ -222,7 +223,8 @@ public function initialize(array $config): void {
       // Actions that operate over a table (ie: do not require an $id)
       'table' => [
         'add' =>      ['platformAdmin', 'coAdmin'],
-        'index' =>    ['platformAdmin', 'coAdmin']
+        'index' =>    ['platformAdmin', 'coAdmin'],
+        'pick' =>     ['platformAdmin', 'coAdmin'],
       ],
       // Related models whose permissions we'll need, typically for table views
       'related' => [

app/templates/GroupMembers/columns.inc

@@ -101,7 +101,6 @@ $peoplePicker = [
   'label' => __d('operation','add.member'),
   'viewConfigParameters' => [
     'for' => 'GroupMembers',
-    'action' => $vv_action,
     'groupId' => $this->getRequest()?->getQuery('group_id')
   ],
   'actionUrl' => [

app/templates/element/peopleAutocomplete.php

@@ -67,7 +67,8 @@
     api: {
       viewConfigParameters: <?= json_encode($viewConfigParameters) ?>,
       webroot: '<?= $this->request->getAttribute('webroot') ?>',
-      searchPeople: `<?= $this->request->getAttribute('webroot') ?>api/ajax/v2/people?co_id=<?= $vv_cur_co->id ?>&picker=on&for=<?= $viewConfigParameters['for'] ?>`
+      // co_id query parameter is required since it is the People's primary link
+      searchPeople: `<?= $this->request->getAttribute('webroot') ?>api/ajax/v2/people/pick?co_id=<?= $vv_cur_co->id ?>&for=<?= $viewConfigParameters['for'] ?>`
     }
   }
 

app/webroot/js/comanage/components/autocomplete/cm-autocomplete-people.js

@@ -69,10 +69,7 @@ export default {
       queryParams.append('given', query)
       queryParams.append('family', query)
       if(this.api.viewConfigParameters.groupId != undefined) {
-        queryParams.append('groupid', this.api.viewConfigParameters.groupId)
-      }
-      if(this.api.viewConfigParameters.action != undefined) {
-        queryParams.append('action', this.api.viewConfigParameters.action)
+        queryParams.append('group_id', this.api.viewConfigParameters.groupId)
       }
       // Pagination
       // XXX Move this to configuration