Skip to content

Commit

Permalink
[#173,#496] Report better errors from LDAP user create DAO.
Browse files Browse the repository at this point in the history
The HybridUserDAO matches user data with the LDAP and keeps them in
sync with the local database, to maintain the relations with other
data tables.

We modify the create operation in this DAO to return OperationResult,
matching the new expectations of the upper layers.

We change the Hybrid DAO to extend the Postgres DAO, so we can reuse
the SQL operations. In this case, we call the create operation from
the Postgres DAO after we do the corresponding LDAP checks.
  • Loading branch information
jaragunde committed Mar 8, 2023
1 parent 2ea7e95 commit 0708ec4
Showing 1 changed file with 13 additions and 24 deletions.
37 changes: 13 additions & 24 deletions model/dao/UserDAO/HybridUserDAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
include_once(PHPREPORT_ROOT . '/util/SQLIncorrectTypeException.php');
include_once(PHPREPORT_ROOT . '/util/DBPostgres.php');
include_once(PHPREPORT_ROOT . '/model/vo/UserVO.php');
include_once(PHPREPORT_ROOT . '/model/dao/UserDAO/UserDAO.php');
include_once(PHPREPORT_ROOT . '/model/dao/UserDAO/PostgreSQLUserDAO.php');
include_once(PHPREPORT_ROOT . '/util/LDAPConnectionErrorException.php');
include_once(PHPREPORT_ROOT . '/util/LDAPInvalidOperationException.php');
include_once(PHPREPORT_ROOT . '/util/LDAPOperationErrorException.php');
Expand All @@ -42,10 +42,11 @@
/** DAO for Users in LDAP/PostgreSQL Hybrid
*
* This is the implementation for LDAP/PostgreSQL Hybrid of {@link UserDAO}.
* It extends PostgreSQLUserDAO so it can reuse the DB operations when needed.
*
* @see UserDAO, UserVO
*/
class HybridUserDAO extends UserDAO{
class HybridUserDAO extends PostgreSQLUserDAO {

protected $ldapConnect;

Expand Down Expand Up @@ -454,12 +455,9 @@ public function update(UserVO $userVO) {
* The internal id of <var>$userVO</var> will be set after its creation.
*
* @param UserVO $userVO the {@link UserVO} with the data we want to insert on database.
* @return int the number of rows that have been affected (it should be 1).
* @throws {@link SQLQueryErrorException}, {@link SQLUniqueViolationException}
* @return OperationResult the result {@link OperationResult} with information about operation status
*/
public function create(UserVO $userVO) {
$affectedRows = 0;

// check if a user with that login exists in LDAP
if (!$sr=ldap_list($this->ldapConnect,"ou=People," .
ConfigurationParametersManager::getParameter('LDAP_BASE'),
Expand All @@ -470,29 +468,20 @@ public function create(UserVO $userVO) {
return $affectedRows;
}
$ldapResult = ldap_get_entries($this->ldapConnect, $sr);
if($ldapResult["count"] == 0)
// user does not exist in LDAP
return $affectedRows;

$sql = "INSERT INTO usr (login) VALUES(" . DBPostgres::checkStringNull($userVO->getLogin()) . ")";

$res = pg_query($this->connect, $sql);

if ($res == NULL)
if (strpos(pg_last_error(), "unique_usr_login"))
throw new SQLUniqueViolationException(pg_last_error());
else throw new SQLQueryErrorException(pg_last_error());
if($ldapResult["count"] == 0) {
$result = new OperationResult(false);
$result->setResponseCode(400);
$result->setErrorNumber(1234);
$result->setMessage("Error creating user:\nLogin does not exist in LDAP.");
return $result;
}

// populate UserVO with newly created ID from DB
$userVO->setID(DBPostgres::getId($this->connect, "usr_id_seq"));
$result = parent::create($userVO);

// populate UserVO with existing group assignation in LDAP
$userVO->setGroups($this->getGroupsByLogin($userVO->getLogin()));

$affectedRows = pg_affected_rows($res);

return $affectedRows;

return $result;
}

/** User deleter for LDAP/PostgreSQL Hybrid.
Expand Down

0 comments on commit 0708ec4

Please sign in to comment.