diff --git a/app/services/user_authenticator.rb b/app/services/user_authenticator.rb index cd75e931220..b53c5de8b95 100644 --- a/app/services/user_authenticator.rb +++ b/app/services/user_authenticator.rb @@ -21,7 +21,10 @@ class UserAuthenticator end def finish - authenticator.after_create_account(@user, @session) if authenticator + if authenticator + authenticator.after_create_account(@user, @session) + confirm_email + end @session = nil end @@ -30,11 +33,18 @@ class UserAuthenticator end def authenticated? - @session && @session[:email] == @user.email && @session[:email_valid] + @session && @session[:email]&.downcase == @user.email.downcase && @session[:email_valid].to_s == "true" end private + def confirm_email + if authenticated? + EmailToken.confirm(@user.email_tokens.first.token) + @user.set_automatic_groups + end + end + def authenticator if authenticator_name @authenticator ||= @authenticator_finder.find_authenticator(authenticator_name) diff --git a/lib/auth/google_oauth2_authenticator.rb b/lib/auth/google_oauth2_authenticator.rb index a2804081935..e477c55edc0 100644 --- a/lib/auth/google_oauth2_authenticator.rb +++ b/lib/auth/google_oauth2_authenticator.rb @@ -44,10 +44,6 @@ class Auth::GoogleOAuth2Authenticator < Auth::Authenticator def after_create_account(user, auth) data = auth[:extra_data] GoogleUserInfo.create({ user_id: user.id }.merge(data)) - if auth[:email_valid].to_s == 'true' && data[:email]&.downcase == user.email - EmailToken.confirm(user.email_tokens.first.token) - user.set_automatic_groups - end end def register_middleware(omniauth) diff --git a/spec/components/auth/google_oauth2_authenticator_spec.rb b/spec/components/auth/google_oauth2_authenticator_spec.rb index e3af7278baf..525d65f9aa9 100644 --- a/spec/components/auth/google_oauth2_authenticator_spec.rb +++ b/spec/components/auth/google_oauth2_authenticator_spec.rb @@ -81,35 +81,4 @@ describe Auth::GoogleOAuth2Authenticator do expect(result.extra_data[:name]).to eq("Jane Doe") end end - - context 'after_create_account' do - it 'confirms email' do - authenticator = Auth::GoogleOAuth2Authenticator.new - user = Fabricate(:user, email: 'realgoogleuser@gmail.com') - session = { - email_valid: "true", - extra_data: { - google_user_id: 1, - email: 'realgoogleuser@gmail.com' - } - } - authenticator.after_create_account(user, session) - expect(user.email_confirmed?).to eq(true) - end - - it "doesn't confirm email if it was changed" do - authenticator = Auth::GoogleOAuth2Authenticator.new - user = Fabricate(:user, email: 'changed@gmail.com') - session = { - email_valid: "true", - extra_data: { - google_user_id: 1, - email: 'realgoogleuser@gmail.com' - } - } - authenticator.after_create_account(user, session) - expect(user.email_confirmed?).to eq(false) - end - end - end diff --git a/spec/services/user_authenticator_spec.rb b/spec/services/user_authenticator_spec.rb new file mode 100644 index 00000000000..3590711a32d --- /dev/null +++ b/spec/services/user_authenticator_spec.rb @@ -0,0 +1,60 @@ +require 'rails_helper' +require_dependency 'user_authenticator' + +def github_auth(email_valid) + { + email: "user53@discourse.org", + username: "joedoe546", + email_valid: email_valid, + omit_username: nil, + name: "Joe Doe 546", + authenticator_name: "github", + extra_data: { + github_user_id: "100", + github_screen_name: "joedoe546" + }, + skip_email_validation: false + } +end + +describe UserAuthenticator do + context "#finish" do + let(:authenticator) { Auth::GithubAuthenticator.new } + let(:group) { Fabricate(:group, automatic_membership_email_domains: "discourse.org") } + + before do + SiteSetting.enable_github_logins = true + end + + it "confirms email and adds the user to appropraite groups based on email" do + user = Fabricate(:user, email: "user53@discourse.org") + expect(group.usernames).not_to include(user.username) + + authentication = github_auth(true) + + UserAuthenticator.new(user, authentication: authentication).finish + expect(user.email_confirmed?).to be_truthy + expect(group.usernames).to include(user.username) + end + + it "doesn't confirm email if email is invalid" do + user = Fabricate(:user, email: "user53@discourse.org") + + authentication = github_auth(false) + + UserAuthenticator.new(user, authentication: authentication).finish + expect(user.email_confirmed?).to be_falsey + expect(group.usernames).not_to include(user.username) + end + + it "doesn't confirm email if it was changed" do + user = Fabricate(:user, email: "changed@discourse.org") + + authentication = github_auth(true) + + UserAuthenticator.new(user, authentication: authentication).finish + expect(user.email_confirmed?).to be_falsey + expect(group.usernames).not_to include(user.username) + end + end +end