From 7dc8f8b794cbb36b14737710ccfe417d1c074d12 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Mon, 2 Aug 2021 17:57:52 +0100
Subject: [PATCH] FEATURE: Allow linking an existing account during
 external-auth signup

When a user signs up via an external auth method, a new link is added to the signup modal which allows them to connect an existing Discourse account. This will only happen if:

- There is at least 1 other auth method available

and

- The current auth method permits users to disconnect/reconnect their accounts themselves
---
 .../app/controllers/create-account.js         | 11 ++++
 .../app/templates/modal/create-account.hbs    |  5 ++
 .../create-account-external-test.js           | 55 ++++++++++++++++---
 .../users/associate_accounts_controller.rb    |  2 +-
 .../users/omniauth_callbacks_controller.rb    | 20 +++++--
 config/locales/client.en.yml                  |  1 +
 .../omniauth_callbacks_controller_spec.rb     | 13 +++++
 7 files changed, 92 insertions(+), 15 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/controllers/create-account.js b/app/assets/javascripts/discourse/app/controllers/create-account.js
index b2e9622923b..52a3f7230a7 100644
--- a/app/assets/javascripts/discourse/app/controllers/create-account.js
+++ b/app/assets/javascripts/discourse/app/controllers/create-account.js
@@ -407,6 +407,17 @@ export default Controller.extend(
       }
     },
 
