From 93cf929178d9c6ec37ef1377aa8bcb6b14297c4e Mon Sep 17 00:00:00 2001
From: Sonia Zorba <sonia.zorba@inaf.it>
Date: Thu, 1 Oct 2020 17:05:51 +0200
Subject: [PATCH] Join improvements (added summary page to confirm the
 operation)

---
 classes/UserHandler.php                     |  7 +-
 classes/datalayer/mysql/MySQLUserDAO.php    |  8 +--
 classes/exceptions/ServerErrorException.php | 13 ++++
 classes/login/LoginHandler.php              | 74 ++++++++++++++-------
 classes/model/SessionData.php               | 20 ++++++
 include/front-controller.php                | 31 ++++++++-
 include/identities.php                      | 40 +++++++++++
 include/user-data.php                       | 40 +----------
 index.php                                   | 23 ++++---
 sql/setup-database.sql                      |  3 +
 views/account-management.php                |  2 +-
 views/confirm-join.php                      | 48 +++++++++++++
 views/error.php                             | 23 +++++++
 views/services-list.php                     |  2 +-
 views/tou-check.php                         |  3 +-
 15 files changed, 254 insertions(+), 83 deletions(-)
 create mode 100644 classes/exceptions/ServerErrorException.php
 create mode 100644 include/identities.php
 create mode 100644 views/confirm-join.php
 create mode 100644 views/error.php

diff --git a/classes/UserHandler.php b/classes/UserHandler.php
index f744588..1963a22 100644
--- a/classes/UserHandler.php
+++ b/classes/UserHandler.php
@@ -95,8 +95,11 @@ class UserHandler {
                 //show information regarding the error
                 curl_close($conn);
                 error_log($response);
-                http_response_code(500);
-                die('Error: GMS response code: ' . $info['http_code'] . "\n");
+                $httpCode = $info['http_code'];
+                if ($httpCode === 0) {
+                    throw new ServerErrorException('GMS service is unreachable');
+                }
+                throw new ServerErrorException('Error: GMS response code: ' . $httpCode);
             }
         }
 
