Merge branch...

Merge branch '332823-registrations-invitescontroller-create-activerecord-statementinvalid-pg-invalidrecursion' into 'master'

Validate new user saved before accepting invite

See merge request gitlab-org/gitlab!67136

app/controllers/registrations_controller.rb

@@ -21,9 +21,10 @@ class RegistrationsController < Devise::RegistrationsController
 
   def create
     set_user_state
-    accept_pending_invitations
 
     super do |new_user|
+      accept_pending_invitations if new_user.persisted?
+
       persist_accepted_terms_if_required(new_user)
       set_role_required(new_user)
 

app/models/member.rb

@@ -278,12 +278,14 @@ class Member < ApplicationRecord
 
   def accept_invite!(new_user)
     return false unless invite?
+    return false unless new_user
+
+    self.user = new_user
+    return false unless self.user.save
 
     self.invite_token = nil
     self.invite_accepted_at = Time.current.utc
 
-    self.user = new_user
-
     saved = self.save
 
     after_accept_invite if saved

spec/controllers/registrations_controller_spec.rb

@@ -283,6 +283,26 @@ RSpec.describe RegistrationsController do
           end
         end
 
+        context 'when the registration fails' do
+          let_it_be(:member) { create(:project_member, :invited) }
+          let_it_be(:missing_user_params) do
+            { username: '', email: member.invite_email, password: 'Any_password' }
+          end
+
+          let_it_be(:user_params) { { user: missing_user_params } }
+
+          let(:session_params) { { invite_email: member.invite_email } }
+
+          subject { post(:create, params: user_params, session: session_params) }
+
+          it 'does not delete the invitation or register the new user' do
+            subject
+
+            expect(member.invite_token).not_to be_nil
+            expect(controller.current_user).to be_nil
+          end
+        end
+
         context 'when soft email confirmation is enabled' do
           before do
             stub_feature_flags(soft_email_confirmation: true)

spec/features/invites_spec.rb

@@ -189,6 +189,16 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
       end
 
       context 'email confirmation enabled' do
+        context 'when user is not valid in sign up form' do
+          let(:new_user) { build_stubbed(:user, first_name: '', last_name: '') }
+
+          it 'fails sign up and redirects back to sign up', :aggregate_failures do
+            expect { fill_in_sign_up_form(new_user) }.not_to change { User.count }
+            expect(page).to have_content('prohibited this user from being saved')
+            expect(current_path).to eq(user_registration_path)
+          end
+        end
+
         context 'with invite email acceptance', :snowplow do
           it 'tracks the accepted invite' do
             fill_in_sign_up_form(new_user)

spec/models/member_spec.rb

@@ -645,6 +645,16 @@ RSpec.describe Member do
 
       expect(user.authorized_projects.reload).to include(project)
     end
+
+    it 'does not accept the invite if saving a new user fails' do
+      invalid_user = User.new(first_name: '', last_name: '')
+
+      member.accept_invite! invalid_user
+
+      expect(member.invite_accepted_at).to be_nil
+      expect(member.invite_token).not_to be_nil
+      expect_any_instance_of(Member).not_to receive(:after_accept_invite)
+    end
   end
 
   describe "#decline_invite!" do