From 47e261f29eca0a8b41a4043b09440629053869d6 Mon Sep 17 00:00:00 2001 From: Ian Neilson Date: Wed, 2 Jun 2021 14:48:56 +0000 Subject: [PATCH] Handle edit and delete api credentials without owning user --- .../controllers/site/edit_api_auth.php | 8 ++-- .../web_portal/views/site/edit_api_auth.php | 48 ++++++++++++++----- htdocs/web_portal/views/site/view_site.php | 40 +++++++++------- lib/Gocdb_Services/Site.php | 48 +++++++++++-------- 4 files changed, 89 insertions(+), 55 deletions(-) diff --git a/htdocs/web_portal/controllers/site/edit_api_auth.php b/htdocs/web_portal/controllers/site/edit_api_auth.php index 16f992abd..e75a2cfec 100644 --- a/htdocs/web_portal/controllers/site/edit_api_auth.php +++ b/htdocs/web_portal/controllers/site/edit_api_auth.php @@ -62,17 +62,13 @@ function draw(\User $user = null, \APIAuthentication $authEnt = null, \Site $sit throw new Exception("Unregistered users can't edit authentication credentials"); } + $params = array(); $params['site'] = $site; $params['authEnt'] = $authEnt; $params['authTypes'] = array(); $params['authTypes'][]='X509'; $params['authTypes'][]='OIDC Subject'; $params['user'] = $user; - $params['allowWrite'] = $authEnt->getAllowAPIWrite(); - // If the user is changing, send in the new user DN - if ($user->getId() != $authEnt->getUser()->getId()) { - $params['currentUserIdent'] = $authEnt->getUser()->getCertificateDn(); - } show_view("site/edit_api_auth.php", $params); die(); @@ -87,6 +83,8 @@ function submit(\User $user, \APIAuthentication $authEnt, \Site $site, org\gocdb show_view('error.php', $e->getMessage()); die(); } + + $params = array(); $params['apiAuthenticationEntity'] = $authEnt; $params['site'] = $site; show_view("site/edited_api_auth.php", $params); diff --git a/htdocs/web_portal/views/site/edit_api_auth.php b/htdocs/web_portal/views/site/edit_api_auth.php index ebe91a703..9663b898b 100644 --- a/htdocs/web_portal/views/site/edit_api_auth.php +++ b/htdocs/web_portal/views/site/edit_api_auth.php @@ -1,20 +1,42 @@
-

Edit API credential for getName());?>

-

This credential is linked to GOCDB user - - getUser()->getFullname())?> - -