+    @discourseComputed("authOptions.associate_url", "authOptions.auth_provider")
+    associateHtml(url, provider) {
+      if (!url) {
+        return;
+      }
+      return I18n.t("create_account.associate", {
+        associate_link: url,
+        provider: I18n.t(`login.${provider}.name`),
+      });
+    },
+
     actions: {
       externalLogin(provider) {
         this.login.send("externalLogin", provider, { signup: true });
diff --git a/app/assets/javascripts/discourse/app/templates/modal/create-account.hbs b/app/assets/javascripts/discourse/app/templates/modal/create-account.hbs
index 846bee341d3..c33f956d23f 100644
--- a/app/assets/javascripts/discourse/app/templates/modal/create-account.hbs
+++ b/app/assets/javascripts/discourse/app/templates/modal/create-account.hbs
@@ -16,6 +16,11 @@
 
           <div class="login-form">
             <form>
+              {{#if associateHtml}}
+                <div class="input-group create-account-associate-link">
+                  <span>{{html-safe associateHtml}}</span>
+                </div>
+              {{/if}}
               <div class="input-group create-account-email">
                 {{input type="email" disabled=emailDisabled value=accountEmail id="new-account-email" name="email" class=(value-entered accountEmail) autofocus="autofocus" focusOut=(action "checkEmailAvailability")}}
                 <label class="alt-placeholder" for="new-account-email">
diff --git a/app/assets/javascripts/discourse/tests/acceptance/create-account-external-test.js b/app/assets/javascripts/discourse/tests/acceptance/create-account-external-test.js
index e679bfcb0c0..e6ec013d541 100644
--- a/app/assets/javascripts/discourse/tests/acceptance/create-account-external-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/create-account-external-test.js
@@ -2,17 +2,24 @@ import { acceptance, exists } from "discourse/tests/helpers/qunit-helpers";
 import { test } from "qunit";
 import { visit } from "@ember/test-helpers";
 
+function setupAuthData(data) {
+  data = {
+    auth_provider: "test",
+    email: "blah@example.com",
+    can_edit_username: true,
+    can_edit_name: true,
+    ...data,
+  };
+
+  const node = document.createElement("meta");
+  node.dataset.authenticationData = JSON.stringify(data);
+  node.id = "data-authentication";
+  document.querySelector("head").appendChild(node);
+}
+
 acceptance("Create Account - external auth", function (needs) {
   needs.hooks.beforeEach(() => {
-    const node = document.createElement("meta");
-    node.dataset.authenticationData = JSON.stringify({
-      auth_provider: "test",
-      email: "blah@example.com",
-      can_edit_username: true,
-      can_edit_name: true,
-    });
-    node.id = "data-authentication";
-    document.querySelector("head").appendChild(node);
+    setupAuthData();
   });
   needs.hooks.afterEach(() => {
     document
@@ -29,6 +36,11 @@ acceptance("Create Account - external auth", function (needs) {
     );
 
     assert.ok(exists("#new-account-username"), "it shows the fields");
+
+    assert.notOk(
+      exists(".create-account-associate-link"),
+      "it does not show the associate link"
+    );
   });
 
   test("when skip is enabled", async function (assert) {
@@ -43,3 +55,28 @@ acceptance("Create Account - external auth", function (needs) {
     assert.not(exists("#new-account-username"), "it does not show the fields");
   });
 });
+
+acceptance("Create account - with associate link", function (needs) {
+  needs.hooks.beforeEach(() => {
+    setupAuthData({ associate_url: "/associate/abcde" });
+  });
+  needs.hooks.afterEach(() => {
+    document
+      .querySelector("head")
+      .removeChild(document.getElementById("data-authentication"));
+  });
+
+  test("displays associate link when allowed", async function (assert) {
+    await visit("/");
+
+    assert.ok(
+      exists("#discourse-modal div.create-account-body"),
+      "it shows the registration modal"
+    );
+    assert.ok(exists("#new-account-username"), "it shows the fields");
+    assert.ok(
+      exists(".create-account-associate-link"),
+      "it shows the associate link"
+    );
+  });
+});
diff --git a/app/controllers/users/associate_accounts_controller.rb b/app/controllers/users/associate_accounts_controller.rb
index b07cadcddfb..9f12727b944 100644
--- a/app/controllers/users/associate_accounts_controller.rb
+++ b/app/controllers/users/associate_accounts_controller.rb
@@ -22,7 +22,7 @@ class Users::AssociateAccountsController < ApplicationController
     end
 
     DiscourseEvent.trigger(:before_auth, authenticator, auth_hash, session, cookies, request)
-    auth_result = authenticator.after_authenticate(auth, existing_account: current_user)
+    auth_result = authenticator.after_authenticate(auth_hash, existing_account: current_user)
     DiscourseEvent.trigger(:after_auth, authenticator, auth_result, session, cookies, request)
 
     secure_session[self.class.key(params[:token])] = nil
diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
index ff5d455857f..6a9253b6f34 100644
--- a/app/controllers/users/omniauth_callbacks_controller.rb
+++ b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -28,10 +28,8 @@ class Users::OmniauthCallbacksController < ApplicationController
     authenticator = self.class.find_authenticator(params[:provider])
 
     if session.delete(:auth_reconnect) && authenticator.can_connect_existing_user? && current_user
-      # Save to redis, with a secret token, then redirect to confirmation screen
-      token = SecureRandom.hex
-      secure_session.set "#{Users::AssociateAccountsController.key(token)}", auth.to_json, expires: 10.minutes
-      return redirect_to "#{Discourse.base_path}/associate/#{token}"
+      path = persist_auth_token(auth)
+      return redirect_to path
     else
       DiscourseEvent.trigger(:before_auth, authenticator, auth, session, cookies, request)
       @auth_result = authenticator.after_authenticate(auth)
@@ -76,9 +74,16 @@ class Users::OmniauthCallbacksController < ApplicationController
 
     return render_auth_result_failure if @auth_result.failed?
 
+    client_hash = @auth_result.to_client_hash
+    if authenticator.can_connect_existing_user? &&
+      (SiteSetting.enable_local_logins || Discourse.enabled_authenticators.count > 1)
+      # There is more than one login method, and users are allowed to manage associations themselves
+      client_hash[:associate_url] = persist_auth_token(auth)
+    end
+
     cookies['_bypass_cache'] = true
     cookies[:authentication_data] = {
-      value: @auth_result.to_client_hash.to_json,
+      value: client_hash.to_json,
       path: Discourse.base_path("/")
     }
     redirect_to @origin
@@ -180,4 +185,9 @@ class Users::OmniauthCallbacksController < ApplicationController
     end
   end
 
+  def persist_auth_token(auth)
+    secret = SecureRandom.hex
+    secure_session.set "#{Users::AssociateAccountsController.key(secret)}", auth.to_json, expires: 10.minutes
+    "#{Discourse.base_path}/associate/#{secret}"
+  end
 end
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 0661ac0942e..a421f328e3c 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1795,6 +1795,7 @@ en:
       disclaimer: "By registering, you agree to the <a href='%{privacy_link}' target='blank'>privacy policy</a> and <a href='%{tos_link}' target='blank'>terms of service</a>."
       title: "Create your account"
       failed: "Something went wrong, perhaps this email is already registered, try the forgot password link"
+      associate: "Already have an account? <a href='%{associate_link}'>Log In</a> to link your %{provider} account."
 
     forgot_password:
       title: "Password Reset"
diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb
index 331678dc5d5..5e774196c3a 100644
--- a/spec/requests/omniauth_callbacks_controller_spec.rb
+++ b/spec/requests/omniauth_callbacks_controller_spec.rb
@@ -246,6 +246,19 @@ RSpec.describe Users::OmniauthCallbacksController do
         expect(data["destination_url"]).to eq(destination_url)
       end
 
+      it 'should return an associate url when multiple login methods are enabled' do
+        get "/auth/google_oauth2/callback.json"
+        expect(response.status).to eq(302)
+
+        data = JSON.parse(cookies[:authentication_data])
+        expect(data["associate_url"]).to start_with('/associate/')
+
+        SiteSetting.enable_local_logins = false
+        get "/auth/google_oauth2/callback.json"
+        data = JSON.parse(cookies[:authentication_data])
+        expect(data["associate_url"]).to eq(nil)
+      end
+
       describe 'when site is invite_only' do
         before do
           SiteSetting.invite_only = true