diff --git a/ee/app/controllers/ee/omniauth_callbacks_controller.rb b/ee/app/controllers/ee/omniauth_callbacks_controller.rb
index 09c5b05eba161fbbed0cef63f25cc9bc421c10dc..9691f33bd0d5a7ab99ea314333f554f4bb51f8e2 100644
--- a/ee/app/controllers/ee/omniauth_callbacks_controller.rb
+++ b/ee/app/controllers/ee/omniauth_callbacks_controller.rb
@@ -40,7 +40,11 @@ def after_sign_up_path
override :perform_registration_tasks
def perform_registration_tasks(user, provider)
- super
+ # We need to do this here since the subscription flow relies on what was set in the stored_location_for(:user)
+ # that was set on initial redirect from the SubscriptionsController#new and super will wipe that out.
+ # Then the IdentityVerificationController#success will get whatever is set in super instead of the subscription
+ # path we desire.
+ super unless helpers.in_subscription_flow?
# This also protects the sub classes group saml and ldap from staring onboarding
# as we don't want those to onboard.
diff --git a/ee/app/controllers/subscriptions_controller.rb b/ee/app/controllers/subscriptions_controller.rb
index 3962b2a05e359897967b4744f46d6ea0e8570280..b1c9325ba7492b5a61c882f0277874900db46ed5 100644
--- a/ee/app/controllers/subscriptions_controller.rb
+++ b/ee/app/controllers/subscriptions_controller.rb
@@ -37,7 +37,7 @@ def new
end
else
store_location_for(:user, request.fullpath)
- redirect_to new_user_registration_path(redirect_from: 'checkout')
+ redirect_to new_user_registration_path
end
end
diff --git a/ee/app/controllers/users/identity_verification_controller.rb b/ee/app/controllers/users/identity_verification_controller.rb
index 5eefd9083bdb8521e89a2d6bb920b59830f5a491..37b2da6188fcbe4a988f3aa4d90a4b85762c1fcd 100644
--- a/ee/app/controllers/users/identity_verification_controller.rb
+++ b/ee/app/controllers/users/identity_verification_controller.rb
@@ -83,7 +83,7 @@ def success
sign_in(@user)
session.delete(:verification_user_id)
- @redirect_url = after_sign_in_path_for(@user)
+ set_redirect_url
render 'devise/sessions/successful_verification'
end
@@ -111,6 +111,16 @@ def verify_credit_card
private
+ def set_redirect_url
+ @redirect_url = if helpers.in_subscription_flow?
+ # Since we need this value to stay in the stored_location_for(user) in order for
+ # us to be properly redirected for subscription signups.
+ session['user_return_to']
+ else
+ after_sign_in_path_for(@user)
+ end
+ end
+
def require_verification_user!
if verification_user_id = session[:verification_user_id]
User.sticking.stick_or_unstick_request(request.env, :user, verification_user_id)
diff --git a/ee/spec/controllers/subscriptions_controller_spec.rb b/ee/spec/controllers/subscriptions_controller_spec.rb
index 401ea2c05caeb8b8e7a029f28060e2f350eefe82..fb6f3e31da097afa29c70e905c1525d2bb42bfe9 100644
--- a/ee/spec/controllers/subscriptions_controller_spec.rb
+++ b/ee/spec/controllers/subscriptions_controller_spec.rb
@@ -10,7 +10,7 @@
context 'for unauthenticated subscription request' do
it { is_expected.to have_gitlab_http_status(:redirect) }
- it { is_expected.to redirect_to new_user_registration_path(redirect_from: 'checkout') }
+ it { is_expected.to redirect_to new_user_registration_path }
it 'stores subscription URL for later' do
get_new
diff --git a/ee/spec/features/registrations/saas/subscription_flow_company_paid_plan_spec.rb b/ee/spec/features/registrations/saas/subscription_flow_company_paid_plan_spec.rb
index 6b5dc9d8470e7e13f5a8428adc3951195215e0ac..0ed48d1cbd9890e51c4bc26d90b54d1930701b95 100644
--- a/ee/spec/features/registrations/saas/subscription_flow_company_paid_plan_spec.rb
+++ b/ee/spec/features/registrations/saas/subscription_flow_company_paid_plan_spec.rb
@@ -5,9 +5,8 @@
RSpec.describe 'Subscription flow for user picking company for paid plan', :js, :saas_registration, feature_category: :onboarding do
where(:case_name, :sign_up_method) do
[
- ['with regular sign up', -> { subscription_regular_sign_up }]
- # TODO: does not work, issue to address: https://gitlab.com/gitlab-org/gitlab/-/issues/413995
- # ['with sso sign up', -> { sso_subscription_sign_up }]
+ ['with regular sign up', -> { subscription_regular_sign_up }],
+ ['with sso sign up', -> { sso_subscription_sign_up }]
]
end
diff --git a/ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb b/ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb
index 940dc9d692dc4ed13872748c976068cac5ff2ec1..dcb6398d1fcb4969f92acbbc8c9c61fda1a06c35 100644
--- a/ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb
+++ b/ee/spec/features/registrations/saas/subscription_flow_just_me_paid_plan_spec.rb
@@ -5,9 +5,8 @@
RSpec.describe 'Subscription flow for user picking just me for paid plan', :js, :saas_registration, feature_category: :onboarding do
where(:case_name, :sign_up_method) do
[
- ['with regular sign up', -> { subscription_regular_sign_up }]
- # TODO: does not work, issue to address: https://gitlab.com/gitlab-org/gitlab/-/issues/413995
- # ['with sso sign up', -> { sso_subscription_sign_up }]
+ ['with regular sign up', -> { subscription_regular_sign_up }],
+ ['with sso sign up', -> { sso_subscription_sign_up }]
]
end
diff --git a/ee/spec/requests/ee/omniauth_callbacks_controller_spec.rb b/ee/spec/requests/ee/omniauth_callbacks_controller_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..5db7e1158c83fbb65ae011d043d74d518607d103
--- /dev/null
+++ b/ee/spec/requests/ee/omniauth_callbacks_controller_spec.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe OmniauthCallbacksController, feature_category: :system_access do
+ include LoginHelpers
+ include SessionHelpers
+
+ context 'with strategies', :aggregate_failures do
+ let(:provider) { :github }
+ let(:check_namespace_plan) { true }
+
+ before do
+ stub_feature_flags(ensure_onboarding: true)
+ stub_omniauth_setting(block_auto_created_users: false)
+ mock_auth_hash(provider.to_s, 'my-uid', user.email)
+ end
+
+ around do |example|
+ with_omniauth_full_host { example.run }
+ end
+
+ context 'when user is not registered yet', :clean_gitlab_redis_sessions do
+ let(:user) { build_stubbed(:user, email: 'new@example.com') }
+ let(:path) { '/user/return/to/path' }
+
+ before do
+ stub_session(user_return_to: path)
+ end
+
+ it 'wipes the previously stored location for user' do
+ expect_next_instance_of(described_class) do |controller|
+ expect(controller).to receive(:store_location_for).with(:user, users_sign_up_welcome_path)
+ end
+
+ post public_send("user_#{provider}_omniauth_callback_path")
+
+ expect(request.env['warden']).to be_authenticated
+ end
+
+ context 'when user is in subscription onboarding' do
+ let(:path) { new_subscriptions_path(plan_id: 'bronze_id') }
+
+ it 'preserves the previously stored location for user' do
+ expect_next_instance_of(described_class) do |controller|
+ expect(controller).not_to receive(:store_location_for).with(:user)
+ end
+
+ post public_send("user_#{provider}_omniauth_callback_path")
+
+ expect(request.env['warden']).to be_authenticated
+ end
+ end
+ end
+ end
+end
diff --git a/ee/spec/requests/users/identity_verification_controller_spec.rb b/ee/spec/requests/users/identity_verification_controller_spec.rb
index a2753734809a1571a941aca8796e4d6c906abcce..2744b2ec4d735de01282f8390a5859bdd1332a62 100644
--- a/ee/spec/requests/users/identity_verification_controller_spec.rb
+++ b/ee/spec/requests/users/identity_verification_controller_spec.rb
@@ -462,15 +462,11 @@
end
describe 'GET success' do
- let(:after_sign_in_path) { '/after/sign/in' }
+ let(:stored_user_return_to_path) { '/user/return/to/path' }
let(:user) { confirmed_user }
before do
- allow_next_instance_of(described_class) do |controller|
- allow(controller).to receive(:after_sign_in_path_for).and_return(after_sign_in_path)
- end
-
- stub_session(verification_user_id: user.id)
+ stub_session(verification_user_id: user.id, user_return_to: stored_user_return_to_path)
get success_identity_verification_path
end
@@ -493,7 +489,18 @@
it 'renders the template with the after_sign_in_path_for variable' do
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template('successful_verification', layout: 'minimal')
- expect(assigns(:redirect_url)).to eq(after_sign_in_path)
+ expect(assigns(:redirect_url)).to eq(stored_user_return_to_path)
+ expect(controller.stored_location_for(:user)).to be_nil
+ end
+
+ context 'when user is in subscription onboarding' do
+ let(:stored_user_return_to_path) { new_subscriptions_path(plan_id: 'bronze_id') }
+
+ it 'does not empty out the stored location for user' do
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(assigns(:redirect_url)).to eq(stored_user_return_to_path)
+ expect(controller.stored_location_for(:user)).to eq(stored_user_return_to_path)
+ end
end
end