Skip to content

Commit

Permalink
Handle edit and delete api credentials without owning user
Browse files Browse the repository at this point in the history
  • Loading branch information
Ian Neilson committed Jun 13, 2022
1 parent 8f46f02 commit 47e261f
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 55 deletions.
8 changes: 3 additions & 5 deletions htdocs/web_portal/controllers/site/edit_api_auth.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand Down
48 changes: 35 additions & 13 deletions htdocs/web_portal/views/site/edit_api_auth.php
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,42 @@
<div class="rightPageContainer">
<h1>Edit API credential for <?php xecho($params['site']->getName());?></h1>
<h4>This credential is linked to GOCDB user
<a href="<?php echo \GocContextPath::getPath()?>index.php?Page_Type=User&id=<?php echo $params['authEnt']->getUser()->getId()?>" >
<?php xecho($params['authEnt']->getUser()->getFullname())?>
</a>
</h4>
<?php
// currentUserIdent is only initialised if the user is changing
if ($params['currentUserIdent']) {

$user = $params['user'];
$entUser = $params['authEnt']->getUser();

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

if (!is_null($entUser)) {

echo('<h4>This credential is linked to GOCDB user ');
echo('<a href="');
xecho(\GocContextPath::getPath());
echo('index.php?Page_Type=User&id=');
xecho($entUser->getId());
echo('">');
xecho($entUser->getFullname());
echo('</a></h4>');

// entities created prior to GOCDB5.8 have a null owning user
if ($entUser->getId() != $user->getId()) {
echo('<div class="input_warning">');
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.</div>");
}
}

} else {
// This clause should be deleted or replaced with exception after all
// authentication entities are assigned a user.
echo('<div class="input_warning">');
echo("WARNING: editing will link user '");
xecho($user->getFullname());
echo("' to this credential. Click the browser Back button to cancel the edit.</div>");
}
?>
<form class="inputForm" method="post" action="index.php?Page_Type=Edit_API_Authentication_Entity&parentid=<?php echo($params['site']->getId())?>&authentityid=<?php xecho($params['authEnt']->getId())?>" name="addAPIAuthenticationEntity">
<div style="margin-bottom: 0.5em;">
Expand All @@ -39,7 +61,7 @@
<div class="input_checkbox">
<input type="checkbox" name="ALLOW_WRITE" id="ALLOW_WRITE" value="checked"
<?php
if ($params['allowWrite']) { echo('checked="checked"');}
if ($params['authEnt']->getAllowAPIWrite()) { echo('checked="checked"');}
?>
/>
<label class="input_label" for="ALLOW_WRITE">Allow API write</label>
Expand Down
40 changes: 22 additions & 18 deletions htdocs/web_portal/views/site/view_site.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,20 +571,17 @@
</thead>
<tbody>
<?php
/** @var \APIAuthentication $APIAuthEnt */
foreach ($params['APIAuthEnts'] as $APIAuthEnt) {
$disableButtons = false;
/* TODO-irn Add configuration switch for disabling buttons - default for now
* to enabled.
if (($APIAuthEnt->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;
}
*/
?>
<tr>
<td>
Expand All @@ -594,10 +591,17 @@
<?php xecho($APIAuthEnt->getIdentifier())?>
</td>
<td>
<a href="index.php?Page_Type=User&amp;id=<?php xecho($APIAuthEnt->getUser()->getId())?>"
title="<?php xecho($APIAuthEnt->getUser()->getFullname())?>">
<?php xecho(substr($APIAuthEnt->getUser()->getSurname(),0,10))?>
</a>
<?php
$disableEdit = false;
$disableDelete = false;
if ($APIAuthEnt->getUser() != null) {
// Credentials added prior to 5.8 have no owning user
echo "<a href=\"index.php?Page_Type=User&amp;id=", $APIAuthEnt->getUser()->getId(), "\" ";
echo "title=\"", $APIAuthEnt->getUser()->getFullname(), "\">";
echo substr($APIAuthEnt->getUser()->getSurname(),0,10);
echo "</a>";
}
?>
</td>
<td style="width: 8%; text-align:center">
<img height="22px" src=
Expand All @@ -611,15 +615,15 @@
<td style="width: 8%;"align = "center">
<?php if(!$portalIsReadOnly):?>
<form action="index.php?Page_Type=Edit_API_Authentication_Entity&amp;authentityid=<?php echo $APIAuthEnt->getId();?>" method="post">
<button type="submit" <?php if ($disableButtons) echo "disabled"; ?>
<button type="submit" <?php if ($disableEdit) echo "disabled"; ?>
>Edit</button>
</form>
<?php endif;?>
</td>
<td style="width: 8%;"align = "center">
<?php if(!$portalIsReadOnly):?>
<form action="index.php?Page_Type=Delete_API_Authentication_Entity&amp;authentityid=<?php echo $APIAuthEnt->getId();?>" method="post">
<button type="submit" <?php if ($disableButtons) echo "disabled"; ?>
<button type="submit" <?php if ($disableDelete) echo "disabled"; ?>
>Delete</button>
</form>
<?php endif;?>
Expand Down
48 changes: 29 additions & 19 deletions lib/Gocdb_Services/Site.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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'];
Expand Down Expand Up @@ -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());
}
}
}

0 comments on commit 47e261f

Please sign in to comment.