FIX: DiscourseConnect login did not auto approve based on email domain (#17006)

This commit resolves a bug where users are not auto approved based on
`SiteSetting.auto_approve_email_domains` when
`SiteSetting.must_approve_users` has been enabled.

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}")

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')