From ae2cd50427633e13ed89177715c3b277d3ea1bf7 Mon Sep 17 00:00:00 2001
From: Christoph Wurst <christoph@winzerhof-wurst.at>
Date: Wed, 3 Oct 2018 22:16:54 +0200
Subject: [PATCH] Improve 2FA login screens

* Show icon if no provider is available
* Reduce shown text if no provider available
* Fix login button icons
* Make backup codes button primary if the only options to log in

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
---
 core/css/guest.css                          |  50 +++++----
 core/templates/twofactorselectchallenge.php | 114 ++++++++++----------
 core/templates/twofactorshowchallenge.php   |  40 +++----
 3 files changed, 101 insertions(+), 103 deletions(-)

diff --git a/core/css/guest.css b/core/css/guest.css
index 00857c2976..200aeda312 100644
--- a/core/css/guest.css
+++ b/core/css/guest.css
@@ -103,7 +103,7 @@ body {
 /* Default FORM */
 form {
 	position: relative;
-	width: 100%;
+	width: 280px;
 	margin: auto;
 	padding: 0;
 }
@@ -178,7 +178,8 @@ input, textarea, select, button, div[contenteditable=true] {
 	font-family: 'Open Sans', Frutiger, Calibri, 'Myriad Pro', Myriad, sans-serif;
 }
 input,
-input:not([type='range']) {
+input:not([type='range']),
+a.button {
 	font-size: 20px;
 	margin: 5px;
 	padding: 11px 10px 9px;
@@ -189,7 +190,8 @@ input:not([type='range']) {
 input[type='submit'],
 input[type='submit'].icon-confirm,
 input[type='button'],
-button, .button,
+button,
+.button,
 select {
 	display: inline-block;
 	width: auto;
@@ -239,7 +241,8 @@ input.update-continue {
 }
 
 input.primary,
-button.primary {
+button.primary,
+a.primary {
 	border: 1px solid #fff;
 	background-color: #0082c9;
 	color: #fff;
@@ -248,7 +251,9 @@ button.primary {
 input.primary:not(:disabled):hover,
 input.primary:not(:disabled):focus,
 button.primary:not(:disabled):hover,
-button.primary:not(:disabled):focus {
+button.primary:not(:disabled):focus,
+a.primary:not(:disabled):hover,
+a.primary:not(:disabled):focus{
 	background-color: #17adff;
 }
 
@@ -395,6 +400,9 @@ form .warning input[type='checkbox']+label {
 	border-radius: 3px;
 	margin: 12px 0;
 	border: 1px solid transparent;
+	text-align: left;
+	align-items: center;
+	text-decoration: none !important;
 }
 .two-factor-provider:hover,
 .two-factor-provider:focus,
@@ -415,18 +423,6 @@ form .warning input[type='checkbox']+label {
 .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;
@@ -439,6 +435,15 @@ form .warning input[type='checkbox']+label {
 	border-radius: 3px;
 	font-size: 20px;
 }
+.two-factor-primary {
+	/* Fix for 'Use backup codes' button not taking correct styles */
+	padding: 14px !important;
+	width: 226px;
+}
+.two-factor-secondary {
+	display: inline-block;
+	padding: 12px;
+}
 
 
 /* Additional login options */
@@ -559,10 +564,12 @@ form #selectDbType label.ui-state-active {
 
 /* Errors */
 /* Warnings and errors are the same */
+.body-login-container,
 .warning,
 .update,
 .error {
 	display: block;
+	margin-top: 15px;
 	padding: 15px;
 	background-color: rgba(0,0,0,.3);
 	color: #fff;
@@ -576,10 +583,7 @@ form #selectDbType label.ui-state-active {
 	user-select: text;
 }
 
-.warning {
-	margin-top: 15px;
-}
-
+.body-login-container h2,
 .warning h2,
 .update h2,
 .error h2 {
@@ -592,6 +596,10 @@ form #selectDbType label.ui-state-active {
 	height: 70px;
 }
 
+.body-login-container form {
+	width: initial;
+}
+
 .warning.updateAnyways {
 	text-align: center;
 }
diff --git a/core/templates/twofactorselectchallenge.php b/core/templates/twofactorselectchallenge.php
index d0bccfabd6..65691f5857 100644
--- a/core/templates/twofactorselectchallenge.php
+++ b/core/templates/twofactorselectchallenge.php
@@ -1,68 +1,66 @@
-<div class="warning">
+<?php
+$noProviders = empty($_['providers']);
+?>
+<div class="body-login-container update">
 	<h2 class="two-factor-header"><?php p($l->t('Two-factor authentication')) ?></h2>
+	<?php if (!$noProviders): ?>
 	<p><?php p($l->t('Enhanced security is enabled for your account. Choose a second factor for authentication:')) ?></p>
+	<?php endif ?>
 	<?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>
 	</p>
 	<?php endif; ?>
+	<?php if ($noProviders): ?>
+	<img class="two-factor-icon" src="<?php p(image_path('core', 'actions/password-white.svg')) ?>" alt="" />
 	<p>
-		<ul>
-			<?php if (empty($_['providers'])): ?>
-			<p>
-				<?php if (is_null($_['backupProvider'])): ?>
-				<strong><?php p($l->t('Two-factor authentication is enforced but has not been configured on your account. Contact your admin for assistance.')) ?></strong>
-				<?php else: ?>
-				<strong><?php p($l->t('Two-factor authentication is enforced but has not been configured on your account. Use one of your backup codes to log in or contact your admin for assistance.')) ?></strong>
-				<?php endif; ?>
-			</p>
-			<?php else: ?>
-			<?php foreach ($_['providers'] as $provider): ?>
-				<li>
-					<a class="two-factor-provider"
-					   href="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.TwoFactorChallenge.showChallenge',
-										[
-											'challengeProviderId' => $provider->getId(),
-											'redirect_url' => $_['redirect_url'],
-										]
-									)) ?>">
-						<?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>
+		<?php if (is_null($_['backupProvider'])): ?>
+		<strong><?php p($l->t('Two-factor authentication is enforced but has not been configured on your account. Contact your admin for assistance.')) ?></strong>
+		<?php else: ?>
+		<strong><?php p($l->t('Two-factor authentication is enforced but has not been configured on your account. Use one of your backup codes to log in or contact your admin for assistance.')) ?></strong>
+		<?php endif; ?>
 	</p>
+	<?php else: ?>
+	<ul>
+	<?php foreach ($_['providers'] as $provider): ?>
+		<li>
+			<a class="two-factor-provider"
+			   href="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.TwoFactorChallenge.showChallenge',
+								[
+									'challengeProviderId' => $provider->getId(),
+									'redirect_url' => $_['redirect_url'],
+								]
+							)) ?>">
+				<?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; ?>
+	</ul>
+	<?php endif ?>
+	<?php if (!is_null($_['backupProvider'])): ?>
+	<p>
+		<a class="<?php if($noProviders): ?>button primary two-factor-primary<?php else: ?>two-factor-secondary<?php endif ?>" 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>
+	</p>
+	<?php endif; ?>
+	<p><a class="two-factor-secondary" href="<?php print_unescaped($_['logout_url']); ?>">
+		<?php p($l->t('Cancel log in')) ?>
+	</a></p>
 </div>
diff --git a/core/templates/twofactorshowchallenge.php b/core/templates/twofactorshowchallenge.php
index 88ac550575..bb962594c1 100644
--- a/core/templates/twofactorshowchallenge.php
+++ b/core/templates/twofactorshowchallenge.php
@@ -11,7 +11,7 @@ $provider = $_['provider'];
 $template = $_['template'];
 ?>
 
-<div class="warning">
+<div class="body-login-container update">
 	<h2 class="two-factor-header"><?php p($provider->getDisplayName()); ?></h2>
 	<?php if ($error): ?>
 			<?php if($error_message): ?>
@@ -21,27 +21,19 @@ $template = $_['template'];
 			<?php endif; ?>
 	<?php endif; ?>
 	<?php print_unescaped($template); ?>
-	<ul>
-		<?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>
+	<?php if (!is_null($_['backupProvider'])): ?>
+	<p>
+		<a class="two-factor-secondary" 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>
+	</p>
+	<?php endif; ?>
+	<p><a class="two-factor-secondary" href="<?php print_unescaped($_['logout_url']); ?>">
+		<?php p($l->t('Cancel log in')) ?>
+	</a></p>
 </div>