diff --git a/app/models/discourse_connect.rb b/app/models/discourse_connect.rb index b152b0209f0..91e0181a349 100644 --- a/app/models/discourse_connect.rb +++ b/app/models/discourse_connect.rb @@ -238,6 +238,7 @@ class DiscourseConnect < DiscourseConnectBase # the same email payload DistributedMutex.synchronize("discourse_single_sign_on_#{email}") do user = User.find_by_email(email) if !require_activation + if !user user_params = { primary_email: UserEmail.new(email: email, primary: true), @@ -250,7 +251,13 @@ class DiscourseConnect < DiscourseConnectBase user_params[:locale] = locale end - user = User.create!(user_params) + user = User.new(user_params) + + if SiteSetting.must_approve_users && EmailValidator.can_auto_approve_user?(email) + ReviewableUser.set_approved_fields!(user, Discourse.system_user) + end + + user.save! if SiteSetting.verbose_discourse_connect_logging Rails.logger.warn("Verbose SSO log: New User (user_id: #{user.id}) Params: #{user_params} User Params: #{user.attributes} User Errors: #{user.errors.full_messages} Email: #{user.primary_email.attributes} Email Error: #{user.primary_email.errors.full_messages}") diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index e9b02ced1a5..0328025f31f 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -827,6 +827,116 @@ describe SessionController do expect(response).to redirect_to('/') end + it 'creates a user but ignores auto_approve_email_domains site setting when must_approve_users site setting is not enabled' do + SiteSetting.auto_approve_email_domains = "discourse.com" + + sso = get_sso('/a/') + sso.external_id = '666' + sso.email = 'sam@discourse.com' + sso.name = 'Sam Saffron' + sso.username = 'sam' + + events = DiscourseEvent.track_events do + get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers + + expect(response).to redirect_to('/a/') + end + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) + + logged_on_user = Discourse.current_user_provider.new(request.env).current_user + + # ensure nothing is transient + logged_on_user = User.find(logged_on_user.id) + + expect(logged_on_user.admin).to eq(false) + expect(logged_on_user.email).to eq('sam@discourse.com') + expect(logged_on_user.name).to eq('Sam Saffron') + expect(logged_on_user.username).to eq('sam') + expect(logged_on_user.approved).to eq(false) + expect(logged_on_user.active).to eq(true) + + expect(logged_on_user.single_sign_on_record.external_id).to eq("666") + expect(logged_on_user.single_sign_on_record.external_username).to eq('sam') + end + + context 'when must_approve_users site setting has been enabled' do + before do + SiteSetting.must_approve_users = true + end + + it "creates a user but does not approve when user's email domain does not match a domain in auto_approve_email_domains site settings" do + SiteSetting.auto_approve_email_domains = "discourse.com" + + sso = get_sso('/a/') + sso.external_id = '666' + sso.email = 'sam@discourse.org' + sso.name = 'Sam Saffron' + sso.username = 'sam' + + expect do + get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers + + expect(response.status).to eq(403) + expect(response.parsed_body).to include(I18n.t("discourse_connect.account_not_approved")) + end.to change { User.count }.by(1) + + logged_on_user = Discourse.current_user_provider.new(request.env).current_user + + expect(logged_on_user).to eq(nil) + + user = User.last + + expect(user.admin).to eq(false) + expect(user.email).to eq('sam@discourse.org') + expect(user.name).to eq('Sam Saffron') + expect(user.username).to eq('sam') + expect(user.approved).to eq(false) + expect(user.active).to eq(true) + + expect(user.single_sign_on_record.external_id).to eq("666") + expect(user.single_sign_on_record.external_username).to eq('sam') + end + + it "creates and approves a user when user's email domain matches a domain in auto_approve_email_domains site settings" do + SiteSetting.auto_approve_email_domains = "discourse.com" + + sso = get_sso('/a/') + sso.external_id = '666' + sso.email = 'sam@discourse.com' + sso.name = 'Sam Saffron' + sso.username = 'sam' + + events = DiscourseEvent.track_events do + get "/session/sso_login", params: Rack::Utils.parse_query(sso.payload), headers: headers + + expect(response).to redirect_to('/a/') + end + + expect(events.map { |event| event[:event_name] }).to include( + :user_logged_in, :user_first_logged_in + ) + + logged_on_user = Discourse.current_user_provider.new(request.env).current_user + + # ensure nothing is transient + logged_on_user = User.find(logged_on_user.id) + + expect(logged_on_user.admin).to eq(false) + expect(logged_on_user.email).to eq('sam@discourse.com') + expect(logged_on_user.name).to eq('Sam Saffron') + expect(logged_on_user.username).to eq('sam') + expect(logged_on_user.approved).to eq(true) + expect(logged_on_user.active).to eq(true) + + expect(logged_on_user.single_sign_on_record.external_id).to eq("666") + expect(logged_on_user.single_sign_on_record.external_username).to eq('sam') + end + + end + it 'allows you to create an account' do group = Fabricate(:group, name: :bob, automatic_membership_email_domains: 'bob.com')