From fbe51d68a7e4c89183415e24e8163dd3f70085df Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Fri, 24 Feb 2017 10:27:41 +0800
Subject: [PATCH] SECURITY: Ensure that user has been authenticated.

---
 app/services/user_authenticator.rb            |  5 ++-
 .../auth/user_authenticator_spec.rb           | 36 +++++++++++++++++++
 spec/controllers/users_controller_spec.rb     |  2 ++
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 spec/components/auth/user_authenticator_spec.rb

diff --git a/app/services/user_authenticator.rb b/app/services/user_authenticator.rb
index 4019fefeadc1a..b0b8f0ccb9407 100644
--- a/app/services/user_authenticator.rb
+++ b/app/services/user_authenticator.rb
@@ -21,7 +21,10 @@ def has_authenticator?
   end
 
   def finish
-    authenticator.after_create_account(@user, @session) if authenticator
+    if authenticator && authenticated?
+      authenticator.after_create_account(@user, @session)
+    end
+
     @session = nil
   end
 
diff --git a/spec/components/auth/user_authenticator_spec.rb b/spec/components/auth/user_authenticator_spec.rb
new file mode 100644
index 0000000000000..1b6bb43019003
--- /dev/null
+++ b/spec/components/auth/user_authenticator_spec.rb
@@ -0,0 +1,36 @@
+require 'rails_helper'
+
+RSpec.describe UserAuthenticator do
+  let(:user) { Fabricate(:user, email: 'test@discourse.org') }
+
+  describe "#finish" do
+    before do
+      SiteSetting.enable_google_oauth2_logins = true
+    end
+
+    it "should execute provider's callback" do
+      user.update!(email: 'test@gmail.com')
+
+      authenticator = UserAuthenticator.new(user, { authentication: {
+        authenticator_name: Auth::GoogleOAuth2Authenticator.new.name,
+        email: user.email,
+        email_valid: true,
+        extra_data: { google_user_id: 1 }
+      }})
+
+      expect { authenticator.finish }.to change { GoogleUserInfo.count }.by(1)
+    end
+
+    describe "when session's email is different from user's email" do
+      it "should not execute provider's callback" do
+        authenticator = UserAuthenticator.new(user, { authentication: {
+          authenticator_name: Auth::GoogleOAuth2Authenticator.new.name,
+          email: 'test@gmail.com',
+          email_valid: true
+        }})
+
+        expect { authenticator.finish }.to_not change { GoogleUserInfo.count }
+      end
+    end
+  end
+end
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 746d26a0712a4..87ee78158f2da 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -611,6 +611,8 @@ def post_user
           auth = session[:authentication] = {}
           auth[:authenticator_name] = 'twitter'
           auth[:extra_data] = twitter_auth
+          auth[:email_valid] = true
+          auth[:email] = @user.email
 
           post_user