From 3d347fb9c4f4927966d70faf836491c6334c550e Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 1 Mar 2017 11:58:24 +0800 Subject: [PATCH] FIX: Don't mark user as `active` if verified email is different. --- .../discourse/controllers/login.js.es6 | 10 +- .../users/omniauth_callbacks_controller.rb | 6 +- config/locales/client.en.yml | 2 +- lib/auth/google_oauth2_authenticator.rb | 2 +- spec/integration/omniauth_callbacks_spec.rb | 98 +++++++++++++++++++ 5 files changed, 109 insertions(+), 9 deletions(-) create mode 100644 spec/integration/omniauth_callbacks_spec.rb diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6 index 5c97bbbf82d..38ea5f9d304 100644 --- a/app/assets/javascripts/discourse/controllers/login.js.es6 +++ b/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, { diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 5295b3a3fba..190c22924fe 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/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) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ef67b977611..23f07901c28 100644 --- a/config/locales/client.en.yml +++ b/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 {{sentTo}}. Please follow the instructions in that email to activate your account." diff --git a/lib/auth/google_oauth2_authenticator.rb b/lib/auth/google_oauth2_authenticator.rb index ec1f386ea72..9b01a80681d 100644 --- a/lib/auth/google_oauth2_authenticator.rb +++ b/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], diff --git a/spec/integration/omniauth_callbacks_spec.rb b/spec/integration/omniauth_callbacks_spec.rb new file mode 100644 index 00000000000..7549e90f776 --- /dev/null +++ b/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