Various improvements and fixes to Pipelines and Person Role managemen…

…t (CFM-33, CFM-344, etc)
COmanage · Dec 6, 2023 · 7193caa · 7193caa
1 parent 1f02168
commit 7193caa
Show file tree

Hide file tree

Showing 40 changed files with 1,384 additions and 278 deletions.
diff --git a/app/resources/locales/en_US/enumeration.po b/app/resources/locales/en_US/enumeration.po
@@ -90,8 +90,8 @@ msgstr "Pending Activation"
 msgid "ExternalIdentityStatusEnum.S"
 msgstr "Suspended"
 
-msgid "ExternalIdentityStatusEnum.XP"
-msgstr "Expired"
+msgid "ExternalIdentityStatusEnum.DX"
+msgstr "Deleted"
 
 msgid "GroupTypeEnum.MA"
 msgstr "Active Members"

diff --git a/app/resources/locales/en_US/error.po b/app/resources/locales/en_US/error.po
@@ -229,6 +229,10 @@ msgstr "Page number must be an integer"
 msgid "perm"
 msgstr "Permission Denied"
 
+msgid "PersonRoles.valid_from.after"
+msgstr "Valid From date must be earlier than Valid Through date"
+
+
 msgid "Plugins.inactive"
 msgstr "The plugin {0} is not active"
 

diff --git a/app/resources/locales/en_US/result.po b/app/resources/locales/en_US/result.po
@@ -48,6 +48,9 @@ msgstr "{0} {1} Deleted: {2}"
 msgid "edited.mvea"
 msgstr "{0} {1} Edited: {2}"
 
+msgid "ExternalIdentities.status.recalculated"
+msgstr "External Identity status recalculated from {0} to {1}"
+
 msgid "ExternalIdentitySources.synced"
 msgstr "External Identity Source sync complete"
 
@@ -108,9 +111,21 @@ msgstr "Started via JobCommand by {0} (uid {1})"
 msgid "People.added.pipeline"
 msgstr "Created new Person via Pipeline {0} ({1}) using Source {2} ({3}) Key {4}"
 
+msgid "People.status.recalculated"
+msgstr "Person status recalculated from {0} to {1}"
+
+msgid "PersonRoles.status.recalculated"
+msgstr "Person Role status recalculated from {0} to {1}"
+
+msgid "Pipelines.complete"
+msgstr "Pipeline {0} complete for EIS {1} source key {2}"
+
 msgid "Pipelines.ei.added"
 msgstr "Created new External Identity via Pipeline {0} ({1}) using Source {2} ({3}) Key {4}"
 
+msgid "Pipelines.started"
+msgstr "Pipeline {0} started for EIS {1} source key {2}"
+
 msgid "saved"
 msgstr "Saved"
 

