From fded36cc21538744e8addb8240a3e1d0e0ac1285 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 17 Jun 2013 12:14:51 -0700
Subject: [PATCH] Improve more crumbs and cancel buttons for auth

Summary:
Ref T1536.

  - When linking accounts after initially failing, make the crumb say "Link Account" instead of "Login".
  - When on the LDAP failure form, show a "Cancel" button returning to start (if logging in) or settings (if linking accounts).
  - Allow providers to distinguish between "start", "login" and "link" rendering.

Test Plan: Linked and logged in with LDAP and other registration mechainsms.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T1536

Differential Revision: https://secure.phabricator.com/D6214
---
 .../PhabricatorAuthLoginController.php        | 17 ++++++++++----
 .../auth/provider/PhabricatorAuthProvider.php | 16 +++++++++-----
 .../provider/PhabricatorAuthProviderLDAP.php  | 22 ++++++++++++-------
 .../provider/PhabricatorAuthProviderOAuth.php |  4 ++--
 4 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/src/applications/auth/controller/PhabricatorAuthLoginController.php b/src/applications/auth/controller/PhabricatorAuthLoginController.php
index 29ecfaa55e..c94180c999 100644
--- a/src/applications/auth/controller/PhabricatorAuthLoginController.php
+++ b/src/applications/auth/controller/PhabricatorAuthLoginController.php
@@ -178,10 +178,19 @@ final class PhabricatorAuthLoginController
     $content) {
 
     $crumbs = $this->buildApplicationCrumbs();
-    $crumbs->addCrumb(
-      id(new PhabricatorCrumbView())
-        ->setName(pht('Login'))
-        ->setHref($this->getApplicationURI('start/')));
+
+    if ($this->getRequest()->getUser()->isLoggedIn()) {
+      $crumbs->addCrumb(
+        id(new PhabricatorCrumbView())
+          ->setName(pht('Link Account'))
+          ->setHref($provider->getSettingsURI()));
+    } else {
+      $crumbs->addCrumb(
+        id(new PhabricatorCrumbView())
+          ->setName(pht('Login'))
+          ->setHref($this->getApplicationURI('start/')));
+    }
+
     $crumbs->addCrumb(
       id(new PhabricatorCrumbView())
         ->setName($provider->getProviderName()));
diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php
index 3d05da8a76..5bbc7d3c20 100644
--- a/src/applications/auth/provider/PhabricatorAuthProvider.php
+++ b/src/applications/auth/provider/PhabricatorAuthProvider.php
@@ -108,7 +108,7 @@ abstract class PhabricatorAuthProvider {
 
   public function buildLoginForm(
     PhabricatorAuthStartController $controller) {
-    return $this->renderLoginForm($controller->getRequest(), $is_link = false);
+    return $this->renderLoginForm($controller->getRequest(), $mode = 'start');
   }
 
   abstract public function processLoginRequest(
@@ -116,12 +116,12 @@ abstract class PhabricatorAuthProvider {
 
   public function buildLinkForm(
     PhabricatorAuthLinkController $controller) {
-    return $this->renderLoginForm($controller->getRequest(), $is_link = true);
+    return $this->renderLoginForm($controller->getRequest(), $mode = 'link');
   }
 
   protected function renderLoginForm(
     AphrontRequest $request,
-    $is_link) {
+    $mode) {
     throw new Exception("Not implemented!");
   }
 
@@ -213,16 +213,22 @@ abstract class PhabricatorAuthProvider {
     return $account;
   }
 
-  protected function getLoginURI() {
+  public function getLoginURI() {
     $app = PhabricatorApplication::getByClass('PhabricatorApplicationAuth');
     $uri = $app->getApplicationURI('/login/'.$this->getProviderKey().'/');
     return PhabricatorEnv::getURI($uri);
   }
 
-  protected function getCancelLinkURI() {
+  public function getSettingsURI() {
     return '/settings/panel/external/';
   }
 
+  public function getStartURI() {
+    $app = PhabricatorApplication::getByClass('PhabricatorApplicationAuth');
+    $uri = $app->getApplicationURI('/start/');
+    return $uri;
+  }
+
   public function isDefaultRegistrationProvider() {
     return false;
   }
diff --git a/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php b/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php
index 8d8c00b877..72ee552466 100644
--- a/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php
+++ b/src/applications/auth/provider/PhabricatorAuthProviderLDAP.php
@@ -63,22 +63,28 @@ final class PhabricatorAuthProviderLDAP
     return true;
   }
 
-  protected function renderLoginForm(AphrontRequest $request, $is_link) {
+  protected function renderLoginForm(AphrontRequest $request, $mode) {
     $viewer = $request->getUser();
 
     $dialog = id(new AphrontDialogView())
       ->setSubmitURI($this->getLoginURI())
       ->setUser($viewer);
 
-    if ($is_link) {
+    if ($mode == 'link') {
       $dialog->setTitle(pht('Link LDAP Account'));
       $dialog->addSubmitButton(pht('Link Accounts'));
-    } else if ($this->shouldAllowRegistration()) {
-      $dialog->setTitle(pht('Login or Register with LDAP'));
-      $dialog->addSubmitButton(pht('Login or Register'));
+      $dialog->addCancelButton($this->getSettingsURI());
     } else {
-      $dialog->setTitle(pht('Login with LDAP'));
-      $dialog->addSubmitButton(pht('Login'));
+      if ($this->shouldAllowRegistration()) {
+        $dialog->setTitle(pht('Login or Register with LDAP'));
+        $dialog->addSubmitButton(pht('Login or Register'));
+      } else {
+        $dialog->setTitle(pht('Login with LDAP'));
+        $dialog->addSubmitButton(pht('Login'));
+      }
+      if ($mode == 'login') {
+        $dialog->addCancelButton($this->getStartURI());
+      }
     }
 
     $v_user = $request->getStr('ldap_username');
@@ -137,7 +143,7 @@ final class PhabricatorAuthProviderLDAP
     if (!strlen($username) || !$has_password) {
       $response = $controller->buildProviderPageResponse(
         $this,
-        $this->renderLoginForm($request));
+        $this->renderLoginForm($request, 'login'));
       return array($account, $response);
     }
 
diff --git a/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php b/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php
index 7bf1b01971..f0c12c1f82 100644
--- a/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php
+++ b/src/applications/auth/provider/PhabricatorAuthProviderOAuth.php
@@ -44,10 +44,10 @@ abstract class PhabricatorAuthProviderOAuth extends PhabricatorAuthProvider {
     return true;
   }
 
-  protected function renderLoginForm(AphrontRequest $request, $is_link) {
+  protected function renderLoginForm(AphrontRequest $request, $mode) {
     $viewer = $request->getUser();
 
-    if ($is_link) {
+    if ($mode == 'link') {
       $button_text = pht('Link External Account');
     } else if ($this->shouldAllowRegistration()) {
       $button_text = pht('Login or Register');