FIX: don't activate un-confirmed email on omniauth authentication (#5176)

app/controllers/users/omniauth_callbacks_controller.rb

@@ -115,7 +115,7 @@ class Users::OmniauthCallbacksController < ApplicationController
     if @auth_result.email_valid && @auth_result.email == user.email
       user.update!(staged: false)
       # ensure there is an active email token
-      user.email_tokens.create(email: user.email) unless user.email_tokens.active.exists?
+      user.email_tokens.create(email: user.email) unless user.email_tokens.active.where(email: user.email).exists?
       user.activate
     end
 

app/models/user.rb

@@ -690,7 +690,7 @@ class User < ActiveRecord::Base
   end
 
   def activate
-    if email_token = self.email_tokens.active.first
+    if email_token = self.email_tokens.active.where(email: self.email).first
       EmailToken.confirm(email_token.token)
     else
       self.active = true

spec/requests/omniauth_callbacks_controller_spec.rb

@@ -129,5 +129,63 @@ RSpec.describe Users::OmniauthCallbacksController do
         end
       end
     end
+
+    context 'after changing email' do
+      require_dependency 'email_updater'
+
+      def login(identity)
+        OmniAuth.config.mock_auth[:google_oauth2] = OmniAuth::AuthHash.new(
+          provider: 'google_oauth2',
+          uid: "123545#{identity[:username]}",
+          info: OmniAuth::AuthHash::InfoHash.new(
+            email: identity[:email],
+            name: 'Some name'
+          ),
+          extra: {
+            raw_info: OmniAuth::AuthHash.new(
+              email_verified: true,
+              email: identity[:email],
+              family_name: 'Huh',
+              given_name: identity[:name],
+              gender: 'male',
+              name: "#{identity[:name]} Huh",
+            )
+          },
+        )
+
+        Rails.application.env_config["omniauth.auth"] = OmniAuth.config.mock_auth[:google_oauth2]
+
+        get "/auth/google_oauth2/callback.json"
+        JSON.parse(response.body)
+      end
+
+      it 'activates the correct email' do
+        old_email = 'old@email.com'
+        old_identity = { name: 'Bob',
+                         username: 'bob',
+                         email: old_email }
+        user = Fabricate(:user, email: old_email)
+        new_email = 'new@email.com'
+        new_identity = { name: 'Bob',
+                         username: 'boguslaw',
+                         email: new_email }
+
+        updater = EmailUpdater.new(user.guardian, user)
+        updater.change_to(new_email)
+
+        user.reload
+        expect(user.email).to eq(old_email)
+
+        response = login(old_identity)
+        expect(response['authenticated']).to eq(true)
+
+        user.reload
+        expect(user.email).to eq(old_email)
+
+        response = login(new_identity)
+        expect(response['authenticated']).to eq(nil)
+        expect(response['email']).to eq(new_email)
+      end
+    end
   end
 end