FEATURE: Use full page redirection for all external auth methods (#8092)

FEATURE: Use full page redirection for all external auth methods (#8092)

Using popups is becoming increasingly rare. Full page redirects are already used on mobile, and for some providers. This commit removes all logic related to popup authentication, leaving only the full page redirect method.

For more info, see Do we need popups for login? - ux - Discourse Meta

diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6
index 5dfc922..7f0f5bd 100644
--- a/app/assets/javascripts/discourse/controllers/login.js.es6
+++ b/app/assets/javascripts/discourse/controllers/login.js.es6
@@ -24,7 +24,6 @@ export default Ember.Controller.extend(ModalFunctionality, {
   forgotPassword: Ember.inject.controller(),
   application: Ember.inject.controller(),
 
-  authenticate: null,
   loggingIn: false,
   loggedIn: false,
   processingEmailLink: false,
@@ -39,7 +38,6 @@ export default Ember.Controller.extend(ModalFunctionality, {
 
   resetForm() {
     this.setProperties({
-      authenticate: null,
       loggingIn: false,
       loggedIn: false,
       secondFactorRequired: false,
@@ -85,12 +83,12 @@ export default Ember.Controller.extend(ModalFunctionality, {
 
   loginDisabled: Ember.computed.or("loggingIn", "loggedIn"),
 
-  @computed("loggingIn", "authenticate", "application.canSignUp")
-  showSignupLink(loggingIn, authenticate, canSignUp) {
-    return canSignUp && !loggingIn && Ember.isEmpty(authenticate);
+  @computed("loggingIn", "application.canSignUp")
+  showSignupLink(loggingIn, canSignUp) {
+    return canSignUp && !loggingIn;
   },
 
-  showSpinner: Ember.computed.or("loggingIn", "authenticate"),
+  showSpinner: Ember.computed.readOnly("loggingIn"),
 
   @computed("canLoginLocalWithEmail", "processingEmailLink")
   showLoginWithEmailLink(canLoginLocalWithEmail, processingEmailLink) {
@@ -233,20 +231,13 @@ export default Ember.Controller.extend(ModalFunctionality, {
       return false;
     },
 
-    externalLogin(loginMethod, { fullScreenLogin = false } = {}) {
-      const capabilities = this.capabilities;
-      // On Mobile, Android or iOS always go with full screen
-      if (
-        this.isMobileDevice ||
-        (capabilities &&
-          (capabilities.isIOS ||
-            capabilities.isAndroid ||
-            capabilities.isSafari))
-      ) {
-        fullScreenLogin = true;
+    externalLogin(loginMethod) {
+      if (this.loginDisabled) {
+        return;
       }
 
-      loginMethod.doLogin({ fullScreenLogin });
+      this.set("loggingIn", true);
+      loginMethod.doLogin().catch(() => this.set("loggingIn", false));
     },
 
     createAccount() {
@@ -324,16 +315,6 @@ export default Ember.Controller.extend(ModalFunctionality, {
     }
   },
 
-  @computed("authenticate")
-  authMessage(authenticate) {
-    if (Ember.isEmpty(authenticate)) return "";
-
-    const method = findAll().findBy("name", authenticate);
-    if (method) {
-      return method.message;
-    }
-  },
-
   authenticationComplete(options) {
     const loginError = (errorMsg, className, callback) => {
       showModal("login");
@@ -341,7 +322,6 @@ export default Ember.Controller.extend(ModalFunctionality, {
       Ember.run.next(() => {
         if (callback) callback();
         this.flash(errorMsg, className || "success");
-        this.set("authenticate", null);
       });
     };
 
diff --git a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6
index 8bf5d24..14fcc4b 100644
--- a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6
+++ b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6
@@ -252,7 +252,7 @@ export default Ember.Controller.extend(
       },
 
       connectAccount(method) {
-        method.doLogin({ reconnect: true, fullScreenLogin: false });
+        method.doLogin({ reconnect: true });
       }
     }
   }
diff --git a/app/assets/javascripts/discourse/initializers/auth-complete.js.es6 b/app/assets/javascripts/discourse/initializers/auth-complete.js.es6
index cecf831..83e0183 100644
--- a/app/assets/javascripts/discourse/initializers/auth-complete.js.es6
+++ b/app/assets/javascripts/discourse/initializers/auth-complete.js.es6
@@ -4,11 +4,7 @@ export default {
   initialize(container) {
     let lastAuthResult;
 
-    if (window.location.search.indexOf("authComplete=true") !== -1) {
-      // Happens when a popup social login loses connection to the parent window
-      lastAuthResult = localStorage.getItem("lastAuthResult");
-      localStorage.removeItem("lastAuthResult");
-    } else if (document.getElementById("data-authentication")) {
+    if (document.getElementById("data-authentication")) {
       // Happens for full screen logins
       lastAuthResult = document.getElementById("data-authentication").dataset
         .authenticationData;
diff --git a/app/assets/javascripts/discourse/models/login-method.js.es6 b/app/assets/javascripts/discourse/models/login-method.js.es6
index 0b12604..ce59db2 100644
--- a/app/assets/javascripts/discourse/models/login-method.js.es6
+++ b/app/assets/javascripts/discourse/models/login-method.js.es6
@@ -17,60 +17,24 @@ const LoginMethod = Ember.Object.extend({
     return this.message_override || I18n.t(`login.${this.name}.message`);
   },
 
-  doLogin({ reconnect = false, fullScreenLogin = true } = {}) {
-    const name = this.name;
-    const customLogin = this.customLogin;
-
-    if (customLogin) {
-      customLogin();
-    } else {
-      if (this.custom_url) {
-        window.location = this.custom_url;
-        return;
-      }
-      let authUrl = Discourse.getURL(`/auth/${name}`);
-
-      if (reconnect) {
-        authUrl += "?reconnect=true";
-      }
-
-      if (reconnect || fullScreenLogin || this.full_screen_login) {
-        LoginMethod.buildPostForm(authUrl).then(form => {
-          document.cookie = "fsl=true";
-          form.submit();
-        });
-      } else {
-        this.set("authenticate", name);
-        const left = this.lastX - 400;
-        const top = this.lastY - 200;
-
-        const height = this.frame_height || 400;
-        const width = this.frame_width || 800;
-
-        if (name === "facebook") {
-          authUrl += authUrl.includes("?") ? "&" : "?";
-          authUrl += "display=popup";
-        }
-        LoginMethod.buildPostForm(authUrl).then(form => {
-          const windowState = window.open(
-            "about:blank",
-            "auth_popup",
-            `menubar=no,status=no,height=${height},width=${width},left=${left},top=${top}`
-          );
-
-          form.target = "auth_popup";
-          form.submit();
-
-          const timer = setInterval(() => {
-            // If the process is aborted, reset state in this window
-            if (!windowState || windowState.closed) {
-              clearInterval(timer);
-              this.set("authenticate", null);
-            }
-          }, 1000);
-        });
-      }
+  doLogin({ reconnect = false } = {}) {
+    if (this.customLogin) {
+      this.customLogin();
+      return Ember.RSVP.resolve();
     }
+
+    if (this.custom_url) {
+      window.location = this.custom_url;
+      return Ember.RSVP.resolve();
+    }
+
+    let authUrl = Discourse.getURL(`/auth/${this.name}`);
+
+    if (reconnect) {
+      authUrl += "?reconnect=true";
+    }
+
+    return LoginMethod.buildPostForm(authUrl).then(form => form.submit());
   }
 });
 
diff --git a/app/assets/javascripts/discourse/routes/application.js.es6 b/app/assets/javascripts/discourse/routes/application.js.es6
index f9cdcab..4c85a42 100644
--- a/app/assets/javascripts/discourse/routes/application.js.es6
+++ b/app/assets/javascripts/discourse/routes/application.js.es6
@@ -265,9 +265,7 @@ const ApplicationRoute = Discourse.Route.extend(OpenComposer, {
     const methods = findAll();
 
     if (!this.siteSettings.enable_local_logins && methods.length === 1) {
-      this.controllerFor("login").send("externalLogin", methods[0], {
-        fullScreenLogin: true
-      });
+      this.controllerFor("login").send("externalLogin", methods[0]);
     } else {
       showModal(modal);
       this.controllerFor("modal").set("modalClass", modalClass);

[... diff too long, it was truncated ...]

GitHub sha: d2bceff1

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there: