From 9f6a4bd67f8eefdff6414a7151c5a859a7fe8844 Mon Sep 17 00:00:00 2001
From: Sonia Zorba <sonia.zorba@inaf.it>
Date: Fri, 5 Mar 2021 20:15:18 +0100
Subject: [PATCH] Implemented rejected join persistence
---
classes/datalayer/UserDAO.php | 4 +-
classes/datalayer/mysql/MySQLUserDAO.php | 22 ++-
classes/login/LoginHandler.php | 39 ++++--
tests/LoginFlowTest.php | 169 ++++++++++++++++++++---
tests/LoginHandlerTest.php | 24 +++-
5 files changed, 215 insertions(+), 43 deletions(-)
diff --git a/classes/datalayer/UserDAO.php b/classes/datalayer/UserDAO.php
index 436f755..0eb2f6a 100644
--- a/classes/datalayer/UserDAO.php
+++ b/classes/datalayer/UserDAO.php
@@ -81,6 +81,8 @@ interface UserDAO {
function updateIdentity(Identity $identity): void;
function findJoinableUsersByEmail(string $email): array;
-
+
function findJoinableUsersByUserId(string $userId): array;
+
+ function insertRejectedJoin(string $userId1, string $userId2): void;
}
diff --git a/classes/datalayer/mysql/MySQLUserDAO.php b/classes/datalayer/mysql/MySQLUserDAO.php
index d65bc7b..f16ce49 100644
--- a/classes/datalayer/mysql/MySQLUserDAO.php
+++ b/classes/datalayer/mysql/MySQLUserDAO.php
@@ -302,9 +302,9 @@ class MySQLUserDAO extends BaseMySQLDAO implements UserDAO {
}
function findJoinableUsersByEmail(string $email): array {
-
+
$dbh = $this->getDBHandler();
-
+
$query = "SELECT DISTINCT(i3.user_id)\n"
. "FROM user u\n"
. "JOIN identity i ON i.user_id = u.id\n"
@@ -323,11 +323,11 @@ class MySQLUserDAO extends BaseMySQLDAO implements UserDAO {
}
return $results;
}
-
+
function findJoinableUsersByUserId(string $userId): array {
$dbh = $this->getDBHandler();
-
+
$query = "SELECT DISTINCT(i3.user_id)\n"
. "FROM user u\n"
. "JOIN identity i ON i.user_id = u.id\n"
@@ -351,4 +351,18 @@ class MySQLUserDAO extends BaseMySQLDAO implements UserDAO {
return $results;
}
+ function insertRejectedJoin(string $userId1, string $userId2): void {
+
+ $dbh = $this->getDBHandler();
+
+ $query = "INSERT INTO keep_separated (user_id1, user_id2) VALUES (:id1, :id2)\n"
+ . "ON DUPLICATE KEY UPDATE user_id1 = user_id1, user_id2 = user_id2";
+
+ $stmt = $dbh->prepare($query);
+ $stmt->bindParam(':id1', $userId1);
+ $stmt->bindParam(':id2', $userId2);
+
+ $stmt->execute();
+ }
+
}
diff --git a/classes/login/LoginHandler.php b/classes/login/LoginHandler.php
index 571421e..b480f27 100644
--- a/classes/login/LoginHandler.php
+++ b/classes/login/LoginHandler.php
@@ -78,11 +78,6 @@ class LoginHandler {
$this->joinUsers();
- $autoJoinStep = $this->checkAutoJoin();
- if ($autoJoinStep !== null) {
- return $autoJoinStep;
- }
-
return $this->getAfterLoginRedirect();
}
@@ -105,6 +100,7 @@ class LoginHandler {
return $this->showConfirmJoin($userToJoin);
}
+ $session->setAutojoin(false);
return null;
}
@@ -120,8 +116,9 @@ class LoginHandler {
$session->setJoinRejected(true);
if ($session->getUser()->id === null) {
- return $this->redirectToTOUCheck($session->getUser()->identities[0]);
+ return $this->redirectToTOUCheck();
} else {
+ $this->saveRejectedJoinIfPossible();
return $this->getAfterLoginRedirect();
}
}
@@ -158,12 +155,8 @@ class LoginHandler {
$user = $session->getUser();
$userToJoin = $session->getUserToJoin();
- if ($user === null) {
- $session->setUser($userToJoin);
- } else {
- $joinedUser = $this->locator->getUserHandler()->joinUsers($userToJoin, $user);
- $session->setUser($joinedUser);
- }
+ $joinedUser = $this->locator->getUserHandler()->joinUsers($userToJoin, $user);
+ $session->setUser($joinedUser);
if ($session->getAction() === 'join') {
$session->setAction('account');
@@ -174,7 +167,15 @@ class LoginHandler {
private function getAfterLoginRedirect(): string {
+ $autoJoinStep = $this->checkAutoJoin();
+ if ($autoJoinStep !== null) {
+ return $autoJoinStep;
+ }
+
$session = $this->locator->getSession();
+
+ $this->saveRejectedJoinIfPossible();
+
$this->locator->getAuditLogger()->info("LOGIN," . $session->getLoginIdentityType() . "," . $session->getUser()->id);
if ($session->getOAuth2RequestData() !== null) {
@@ -193,4 +194,18 @@ class LoginHandler {
throw new \Exception("Unable to find a proper redirect");
}
+ private function saveRejectedJoinIfPossible(): void {
+
+ $session = $this->locator->getSession();
+
+ if ($session->isJoinRejected() && $session->getUserToJoin() !== null) {
+ $id1 = $session->getUser()->id;
+ $id2 = $session->getUserToJoin()->id;
+ if ($id1 !== null && $id2 !== null) {
+ $this->locator->getUserDAO()->insertRejectedJoin($id1, $id2);
+ $session->setJoinRejected(false);
+ }
+ }
+ }
+
}
diff --git a/tests/LoginFlowTest.php b/tests/LoginFlowTest.php
index 80b3086..e784ecb 100644
--- a/tests/LoginFlowTest.php
+++ b/tests/LoginFlowTest.php
@@ -37,20 +37,20 @@ final class LoginFlowTest extends TestCase {
$this->gmsClientStub = $this->createMock(\RAP\GmsClient::class);
$this->locatorStub->method('getGmsClient')->willReturn($this->gmsClientStub);
-
+
$this->sessionData = new \RAP\SessionData();
$this->locatorStub->method('getSession')->willReturn($this->sessionData);
$this->userHandler = new \RAP\UserHandler($this->locatorStub);
$this->locatorStub->method('getUserHandler')->willReturn($this->userHandler);
-
+
$this->loginHandler = new \RAP\LoginHandler($this->locatorStub);
}
public function testNewIdentityLogin(): void {
$this->sessionData->setAction('account');
-
+
$identity = $this->getFakeIdentity1();
// verify DAO is called for checking user existence
@@ -79,7 +79,7 @@ final class LoginFlowTest extends TestCase {
public function testExistingUserLogin(): void {
$this->sessionData->setAction('account');
-
+
$user = $this->getFakeUser1();
$this->assertEquals('test@example.com', $user->identities[0]->email);
@@ -95,35 +95,43 @@ final class LoginFlowTest extends TestCase {
$this->assertEquals('http://rap-ia2/account', $redirect);
}
-
+
public function testNewIdentityAutojoin(): void {
-
+
$this->oAuth2RequestHandler->method('getRedirectResponseUrl')->willReturn('http://redirect-url');
-
+ $this->sessionData->setOAuth2RequestData(new \RAP\OAuth2RequestData());
+
$this->userDaoStub->method('findJoinableUsersByEmail')->willReturn(['1', '2']);
- $this->userDaoStub->method('findUserById')->willReturn($this->getFakeUser1());
-
+
+ $this->userDaoStub->method('findUserById')->will($this->returnValueMap([
+ ['1', $this->getFakeUser1()],
+ ['2', $this->getFakeUser2()]
+ ]));
+
+ // Login: two joinable users detected
$redirect1 = $this->loginHandler->onIdentityDataReceived($this->getFakeIdentity3());
-
+
$this->assertTrue($this->sessionData->isAutojoin());
$this->assertEquals('http://rap-ia2/confirm-join', $redirect1);
$this->assertNotNull($this->sessionData->getUserToJoin());
$this->assertNull($this->sessionData->getUser()->id);
-
- $this->userDaoStub->method('findJoinableUsersByUserId')->willReturn(['2']);
-
+
+ $this->userDaoStub->method('findJoinableUsersByUserId')->will($this->onConsecutiveCalls(['2'], []));
+
+ // First confirm join
$redirect2 = $this->loginHandler->confirmJoin();
-
+
$this->assertEquals('1', $this->sessionData->getUser()->id);
$this->assertEquals('http://rap-ia2/confirm-join', $redirect2);
-
+
$this->gmsClientStub->method('joinGroups')->willReturn('2');
-
+
$this->gmsClientStub->expects($this->once())
->method('joinGroups')->with($this->anything());
-
+
+ // Second confirm join, then redirect to caller application
$redirect3 = $this->loginHandler->confirmJoin();
-
+
$this->assertEquals('2', $this->sessionData->getUser()->id);
$this->assertEquals('http://redirect-url', $redirect3);
}
@@ -141,6 +149,129 @@ final class LoginFlowTest extends TestCase {
$this->assertEquals('http://rap-ia2/account', $redirect);
}
+ public function testRejectJoinExistingUser(): void {
+
+ $this->sessionData->setAction('account');
+
+ $this->userDaoStub->method('findUserByIdentity')->willReturn($this->getFakeUser1());
+ $this->userDaoStub->method('findUserById')->willReturn($this->getFakeUser2());
+ $this->userDaoStub->method('findJoinableUsersByUserId')->will($this->onConsecutiveCalls(['2'], []));
+
+ $redirect1 = $this->loginHandler->onIdentityDataReceived($this->getFakeIdentity1());
+
+ $this->assertTrue($this->sessionData->isAutojoin());
+ $this->assertEquals('http://rap-ia2/confirm-join', $redirect1);
+
+ $this->userDaoStub->expects($this->once())->method('insertRejectedJoin');
+
+ // User rejects join, redirect to account page
+ $redirect2 = $this->loginHandler->rejectJoin();
+
+ $this->assertEquals('http://rap-ia2/account', $redirect2);
+ }
+
+ public function testRejectJoinNewUser(): void {
+
+ $this->sessionData->setAction('account');
+
+ $this->userDaoStub->method('findJoinableUsersByEmail')->willReturn(['1']);
+ $this->userDaoStub->method('findUserById')->willReturn($this->getFakeUser1());
+
+ // Login: one joinable user detected
+ $redirect1 = $this->loginHandler->onIdentityDataReceived($this->getFakeIdentity3());
+
+ $this->assertTrue($this->sessionData->isAutojoin());
+ $this->assertEquals('http://rap-ia2/confirm-join', $redirect1);
+
+ // User rejects join, redirect to TOU check
+ $redirect2 = $this->loginHandler->rejectJoin();
+
+ $this->assertTrue($this->sessionData->isJoinRejected());
+ $this->assertEquals('http://rap-ia2/tou-check', $redirect2);
+
+ $this->userDaoStub->method('createUser')->willReturn('5');
+ $this->userDaoStub->expects($this->once())->method('insertRejectedJoin');
+
+ // User accepts TOU
+ $redirect3 = $this->loginHandler->register();
+ $this->assertEquals('http://rap-ia2/account', $redirect3);
+ }
+
+ public function testExplicitJoin(): void {
+
+ // First login result
+ $this->sessionData->setUser($this->getFakeUser2());
+
+ $this->sessionData->setAction('join');
+
+ $this->userDaoStub->method('findUserById')->willReturn($this->getFakeUser1());
+
+ // Second login
+ $redirect1 = $this->loginHandler->onIdentityDataReceived($this->getFakeIdentity1());
+
+ $this->assertFalse($this->sessionData->isAutojoin());
+ $this->assertEquals('http://rap-ia2/confirm-join', $redirect1);
+
+ // User confirms the join
+ $redirect2 = $this->loginHandler->confirmJoin();
+ $this->assertEquals('http://rap-ia2/account', $redirect2);
+ }
+
+ public function testExplicitJoinAndAutojoin(): void {
+ // First login result
+ $this->sessionData->setUser($this->getFakeUser2());
+
+ $this->sessionData->setAction('join');
+
+ $this->userDaoStub->method('findUserById')->willReturn($this->getFakeUser1());
+ $this->userDaoStub->method('findJoinableUsersByUserId')->will($this->onConsecutiveCalls(['2'], []));
+
+ // Second login
+ $redirect1 = $this->loginHandler->onIdentityDataReceived($this->getFakeIdentity1());
+
+ $this->assertFalse($this->sessionData->isAutojoin());
+ $this->assertEquals('http://rap-ia2/confirm-join', $redirect1);
+
+ // User confirms the join, another join is detected
+ $redirect2 = $this->loginHandler->confirmJoin();
+
+ $this->assertTrue($this->sessionData->isAutojoin());
+ $this->assertEquals('http://rap-ia2/confirm-join', $redirect2);
+
+ // User confirm the second join too
+ $redirect3 = $this->loginHandler->confirmJoin();
+ $this->assertEquals('http://rap-ia2/account', $redirect3);
+ }
+
+ public function testExplicitJoinAndRejectedAutojoin(): void {
+
+ // First login result
+ $this->sessionData->setUser($this->getFakeUser2());
+
+ $this->sessionData->setAction('join');
+
+ $this->userDaoStub->method('findUserById')->willReturn($this->getFakeUser1());
+ $this->userDaoStub->method('findJoinableUsersByUserId')->will($this->onConsecutiveCalls(['2'], []));
+
+ // Second login
+ $redirect1 = $this->loginHandler->onIdentityDataReceived($this->getFakeIdentity1());
+
+ $this->assertFalse($this->sessionData->isAutojoin());
+ $this->assertEquals('http://rap-ia2/confirm-join', $redirect1);
+
+ // User confirms the join, another join is detected
+ $redirect2 = $this->loginHandler->confirmJoin();
+
+ $this->assertTrue($this->sessionData->isAutojoin());
+ $this->assertEquals('http://rap-ia2/confirm-join', $redirect2);
+
+ $this->userDaoStub->expects($this->once())->method('insertRejectedJoin');
+
+ // User reject the second join
+ $redirect3 = $this->loginHandler->rejectJoin();
+ $this->assertEquals('http://rap-ia2/account', $redirect3);
+ }
+
private function getFakeUser1(): \RAP\User {
$user = new \RAP\User();
@@ -152,7 +283,7 @@ final class LoginFlowTest extends TestCase {
$user->addIdentity($identity);
return $user;
}
-
+
private function getFakeUser2(): \RAP\User {
$user = new \RAP\User();
diff --git a/tests/LoginHandlerTest.php b/tests/LoginHandlerTest.php
index 3540aa4..7f7d690 100644
--- a/tests/LoginHandlerTest.php
+++ b/tests/LoginHandlerTest.php
@@ -12,7 +12,6 @@ final class LoginHandlerTest extends TestCase {
private $userDaoStub;
private $sessionStub;
private $userHandlerStub;
- private $oAuth2RequestHandler;
private $auditLogger;
private $loginHandler;
@@ -29,9 +28,6 @@ final class LoginHandlerTest extends TestCase {
$this->userHandlerStub = $this->createMock(\RAP\UserHandler::class);
$this->locatorStub->method('getUserHandler')->willReturn($this->userHandlerStub);
- $this->oAuth2RequestHandler = $this->createMock(\RAP\OAuth2RequestHandler::class);
- $this->locatorStub->method('getOAuth2RequestHandler')->willReturn($this->oAuth2RequestHandler);
-
$this->auditLogger = $this->createMock(\Monolog\Logger::class);
$this->locatorStub->method('getAuditLogger')->willReturn($this->auditLogger);
@@ -44,14 +40,13 @@ final class LoginHandlerTest extends TestCase {
$this->userDaoStub->method('findUserByIdentity')->willReturn($user);
- $this->oAuth2RequestHandler->method('getRedirectResponseUrl')->willReturn('http://redirect-url');
+ $this->sessionStub->method('getAction')->willReturn('account');
- $this->sessionStub->method('getOAuth2RequestData')->willReturn(new \RAP\OAuth2RequestData());
$this->sessionStub->method('getUser')->willReturn($user);
$redirect = $this->loginHandler->onIdentityDataReceived($this->getFakeIdentity1());
- $this->assertEquals('http://redirect-url', $redirect);
+ $this->assertEquals('http://rap-ia2/account', $redirect);
}
public function testShowConfirmJoinNewIdentity(): void {
@@ -85,6 +80,21 @@ final class LoginHandlerTest extends TestCase {
$this->assertEquals('http://rap-ia2/confirm-join', $redirect);
}
+ public function testRegisterFailWithoutSession(): void {
+ $this->expectException(\RAP\BadRequestException::class);
+ $this->loginHandler->register();
+ }
+
+ public function testConfirmJoinFailWithoutSession(): void {
+ $this->expectException(\RAP\BadRequestException::class);
+ $this->loginHandler->confirmJoin();
+ }
+
+ public function testRejectJoinFailWithoutSession(): void {
+ $this->expectException(\RAP\BadRequestException::class);
+ $this->loginHandler->rejectJoin();
+ }
+
private function getFakeUser(): \RAP\User {
$user = new \RAP\User();
--
GitLab