From 198c960b5238cd893113399a0323c7233a966d74 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Wed, 31 Mar 2021 13:40:58 +0100 Subject: [PATCH] FIX: Handle staged users as unregistered users for external auth (#12567) For 'local logins', the UX for staged users is designed to be identical to unregistered users. However, staged users logging in via external auth were being automatically unstaged, and skipping the registration/invite flow. In the past this made sense because the registration/invite flows didn't work perfectly with external auth. Now, both registration and invites work well with external auth, so it's best to leave the 'unstage' logic to those endpoints. This problem was particularly noticeable when using the 'bulk invite' feature to invite users with pre-configured User Fields. In that situation, staged user accounts are used to preserve the user field data. --- .../users/omniauth_callbacks_controller.rb | 5 ++-- .../omniauth_callbacks_controller_spec.rb | 23 +++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 32b9bc3639c..2b702962c38 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -35,6 +35,7 @@ class Users::OmniauthCallbacksController < ApplicationController else DiscourseEvent.trigger(:before_auth, authenticator, auth) @auth_result = authenticator.after_authenticate(auth) + @auth_result.user = nil if @auth_result&.user&.staged # Treat staged users the same as unregistered users DiscourseEvent.trigger(:after_auth, authenticator, @auth_result) end @@ -137,10 +138,8 @@ class Users::OmniauthCallbacksController < ApplicationController return end - # automatically activate/unstage any account if a provider marked the email valid + # automatically activate any account if a provider marked the email valid if @auth_result.email_valid && @auth_result.email == user.email - user.unstage! - if !user.active || !user.email_confirmed? user.update!(password: SecureRandom.hex) diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb index 33359a2c718..bd7421505a9 100644 --- a/spec/requests/omniauth_callbacks_controller_spec.rb +++ b/spec/requests/omniauth_callbacks_controller_spec.rb @@ -356,7 +356,7 @@ RSpec.describe Users::OmniauthCallbacksController do expect(user.email_confirmed?).to eq(true) end - it "should unstage staged user" do + it "should treat a staged user the same as an unregistered user" do user.update!(staged: true, registration_ip_address: nil) user.reload @@ -367,13 +367,32 @@ RSpec.describe Users::OmniauthCallbacksController do get "/auth/google_oauth2/callback.json" end - expect(events.map { |event| event[:event_name] }).to include(:user_logged_in, :user_first_logged_in) + expect(events.map { |event| event[:event_name] }).to include(:before_auth, :after_auth) expect(response.status).to eq(302) + data = JSON.parse(cookies[:authentication_data]) + expect(data["username"]).to eq("Somenickname") + + user.reload + expect(user.staged).to eq(true) + expect(user.registration_ip_address).to eq(nil) + + # Now register + UsersController.any_instance.stubs(:honeypot_value).returns(nil) + UsersController.any_instance.stubs(:challenge_value).returns(nil) + post "/u.json", params: { + name: "My new name", + username: "mynewusername", + email: user.email + } + + expect(response.status).to eq(200) + user.reload expect(user.staged).to eq(false) expect(user.registration_ip_address).to be_present + expect(user.name).to eq("My new name") end it "should activate user with matching email" do