Commit 01275667 authored by 🙈  jacopo beschi 🙉's avatar 🙈 jacopo beschi 🙉 Committed by Douwe Maan

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

parent bbd8d5b2
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
class ConfirmationsController < Devise::ConfirmationsController class ConfirmationsController < Devise::ConfirmationsController
include AcceptsPendingInvitations
def almost_there def almost_there
flash[:notice] = nil flash[:notice] = nil
render layout: "devise_empty" render layout: "devise_empty"
...@@ -11,6 +13,8 @@ class ConfirmationsController < Devise::ConfirmationsController ...@@ -11,6 +13,8 @@ class ConfirmationsController < Devise::ConfirmationsController
end end
def after_confirmation_path_for(resource_name, resource) def after_confirmation_path_for(resource_name, resource)
accept_pending_invitations
# incoming resource can either be a :user or an :email # incoming resource can either be a :user or an :email
if signed_in?(:user) if signed_in?(:user)
after_sign_in(resource) after_sign_in(resource)
......
class RegistrationsController < Devise::RegistrationsController class RegistrationsController < Devise::RegistrationsController
include Recaptcha::Verify include Recaptcha::Verify
include AcceptsPendingInvitations
before_action :whitelist_query_limiting, only: [:destroy] before_action :whitelist_query_limiting, only: [:destroy]
...@@ -16,6 +17,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -16,6 +17,7 @@ class RegistrationsController < Devise::RegistrationsController
end end
if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha
accept_pending_invitations
super super
else else
flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
...@@ -60,7 +62,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -60,7 +62,7 @@ class RegistrationsController < Devise::RegistrationsController
def after_sign_up_path_for(user) 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?}") 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 end
def after_inactive_sign_up_path_for(resource) def after_inactive_sign_up_path_for(resource)
......
...@@ -860,6 +860,16 @@ class User < ActiveRecord::Base ...@@ -860,6 +860,16 @@ class User < ActiveRecord::Base
confirmed? && !temp_oauth_email? confirmed? && !temp_oauth_email?
end 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 def all_emails
all_emails = [] all_emails = []
all_emails << email unless temp_oauth_email? all_emails << email unless temp_oauth_email?
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
by by
= link_to member.created_by.name, user_url(member.created_by) = link_to member.created_by.name, user_url(member.created_by)
to join the 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}. #{member_source.model_name.singular} as #{member.human_access}.
%p %p
......
---
title: Automatically accepts project/group invite by email after user signup
merge_request: 17634
author: Jacopo Beschi @jacopo-beschi
type: changed
...@@ -5,18 +5,41 @@ describe 'Invites' do ...@@ -5,18 +5,41 @@ describe 'Invites' do
let(:owner) { create(:user, name: 'John Doe') } let(:owner) { create(:user, name: 'John Doe') }
let(:group) { create(:group, name: 'Owned') } let(:group) { create(:group, name: 'Owned') }
let(:project) { create(:project, :repository, namespace: group) } let(:project) { create(:project, :repository, namespace: group) }
let(:invite) { group.group_members.invite.last } let(:group_invite) { group.group_members.invite.last }
before do before do
project.add_master(owner) project.add_master(owner)
group.add_user(owner, Gitlab::Access::OWNER) group.add_user(owner, Gitlab::Access::OWNER)
group.add_developer('user@example.com', 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 end
context 'when signed out' do context 'when signed out' do
before do before do
visit invite_path(invite.raw_invite_token) visit invite_path(group_invite.raw_invite_token)
end end
it 'renders sign in page with sign in notice' do it 'renders sign in page with sign in notice' do
...@@ -25,12 +48,9 @@ describe 'Invites' do ...@@ -25,12 +48,9 @@ describe 'Invites' do
end end
it 'sign in and redirects to invitation page' do it 'sign in and redirects to invitation page' do
fill_in 'user_login', with: user.email fill_in_sign_in_form(user)
fill_in 'user_password', with: user.password
check 'user_remember_me'
click_button 'Sign in'
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( expect(page).to have_content(
'You have been invited by John Doe to join group Owned as Developer.' 'You have been invited by John Doe to join group Owned as Developer.'
) )
...@@ -45,7 +65,7 @@ describe 'Invites' do ...@@ -45,7 +65,7 @@ describe 'Invites' do
end end
it 'shows message user already a member' do 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.') expect(page).to have_content('However, you are already a member of this group.')
end end
end end
...@@ -53,7 +73,7 @@ describe 'Invites' do ...@@ -53,7 +73,7 @@ describe 'Invites' do
describe 'accepting the invitation' do describe 'accepting the invitation' do
before do before do
sign_in(user) sign_in(user)
visit invite_path(invite.raw_invite_token) visit invite_path(group_invite.raw_invite_token)
end end
it 'grants access and redirects to group page' do it 'grants access and redirects to group page' do
...@@ -69,7 +89,7 @@ describe 'Invites' do ...@@ -69,7 +89,7 @@ describe 'Invites' do
context 'when signed in' do context 'when signed in' do
before do before do
sign_in(user) sign_in(user)
visit invite_path(invite.raw_invite_token) visit invite_path(group_invite.raw_invite_token)
end end
it 'declines application and redirects to dashboard' do it 'declines application and redirects to dashboard' do
...@@ -83,7 +103,7 @@ describe 'Invites' do ...@@ -83,7 +103,7 @@ describe 'Invites' do
context 'when signed out' do context 'when signed out' do
before do before do
visit decline_invite_path(invite.raw_invite_token) visit decline_invite_path(group_invite.raw_invite_token)
end end
it 'declines application and redirects to sign in page' do it 'declines application and redirects to sign in page' do
...@@ -94,4 +114,72 @@ describe 'Invites' do ...@@ -94,4 +114,72 @@ describe 'Invites' do
end end
end 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 end
...@@ -594,7 +594,7 @@ describe Notify do ...@@ -594,7 +594,7 @@ describe Notify do
it 'contains all the useful information' 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_subject "Invitation to join the #{project.full_name} project"
is_expected.to have_html_escaped_body_text project.full_name 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.human_access
is_expected.to have_body_text project_member.invite_token is_expected.to have_body_text project_member.invite_token
end end
......
...@@ -1223,6 +1223,24 @@ describe User do ...@@ -1223,6 +1223,24 @@ describe User do
end end
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 describe '#all_emails' do
let(:user) { create(:user) } let(:user) { create(:user) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment