Merge branch '42531-open-invite-404' into 'master'

Resolve "Opening Project with invite but without accepting leads to 404 error page"

Closes #42531

See merge request gitlab-org/gitlab-ce!17634

9 files changed, 157 insertions, 15 deletions

diff --git a/app/controllers/concerns/accepts_pending_invitations.rb b/app/controllers/concerns/accepts_pending_invitations.rb
new file mode 100644
index 00000000000..6e8aef52b52
--- /dev/null
+++ b/app/controllers/concerns/accepts_pending_invitations.rb
@@ -0,0 +1,15 @@
+module AcceptsPendingInvitations
+  extend ActiveSupport::Concern
+
+  def accept_pending_invitations
+    return unless resource.active_for_authentication?
+
+    clear_stored_location_for_resource if resource.accept_pending_invitations!.any?
+  end
+
+  def clear_stored_location_for_resource
+    session_key = stored_location_key_for(resource)
+
+    session.delete(session_key)
+  end
+end
diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb
index 6d9c38d9581..7bc46a6ccc0 100644
--- a/app/controllers/confirmations_controller.rb
+++ b/app/controllers/confirmations_controller.rb
@@ -1,4 +1,6 @@
 class ConfirmationsController < Devise::ConfirmationsController
+  include AcceptsPendingInvitations
+
   def almost_there
     flash[:notice] = nil
     render layout: "devise_empty"
@@ -11,6 +13,8 @@ class ConfirmationsController < Devise::ConfirmationsController
   end
 
   def after_confirmation_path_for(resource_name, resource)
+    accept_pending_invitations
+
     # incoming resource can either be a :user or an :email
     if signed_in?(:user)
       after_sign_in(resource)
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 1848c806c41..f5a222b3a48 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -1,5 +1,6 @@
 class RegistrationsController < Devise::RegistrationsController
   include Recaptcha::Verify
+  include AcceptsPendingInvitations
 
   before_action :whitelist_query_limiting, only: [:destroy]
 
@@ -16,6 +17,7 @@ class RegistrationsController < Devise::RegistrationsController
     end
 
     if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
+      accept_pending_invitations
       super
     else
       flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
@@ -60,7 +62,7 @@ class RegistrationsController < Devise::RegistrationsController
 
   def after_sign_up_path_for(user)
     Gitlab::AppLogger.info("User Created: username=#{user.username} email=#{user.email} ip=#{request.remote_ip} confirmed:#{user.confirmed?}")
-    user.confirmed? ? dashboard_projects_path : users_almost_there_path
+    user.confirmed? ? stored_location_for(user) || dashboard_projects_path : users_almost_there_path
   end
 
   def after_inactive_sign_up_path_for(resource)
diff --git a/app/models/user.rb b/app/models/user.rb
index 474fde36c02..8ef3c3ceff0 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -860,6 +860,16 @@ class User < ActiveRecord::Base
     confirmed? && !temp_oauth_email?
   end
 
+  def accept_pending_invitations!
+    pending_invitations.select do |member|
+      member.accept_invite!(self)
+    end
+  end
+
+  def pending_invitations
+    Member.where(invite_email: verified_emails).invite
+  end
+
   def all_emails
     all_emails = []
     all_emails << email unless temp_oauth_email?
diff --git a/app/views/notify/member_invited_email.html.haml b/app/views/notify/member_invited_email.html.haml
index b8b75da3f2f..6730172242b 100644
--- a/app/views/notify/member_invited_email.html.haml
+++ b/app/views/notify/member_invited_email.html.haml
@@ -4,7 +4,7 @@
     by
     = link_to member.created_by.name, user_url(member.created_by)
   to join the
-  = link_to member_source.human_name, member_source.web_url
+  = link_to member_source.human_name, member_source.public? ? member_source.web_url : invite_url(@token)
   #{member_source.model_name.singular} as #{member.human_access}.
 
 %p
