From dd9bcdb5a92c2e20ee8fab0f66aa62f10f2e8fb2 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 2 Aug 2024 15:10:16 +0200 Subject: [PATCH 1/7] Pass Current.organization_id to user creation during registration --- app/controllers/registrations_controller.rb | 15 +++++++-------- .../controllers/trial_registrations_controller.rb | 4 +++- .../ee/registrations_controller_spec.rb | 2 +- .../requests/ee/registrations_controller_spec.rb | 2 +- .../trial_registrations_controller_spec.rb | 2 +- spec/controllers/registrations_controller_spec.rb | 2 +- spec/requests/registrations_controller_spec.rb | 2 +- .../disable_namespace_organization_validation.yml | 2 -- 8 files changed, 15 insertions(+), 16 deletions(-) diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 36306f2ef12273..87be68db1d39aa 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -38,13 +38,11 @@ def new def create set_resource_fields - Namespace.with_disabled_organization_validation do - super do |new_user| - if new_user.persisted? - after_successful_create_hook(new_user) - else - track_error(new_user) - end + super do |new_user| + if new_user.persisted? + after_successful_create_hook(new_user) + else + track_error(new_user) end end # Devise sets a flash message on both successful & failed signups, @@ -236,7 +234,8 @@ def resource_name def resource @resource ||= Users::RegistrationsBuildService .new(current_user, sign_up_params.merge({ skip_confirmation: skip_confirmation?, - preferred_language: preferred_language })) + preferred_language: preferred_language, + organization_id: Current.organization_id })) .execute end diff --git a/ee/app/controllers/trial_registrations_controller.rb b/ee/app/controllers/trial_registrations_controller.rb index cfc0c0141193ab..00cb869990dd56 100644 --- a/ee/app/controllers/trial_registrations_controller.rb +++ b/ee/app/controllers/trial_registrations_controller.rb @@ -59,7 +59,9 @@ def sign_up_params_attributes override :resource def resource - @resource ||= Users::AuthorizedBuildService.new(current_user, sign_up_params).execute + @resource ||= Users::AuthorizedBuildService.new( + current_user, sign_up_params.merge(organization_id: Current.organization_id) + ).execute end override :preregistration_tracking_label diff --git a/ee/spec/controllers/ee/registrations_controller_spec.rb b/ee/spec/controllers/ee/registrations_controller_spec.rb index 4416b5061002c0..a4cf67b3082077 100644 --- a/ee/spec/controllers/ee/registrations_controller_spec.rb +++ b/ee/spec/controllers/ee/registrations_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe RegistrationsController, feature_category: :system_access do +RSpec.describe RegistrationsController, :with_current_organization, feature_category: :system_access do let(:member) { nil } shared_examples 'an unrestricted IP address' do diff --git a/ee/spec/requests/ee/registrations_controller_spec.rb b/ee/spec/requests/ee/registrations_controller_spec.rb index ec2b62162e3b36..245b1e005c2233 100644 --- a/ee/spec/requests/ee/registrations_controller_spec.rb +++ b/ee/spec/requests/ee/registrations_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe RegistrationsController, type: :request, feature_category: :system_access do +RSpec.describe RegistrationsController, :with_current_organization, type: :request, feature_category: :system_access do include SessionHelpers let_it_be(:user_attrs) do diff --git a/ee/spec/requests/trial_registrations_controller_spec.rb b/ee/spec/requests/trial_registrations_controller_spec.rb index 90b223dc0c155f..5dbe20b318d819 100644 --- a/ee/spec/requests/trial_registrations_controller_spec.rb +++ b/ee/spec/requests/trial_registrations_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe TrialRegistrationsController, :saas, feature_category: :onboarding do +RSpec.describe TrialRegistrationsController, :with_current_organization, :saas, feature_category: :onboarding do include FullNameHelper describe 'GET new' do diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 37c48a088c287e..5a4e2cc7eca78a 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe RegistrationsController, feature_category: :user_profile do +RSpec.describe RegistrationsController, :with_current_organization, feature_category: :user_profile do include TermsHelper include FullNameHelper diff --git a/spec/requests/registrations_controller_spec.rb b/spec/requests/registrations_controller_spec.rb index 467feb41f1ff7f..cc75046a1ea875 100644 --- a/spec/requests/registrations_controller_spec.rb +++ b/spec/requests/registrations_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe RegistrationsController, type: :request, feature_category: :system_access do - describe '#create' do + describe '#create', :with_current_organization do let_it_be(:user_attrs) { build_stubbed(:user).slice(:first_name, :last_name, :username, :email, :password) } let(:expected_context) do { 'meta.caller_id' => 'RegistrationsController#create' } diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml index 5073522243d3cb..60b6a13639200b 100644 --- a/spec/support/helpers/disable_namespace_organization_validation.yml +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -1,6 +1,5 @@ --- - ee/spec/controllers/ee/groups_controller_spec.rb -- ee/spec/controllers/ee/registrations_controller_spec.rb - ee/spec/controllers/ee/omniauth_callbacks_controller_spec.rb - ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb - ee/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb @@ -31,7 +30,6 @@ - spec/controllers/admin/groups_controller_spec.rb - spec/controllers/admin/users_controller_spec.rb - spec/controllers/groups_controller_spec.rb -- spec/controllers/registrations_controller_spec.rb - spec/features/admin/admin_groups_spec.rb - spec/features/dashboard/group_spec.rb - spec/features/groups_spec.rb -- GitLab From 50d830cc346bcac01759655563325b62cf0ddffa Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 2 Aug 2024 15:41:28 +0200 Subject: [PATCH 2/7] Pass Organization ID to Users::RegistrationsBuildService --- spec/services/users/registrations_build_service_spec.rb | 5 +++-- .../helpers/disable_namespace_organization_validation.yml | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/services/users/registrations_build_service_spec.rb b/spec/services/users/registrations_build_service_spec.rb index f8e0b83a896a7b..a3892c9c1df4c8 100644 --- a/spec/services/users/registrations_build_service_spec.rb +++ b/spec/services/users/registrations_build_service_spec.rb @@ -4,9 +4,10 @@ RSpec.describe Users::RegistrationsBuildService, feature_category: :system_access do describe '#execute' do + let_it_be(:organization) { create(:organization) } let(:base_params) { build_stubbed(:user).slice(:first_name, :last_name, :username, :email, :password) } let(:skip_param) { {} } - let(:params) { base_params.merge(skip_param) } + let(:params) { base_params.merge(skip_param).merge(organization_id: organization.id) } subject(:service) { described_class.new(nil, params) } @@ -18,7 +19,7 @@ it 'creates the user_detail record' do user = service.execute - expect { Namespace.with_disabled_organization_validation { user.save! } }.to change { UserDetail.count }.by(1) + expect { user.save! }.to change { UserDetail.count }.by(1) end end diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml index 60b6a13639200b..49d9747c3f9ea8 100644 --- a/spec/support/helpers/disable_namespace_organization_validation.yml +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -41,6 +41,5 @@ - spec/models/hooks/system_hook_spec.rb - spec/requests/api/groups_spec.rb - spec/services/users/create_service_spec.rb -- spec/services/users/registrations_build_service_spec.rb - spec/services/groups/create_service_spec.rb - spec/services/resource_access_tokens/create_service_spec.rb -- GitLab From 5b441f746b9f9274c26738cf68118419eb1a5cbd Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Thu, 8 Aug 2024 17:08:26 +0200 Subject: [PATCH 3/7] Set 'with_current_organization' for all methods --- spec/requests/registrations_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/registrations_controller_spec.rb b/spec/requests/registrations_controller_spec.rb index cc75046a1ea875..346413b6c48c5e 100644 --- a/spec/requests/registrations_controller_spec.rb +++ b/spec/requests/registrations_controller_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' -RSpec.describe RegistrationsController, type: :request, feature_category: :system_access do - describe '#create', :with_current_organization do +RSpec.describe RegistrationsController, :with_current_organization, type: :request, feature_category: :system_access do + describe '#create' do let_it_be(:user_attrs) { build_stubbed(:user).slice(:first_name, :last_name, :username, :email, :password) } let(:expected_context) do { 'meta.caller_id' => 'RegistrationsController#create' } -- GitLab From cda10d755ec4068921f32a64f6fb905c3fb40ba3 Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 9 Aug 2024 09:58:50 +0200 Subject: [PATCH 4/7] Add with_current_organization context to feature specs --- ee/spec/features/invites_spec.rb | 2 +- ee/spec/features/registrations/saas/invite_flow_spec.rb | 2 +- .../registrations/saas/standard_flow_with_2fa_spec.rb | 2 +- ...h_trial_from_external_site_without_confirmation_spec.rb | 2 ++ ...t_trial_from_external_site_without_confirmation_spec.rb | 2 +- ee/spec/features/signup_spec.rb | 2 +- ee/spec/features/trial_registrations/signup_spec.rb | 2 +- ee/spec/features/users/signup_spec.rb | 2 +- spec/features/invites_spec.rb | 2 +- spec/features/registrations/registration_spec.rb | 2 +- spec/features/users/signup_spec.rb | 2 +- .../helpers/disable_namespace_organization_validation.yml | 7 ------- 12 files changed, 12 insertions(+), 17 deletions(-) diff --git a/ee/spec/features/invites_spec.rb b/ee/spec/features/invites_spec.rb index 0c414239badab7..c4118c2cdf3dd1 100644 --- a/ee/spec/features/invites_spec.rb +++ b/ee/spec/features/invites_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Group or Project invitations', :js, feature_category: :onboarding do +RSpec.describe 'Group or Project invitations', :with_current_organization, :js, feature_category: :onboarding do let(:group) { create(:group, name: 'Owned') } let(:project) { create(:project, :repository, namespace: group) } let(:group_invite) { create(:group_member, :invited, group: group) } diff --git a/ee/spec/features/registrations/saas/invite_flow_spec.rb b/ee/spec/features/registrations/saas/invite_flow_spec.rb index 88770237cda43e..4b924d012bb099 100644 --- a/ee/spec/features/registrations/saas/invite_flow_spec.rb +++ b/ee/spec/features/registrations/saas/invite_flow_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'SaaS registration from an invite', :js, :saas_registration, :sidekiq_inline, feature_category: :onboarding do +RSpec.describe 'SaaS registration from an invite', :with_current_organization, :js, :saas_registration, :sidekiq_inline, feature_category: :onboarding do it 'registers the user and sends them to the group page' do group = create(:group, name: 'Test Group', organization: create(:organization)) diff --git a/ee/spec/features/registrations/saas/standard_flow_with_2fa_spec.rb b/ee/spec/features/registrations/saas/standard_flow_with_2fa_spec.rb index 01867197f92902..f432e24a6975a9 100644 --- a/ee/spec/features/registrations/saas/standard_flow_with_2fa_spec.rb +++ b/ee/spec/features/registrations/saas/standard_flow_with_2fa_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'SaaS registration from an invite', :js, :saas_registration, feature_category: :onboarding do +RSpec.describe 'SaaS registration from an invite', :with_current_organization, :js, :saas_registration, feature_category: :onboarding do context 'when user has not completed welcome step before being added to group', :sidekiq_inline do it 'registers the user, completes 2fa and sends them to the profile account page' do group = create(:group, name: 'Test Group', require_two_factor_authentication: true) diff --git a/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb b/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb index 67719f8777d9d1..f55960d2f75fee 100644 --- a/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb +++ b/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb @@ -10,6 +10,8 @@ { glm_source: 'some_source', glm_content: 'some_content' } end + let_it_be(:default_organization) { create(:organization, :default) } + before do stub_application_setting(require_admin_approval_after_user_signup: false) stub_application_setting(import_sources: %w[github gitlab_project]) diff --git a/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb b/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb index 46d6b059e26654..1e2eefef2ed0d4 100644 --- a/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb +++ b/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Start trial from external site without confirmation', :saas, :js, feature_category: :onboarding do +RSpec.describe 'Start trial from external site without confirmation', :with_current_organization, :saas, :js, feature_category: :onboarding do include SaasRegistrationHelpers let_it_be(:glm_params) do diff --git a/ee/spec/features/signup_spec.rb b/ee/spec/features/signup_spec.rb index 481346af037f87..381b17d965fbf9 100644 --- a/ee/spec/features/signup_spec.rb +++ b/ee/spec/features/signup_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Signup on EE', :js, feature_category: :user_profile do +RSpec.describe 'Signup on EE', :with_current_organization, :js, feature_category: :user_profile do let(:new_user) { build_stubbed(:user) } before do diff --git a/ee/spec/features/trial_registrations/signup_spec.rb b/ee/spec/features/trial_registrations/signup_spec.rb index 736efef1a6986d..f01c27e64b619f 100644 --- a/ee/spec/features/trial_registrations/signup_spec.rb +++ b/ee/spec/features/trial_registrations/signup_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Trial Sign Up', :saas, feature_category: :acquisition do +RSpec.describe 'Trial Sign Up', :with_current_organization, :saas, feature_category: :acquisition do before do stub_application_setting(require_admin_approval_after_user_signup: false) end diff --git a/ee/spec/features/users/signup_spec.rb b/ee/spec/features/users/signup_spec.rb index 05f6f662117d2a..c76e23b7bf9da3 100644 --- a/ee/spec/features/users/signup_spec.rb +++ b/ee/spec/features/users/signup_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Signup', feature_category: :system_access do +RSpec.describe 'Signup', :with_current_organization, feature_category: :system_access do context 'almost there page' do context 'when public visibility is restricted' do before do diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb index 3393cdf2f6cf55..2b7cc3a57af751 100644 --- a/spec/features/invites_spec.rb +++ b/spec/features/invites_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Group or Project invitations', :aggregate_failures, feature_category: :acquisition do +RSpec.describe 'Group or Project invitations', :with_current_organization, :aggregate_failures, feature_category: :acquisition do let_it_be(:owner) { create(:user, name: 'John Doe') } # private will ensure we really have access to the group when we land on the group page let_it_be(:group) { create(:group, :private, name: 'Owned') } diff --git a/spec/features/registrations/registration_spec.rb b/spec/features/registrations/registration_spec.rb index 88cbf8145bb498..efbd63e1321c41 100644 --- a/spec/features/registrations/registration_spec.rb +++ b/spec/features/registrations/registration_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Registrations', feature_category: :system_access do +RSpec.describe 'Registrations', :with_current_organization, feature_category: :system_access do let_it_be(:user) { create(:user) } context 'when the user visits the registration page when already signed in', :clean_gitlab_redis_sessions do diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb index 3bca31dcaa192b..4abe90dc463fb2 100644 --- a/spec/features/users/signup_spec.rb +++ b/spec/features/users/signup_spec.rb @@ -40,7 +40,7 @@ end end -RSpec.describe 'Signup', :js, feature_category: :user_management do +RSpec.describe 'Signup', :with_current_organization, :js, feature_category: :user_management do include TermsHelper let(:new_user) { build_stubbed(:user) } diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml index 49d9747c3f9ea8..84a5546ce16df5 100644 --- a/spec/support/helpers/disable_namespace_organization_validation.yml +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -8,15 +8,8 @@ - ee/spec/features/registrations/saas/sso_signin_standard_flow_company_creating_project_spec.rb - ee/spec/features/registrations/saas/standard_flow_company_creating_project_spec.rb - ee/spec/features/registrations/saas/standard_flow_just_me_creating_project_spec.rb -- ee/spec/features/registrations/saas/standard_flow_just_me_importing_project_spec.rb -- ee/spec/features/registrations/saas/subscription_flow_company_paid_plan_spec.rb -- ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb - ee/spec/features/registrations/saas/trial_flow_company_creating_project_spec.rb -- ee/spec/features/registrations/saas/trial_flow_company_importing_project_spec.rb - ee/spec/features/registrations/saas/trial_flow_just_me_creating_project_spec.rb -- ee/spec/features/registrations/saas/trial_flow_just_me_importing_project_spec.rb -- ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb -- ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb - ee/spec/features/trials/saas/creation_with_no_existing_namespace_flow_spec.rb - ee/spec/lib/gitlab/auth/group_saml/user_spec.rb - ee/spec/lib/gitlab/auth/saml/user_spec.rb -- GitLab From 54f9725ad599b7c0042e1731fdbd62f5f332a61e Mon Sep 17 00:00:00 2001 From: Rutger Wessels Date: Fri, 9 Aug 2024 16:11:25 +0200 Subject: [PATCH 5/7] Remove additional specs from disable validation yaml --- .../helpers/disable_namespace_organization_validation.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/spec/support/helpers/disable_namespace_organization_validation.yml b/spec/support/helpers/disable_namespace_organization_validation.yml index 84a5546ce16df5..dcc1fb0ea06f1e 100644 --- a/spec/support/helpers/disable_namespace_organization_validation.yml +++ b/spec/support/helpers/disable_namespace_organization_validation.yml @@ -4,13 +4,6 @@ - ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb - ee/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb - ee/spec/features/groups_spec.rb -- ee/spec/features/registrations/combined_registration_spec.rb -- ee/spec/features/registrations/saas/sso_signin_standard_flow_company_creating_project_spec.rb -- ee/spec/features/registrations/saas/standard_flow_company_creating_project_spec.rb -- ee/spec/features/registrations/saas/standard_flow_just_me_creating_project_spec.rb -- ee/spec/features/registrations/saas/trial_flow_company_creating_project_spec.rb -- ee/spec/features/registrations/saas/trial_flow_just_me_creating_project_spec.rb -- ee/spec/features/trials/saas/creation_with_no_existing_namespace_flow_spec.rb - ee/spec/lib/gitlab/auth/group_saml/user_spec.rb - ee/spec/lib/gitlab/auth/saml/user_spec.rb - ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb -- GitLab From 0db298f0853331ec69197ddbf279b2cfa58de160 Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Fri, 30 Aug 2024 21:08:53 +0530 Subject: [PATCH 6/7] Use with_default_organization to create the default organization --- ...with_trial_from_external_site_without_confirmation_spec.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb b/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb index f55960d2f75fee..99aa5fa1b25e53 100644 --- a/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb +++ b/ee/spec/features/registrations/sign_up_with_trial_from_external_site_without_confirmation_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Sign up with trial from external site without confirmation', :saas, :js, +RSpec.describe 'Sign up with trial from external site without confirmation', :saas, :js, :with_default_organization, feature_category: :onboarding do include SaasRegistrationHelpers @@ -10,8 +10,6 @@ { glm_source: 'some_source', glm_content: 'some_content' } end - let_it_be(:default_organization) { create(:organization, :default) } - before do stub_application_setting(require_admin_approval_after_user_signup: false) stub_application_setting(import_sources: %w[github gitlab_project]) -- GitLab From 28f5256d08cf58f6c4c87f031b6d732a0e8e386f Mon Sep 17 00:00:00 2001 From: Abdul Wadood Date: Mon, 2 Sep 2024 10:50:56 +0530 Subject: [PATCH 7/7] Use with_default_organization instead of with_current_organization As the user by default has access to the default organization. --- .../start_trial_from_external_site_without_confirmation_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb b/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb index 1e2eefef2ed0d4..6b9c5a85277e91 100644 --- a/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb +++ b/ee/spec/features/registrations/start_trial_from_external_site_without_confirmation_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Start trial from external site without confirmation', :with_current_organization, :saas, :js, feature_category: :onboarding do +RSpec.describe 'Start trial from external site without confirmation', :with_default_organization, :saas, :js, feature_category: :onboarding do include SaasRegistrationHelpers let_it_be(:glm_params) do -- GitLab