Merge pull request #4729 from tgxworld/dont_mark_user_as_valid

FIX: Don't mark user as `active` if verified email is different.

app/assets/javascripts/discourse/controllers/login.js.es6

@@ -6,9 +6,13 @@ import { findAll } from 'discourse/models/login-method';
 import { escape } from 'pretty-text/sanitizer';
 
 // This is happening outside of the app via popup
-const AuthErrors =
-  ['requires_invite', 'awaiting_approval', 'awaiting_confirmation', 'admin_not_allowed_from_ip_address',
-   'not_allowed_from_ip_address'];
+const AuthErrors = [
+  'requires_invite',
+  'awaiting_approval',
+  'awaiting_activation',
+  'admin_not_allowed_from_ip_address',
+  'not_allowed_from_ip_address'
+];
 
 export default Ember.Controller.extend(ModalFunctionality, {
 

app/controllers/users/omniauth_callbacks_controller.rb

@@ -111,10 +111,8 @@ class Users::OmniauthCallbacksController < ApplicationController
 
   def user_found(user)
     # automatically activate/unstage any account if a provider marked the email valid
-    if @auth_result.email_valid
-      user.staged = false
-      user.active = true
-      user.save
+    if @auth_result.email_valid && @auth_result.email == user.email
+      user.update!(staged: false, active: true)
     end
 
     if ScreenedIpAddress.should_block?(request.remote_ip)

config/locales/client.en.yml

@@ -1030,7 +1030,7 @@ en:
       logging_in: "Signing In..."
       or: "Or"
       authenticating: "Authenticating..."
-      awaiting_confirmation: "Your account is awaiting activation, use the forgot password link to issue another activation email."
+      awaiting_activation: "Your account is awaiting activation, use the forgot password link to issue another activation email."
       awaiting_approval: "Your account has not been approved by a staff member yet. You will be sent an email when it is approved."
       requires_invite: "Sorry, access to this forum is by invite only."
       not_activated: "You can't log in yet. We previously sent an activation email to you at <b>{{sentTo}}</b>. Please follow the instructions in that email to activate your account."

lib/auth/google_oauth2_authenticator.rb

@@ -54,7 +54,7 @@ class Auth::GoogleOAuth2Authenticator < Auth::Authenticator
 
     h[:email] = hash[:info][:email]
     h[:name] = hash[:info][:name]
-    h[:email_valid] = hash[:extra][:raw_info][:email_verified]
+    h[:email_valid] = extra[:email_verified]
 
     h[:google] = {
       google_user_id: hash[:uid] || extra[:sub],

spec/integration/omniauth_callbacks_spec.rb

@@ -0,0 +1,98 @@
+require 'rails_helper'
+
+RSpec.describe "OmniAuth Callbacks" do
+  let(:user) { Fabricate(:user) }
+
+  before do
+    OmniAuth.config.test_mode = true
+  end
+
+  after do
+    OmniAuth.config.test_mode = false
+  end
+
+  context 'Google Oauth2' do
+    before do
+      SiteSetting.enable_google_oauth2_logins = true
+    end
+
+    describe 'when user has been verified' do
+      before do
+        OmniAuth.config.mock_auth[:google_oauth2] = OmniAuth::AuthHash.new(
+          provider: 'google_oauth2',
+          uid: '123545',
+          info: OmniAuth::AuthHash::InfoHash.new(
+            email: user.email,
+            name: 'Some name'
+          ),
+          extra: {
+            raw_info: OmniAuth::AuthHash.new(
+              email_verified: true,
+              email: user.email,
+              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 return the right response' do
+        get "/auth/google_oauth2/callback.json"
+
+        expect(response).to be_success
+
+        response_body = JSON.parse(response.body)
+
+        expect(response_body["authenticated"]).to eq(true)
+        expect(response_body["awaiting_activation"]).to eq(false)
+        expect(response_body["awaiting_approval"]).to eq(false)
+        expect(response_body["not_allowed_from_ip_address"]).to eq(false)
+        expect(response_body["admin_not_allowed_from_ip_address"]).to eq(false)
+      end
+
+      context 'when user has not verified his email' do
+        before do
+          GoogleUserInfo.create!(google_user_id: '12345', user: user)
+          user.update!(active: false)
+
+          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 return the right response' do
+          get "/auth/google_oauth2/callback.json"
+
+          expect(response).to be_success
+
+          response_body = JSON.parse(response.body)
+
+          expect(user.reload.active).to eq(false)
+          expect(response_body["authenticated"]).to eq(false)
+          expect(response_body["awaiting_activation"]).to eq(true)
+        end
+      end
+    end
+  end
+end