FEATURE: Allow connecting associated accounts when two-factor is enabled (#6754)

Previously the 'reconnect' process was a bit magic - IF you were already logged into discourse, and followed the auth flow, your account would be reconnected and you would be 'logged in again'.

Now, we explicitly check for a reconnect=true parameter when the flow is started, store it in the session, and then only follow the reconnect logic if that variable is present. Setting this parameter also skips the 'logged in again' step, which means reconnect now works with 2fa enabled.

app/assets/javascripts/discourse/controllers/preferences/account.js.es6

@@ -243,7 +243,7 @@ export default Ember.Controller.extend(
       },
 
       connectAccount(method) {
-        method.doLogin();
+        method.doLogin(true);
       }
     }
   }

app/assets/javascripts/discourse/models/login-method.js.es6

@@ -24,7 +24,7 @@ const LoginMethod = Ember.Object.extend({
     );
   },
 
-  doLogin() {
+  doLogin(reconnect = false) {
     const name = this.get("name");
     const customLogin = this.get("customLogin");
 
@@ -33,6 +33,10 @@ const LoginMethod = Ember.Object.extend({
     } else {
       let authUrl = this.get("custom_url") || Discourse.getURL("/auth/" + name);
 
+      if (reconnect) {
+        authUrl += "?reconnect=true";
+      }
+
       if (this.get("full_screen_login")) {
         document.cookie = "fsl=true";
         window.location = authUrl;
@@ -45,7 +49,8 @@ const LoginMethod = Ember.Object.extend({
         const width = this.get("frame_width") || 800;
 
         if (name === "facebook") {
-          authUrl = authUrl + "?display=popup";
+          authUrl += authUrl.includes("?") ? "&" : "?";
+          authUrl += "display=popup";
         }
 
         const w = window.open(

app/controllers/users/omniauth_callbacks_controller.rb

@@ -25,8 +25,18 @@ class Users::OmniauthCallbacksController < ApplicationController
     authenticator = self.class.find_authenticator(params[:provider])
     provider = DiscoursePluginRegistry.auth_providers.find { |p| p.name == params[:provider] }
 
-    if authenticator.can_connect_existing_user? && current_user
+    if session.delete(:auth_reconnect) && authenticator.can_connect_existing_user? && current_user
+      # If we're reconnecting, don't actually try and log the user in
       @auth_result = authenticator.after_authenticate(auth, existing_account: current_user)
+      if provider&.full_screen_login || cookies['fsl']
+        cookies.delete('fsl')
+        return redirect_to Discourse.base_uri("/my/preferences/account")
+      else
+        return respond_to do |format|
+          format.html
+          format.json { render json: @auth_result.to_client_hash }
+        end
+      end
     else
       @auth_result = authenticator.after_authenticate(auth)
     end
@@ -64,7 +74,7 @@ class Users::OmniauthCallbacksController < ApplicationController
       @auth_result.authenticator_name = authenticator.name
       complete_response_data
 
-      if (provider && provider.full_screen_login) || cookies['fsl']
+      if provider&.full_screen_login || cookies['fsl']
         cookies.delete('fsl')
         cookies['_bypass_cache'] = true
         cookies[:authentication_data] = @auth_result.to_client_hash.to_json

lib/middleware/omniauth_bypass_middleware.rb

@@ -17,6 +17,12 @@ class Middleware::OmniauthBypassMiddleware
         authenticator.register_middleware(self)
       end
     end
+
+    @omniauth.before_request_phase do |env|
+      # If the user is trying to reconnect to an existing account, store in session
+      request = ActionDispatch::Request.new(env)
+      request.session[:auth_reconnect] = !!request.params["reconnect"]
+    end
   end
 
   def call(env)

spec/requests/omniauth_callbacks_controller_spec.rb

@@ -338,6 +338,86 @@ RSpec.describe Users::OmniauthCallbacksController do
       end
     end
 
+    context 'when attempting reconnect' do
+      let(:user2) { Fabricate(:user) }
+      before do
+        GoogleUserInfo.create!(google_user_id: '12345', user: user)
+        GoogleUserInfo.create!(google_user_id: '123456', user: user2)
+
+        OmniAuth.config.mock_auth[:google_oauth2] = OmniAuth::AuthHash.new(
+          provider: 'google_oauth2',
+          uid: '12345',
+          info: OmniAuth::AuthHash::InfoHash.new(
+            email: 'someother_email@test.com',
+            name: 'Some name'
+          ),
+          extra: {
+            raw_info: OmniAuth::AuthHash.new(
+              email_verified: true,
+              email: 'someother_email@test.com',
+              family_name: 'Huh',
+              given_name: user.name,
+              gender: 'male',
+              name: "#{user.name} Huh",
+            )
+          },
+        )
+
+        Rails.application.env_config["omniauth.auth"] = OmniAuth.config.mock_auth[:google_oauth2]
+      end
+
+      it 'should not reconnect normally' do
+        # Log in normally
+        get "/auth/google_oauth2"
+        expect(response.status).to eq(302)
+        expect(session[:auth_reconnect]).to eq(false)
+
+        get "/auth/google_oauth2/callback.json"
+        expect(response.status).to eq(200)
+        expect(session[:current_user_id]).to eq(user.id)
+
+        # Log into another user
+        OmniAuth.config.mock_auth[:google_oauth2].uid = "123456"
+        get "/auth/google_oauth2"
+        expect(response.status).to eq(302)
+        expect(session[:auth_reconnect]).to eq(false)
+
+        get "/auth/google_oauth2/callback.json"
+        expect(response.status).to eq(200)
+        expect(session[:current_user_id]).to eq(user2.id)
+        expect(GoogleUserInfo.count).to eq(2)
+      end
+
+      it 'should reconnect if parameter supplied' do
+        # Log in normally
+        get "/auth/google_oauth2?reconnect=true"
+        expect(response.status).to eq(302)
+        expect(session[:auth_reconnect]).to eq(true)
+
+        get "/auth/google_oauth2/callback.json"
+        expect(response.status).to eq(200)
+        expect(session[:current_user_id]).to eq(user.id)
+
+        # Clear cookie after login
+        expect(session[:auth_reconnect]).to eq(nil)
+
+        # Disconnect
+        GoogleUserInfo.find_by(user_id: user.id).destroy
+
+        # Reconnect flow:
+        get "/auth/google_oauth2?reconnect=true"
+        expect(response.status).to eq(302)
+        expect(session[:auth_reconnect]).to eq(true)
+
+        OmniAuth.config.mock_auth[:google_oauth2].uid = "123456"
+        get "/auth/google_oauth2/callback.json"
+        expect(response.status).to eq(200)
+        expect(session[:current_user_id]).to eq(user.id)
+        expect(GoogleUserInfo.count).to eq(1)
+      end
+
+    end
+
     context 'after changing email' do
       require_dependency 'email_updater'