From 177c8972cc138ad759b02dc06a935b2b11e61a1f Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <roeland@famdouma.nl>
Date: Fri, 6 Apr 2018 22:09:20 +0200
Subject: [PATCH] Improve login flow

* Add page explaining you are about to grant access
* Show grant access page after login

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
---
 core/Controller/ClientFlowLoginController.php | 38 +++++++++++++++
 core/routes.php                               |  1 +
 core/templates/loginflow/authpicker.php       | 10 ++--
 core/templates/loginflow/grant.php            | 46 +++++++++++++++++++
 4 files changed, 90 insertions(+), 5 deletions(-)
 create mode 100644 core/templates/loginflow/grant.php

diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php
index 23bd42a0f1..ab9d98df8d 100644
--- a/core/Controller/ClientFlowLoginController.php
+++ b/core/Controller/ClientFlowLoginController.php
@@ -204,6 +204,44 @@ class ClientFlowLoginController extends Controller {
 		);
 	}
 
+	/**
+	 * @NoAdminRequired
+	 * @NoCSRFRequired
+	 * @UseSession
+	 *
+	 * @param string $stateToken
+	 * @param string $clientIdentifier
+	 * @return TemplateResponse
+	 */
+	public function grantPage($stateToken = '',
+								 $clientIdentifier = '') {
+		if(!$this->isValidToken($stateToken)) {
+			return $this->stateTokenForbiddenResponse();
+		}
+
+		$clientName = $this->getClientName();
+		$client = null;
+		if($clientIdentifier !== '') {
+			$client = $this->clientMapper->getByIdentifier($clientIdentifier);
+			$clientName = $client->getName();
+		}
+
+		return new TemplateResponse(
+			$this->appName,
+			'loginflow/grant',
+			[
+				'client' => $clientName,
+				'clientIdentifier' => $clientIdentifier,
+				'instanceName' => $this->defaults->getName(),
+				'urlGenerator' => $this->urlGenerator,
+				'stateToken' => $stateToken,
+				'serverHost' => $this->request->getServerHost(),
+				'oauthState' => $this->session->get('oauth.state'),
+			],
+			'guest'
+		);
+	}
+
 	/**
 	 * @NoAdminRequired
 	 * @NoCSRFRequired
diff --git a/core/routes.php b/core/routes.php
index 1c7be78bc7..cc1bd34d89 100644
--- a/core/routes.php
+++ b/core/routes.php
@@ -54,6 +54,7 @@ $application->registerRoutes($this, [
 		['name' => 'ClientFlowLogin#showAuthPickerPage', 'url' => '/login/flow', 'verb' => 'GET'],
 		['name' => 'ClientFlowLogin#redirectPage', 'url' => '/login/flow/redirect', 'verb' => 'GET'],
 		['name' => 'ClientFlowLogin#generateAppPassword', 'url' => '/login/flow', 'verb' => 'POST'],
+		['name' => 'ClientFlowLogin#grantPage', 'url' => '/login/flow/grant', 'verb' => 'GET'],
 		['name' => 'TwoFactorChallenge#selectChallenge', 'url' => '/login/selectchallenge', 'verb' => 'GET'],
 		['name' => 'TwoFactorChallenge#showChallenge', 'url' => '/login/challenge/{challengeProviderId}', 'verb' => 'GET'],
 		['name' => 'TwoFactorChallenge#solveChallenge', 'url' => '/login/challenge/{challengeProviderId}', 'verb' => 'POST'],
diff --git a/core/templates/loginflow/authpicker.php b/core/templates/loginflow/authpicker.php
index 1858f8bcb6..4e2282a180 100644
--- a/core/templates/loginflow/authpicker.php
+++ b/core/templates/loginflow/authpicker.php
@@ -28,9 +28,9 @@ $urlGenerator = $_['urlGenerator'];
 ?>
 
 <div class="picker-window">
-	<h2><?php p($l->t('Account access')) ?></h2>
+	<h2><?php p($l->t('Connect to your account')) ?></h2>
 	<p class="info">
-		<?php print_unescaped($l->t('You are about to grant %s access to your %s account.', [
+		<?php print_unescaped($l->t('Please log in before granting %s access to your %s account.', [
 								'<strong>' . \OCP\Util::sanitizeHTML($_['client']) . '</strong>',
 								\OCP\Util::sanitizeHTML($_['instanceName'])
 							])) ?>
@@ -39,8 +39,8 @@ $urlGenerator = $_['urlGenerator'];
 	<br/>
 
 	<p id="redirect-link">
-		<a href="<?php p($urlGenerator->linkToRouteAbsolute('core.ClientFlowLogin.redirectPage', ['stateToken' => $_['stateToken'], 'clientIdentifier' => $_['clientIdentifier'], 'oauthState' => $_['oauthState']])) ?>">
-			<input type="submit" class="login primary icon-confirm-white" value="<?php p($l->t('Grant access')) ?>">
+		<a href="<?php p($urlGenerator->linkToRouteAbsolute('core.ClientFlowLogin.grantPage', ['stateToken' => $_['stateToken'], 'clientIdentifier' => $_['clientIdentifier'], 'oauthState' => $_['oauthState']])) ?>">
+			<input type="submit" class="login primary icon-confirm-white" value="<?php p($l->t('Log in')) ?>">
 		</a>
 	</p>
 
@@ -59,5 +59,5 @@ $urlGenerator = $_['urlGenerator'];
 </div>
 
 <?php if(empty($_['oauthState'])): ?>
-<a id="app-token-login" class="warning" href="#"><?php p($l->t('Alternative login using app token')) ?></a>
+<a id="app-token-login" class="warning" href="#"><?php p($l->t('Alternative log in using app token')) ?></a>
 <?php endif; ?>
diff --git a/core/templates/loginflow/grant.php b/core/templates/loginflow/grant.php
new file mode 100644
index 0000000000..6278e910bd
--- /dev/null
+++ b/core/templates/loginflow/grant.php
@@ -0,0 +1,46 @@
+<?php
+/**
+ * @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+script('core', 'login/authpicker');
+style('core', 'login/authpicker');
+
+/** @var array $_ */
+/** @var \OCP\IURLGenerator $urlGenerator */
+$urlGenerator = $_['urlGenerator'];
+?>
+
+<div class="picker-window">
+	<h2><?php p($l->t('Account access')) ?></h2>
+	<p class="info">
+		<?php print_unescaped($l->t('You are about to grant %s access to your %s account.', [
+								'<strong>' . \OCP\Util::sanitizeHTML($_['client']) . '</strong>',
+								\OCP\Util::sanitizeHTML($_['instanceName'])
+							])) ?>
+	</p>
+
+	<br/>
+
+	<p id="redirect-link">
+		<a href="<?php p($urlGenerator->linkToRouteAbsolute('core.ClientFlowLogin.redirectPage', ['stateToken' => $_['stateToken'], 'clientIdentifier' => $_['clientIdentifier'], 'oauthState' => $_['oauthState']])) ?>">
+			<input type="submit" class="login primary icon-confirm-white" value="<?php p($l->t('Grant access')) ?>">
+		</a>
+	</p>
+</div>