From feb5842a7df71f1c8747446f2ff6293cea2a9e30 Mon Sep 17 00:00:00 2001
From: Christoph Wurst <christoph@winzerhof-wurst.at>
Date: Mon, 1 Oct 2018 11:30:32 +0200
Subject: [PATCH] Restructure and redesign two-factor auth provider selection

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
---
 .../templates/challenge.php                   |  8 ++-
 core/css/guest.css                            | 71 ++++++++++++-------
 core/img/actions/password-white.svg           |  1 +
 core/templates/twofactorselectchallenge.php   | 51 +++++++++----
 core/templates/twofactorshowchallenge.php     | 30 +++++---
 5 files changed, 112 insertions(+), 49 deletions(-)
 create mode 100644 core/img/actions/password-white.svg

diff --git a/apps/twofactor_backupcodes/templates/challenge.php b/apps/twofactor_backupcodes/templates/challenge.php
index e6ec7bb52e..d269fb4b47 100644
--- a/apps/twofactor_backupcodes/templates/challenge.php
+++ b/apps/twofactor_backupcodes/templates/challenge.php
@@ -2,7 +2,13 @@
 style('twofactor_backupcodes', 'style');
 ?>
 
+<img class="two-factor-icon" src="<?php p(image_path('core', 'actions/more-white.svg')) ?>" alt="" />
+
+<p><?php p($l->t('Use one of the backup codes you saved when setting up two-factor authentication.')) ?></p>
+
 <form method="POST" class="challenge-form">
 	<input type="text" class="challenge" name="challenge" required="required" autofocus autocomplete="off" autocapitalize="off" placeholder="<?php p($l->t('Backup code')) ?>">
-	<input type="submit" class="confirm-inline icon-confirm" value="">
+	<button class="two-factor-submit primary" type="submit">
+		<?php p($l->t('Submit')); ?>
+	</button>
 </form>
diff --git a/core/css/guest.css b/core/css/guest.css
index fc926688c4..5d1e164a4f 100644
--- a/core/css/guest.css
+++ b/core/css/guest.css
@@ -372,33 +372,56 @@ form .warning input[type='checkbox']+label {
 	color: #fff !important;
 }
 
-/* TOTP */
-.two-factor-header {
-	text-align: center;
-}
 .two-factor-provider {
-	text-align: center;
-	width: 258px !important;
-	display: inline-block;
-	margin-bottom: 0 !important;
-	background-color: rgba(0, 0, 0, 0.3) !important;
-	border: none !important;
-}
-.two-factor-link {
-	display: inline-block;
-	color: rgba(255, 255, 255, 0.75);
-	width: 100%;
-}
-.two-factor-link .button {
-	padding: 10px;
+	display: flex;
 	border-radius: 3px;
-	color: #555 !important;
-	display: inline-block;
-	margin: 5px 0;
-	text-align: center;
-	width: 100%;
-	box-sizing: border-box
+	margin: 12px 0;
+	border: 1px solid transparent;
 }
+.two-factor-provider:hover,
+.two-factor-provider:focus,
+.two-factor-provider:active {
+	border: 1px solid #fff;
+}
+.two-factor-provider img {
+	width: 64px;
+	height: 64px;
+	padding: 0 12px;
+}
+.two-factor-provider div {
+	margin: 12px 0;
+}
+.two-factor-provider h3 {
+	margin: 0;
+}
+.two-factor-provider p {
+	font-weight: 300;
+}
+.two-factor-provider.two-factor-secondary {
+	margin: 0;
+}
+.two-factor-provider.two-factor-secondary img {
+	width: 16px;
+	height: 16px;
+	padding: 14px 12px 14px 60px;
+}
+.two-factor-provider.two-factor-secondary div {
+	margin: 12px auto;
+}
+
+.two-factor-icon {
+	width: 100px;
+	display: block;
+	margin: 0 auto;
+}
+.two-factor-submit {
+	width: 280px;
+	padding: 10px;
+	margin: 0 0 5px 0;
+	border-radius: 3px;
+	font-size: 20px;
+}
+
 
 /* Additional login options */
 #remember_login {
diff --git a/core/img/actions/password-white.svg b/core/img/actions/password-white.svg
new file mode 100644
index 0000000000..d0ffdcc26d
--- /dev/null
+++ b/core/img/actions/password-white.svg
@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" version="1.1" viewBox="0 0 16 16" width="16" height="16"><path fill="#fff" d="m8 1c-2.319 0-3.967 1.8644-4 4v2.5h-1.5v7.5h11v-7.5h-1.5v-2.5c0-2.27-1.8-3.9735-4-4zm0 2c1.25 0 2 0.963 2 2v2.5h-4v-2.5c0-1.174 0.747-2 2-2z"/></svg>
diff --git a/core/templates/twofactorselectchallenge.php b/core/templates/twofactorselectchallenge.php
index c3c35c7fec..a2fc97495a 100644
--- a/core/templates/twofactorselectchallenge.php
+++ b/core/templates/twofactorselectchallenge.php
@@ -1,6 +1,6 @@
 <div class="warning">
 	<h2 class="two-factor-header"><?php p($l->t('Two-factor authentication')) ?></h2>