diff --git a/changelogs/unreleased/42531-open-invite-404.yml b/changelogs/unreleased/42531-open-invite-404.yml
new file mode 100644
index 00000000000..73729f4a929
--- /dev/null
+++ b/changelogs/unreleased/42531-open-invite-404.yml
@@ -0,0 +1,5 @@
+---
+title: Automatically accepts project/group invite by email after user signup
+merge_request: 17634
+author: Jacopo Beschi @jacopo-beschi
+type: changed
diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb
index e4be6193b8b..a986ddc4abc 100644
--- a/spec/features/invites_spec.rb
+++ b/spec/features/invites_spec.rb
@@ -5,18 +5,41 @@ describe 'Invites' do
   let(:owner) { create(:user, name: 'John Doe') }
   let(:group) { create(:group, name: 'Owned') }
   let(:project) { create(:project, :repository, namespace: group) }
-  let(:invite) { group.group_members.invite.last }
+  let(:group_invite) { group.group_members.invite.last }
 
   before do
     project.add_master(owner)
     group.add_user(owner, Gitlab::Access::OWNER)
     group.add_developer('user@example.com', owner)
-    invite.generate_invite_token!
+    group_invite.generate_invite_token!
+  end
+
+  def confirm_email_and_sign_in(new_user)
+    new_user_token = User.find_by_email(new_user.email).confirmation_token
+
+    visit user_confirmation_path(confirmation_token: new_user_token)
+    fill_in_sign_in_form(new_user)
+  end
+
+  def fill_in_sign_up_form(new_user)
+    fill_in 'new_user_name',                with: new_user.name
+    fill_in 'new_user_username',            with: new_user.username
+    fill_in 'new_user_email',               with: new_user.email
+    fill_in 'new_user_email_confirmation',  with: new_user.email
+    fill_in 'new_user_password',            with: new_user.password
+    click_button "Register"
+  end
+
+  def fill_in_sign_in_form(user)
+    fill_in 'user_login', with: user.email
+    fill_in 'user_password', with: user.password
+    check 'user_remember_me'
+    click_button 'Sign in'
   end
 
   context 'when signed out' do
     before do
-      visit invite_path(invite.raw_invite_token)
+      visit invite_path(group_invite.raw_invite_token)
     end
 
     it 'renders sign in page with sign in notice' do
@@ -25,12 +48,9 @@ describe 'Invites' do
     end
 
     it 'sign in and redirects to invitation page' do
-      fill_in 'user_login', with: user.email
-      fill_in 'user_password', with: user.password
-      check 'user_remember_me'
-      click_button 'Sign in'
+      fill_in_sign_in_form(user)
 
-      expect(current_path).to eq(invite_path(invite.raw_invite_token))
+      expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
       expect(page).to have_content(
         'You have been invited by John Doe to join group Owned as Developer.'
       )
@@ -45,7 +65,7 @@ describe 'Invites' do
     end
 
     it 'shows message user already a member' do
-      visit invite_path(invite.raw_invite_token)
+      visit invite_path(group_invite.raw_invite_token)
       expect(page).to have_content('However, you are already a member of this group.')
     end
   end
@@ -53,7 +73,7 @@ describe 'Invites' do
   describe 'accepting the invitation' do
     before do
       sign_in(user)
-      visit invite_path(invite.raw_invite_token)
+      visit invite_path(group_invite.raw_invite_token)
     end
 
     it 'grants access and redirects to group page' do
@@ -69,7 +89,7 @@ describe 'Invites' do
     context 'when signed in' do
       before do
         sign_in(user)
-        visit invite_path(invite.raw_invite_token)
+        visit invite_path(group_invite.raw_invite_token)
       end
 
       it 'declines application and redirects to dashboard' do
@@ -83,7 +103,7 @@ describe 'Invites' do
 
     context 'when signed out' do
       before do
-        visit decline_invite_path(invite.raw_invite_token)
+        visit decline_invite_path(group_invite.raw_invite_token)
       end
 
       it 'declines application and redirects to sign in page' do
