diff --git a/app/assets/javascripts/discourse/controllers/associate-account-confirm.js.es6 b/app/assets/javascripts/discourse/controllers/associate-account-confirm.js.es6
new file mode 100644
index 0000000000..e9685e0cf9
--- /dev/null
+++ b/app/assets/javascripts/discourse/controllers/associate-account-confirm.js.es6
@@ -0,0 +1,26 @@
+import { ajax } from "discourse/lib/ajax";
+import { popupAjaxError } from "discourse/lib/ajax-error";
+import ModalFunctionality from "discourse/mixins/modal-functionality";
+
+export default Ember.Controller.extend(ModalFunctionality, {
+ actions: {
+ finishConnect() {
+ ajax({
+ url: `/associate/${encodeURIComponent(this.model.token)}`,
+ type: "POST"
+ })
+ .then(result => {
+ if (result.success) {
+ this.transitionToRoute(
+ "preferences.account",
+ this.currentUser.findDetails()
+ );
+ this.send("closeModal");
+ } else {
+ this.set("model.error", result.error);
+ }
+ })
+ .catch(popupAjaxError);
+ }
+ }
+});
diff --git a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6
index ac56e0ec37..952788273f 100644
--- a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6
+++ b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6
@@ -20,6 +20,7 @@ export default Ember.Controller.extend(
this._super(...arguments);
this.saveAttrNames = ["name", "title"];
+ this.set("revoking", {});
},
canEditName: setting("enable_names"),
@@ -32,6 +33,8 @@ export default Ember.Controller.extend(
showAllAuthTokens: false,
+ revoking: null,
+
cannotDeleteAccount: Ember.computed.not("currentUser.can_delete_account"),
deleteDisabled: Ember.computed.or(
"model.isSaving",
@@ -200,7 +203,7 @@ export default Ember.Controller.extend(
},
revokeAccount(account) {
- this.set("revoking", true);
+ this.set(`revoking.${account.name}`, true);
this.model
.revokeAssociatedAccount(account.name)
@@ -212,7 +215,7 @@ export default Ember.Controller.extend(
}
})
.catch(popupAjaxError)
- .finally(() => this.set("revoking", false));
+ .finally(() => this.set(`revoking.${account.name}`, false));
},
toggleShowAllAuthTokens() {
diff --git a/app/assets/javascripts/discourse/models/login-method.js.es6 b/app/assets/javascripts/discourse/models/login-method.js.es6
index 083559ade0..a9166633f4 100644
--- a/app/assets/javascripts/discourse/models/login-method.js.es6
+++ b/app/assets/javascripts/discourse/models/login-method.js.es6
@@ -29,7 +29,7 @@ const LoginMethod = Ember.Object.extend({
authUrl += "?reconnect=true";
}
- if (fullScreenLogin || this.full_screen_login) {
+ if (reconnect || fullScreenLogin || this.full_screen_login) {
document.cookie = "fsl=true";
window.location = authUrl;
} else {
diff --git a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 b/app/assets/javascripts/discourse/routes/app-route-map.js.es6
index bfec70df96..654c683a45 100644
--- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6
+++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6
@@ -178,6 +178,7 @@ export default function() {
this.route("signup", { path: "/signup" });
this.route("login", { path: "/login" });
this.route("email-login", { path: "/session/email-login/:token" });
+ this.route("associate-account", { path: "/associate/:token" });
this.route("login-preferences");
this.route("forgot-password", { path: "/password-reset" });
this.route("faq", { path: "/faq" });
diff --git a/app/assets/javascripts/discourse/routes/associate-account.js.es6 b/app/assets/javascripts/discourse/routes/associate-account.js.es6
new file mode 100644
index 0000000000..dfbe2e52f7
--- /dev/null
+++ b/app/assets/javascripts/discourse/routes/associate-account.js.es6
@@ -0,0 +1,16 @@
+import { ajax } from "discourse/lib/ajax";
+import showModal from "discourse/lib/show-modal";
+import { popupAjaxError } from "discourse/lib/ajax-error";
+
+export default Discourse.Route.extend({
+ beforeModel() {
+ const params = this.paramsFor("associate-account");
+ this.replaceWith(`preferences.account`, this.currentUser).then(() =>
+ Ember.run.next(() =>
+ ajax(`/associate/${encodeURIComponent(params.token)}`)
+ .then(model => showModal("associate-account-confirm", { model }))
+ .catch(popupAjaxError)
+ )
+ );
+ }
+});
diff --git a/app/assets/javascripts/discourse/templates/modal/associate-account-confirm.hbs b/app/assets/javascripts/discourse/templates/modal/associate-account-confirm.hbs
new file mode 100644
index 0000000000..3fec07f02e
--- /dev/null
+++ b/app/assets/javascripts/discourse/templates/modal/associate-account-confirm.hbs
@@ -0,0 +1,21 @@
+{{#d-modal-body
+ rawTitle=(
+ i18n "user.associated_accounts.confirm_modal_title"
+ provider=(i18n (concat "login." model.provider_name ".name"))
+ )
+}}
+ {{#if model.error}}
+
+ {{model.error}}
+
+ {{/if}}
+
+ {{i18n "user.associated_accounts.confirm_description"
+ provider=(i18n (concat "login." model.provider_name ".name"))
+ account_description=model.account_description}}
+{{/d-modal-body}}
+
+
diff --git a/app/assets/javascripts/discourse/templates/preferences/account.hbs b/app/assets/javascripts/discourse/templates/preferences/account.hbs
index 3f3024d0e3..3fc4c5a2fa 100644
--- a/app/assets/javascripts/discourse/templates/preferences/account.hbs
+++ b/app/assets/javascripts/discourse/templates/preferences/account.hbs
@@ -112,9 +112,7 @@
{{authProvider.account.description}} |
{{#if authProvider.method.can_revoke}}
- {{#conditional-loading-spinner condition=revoking size='small'}}
- {{d-button action=(action "revokeAccount") actionParam=authProvider.account title="user.associated_accounts.revoke" class="btn-danger no-text" icon="trash-alt" }}
- {{/conditional-loading-spinner}}
+ {{d-button action=(action "revokeAccount") actionParam=authProvider.account title="user.associated_accounts.revoke" class="btn-danger no-text" icon="trash-alt" disabled=(get revoking authProvider.method.name) }}
{{/if}}
|
diff --git a/app/controllers/users/associate_accounts_controller.rb b/app/controllers/users/associate_accounts_controller.rb
new file mode 100644
index 0000000000..6505afa6a7
--- /dev/null
+++ b/app/controllers/users/associate_accounts_controller.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+class Users::AssociateAccountsController < ApplicationController
+ REDIS_PREFIX ||= "omniauth_reconnect"
+
+ ##
+ # Presents a confirmation screen to the user. Accessed via GET, with no CSRF checks
+ def connect_info
+ auth = get_auth_hash
+
+ provider_name = auth.provider
+ authenticator = Discourse.enabled_authenticators.find { |a| a.name == provider_name }
+ raise Discourse::InvalidAccess.new(I18n.t('authenticator_not_found')) if authenticator.nil?
+
+ account_description = authenticator.description_for_auth_hash(auth)
+
+ render json: { token: params[:token], provider_name: provider_name, account_description: account_description }
+ end
+
+ ##
+ # Presents a confirmation screen to the user. Accessed via GET, with no CSRF checks
+ def connect
+ auth = get_auth_hash
+ $redis.del "#{REDIS_PREFIX}_#{current_user&.id}_#{params[:token]}"
+
+ provider_name = auth.provider
+ authenticator = Discourse.enabled_authenticators.find { |a| a.name == provider_name }
+ raise Discourse::InvalidAccess.new(I18n.t('authenticator_not_found')) if authenticator.nil?
+
+ auth_result = authenticator.after_authenticate(auth, existing_account: current_user)
+ DiscourseEvent.trigger(:after_auth, authenticator, auth_result)
+
+ render json: success_json
+ end
+
+ private
+
+ def get_auth_hash
+ token = params[:token]
+ json = $redis.get "#{REDIS_PREFIX}_#{current_user&.id}_#{token}"
+ raise Discourse::NotFound if json.nil?
+
+ OmniAuth::AuthHash.new(JSON.parse(json))
+ end
+end
diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb
index 4a9b0916e2..b23cc593df 100644
--- a/app/controllers/users/omniauth_callbacks_controller.rb
+++ b/app/controllers/users/omniauth_callbacks_controller.rb
@@ -28,20 +28,10 @@ class Users::OmniauthCallbacksController < ApplicationController
provider = DiscoursePluginRegistry.auth_providers.find { |p| p.name == params[:provider] }
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')
- DiscourseEvent.trigger(:after_auth, authenticator, @auth_result)
- return redirect_to Discourse.base_uri("/my/preferences/account")
- else
- @auth_result.authenticated = true
- DiscourseEvent.trigger(:after_auth, authenticator, @auth_result)
- return respond_to do |format|
- format.html
- format.json { render json: @auth_result.to_client_hash }
- end
- end
+ # Save to redis, with a secret token, then redirect to confirmation screen
+ token = SecureRandom.hex
+ $redis.setex "#{Users::AssociateAccountsController::REDIS_PREFIX}_#{current_user.id}_#{token}", 10.minutes, auth.to_json
+ return redirect_to Discourse.base_uri("/associate/#{token}")
else
@auth_result = authenticator.after_authenticate(auth)
DiscourseEvent.trigger(:after_auth, authenticator, @auth_result)
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index fd5a24d256..20ff0dba59 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1021,7 +1021,10 @@ en:
title: "Associated Accounts"
connect: "Connect"
revoke: "Revoke"
+ cancel: "Cancel"
not_connected: "(not connected)"
+ confirm_modal_title: "Connect %{provider} Account"
+ confirm_description: "Your %{provider} account '%{account_description}' will be used for authentication."
name:
title: "Name"
diff --git a/config/routes.rb b/config/routes.rb
index 44a26637ff..2723cc63aa 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -589,6 +589,8 @@ Discourse::Application.routes.draw do
match "/auth/:provider/callback", to: "users/omniauth_callbacks#complete", via: [:get, :post]
match "/auth/failure", to: "users/omniauth_callbacks#failure", via: [:get, :post]
+ get "/associate/:token", to: "users/associate_accounts#connect_info", constraints: { token: /\h{32}/ }
+ post "/associate/:token", to: "users/associate_accounts#connect", constraints: { token: /\h{32}/ }
resources :clicks do
collection do
diff --git a/lib/auth/authenticator.rb b/lib/auth/authenticator.rb
index b306b2c206..cd6fd4fd5e 100644
--- a/lib/auth/authenticator.rb
+++ b/lib/auth/authenticator.rb
@@ -40,6 +40,13 @@ class Auth::Authenticator
""
end
+ # return a string describing the connected account
+ # for a given OmniAuth::AuthHash. Used in the confirmation screen
+ # when connecting accounts
+ def description_for_auth_hash(user)
+ ""
+ end
+
# can authorisation for this provider be revoked?
def can_revoke?
false
diff --git a/lib/auth/managed_authenticator.rb b/lib/auth/managed_authenticator.rb
index bdc7f0951c..9fad79f0c4 100644
--- a/lib/auth/managed_authenticator.rb
+++ b/lib/auth/managed_authenticator.rb
@@ -2,9 +2,15 @@
class Auth::ManagedAuthenticator < Auth::Authenticator
def description_for_user(user)
- info = UserAssociatedAccount.find_by(provider_name: name, user_id: user.id)&.info
- return "" if info.nil?
- info["email"] || info["nickname"] || info["name"] || I18n.t("associated_accounts.connected")
+ associated_account = UserAssociatedAccount.find_by(provider_name: name, user_id: user.id)
+ return "" if associated_account.nil?
+ description_for_auth_hash(associated_account) || I18n.t("associated_accounts.connected")
+ end
+
+ def description_for_auth_hash(auth_token)
+ return if auth_token&.info.nil?
+ info = auth_token.info
+ info["email"] || info["nickname"] || info["name"]
end
# These three methods are designed to be overriden by child classes
diff --git a/spec/requests/associate_accounts_spec.rb b/spec/requests/associate_accounts_spec.rb
new file mode 100644
index 0000000000..41b199c838
--- /dev/null
+++ b/spec/requests/associate_accounts_spec.rb
@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Users::AssociateAccountsController do
+ fab!(:user) { Fabricate(:user) }
+ fab!(:user2) { Fabricate(:user) }
+
+ before do
+ OmniAuth.config.test_mode = true
+ end
+
+ after do
+ Rails.application.env_config["omniauth.auth"] = OmniAuth.config.mock_auth[:google_oauth2] = nil
+ OmniAuth.config.test_mode = false
+ end
+
+ context 'when attempting reconnect' do
+ before do
+ SiteSetting.enable_google_oauth2_logins = true
+ OmniAuth.config.mock_auth[:google_oauth2] = OmniAuth::AuthHash.new(
+ provider: 'google_oauth2',
+ uid: '12345',
+ info: {
+ email: 'someemail@test.com',
+ },
+ extra: {
+ raw_info: {
+ email_verified: true,
+ }
+ },
+ )
+
+ Rails.application.env_config["omniauth.auth"] = OmniAuth.config.mock_auth[:google_oauth2]
+ end
+
+ it 'should work correctly' do
+ sign_in(user)
+
+ # 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(302)
+
+ expect(session[:current_user_id]).to eq(user.id) # Still logged in
+ expect(UserAssociatedAccount.count).to eq(0) # Reconnect has not yet happened
+
+ # Request associate info
+ uri = URI.parse(response.redirect_url)
+ get "#{uri.path}.json"
+ data = JSON.parse(response.body)
+ expect(data["provider_name"]).to eq("google_oauth2")
+ expect(data["account_description"]).to eq("someemail@test.com")
+
+ # Request as different user, should not work
+ sign_in(user2)
+ get "#{uri.path}.json"
+ expect(response.status).to eq(404)
+
+ # Back to first user
+ sign_in(user)
+ get "#{uri.path}.json"
+ data = JSON.parse(response.body)
+ expect(data["provider_name"]).to eq("google_oauth2")
+
+ # Make the connection
+ post "#{uri.path}.json"
+ expect(response.status).to eq(200)
+ expect(UserAssociatedAccount.count).to eq(1)
+
+ # Token cannot be reused
+ get "#{uri.path}.json"
+ expect(response.status).to eq(404)
+ end
+
+ it "returns the correct response for non-existant tokens" do
+ get "/associate/12345678901234567890123456789012.json"
+ expect(response.status).to eq(404)
+
+ get "/associate/shorttoken.json"
+ expect(response.status).to eq(404)
+ end
+
+ end
+end
diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb
index 0cb2d54a40..c5a093f82c 100644
--- a/spec/requests/omniauth_callbacks_controller_spec.rb
+++ b/spec/requests/omniauth_callbacks_controller_spec.rb
@@ -460,7 +460,7 @@ RSpec.describe Users::OmniauthCallbacksController do
expect(UserAssociatedAccount.count).to eq(2)
end
- it 'should reconnect if parameter supplied' do
+ it 'should redirect to associate URL if parameter supplied' do
# Log in normally
get "/auth/google_oauth2?reconnect=true"
expect(response.status).to eq(302)
@@ -483,10 +483,11 @@ RSpec.describe Users::OmniauthCallbacksController do
OmniAuth.config.mock_auth[:google_oauth2].uid = "123456"
get "/auth/google_oauth2/callback.json"
- expect(response.status).to eq(200)
- expect(JSON.parse(response.body)["authenticated"]).to eq(true)
+ expect(response.status).to eq(302)
+ expect(response.redirect_url).to start_with("http://test.localhost/associate/")
+
expect(session[:current_user_id]).to eq(user.id)
- expect(UserAssociatedAccount.count).to eq(1)
+ expect(UserAssociatedAccount.count).to eq(1) # Reconnect has not yet happened
end
end