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.
This commit is contained in:
David Taylor 2021-03-31 13:40:58 +01:00 committed by GitHub
parent e8c576cca9
commit 198c960b52
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 23 additions and 5 deletions

View File

@ -35,6 +35,7 @@ class Users::OmniauthCallbacksController < ApplicationController
else else
DiscourseEvent.trigger(:before_auth, authenticator, auth) DiscourseEvent.trigger(:before_auth, authenticator, auth)
@auth_result = authenticator.after_authenticate(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) DiscourseEvent.trigger(:after_auth, authenticator, @auth_result)
end end
@ -137,10 +138,8 @@ class Users::OmniauthCallbacksController < ApplicationController
return return
end 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 if @auth_result.email_valid && @auth_result.email == user.email
user.unstage!
if !user.active || !user.email_confirmed? if !user.active || !user.email_confirmed?
user.update!(password: SecureRandom.hex) user.update!(password: SecureRandom.hex)

View File

@ -356,7 +356,7 @@ RSpec.describe Users::OmniauthCallbacksController do
expect(user.email_confirmed?).to eq(true) expect(user.email_confirmed?).to eq(true)
end 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.update!(staged: true, registration_ip_address: nil)
user.reload user.reload
@ -367,13 +367,32 @@ RSpec.describe Users::OmniauthCallbacksController do
get "/auth/google_oauth2/callback.json" get "/auth/google_oauth2/callback.json"
end 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) 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 user.reload
expect(user.staged).to eq(false) expect(user.staged).to eq(false)
expect(user.registration_ip_address).to be_present expect(user.registration_ip_address).to be_present
expect(user.name).to eq("My new name")
end end
it "should activate user with matching email" do it "should activate user with matching email" do