diff --git a/app/src/Controller/ApiV2Controller.php b/app/src/Controller/ApiV2Controller.php
@@ -97,7 +97,7 @@ public function add() {
           $results[] = ['id' => $obj->id];
 
           // Trigger provisioning, letting errors bubble up (AR-GMR-5)
-          if(method_exists($table, "requestProvisioning")) {
+          if(method_exists($this->modelsName, "requestProvisioning")) {
             $this->llog('rule', "AR-GMR-5 Requesting provisioning for $modelsName " . $obj->id);
             $table->requestProvisioning(id: $obj->id, context: ProvisioningContextEnum::Automatic);
           }

diff --git a/app/src/Controller/AppController.php b/app/src/Controller/AppController.php
@@ -33,6 +33,7 @@
 use App\Lib\Events\ChangelogEventListener;
 use App\Lib\Events\CoIdEventListener;
 use App\Lib\Events\RuleBuilderEventListener;
+use App\Lib\Util\StringUtilities;
 use Cake\Controller\Controller;
 use Cake\Core\Configure;
 use Cake\Datasource\Exception;
@@ -219,6 +220,20 @@ public function getPrimaryLink(bool $lookup=false) {
       if($lookup) {
         foreach($availablePrimaryLinks as $potentialPrimaryLink) {
           // Try to find a value
+
+          if(strstr($potentialPrimaryLink, '.')) {
+            // For looking up values in records here, we want only the attribute
+            // itself and not the plugin name (used for hacky notation by
+            // PrimaryLinkTrait::setPrimaryLink(). Note this is a field and not
+            // a model, but pluginModel() gets us the bit we need.
+
+            // Store the plugin for possible later reference.
+            $potentialPlugin = StringUtilities::pluginPlugin($potentialPrimaryLink);
+
+            // We clobber $potentialPrimaryLink to avoid rewriting a bunch of code,
+            // but probably we should rewrite it.
+            $potentialPrimaryLink = StringUtilities::pluginModel($potentialPrimaryLink);
+          }
 
           if($this->request->is('get')) {
             // If this action allows unkeyed, asserted primary link IDs, check the query
@@ -269,7 +284,13 @@ public function getPrimaryLink(bool $lookup=false) {
 
                 $this->cur_pl->value = $linkValue;
               }
-            } elseif($this->$modelsName->allowLookupPrimaryLink($this->request->getParam('action'))) {
+            }
+
+            // If we didn't find the primary link in the submitted form or API
+            // request, it might be available via the URL.
+
+            if(!$linkValue
+               && $this->$modelsName->allowLookupPrimaryLink($this->request->getParam('action'))) {
               // Try to map the requested object ID (this is probably a delete, so no attribute in post body)
               $param = (int)$this->request->getParam('pass.0');
 
@@ -439,40 +460,48 @@ protected function setCO() {
       // Nothing to do...
       return;
     }
-
-    // $this->name = Models, unless we're in an API call
-    $modelsName = $this->name;
-
-    $attrs = $this->request->getAttributes();
-
-    // Unlike Match, where the Matchgrid is embedded in the request API URL,
-    // Registry API calls are more similar to UI calls, where we may or may
-    // not be able to find the CO ID directly in the URL.
-    if($this->request->is('restful') 
-       && !empty($attrs['params']['model'])) {
-      $modelsName = \Cake\Utility\Inflector::camelize($attrs['params']['model']);
-      $this->$modelsName = TableRegistry::getTableLocator()->get($modelsName);
-    }
-
-    if(!method_exists($this->$modelsName, "requiresCO")
-       || !$this->$modelsName->requiresCO()) {
-      // Nothing to do, CO not required by this model/controller
-      return;
-    }
-
-    // Not all models have CO as their primary link. This will also
-    // trigger setting of the viewVar for breadcrumbs and anything else.
-    $link = $this->getPrimaryLink(true);
-
+
     // Try to find the requested CO
     $coid = null;
 
-    // getPrimaryLink has already done our work
-    if($link->attr == 'co_id') {
-      $coid = $link->value;
-    } else {
-      if(!empty($link->co_id)) {
-        $coid = $link->co_id;
+    if(method_exists($this, 'calculateRequestedCOID')) {
+      // This controller implements special logic
+
+      $coid = $this->calculateRequestedCOID();
+    }
+
+    if(!$coid) {
+      // $this->name = Models, unless we're in an API call
+      $modelsName = $this->name;
+
+      $attrs = $this->request->getAttributes();
+
+      // Unlike Match, where the Matchgrid is embedded in the request API URL,
+      // Registry API calls are more similar to UI calls, where we may or may
+      // not be able to find the CO ID directly in the URL.
+      if($this->request->is('restful') 
+        && !empty($attrs['params']['model'])) {
+        $modelsName = \Cake\Utility\Inflector::camelize($attrs['params']['model']);
+        $this->$modelsName = TableRegistry::getTableLocator()->get($modelsName);
+      }
+
+      if(!method_exists($this->$modelsName, "requiresCO")
+        || !$this->$modelsName->requiresCO()) {
+        // Nothing to do, CO not required by this model/controller
+        return;
+      }
+
+      // Not all models have CO as their primary link. This will also
+      // trigger setting of the viewVar for breadcrumbs and anything else.
+      $link = $this->getPrimaryLink(true);
+
+      // getPrimaryLink has already done our work
+      if($link->attr == 'co_id') {
+        $coid = $link->value;
+      } else {
+        if(!empty($link->co_id)) {
+          $coid = $link->co_id;
+        }
       }
     }
 
@@ -499,17 +528,21 @@ protected function setCO() {
         throw new \InvalidArgumentException(__d('error', 'inactive', [__d('controller', 'Cos', [1]), $coid]));
       }
 
-      // We store the CO ID in Configuration to facilitate its access from
-      // model contexts such as validation where passing the value via the
-      // Controller is not particularly feasible.
+      if(!empty($modelsName) && !empty($this->$modelsName)) {
+        // We store the CO ID in Configuration to facilitate its access from
+        // model contexts such as validation where passing the value via the
+        // Controller is not particularly feasible. Note that for API calls
+        // $modelsName may not be set, so (eg) StandardApiController does
+        // something similar.
 
-      // This only works for the current model, not related models. If/when we
-      // need to support relatedmodels, we could have setCurCoId() cascade the
-      // CO to any of its related models that require it, or use the event
-      // listener approach commented out below.
-      if(method_exists($this->$modelsName, "acceptsCoId") 
-         && $this->$modelsName->acceptsCoId()) {
-        $this->$modelsName->setCurCoId((int)$coid);
+        // This only works for the current model, not related models. If/when we
+        // need to support relatedmodels, we could have setCurCoId() cascade the
+        // CO to any of its related models that require it, or use the event
+        // listener approach commented out below.
+        if(method_exists($this->$modelsName, "acceptsCoId") 
+          && $this->$modelsName->acceptsCoId()) {
+          $this->$modelsName->setCurCoId((int)$coid);
+        }
 
         /* This doesn't work for the current model since it has already been
            initialized, but it could be an option for related models later...

diff --git a/app/src/Controller/Component/BreadcrumbComponent.php b/app/src/Controller/Component/BreadcrumbComponent.php
@@ -32,6 +32,7 @@
 use \Cake\Controller\Component;
 use \Cake\Event\EventInterface;
 use \Cake\ORM\TableRegistry;
+use \Cake\Utility\Inflector;
 use \App\Lib\Util\StringUtilities;
 
 class BreadcrumbComponent extends Component
@@ -179,11 +180,7 @@ public function injectPrimaryLink(object $link) {
     $modelsName = StringUtilities::foreignKeyToClassName($link->attr);
 
     $contain = [];
-
-    if($modelsName == 'People' || $modelsName == 'ExternalIdentities') {
-      // We need the Primary Name to render it
-      $contain[] = 'PrimaryName';
-    }
+    $primaryName = null;
 
     $linkTable = TableRegistry::getTableLocator()->get($modelsName);
     $linkObj = $linkTable->get($link->value, ['contain' => $contain]);
@@ -203,8 +200,15 @@ public function injectPrimaryLink(object $link) {
 
     $label = $linkObj->$displayField;
 
-    if(!empty($linkObj->primary_name)) {
-      $label = $linkObj->primary_name->full_name;
+    if($modelsName == 'People' || $modelsName == 'ExternalIdentities') {
+      // We need the Primary Name (or first name found) to render it
+
+      $Names = TableRegistry::getTableLocator()->get('Names');
+
+      // This will throw an error on failure
+      $primaryName = $Names->primaryName($linkObj->id, Inflector::underscore(Inflector::singularize($modelsName)));
+
+      $label = $primaryName->full_name;
     }
 
     // If we don't have a visible label use the record ID

diff --git a/app/src/Controller/Component/RegistryAuthComponent.php b/app/src/Controller/Component/RegistryAuthComponent.php
@@ -130,6 +130,41 @@ public function beforeFilter(EventInterface $event) {
 
     // Perform authorization check
 
+    // Controllers can handle their own authn and/or authz, as indicated
+    // by implementing the willHandleAuth() function. This applies to both
+    // regular and API requests.
+
+    $controllerAuthz = false;
+
+    if(method_exists($controller, 'willHandleAuth')) {
+      // The Controller might handle its own authn/z
+
+      $mode = $controller->willHandleAuth($event);
+
+      switch($mode) {
+        case 'authz':
+          // The controller will handle authorization, but we still need
+          // to make sure we have an authenticated user
+          $controllerAuthz = true;
+          break;
+        case 'open':
+          // The current request is open/public, no auth required
+          return true;
+          break;
+        case 'no':
+          // The controller will not do either authn or authz, so apply
+          // standard behavior
+          break;
+        case 'yes':
+          // The controller will handle both authn and authz, simply return
+          return true;
+          break;
+        default:
+          throw new \InvalidArgumentException("Unknown willHandleAuth return value $mode");
+          break;
+      }
+    }
+
     // Do we have an authenticated user session?
 
     // Note we don't stuff anything into the session anymore, the only attribute
@@ -146,9 +181,22 @@ public function beforeFilter(EventInterface $event) {
 
       try {
         if($this->authenticateApiUser()) {
-          if($this->calculatePermission(action: $request->getParam('action'), id: $id)) {
+          $authok = false;
+
+          if($controllerAuthz) {
+            // Don't merge these if statements together! We want to hand off
+            // to the controller to determine if authz was met, and if not redirect
+            // appropriately. We _don't_ want to call our own calculatePermission().
+            if($controller->calculatePermission()) {
+              // Controller asserts authorization successful
+              $authok = true;
+            }
+          } elseif($this->calculatePermission(action: $request->getParam('action'), id: $id)) {
             // Authorization successful
-
+            $authok = true;
+          }
+
+          if($authok) {
             $AuthenticationEvents = TableRegistry::getTableLocator()->get('AuthenticationEvents');
 
             $AuthenticationEvents->record(identifier: $this->authenticatedUser,
@@ -182,12 +230,6 @@ public function beforeFilter(EventInterface $event) {
       }
     } else {
       // Certain requests do not require authentication
-
-      // XXX is this too broad, or are all Pages permitted? Also, should this move
-      // into Controller::isAuthorized?
-      if($controller->getName() == 'Pages') {
-        return true;
-      }
 
       if(!empty($auth['external']['user'])) {
         // We have a valid username that is *authenticated* for the current request.
@@ -196,7 +238,14 @@ public function beforeFilter(EventInterface $event) {
         $controller->set('vv_user', ['username' => $auth['external']['user']]);
         $this->authenticatedUser = $auth['external']['user'];
 
-        if($this->calculatePermission($request->getParam('action'), $id)) {
+        if($controllerAuthz) {
+          // Don't merge these if statements together! We want to hand off
+          // to the controller to determine if authz was met, and if not redirect
+          // appropriately. We _don't_ want to call our own calculatePermission().
+          if($controller->calculatePermission()) {
+            return true;
+          }
+        } elseif($this->calculatePermission($request->getParam('action'), $id)) {
           // Authorization successful
           return true;
         }

diff --git a/app/src/Controller/DashboardsController.php b/app/src/Controller/DashboardsController.php
@@ -76,14 +76,14 @@ public function artifacts() {
     // could be delegated to (eg) a COU Admin at some point...
 
     $artifactMenuItems = [
-      __d('controller', 'Jobs', [99]) => [
+      __d('controller', 'ExtIdentitySourceRecords', [99]) => [
         'icon'          => 'assignment',
-        'controller'    => 'jobs',
+        'controller'    => 'ext_identity_source_records',
         'action'        => 'index'
       ],
-      __d('controller', 'ExternalIdentitySourceRecords', [99]) => [
+      __d('controller', 'Jobs', [99]) => [
         'icon'          => 'assignment',
-        'controller'    => 'external_identity_source_records',
+        'controller'    => 'jobs',
         'action'        => 'index'
       ]
     ];

diff --git a/app/src/Controller/ExternalIdentitiesController.php b/app/src/Controller/ExternalIdentitiesController.php
@@ -38,11 +38,11 @@
 class ExternalIdentitiesController extends MVEAController {
   public $paginate = [
     'order' => [
-      'PrimaryName.family' => 'asc'
+      'Name.family' => 'asc'
     ],
     'sortableFields' => [
-      'PrimaryName.given',
-      'PrimaryName.family'
+      'Names.given',
+      'Names.family'
     ]
   ];
 }