-	<p><?php p($l->t('Enhanced security is enabled for your account. Please authenticate using a second factor.')) ?></p>
+	<p><?php p($l->t('Enhanced security is enabled for your account. Choose a second factor for authentication:')) ?></p>
 	<?php if ($_['providerMissing']): ?>
 	<p>
 		<strong><?php p($l->t('Could not load at least one of your enabled two-factor auth methods. Please contact your admin.')) ?></strong>
@@ -19,29 +19,50 @@
 		<ul>
 			<?php foreach ($_['providers'] as $provider): ?>
 				<li>
-					<a class="button two-factor-provider"
+					<a class="two-factor-provider"
 					   href="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.TwoFactorChallenge.showChallenge',
 										[
 											'challengeProviderId' => $provider->getId(),
 											'redirect_url' => $_['redirect_url'],
 										]
 									)) ?>">
-						<?php p($provider->getDescription()) ?>
+						<?php
+						if ($provider instanceof \OCP\Authentication\TwoFactorAuth\IProvidesIcons) {
+							$icon = $provider->getLightIcon();
+						} else {
+							$icon = image_path('core', 'actions/password-white.svg');
+						}
+						?>
+						<img src="<?php p($icon) ?>" alt="" />
+						<div>
+							<h3><?php p($provider->getDisplayName()) ?></h3>
+							<p><?php p($provider->getDescription()) ?></p>
+						</div>
 					</a>
 				</li>
 			<?php endforeach; ?>
+	<?php endif ?>
+			<?php if (!is_null($_['backupProvider'])): ?>
+				<li>
+					<a class="two-factor-provider two-factor-secondary" href="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.TwoFactorChallenge.showChallenge',
+						[
+							'challengeProviderId' => $_['backupProvider']->getId(),
+							'redirect_url' => $_['redirect_url'],
+						]
+					)) ?>">
+						<div>
+							<p><?php p($l->t('Use backup code')) ?></p>
+						</div>
+					</a>
+				</li>
+			<?php endif; ?>
+			<li>
+				<a class="two-factor-provider two-factor-secondary" href="<?php print_unescaped($_['logout_url']); ?>">
+					<div>
+						<p><?php p($l->t('Cancel log in')) ?></p>
+					</div>
+				</a>
+			</li>
 		</ul>
 	</p>
-	<?php endif ?>
-	<p class="two-factor-link">
-		<a class="button" href="<?php print_unescaped($_['logout_url']); ?>"><?php p($l->t('Cancel log in')) ?></a>
-		<?php if (!is_null($_['backupProvider'])): ?>
-		<a class="button" href="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.TwoFactorChallenge.showChallenge',
-												[
-													'challengeProviderId' => $_['backupProvider']->getId(),
-													'redirect_url' => $_['redirect_url'],
-												]
-											)) ?>"><?php p($l->t('Use backup code')) ?></a>
-		<?php endif; ?>
-	</p>
 </div>
diff --git a/core/templates/twofactorshowchallenge.php b/core/templates/twofactorshowchallenge.php
index fbfeeb4255..88ac550575 100644
--- a/core/templates/twofactorshowchallenge.php
+++ b/core/templates/twofactorshowchallenge.php
@@ -21,15 +21,27 @@ $template = $_['template'];
 			<?php endif; ?>
 	<?php endif; ?>
 	<?php print_unescaped($template); ?>
-	<p class="two-factor-link">
-		<a class="button" href="<?php print_unescaped($_['logout_url']); ?>"><?php p($l->t('Cancel log in')) ?></a>
+	<ul>
 		<?php if (!is_null($_['backupProvider'])): ?>
-		<a class="button" href="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.TwoFactorChallenge.showChallenge',
-												[
-													'challengeProviderId' => $_['backupProvider']->getId(),
-													'redirect_url' => $_['redirect_url'],
-												]
-											)) ?>"><?php p($l->t('Use backup code')) ?></a>
+			<li>
+				<a class="two-factor-provider two-factor-secondary" href="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.TwoFactorChallenge.showChallenge',
+					[
+						'challengeProviderId' => $_['backupProvider']->getId(),
+						'redirect_url' => $_['redirect_url'],
+					]
+				)) ?>">
+					<div>
+						<p><?php p($l->t('Use backup code')) ?></p>
+					</div>
+				</a>
+			</li>
 		<?php endif; ?>
-	</p>
+		<li>
+			<a class="two-factor-provider two-factor-secondary" href="<?php print_unescaped($_['logout_url']); ?>">
+				<div>
+					<p><?php p($l->t('Cancel log in')) ?></p>
+				</div>
+			</a>
+		</li>
+	</ul>
 </div>