@@ -94,4 +114,72 @@ describe 'Invites' do
       end
     end
   end
+
+  describe 'invite an user using their email address' do
+    let(:new_user) { build_stubbed(:user) }
+    let(:invite_email) { new_user.email }
+    let(:group_invite) { create(:group_member, :invited, group: group, invite_email: invite_email) }
+    let!(:project_invite) { create(:project_member, :invited, project: project, invite_email: invite_email) }
+
+    before do
+      stub_application_setting(send_user_confirmation_email: send_email_confirmation)
+      visit invite_path(group_invite.raw_invite_token)
+    end
+
+    context 'email confirmation disabled' do
+      let(:send_email_confirmation) { false }
+
+      it 'signs up and redirects to the dashboard page with all the projects/groups invitations automatically accepted' do
+        fill_in_sign_up_form(new_user)
+
+        expect(current_path).to eq(dashboard_projects_path)
+        expect(page).to have_content(project.full_name)
+        visit group_path(group)
+        expect(page).to have_content(group.full_name)
+      end
+
+      context 'the user sign-up using a different email address' do
+        let(:invite_email) { build_stubbed(:user).email }
+
+        it 'signs up and redirects to the invitation page' do
+          fill_in_sign_up_form(new_user)
+
+          expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
+        end
+      end
+    end
+
+    context 'email confirmation enabled' do
+      let(:send_email_confirmation) { true }
+
+      it 'signs up and redirects to root page with all the project/groups invitation automatically accepted' do
+        fill_in_sign_up_form(new_user)
+        confirm_email_and_sign_in(new_user)
+
+        expect(current_path).to eq(root_path)
+        expect(page).to have_content(project.full_name)
+        visit group_path(group)
+        expect(page).to have_content(group.full_name)
+      end
+
+      it "doesn't accept invitations until the user confirm his email" do
+        fill_in_sign_up_form(new_user)
+        sign_in(owner)
+
+        visit project_project_members_path(project)
+        expect(page).to have_content 'Invited'
+      end
+
+      context 'the user sign-up using a different email address' do
+        let(:invite_email) { build_stubbed(:user).email }
+
+        it 'signs up and redirects to the invitation page' do
+          fill_in_sign_up_form(new_user)
+          confirm_email_and_sign_in(new_user)
+
+          expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
+        end
+      end
+    end
+  end
 end
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 84ddbbbf2ee..8a52c151cc4 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -594,7 +594,7 @@ describe Notify do
       it 'contains all the useful information' do
         is_expected.to have_subject "Invitation to join the #{project.full_name} project"
         is_expected.to have_html_escaped_body_text project.full_name
-        is_expected.to have_body_text project.web_url
+        is_expected.to have_body_text project.full_name
         is_expected.to have_body_text project_member.human_access
         is_expected.to have_body_text project_member.invite_token
       end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 8d3ddd1f87d..684fa030baf 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1223,6 +1223,24 @@ describe User do
     end
   end
 
+  describe '#accept_pending_invitations!' do
+    let(:user) { create(:user, email: 'user@email.com') }
+    let!(:project_member_invite) { create(:project_member, :invited, invite_email: user.email) }
+    let!(:group_member_invite) { create(:group_member, :invited, invite_email: user.email) }
+    let!(:external_project_member_invite) { create(:project_member, :invited, invite_email: 'external@email.com') }
+    let!(:external_group_member_invite) { create(:group_member, :invited, invite_email: 'external@email.com') }
+
+    it 'accepts all the user members pending invitations and returns the accepted_members' do
+      accepted_members = user.accept_pending_invitations!
+
+      expect(accepted_members).to match_array([project_member_invite, group_member_invite])
+      expect(group_member_invite.reload).not_to be_invite
+      expect(project_member_invite.reload).not_to be_invite
+      expect(external_project_member_invite.reload).to be_invite
+      expect(external_group_member_invite.reload).to be_invite
+    end
+  end
+
   describe '#all_emails' do
     let(:user) { create(:user) }