getUser(); + + echo('

Edit API credential for '); + xecho($params['site']->getName()); + echo('

'); + + if (!is_null($entUser)) { + + echo('

This credential is linked to GOCDB user '); + echo(''); + xecho($entUser->getFullname()); + echo('

'); + + // entities created prior to GOCDB5.8 have a null owning user + if ($entUser->getId() != $user->getId()) { echo('
'); - echo("WARNING: editing will change the linked identity from '"); - xecho($params['currentUserIdent']); + echo("WARNING: editing will change the linked user from '"); + xecho($entUser->getFullname()); echo("' to '"); - xecho($params['user']->getCertificateDn()); + xecho($user->getFullname()); echo("'. Click the browser Back button to cancel the edit.
"); - } + } + + } else { + // This clause should be deleted or replaced with exception after all + // authentication entities are assigned a user. + echo('
'); + echo("WARNING: editing will link user '"); + xecho($user->getFullname()); + echo("' to this credential. Click the browser Back button to cancel the edit.
"); + } ?>
@@ -39,7 +61,7 @@
getAllowAPIWrite()) { echo('checked="checked"');} ?> /> diff --git a/htdocs/web_portal/views/site/view_site.php b/htdocs/web_portal/views/site/view_site.php index b5eb32ae2..32ab5f00a 100644 --- a/htdocs/web_portal/views/site/view_site.php +++ b/htdocs/web_portal/views/site/view_site.php @@ -571,20 +571,17 @@ getIdentifier() == $APIAuthEnt->getUser()->getCertificateDn())) { - // This credential is the owning user's own. - // isable edit and delete - // for other users. - // TODO-irn: decide if this is sensible?? - if ($params['userId'] == $APIAuthEnt->getUser()->getId()) { - $disableButtons = true; - } + // Finer grain control of edit or delete could be put here + // Currently work around pre-5.8 credentials having no owning user. + $disableEdit = true; + $disableDelete = true; + if ($APIAuthEnt->getIdentifier() == Get_User_Principle()) { + // If the owning user is making the request, we always allow them to + // delete the credential + $disableDelete = false; } - */ ?> @@ -594,10 +591,17 @@ getIdentifier())?> - - getUser()->getSurname(),0,10))?> - + getUser() != null) { + // Credentials added prior to 5.8 have no owning user + echo "getUser()->getId(), "\" "; + echo "title=\"", $APIAuthEnt->getUser()->getFullname(), "\">"; + echo substr($APIAuthEnt->getUser()->getSurname(),0,10); + echo ""; + } + ?> - @@ -619,7 +623,7 @@
-
diff --git a/lib/Gocdb_Services/Site.php b/lib/Gocdb_Services/Site.php index 7fb5a9cd8..b07874d4a 100644 --- a/lib/Gocdb_Services/Site.php +++ b/lib/Gocdb_Services/Site.php @@ -981,11 +981,16 @@ public function validatePropertyActions(\User $user, \Site $site) { * @return boolian */ public function userCanEditSite(\User $user, \Site $site) { - if ($this->roleActionAuthorisationService->authoriseAction(\Action::EDIT_OBJECT, $site, $user)->getGrantAction() == FALSE) { + + if (is_null($user)) { return false; - } else { + } + + if ($this->roleActionAuthorisationService->authoriseAction(\Action::EDIT_OBJECT, $site, $user)->getGrantAction() == TRUE) { return true; } + + return false; } /** @@ -1398,13 +1403,9 @@ public function getAPIAuthenticationEntity($id) { } public function addAPIAuthEntity(\Site $site, \User $user, $newValues) { - //Check the portal is not in read only mode, throws exception if it is - $this->checkPortalIsNotReadOnlyOrUserIsAdmin($user); // Validate the user has permission to add properties - if (!$this->userCanEditSite($user, $site)) { - throw new \Exception("You don't have permission to add authentication entties to " . $site->getShortName()); - } + $this->checkUserAuthz ($user, $site); $authEntServ = \Factory::getAPIAuthenticationService(); $authEntServ->setEntityManager($this->em); @@ -1415,14 +1416,12 @@ public function addAPIAuthEntity(\Site $site, \User $user, $newValues) { } public function deleteAPIAuthEntity(\APIAuthentication $authEntity, \User $user) { - //Check the portal is not in read only mode, throws exception if it is - $this->checkPortalIsNotReadOnlyOrUserIsAdmin($user); // Validate the user has permission to delete properties - $site = $authEntity->getParentSite(); - if (!$this->userCanEditSite($user, $site)) { - throw new \Exception("You don't have permission to add authentication entities to " . $site->getShortName()); - } + $parentSite = $authEntity->getParentSite(); + + // Check the user can do this. Thows exception if not. + $this->checkUserAuthz($user, $parentSite); $authEntServ = \Factory::getAPIAuthenticationService(); $authEntServ->setEntityManager($this->em); @@ -1431,15 +1430,11 @@ public function deleteAPIAuthEntity(\APIAuthentication $authEntity, \User $user) } public function editAPIAuthEntity(\APIAuthentication $authEntity, \User $user, $newValues) { - //Check the portal is not in read only mode, throws exception if it is - $this->checkPortalIsNotReadOnlyOrUserIsAdmin($user); $parentSite = $authEntity->getParentSite(); - // Validate the user has permission to edit properties - if (!$this->userCanEditSite($user, $parentSite)) { - throw new \Exception("Permission denied: a site role is required to add authentication entities to " . $parentSite->getShortName()); - } + // Check the user can do this. Thows exception if not. + $this->checkUserAuthz($user, $parentSite); $identifier = $newValues['IDENTIFIER']; $type = $newValues['TYPE']; @@ -1468,4 +1463,19 @@ public function editAPIAuthEntity(\APIAuthentication $authEntity, \User $user, $ return $authEntity; } + /** + * Helper combines admin check and authz check to make sure a user + * can modify properties of the site. + * + * @throws \Exception + */ + private function checkUserAuthz (\User $user, \Site $site) { + + $this->checkPortalIsNotReadOnlyOrUserIsAdmin($user); + + // Validate the user has permission to edit properties + if (!$this->userCanEditSite($user, $site)) { + throw new \Exception("Permission denied: a site role is required to modify authentication entities on site " . $site->getShortName()); + } + } }