diff --git a/classes/datalayer/mysql/MySQLUserDAO.php b/classes/datalayer/mysql/MySQLUserDAO.php
index cd53354..02f8b8a 100644
--- a/classes/datalayer/mysql/MySQLUserDAO.php
+++ b/classes/datalayer/mysql/MySQLUserDAO.php
@@ -37,8 +37,8 @@ class MySQLUserDAO extends BaseMySQLDAO implements UserDAO {
 
         $dbh = $this->getDBHandler();
 
-        $stmt = $dbh->prepare("INSERT INTO identity(`user_id`, `type`, `email`, `name`, `surname`, `institution`, `typed_id`, `eppn`)"
-                . " VALUES(:user_id, :type, :email, :name, :surname, :institution, :typed_id, :eppn)");
+        $stmt = $dbh->prepare("INSERT INTO identity(`user_id`, `type`, `email`, `name`, `surname`, `institution`, `typed_id`, `eppn`, `last_login`)"
+                . " VALUES(:user_id, :type, :email, :name, :surname, :institution, :typed_id, :eppn, NOW())");
 
         $stmt->bindParam(':user_id', $userId);
         $stmt->bindParam(':type', $identity->type);
@@ -288,8 +288,8 @@ class MySQLUserDAO extends BaseMySQLDAO implements UserDAO {
 
         $dbh = $this->getDBHandler();
 
-        $query = "UPDATE identity SET email = :email, name = :name, surname = :surname, institution = :institution"
-                . " WHERE id = :id";
+        $query = "UPDATE identity SET email = :email, name = :name, surname = :surname, institution = :institution,"
+                . " last_login = NOW() WHERE id = :id";
 
         $stmt = $dbh->prepare($query);
         $stmt->bindParam(':email', $identity->email);
diff --git a/classes/exceptions/ServerErrorException.php b/classes/exceptions/ServerErrorException.php
new file mode 100644
index 0000000..9aeb3d6
--- /dev/null
+++ b/classes/exceptions/ServerErrorException.php
@@ -0,0 +1,13 @@
+<?php
+
+namespace RAP;
+
+class ServerErrorException extends \Exception {
+
+    public $message;
+
+    public function __construct($message) {
+        $this->message = $message;
+    }
+
+}
diff --git a/classes/login/LoginHandler.php b/classes/login/LoginHandler.php
index 8403f65..5232ae0 100644
--- a/classes/login/LoginHandler.php
+++ b/classes/login/LoginHandler.php
@@ -10,6 +10,7 @@ class LoginHandler {
     public function __construct(Locator $locator, string $identityType) {
         $this->locator = $locator;
         $this->identityType = $identityType;
+        $this->locator->getSession()->setLoginIdentityType($identityType);
     }
 
     public function onIdentityDataReceived(string $typedId, \Closure $fillIdentityData): string {
@@ -22,6 +23,12 @@ class LoginHandler {
             $this->updateUser($user, $typedId, $fillIdentityData);
         }
 
+        $session = $this->locator->getSession();
+        if ($session->getOAuth2RequestData() === null && $session->getAction() === 'join' &&
+                $session->getUser() !== null && $session->getUser()->id !== $user->id) {
+            return $this->showConfirmJoin($user);
+        }
+
         return $this->getAfterLoginRedirect($user);
     }
 
@@ -30,25 +37,16 @@ class LoginHandler {
         $session = $this->locator->getSession();
 
         if ($session->getUser() !== null && $session->getAction() === 'join') {
-            return $this->joinToPreviousUser($session->getUser(), $typedId, $fillIdentityData);
+            $userToJoin = $this->getNewUser($typedId, $fillIdentityData);
+            return $this->showConfirmJoin($userToJoin);
         } else {
             return $this->redirectToTOUCheck($typedId, $fillIdentityData);
         }
     }
 
-    private function joinToPreviousUser(User $user, string $typedId, \Closure $fillIdentityData): string {
-
-        $identity = new Identity($this->identityType);
-        $identity->typedId = $typedId;
-        $fillIdentityData($identity);
-
-        $user->addIdentity($identity);
-
-        $this->locator->getUserHandler()->saveUser($user);
-
-        $this->locator->getSession()->setUser($user);
-
-        return $this->getAfterLoginRedirect($user);
+    private function showConfirmJoin(User $userToJoin): string {
+        $this->locator->getSession()->setUserToJoin($userToJoin);
+        return $this->locator->getBasePath() . '/confirm-join';
     }
 
     /**
@@ -57,19 +55,25 @@ class LoginHandler {
     private function redirectToTOUCheck(string $typedId, \Closure $fillIdentityData): string {
 
         // Create new user
-        $user = new \RAP\User();
+        $user = $this->getNewUser($typedId, $fillIdentityData);
+
+        $this->locator->getSession()->setUser($user);
+
+        return $this->locator->getBasePath() . '/tou-check';
+    }
 
+    private function getNewUser(string $typedId, \Closure $fillIdentityData): User {
+        $user = new User();
         $identity = new Identity($this->identityType);
         $identity->typedId = $typedId;
         $fillIdentityData($identity);
-
         $user->addIdentity($identity);
-
-        $this->locator->getSession()->setUser($user);
-
-        return $this->locator->getBasePath() . '/tou-check';
+        return $user;
     }
 
+    /**
+     * Update user with fresh information received by IdP. Useful for keeping email address always updated.
+     */
     private function updateUser(User $user, string $typedId, \Closure $fillIdentityData): void {
         $identity = $user->getIdentityByTypedId($typedId);
         $fillIdentityData($identity);
@@ -93,11 +97,7 @@ class LoginHandler {
             $action = $session->getAction();
 
             if ($action === 'join') {
-                if ($session->getUser()->id !== $user->id) {
-                    $user = $this->locator->getUserHandler()->joinUsers($session->getUser(), $user);
-                }
-
-                // the join is completed
+                $user = $this->joinTo($user);
                 $action = 'account';
                 $session->setAction($action);
             }
@@ -112,4 +112,28 @@ class LoginHandler {
         throw new \Exception("Unable to find a proper redirect");
     }
 
+    private function joinTo(User $userToJoin): User {
+
+        $session = $this->locator->getSession();
+        $user = $session->getUser();
+
+        if ($user === null) {
+            return $userToJoin;
+        }
+
+        if ($userToJoin->id === null) {
+            // New identity, not yet associated with an user: simply add it to
+            // previously logged in user.
+            $identity = $userToJoin->identities[0];
+            $user->addIdentity($identity);
+            $this->locator->getUserHandler()->saveUser($user);
+        } else if ($user->id !== $userToJoin->id) {
+            $user = $this->locator->getUserHandler()->joinUsers($user, $userToJoin);
+        }
+
+        $session->setUserToJoin(null);
+
+        return $user;
+    }
+
 }
diff --git a/classes/model/SessionData.php b/classes/model/SessionData.php
index a560b6a..54806ae 100644
--- a/classes/model/SessionData.php
+++ b/classes/model/SessionData.php
@@ -33,9 +33,11 @@ class SessionData {
     const KEY = "SessionData";
 
     private $user;
+    private $userToJoin;
     private $x509DataToRegister;
     private $oauth2RequestData;
     private $action;
+    private $loginIdentityType;
 
     public function setUser(?User $user): void {
         $this->user = $user;
@@ -46,6 +48,24 @@ class SessionData {
         return $this->user;
     }
 
+    public function setUserToJoin(?User $userToJoin): void {
+        $this->userToJoin = $userToJoin;
+        $this->save();
+    }
+
+    public function getUserToJoin(): ?User {
+        return $this->userToJoin;
+    }
+
+    public function setLoginIdentityType(string $loginIdentityType): void {
+        $this->loginIdentityType = $loginIdentityType;
+        $this->save();
+    }
+
+    public function getLoginIdentityType(): ?string {
+        return $this->loginIdentityType;
+    }
+
     /**
      * Update the user data model stored into the session after the primary
      * identity has changed, in order to avoid reading again the user data from
diff --git a/include/front-controller.php b/include/front-controller.php
index 540a16d..1358528 100644
--- a/include/front-controller.php
+++ b/include/front-controller.php
@@ -290,6 +290,35 @@ Flight::route('GET /tou-check', function() {
     }
 });
 
+Flight::route('GET /confirm-join', function() {
+
+    session_start();
+    global $locator;
+
+    if ($locator->getSession()->getUser() === null) {
+        die("User data not retrieved.");
+    } else {
+        Flight::render('confirm-join.php', array('title' => 'Confirm join',
+            'user' => $locator->getSession()->getUser(),
+            'user_to_join' => $locator->getSession()->getUserToJoin(),
+            'version' => $locator->getVersion()));
+    }
+});
+
+Flight::route('POST /confirm-join', function() {
+
+    session_start();
+    global $locator;
+
+    $user = $locator->getSession()->getUserToJoin();
+    if ($user === null) {
+        die("Unable to find user to join");
+    } else {
+        $loginHandler = new \RAP\LoginHandler($locator, $locator->getSession()->getLoginIdentityType());
+        Flight::redirect($loginHandler->getAfterLoginRedirect($user));
+    }
+});
+
 /**
  * Stores the user data into the database after he/she accepted the Terms of Use.
  */
@@ -305,7 +334,7 @@ Flight::route('GET /register', function() {
     } else {
         $locator->getUserHandler()->saveUser($user);
 
-        $loginHandler = new \RAP\LoginHandler($locator, $user->identities[0]->type);
+        $loginHandler = new \RAP\LoginHandler($locator, $locator->getSession()->getLoginIdentityType());
         Flight::redirect($loginHandler->getAfterLoginRedirect($user));
     }
 });
diff --git a/include/identities.php b/include/identities.php
new file mode 100644
index 0000000..db5355b
--- /dev/null
+++ b/include/identities.php
@@ -0,0 +1,40 @@
+<?php /**
+ * This fragment represent a panel containing information about an identity.
+ */ ?>
+<dl class="dl-horizontal">
+    <dt>
+        <?php if (!isset($readOnly)) { ?>
+            <?php if ($identity->primary) { ?>
+                <span class="primary-identity-icon" data-toggle="tooltip" data-placement="left" title="This is your primary identity. You will receive email messages on the address related to this identity.">
+                    <span class="glyphicon glyphicon-star"></span>
+                </span>
+            <?php } else { ?>
+                <span class="primary-identity-icon" data-toggle="tooltip" data-placement="left" title="Click on this icon to set this as the primary identity">
+                    <a href="#" onclick="setPrimaryIdentity(<?php echo $i; ?>);">
+                        <span class="glyphicon glyphicon-star-empty"></span>
+                    </a>
+                </span>
+            <?php } ?>
+        <?php } ?>
+        Type
+    </dt>
+    <dd><?php echo $identity->getUIType(); ?></dd>
+    <dt>E-mail</dt>
+    <dd><?php echo $identity->email; ?></dd>
+    <?php if ($identity->eppn !== null) { ?>
+        <dt><abbr title="EduPerson Principal Name, an unique identifier used into federations.">EPPN</abbr></dt>
+        <dd><?php echo $identity->eppn; ?></dd>
+    <?php } ?>
+    <?php if ($identity->name !== null) { ?>
+        <dt>Name</dt>
+        <dd><?php echo $identity->name; ?></dd>
+    <?php } ?>
+    <?php if ($identity->surname !== null) { ?>
+        <dt>Surname</dt>
+        <dd><?php echo $identity->surname; ?></dd>
+    <?php } ?>
+    <?php if ($identity->institution !== null) { ?>
+        <dt>Institution</dt>
+        <dd><?php echo $identity->institution; ?></dd>
+    <?php } ?>
+</dl>
diff --git a/include/user-data.php b/include/user-data.php
index 87df4a1..53edced 100644
--- a/include/user-data.php
+++ b/include/user-data.php
@@ -5,45 +5,7 @@
  */
 $i = 0; // identity index
 foreach ($user->identities as $identity) {
-    ?>
-    <dl class="dl-horizontal">
-        <dt>
-            <?php if (!isset($readOnly)) { ?>
-        <?php if ($identity->primary) { ?>
-                    <span class="primary-identity-icon" data-toggle="tooltip" data-placement="left" title="This is your primary identity. You will receive email messages on the address related to this identity.">
-                        <span class="glyphicon glyphicon-star"></span>
-                    </span>
-        <?php } else { ?>
-                    <span class="primary-identity-icon" data-toggle="tooltip" data-placement="left" title="Click on this icon to set this as the primary identity">
-                        <a href="#" onclick="setPrimaryIdentity(<?php echo $i; ?>);">
-                            <span class="glyphicon glyphicon-star-empty"></span>
-                        </a>
-                    </span>
-                <?php } ?>
-    <?php } ?>
-            Type
-        </dt>
-        <dd><?php echo $identity->getUIType(); ?></dd>
-        <dt>E-mail</dt>
-        <dd><?php echo $identity->email; ?></dd>
-    <?php if ($identity->eppn !== null) { ?>
-            <dt><abbr title="EduPerson Principal Name, an unique identifier used into federations.">EPPN</abbr></dt>
-            <dd><?php echo $identity->eppn; ?></dd>
-        <?php } ?>
-    <?php if ($identity->name !== null) { ?>
-            <dt>Name</dt>
-            <dd><?php echo $identity->name; ?></dd>
-        <?php } ?>
-    <?php if ($identity->surname !== null) { ?>
-            <dt>Surname</dt>
-            <dd><?php echo $identity->surname; ?></dd>
-        <?php } ?>
-    <?php if ($identity->institution !== null) { ?>
-            <dt>Institution</dt>
-            <dd><?php echo $identity->institution; ?></dd>
-    <?php } ?>
-    </dl>
-    <?php
+    include 'identities.php';
     $i++;
 }
 ?>
diff --git a/index.php b/index.php
index 3f05461..4517350 100644
--- a/index.php
+++ b/index.php
@@ -34,26 +34,31 @@ Flight::set('flight.log_errors', true);
 Flight::map('error', function($ex) {
     if ($ex instanceof \Exception) {
         error_log($ex->getTraceAsString());
+    } else {
+        http_response_code(500);
+        throw $ex;
     }
+    $message = "A fatal error happened";
     if ($ex instanceof \RAP\BadRequestException) {
         http_response_code(400);
-        echo "Bad request: " . $ex->message;
+        $message = "Bad request: " . $ex->message;
     } else if ($ex instanceof \RAP\UnauthorizedException) {
         http_response_code(401);
-        echo "Unauthorized: " . $ex->message;
+        $message = "Unauthorized: " . $ex->message;
     } else if ($ex instanceof \Exception) {
         http_response_code(500);
         if ($ex->getMessage() !== null) {
-            echo $ex->getMessage();
-        } else {
-            echo $ex->getTraceAsString();
+            $message = $ex->getMessage();
         }
-    } else {
-        http_response_code(500);
-        throw $ex;
     }
+
+    global $locator;
+    Flight::render('error.php', array('title' => 'Error',
+        'version' => $locator->getVersion(), 'error' => $message,
+        'contactEmail' => isset($locator->config->contactEmail) ? $locator->config->contactEmail : null,
+        'contactLabel' => isset($locator->config->contactLabel) ? $locator->config->contactLabel : null,
+        'contextRoot' => $locator->config->contextRoot));
 });
 
 // Starting Flight framework
 Flight::start();
-
diff --git a/sql/setup-database.sql b/sql/setup-database.sql
index 6e8b7f6..e87e196 100644
--- a/sql/setup-database.sql
+++ b/sql/setup-database.sql
@@ -51,9 +51,12 @@ CREATE TABLE `identity` (
   `institution` varchar(255) DEFAULT NULL,
   `eppn` varchar(255) DEFAULT NULL,
   `tou_accepted` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
+  `last_login` timestamp NOT NULL DEFAULT '0000-00-00 00:00:00',
   PRIMARY KEY (`id`),
   FOREIGN KEY (`user_id`) REFERENCES `user`(`id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+ALTER TABLE identity ADD CONSTRAINT eppn_unique UNIQUE(eppn);
+ALTER TABLE identity ADD CONSTRAINT typed_id_unique UNIQUE(typed_id);
 
 SET FOREIGN_KEY_CHECKS=0;
 ALTER TABLE `user` ADD FOREIGN KEY (`primary_identity`) REFERENCES `identity`(`id`);
diff --git a/views/account-management.php b/views/account-management.php
index ab8db20..39712ab 100644
--- a/views/account-management.php
+++ b/views/account-management.php
@@ -20,7 +20,7 @@ include 'include/header.php';
     <div class="col-sm-2">
         <div class="row">
             <div class="col-sm-12">
-                <a class="btn btn-success disabled" id="join-btn" href="<?php echo $contextRoot; ?>?action=join" title="Perform an additional login to join your identities" data-toggle="tooltip" data-placement="bottom">
+                <a class="btn btn-success" id="join-btn" href="<?php echo $contextRoot; ?>?action=join" title="Perform an additional login to join your identities" data-toggle="tooltip" data-placement="bottom">
                     Join with another identity
                 </a>
             </div>
diff --git a/views/confirm-join.php b/views/confirm-join.php
new file mode 100644
index 0000000..7eb2095
--- /dev/null
+++ b/views/confirm-join.php
@@ -0,0 +1,48 @@
+<?php
+include 'include/header.php';
+?>
+
+<br/>
+<div class="text-center">
+    <h3>Following identities will be joined:</h3><br/>
+</div>
+
+<div class="row">
+    <div class="col-xs-12 col-md-6">
+        <h4>User id: <?php echo $user->id; ?></h4>
+        <div class="panel">
+            <div class="panel-body">
+                <?php
+                $readOnly = true;
+                include 'include/user-data.php';
+                ?>        
+            </div>
+        </div>
+    </div>
+    <div class="col-xs-12 col-md-6">
+        <h4><?php echo $user_to_join->id === null ? '&nbsp;' : ('User id: ' . $user_to_join->id); ?></h4>
+        <div class="panel">
+            <div class="panel-body">
+                <?php
+                $readOnly = true;
+                $i = 0; // identity index
+                foreach ($user_to_join->identities as $identity) {
+                    include 'include/identities.php';
+                    $i++;
+                }
+                ?>        
+            </div>
+        </div>
+    </div>
+</div>
+
+<div class="text-center">
+    <br/>
+    <form action="confirm-join" method="POST">
+        <input type="submit" value="Confirm join" class="btn btn-success btn-lg" />
+    </form>
+    <br/><br/><br/>
+</div>
+
+<?php
+include 'include/footer.php';
diff --git a/views/error.php b/views/error.php
new file mode 100644
index 0000000..d2dc1ea
--- /dev/null
+++ b/views/error.php
@@ -0,0 +1,23 @@
+<?php
+include 'include/header.php';
+?>
+
+<br/>
+<h2 class="text-danger">Error</h2>
+
+<div class="row">
+    <div class="col-xs-12">        
+        <p>
+            <strong>An error happened:</strong>&nbsp;
+            <span class="text-danger"><?php echo $error; ?></span>
+        </p>
+        <?php if (isset($contactEmail) && isset($contactLabel)) { ?>
+            <br/>
+            <p>If you need support please contact <a href="mailto:<?php echo $contactEmail; ?>"><?php echo $contactLabel; ?></a>.</p>
+        <?php } ?>
+        <br/><br/><br/><br/>
+    </div>
+</div>
+
+<?php
+include 'include/footer.php';
diff --git a/views/services-list.php b/views/services-list.php
index 5929fd7..991237a 100644
--- a/views/services-list.php
+++ b/views/services-list.php
@@ -17,7 +17,7 @@ include 'include/header.php';
         }
         ?>
         <li>
-            <a href="?action=account">RAP Account Management</a>
+            <a href="?action=account">RAP Account Management and Join</a>
         </li>
     </ul>
     <br/>
diff --git a/views/tou-check.php b/views/tou-check.php
index a72f88a..5672f26 100644
--- a/views/tou-check.php
+++ b/views/tou-check.php
@@ -27,7 +27,8 @@ include 'include/header.php';
     </strong>
     <div class="checkbox">
         <label>
-            <input type="checkbox" id="tou-ck" />
+            <input type="checkbox" id="tou-ck" style="margin-top: -2px" />
+            &nbsp;
             I accept IA2 services Terms of Use.
         </label>
     </div>
-- 
GitLab