Skip to content
Snippets Groups Projects
Commit 33803656 authored by Doug Stull's avatar Doug Stull :two:
Browse files

Merge branch '411858-remove-role-required-redirect' into 'master'

Remove role required before action

See merge request !129519



Merged-by: Doug Stull's avatarDoug Stull <dstull@gitlab.com>
Approved-by: default avatarEugie Limpin <elimpin@gitlab.com>
Reviewed-by: Doug Stull's avatarDoug Stull <dstull@gitlab.com>
parents 13941537 7cfac1cd
No related branches found
No related tags found
No related merge requests found
Pipeline #983638778 passed
Pipeline: E2E GDK

#983663537

    Pipeline: GitLab

    #983646443

      Showing
      with 13 additions and 131 deletions
      ......@@ -38,7 +38,6 @@ class ApplicationController < ActionController::Base
      before_action :active_user_check, unless: :devise_controller?
      before_action :set_usage_stats_consent_flag
      before_action :check_impersonation_availability
      before_action :required_signup_info
      # Make sure the `auth_user` is memoized so it can be logged, we do this after
      # all other before filters that could have set the user.
      ......@@ -555,15 +554,6 @@ def allow_gitaly_ref_name_caching
      def context_user
      auth_user if strong_memoized?(:auth_user)
      end
      def required_signup_info
      return unless current_user
      return unless current_user.role_required?
      store_location_for :user, request.fullpath
      redirect_to users_sign_up_welcome_path
      end
      end
      ApplicationController.prepend_mod
      ......@@ -9,7 +9,6 @@ module VerifiesWithEmail
      included do
      prepend_before_action :verify_with_email, only: :create, unless: -> { skip_verify_with_email? }
      skip_before_action :required_signup_info, only: :successful_verification
      end
      # rubocop:disable Metrics/PerceivedComplexity
      ......
      ......@@ -7,7 +7,6 @@ class ConfirmationsController < Devise::ConfirmationsController
      include GoogleAnalyticsCSP
      include GoogleSyndicationCSP
      skip_before_action :required_signup_info
      prepend_before_action :check_recaptcha, only: :create
      before_action :load_recaptcha, only: :new
      ......
      ......@@ -6,7 +6,7 @@ class PwaController < ApplicationController # rubocop:disable Gitlab/NamespacedC
      feature_category :navigation
      urgency :low
      skip_before_action :authenticate_user!, :required_signup_info
      skip_before_action :authenticate_user!
      def manifest
      end
      ......
      ......@@ -8,7 +8,7 @@ class WelcomeController < ApplicationController
      include ::Gitlab::Utils::StrongMemoize
      layout 'minimal'
      skip_before_action :required_signup_info, :check_two_factor_requirement
      skip_before_action :check_two_factor_requirement
      helper_method :welcome_update_params
      helper_method :onboarding_status
      ......@@ -43,7 +43,7 @@ def authenticate_user!
      end
      def completed_welcome_step?
      current_user.role.present? && !current_user.setup_for_company.nil?
      !current_user.setup_for_company.nil?
      end
      def update_params
      ......
      ......@@ -47,7 +47,6 @@ def create
      accept_pending_invitations if new_user.persisted?
      persist_accepted_terms_if_required(new_user)
      set_role_required(new_user)
      send_custom_confirmation_instructions
      track_weak_password_error(new_user, self.class.name, 'create')
      ......@@ -90,10 +89,6 @@ def persist_accepted_terms_if_required(new_user)
      Users::RespondToTermsService.new(new_user, terms).execute(accepted: true)
      end
      def set_role_required(new_user)
      new_user.set_role_required! if new_user.persisted?
      end
      def destroy_confirmation_valid?
      if current_user.confirm_deletion_with_password?
      current_user.valid_password?(params[:password])
      ......
      ......@@ -2085,16 +2085,6 @@ def last_active_at
      [last_activity, last_sign_in].compact.max
      end
      REQUIRES_ROLE_VALUE = 99
      def role_required?
      role_before_type_cast == REQUIRES_ROLE_VALUE
      end
      def set_role_required!
      update_column(:role, REQUIRES_ROLE_VALUE)
      end
      def dismissed_callout?(feature_name:, ignore_dismissal_earlier_than: nil)
      callout = callouts_by_feature_name[feature_name]
      ......
      ......@@ -20,7 +20,7 @@
      ensure_onboarding_is_finished
      end
      it 'registers the user with multiple invites and sends them to the root page' do
      it 'registers the user with multiple invites and sends them to the last group activity page' do
      new_user = build(:user, name: 'Registering User', email: user_email)
      group = create(:group, name: 'Test Group')
      ......@@ -40,7 +40,7 @@
      fill_in_welcome_form
      click_on 'Get started!'
      expect(page).to have_current_path(root_path)
      expect(page).to have_current_path(activity_group_path(group), ignore_query: true)
      ensure_onboarding_is_finished
      end
      ......
      ......@@ -20,9 +20,9 @@ def fill_in_signup_form
      wait_for_all_requests
      end
      context 'for Gitlab.com' do
      context 'for SaaS', :saas do
      before do
      expect(Gitlab).to receive(:com?).and_return(true).at_least(:once)
      stub_ee_application_setting(should_check_namespace_plan: true)
      visit new_user_registration_path
      end
      ......@@ -81,7 +81,7 @@ def fill_in_signup_form
      end
      end
      it 'redirects to step 2 of the signup process when not completed' do
      it 'allows visiting of a page after initial registration' do
      fill_in_signup_form
      click_button 'Register'
      visit new_project_path
      ......@@ -99,9 +99,8 @@ def fill_in_signup_form
      end
      end
      context 'not for Gitlab.com' do
      context 'when not for SaaS' do
      before do
      expect(Gitlab).to receive(:com?).and_return(false).at_least(:once)
      visit new_user_registration_path
      end
      ......@@ -117,19 +116,6 @@ def fill_in_signup_form
      expect(user.email_opted_in_source).to be_blank
      expect(user.email_opted_in_at).to be_nil
      end
      it 'redirects to step 2 of the signup process when not completed and redirects back' do
      fill_in_signup_form
      click_button 'Register'
      visit new_project_path
      expect(page).to have_current_path(users_sign_up_welcome_path)
      select 'Software Developer', from: 'user_role'
      click_button 'Get started!'
      expect(page).to have_current_path(new_project_path)
      end
      end
      describe 'password complexity', :js do
      ......
      ......@@ -865,33 +865,6 @@ def index
      end
      end
      describe '#required_signup_info' do
      controller(described_class) do
      def index; end
      end
      let(:user) { create(:user) }
      context 'user with required role' do
      before do
      user.set_role_required!
      sign_in(user)
      get :index
      end
      it { is_expected.to redirect_to users_sign_up_welcome_path }
      end
      context 'user without a required role' do
      before do
      sign_in(user)
      get :index
      end
      it { is_expected.not_to redirect_to users_sign_up_welcome_path }
      end
      end
      describe 'rescue_from Gitlab::Auth::IpBlocked' do
      controller(described_class) do
      skip_before_action :authenticate_user!
      ......
      ......@@ -19,17 +19,6 @@ def perform_request
      get :show, params: { confirmation_token: confirmation_token }
      end
      context 'when signup info is required' do
      before do
      allow(controller).to receive(:current_user) { user }
      user.set_role_required!
      end
      it 'does not redirect' do
      expect(perform_request).not_to redirect_to(users_sign_up_welcome_path)
      end
      end
      context 'user is already confirmed' do
      before do
      user.confirm
      ......@@ -137,17 +126,6 @@ def perform_request
      stub_feature_flags(identity_verification: false)
      end
      context 'when signup info is required' do
      before do
      allow(controller).to receive(:current_user) { user }
      user.set_role_required!
      end
      it 'does not redirect' do
      expect(perform_request).not_to redirect_to(users_sign_up_welcome_path)
      end
      end
      context "when `email_confirmation_setting` is set to `soft`" do
      before do
      stub_application_setting_enum('email_confirmation_setting', 'soft')
      ......
      ......@@ -12,21 +12,12 @@
      it { is_expected.to redirect_to new_user_registration_path }
      end
      context 'when role or setup_for_company is not set' do
      context 'when setup_for_company is not set' do
      before do
      sign_in(user)
      end
      it { is_expected.to render_template(:show) }
      end
      context 'when role is required and setup_for_company is not set' do
      before do
      user.set_role_required!
      sign_in(user)
      end
      it { is_expected.to render_template(:show) }
      render_views
      ......@@ -37,7 +28,7 @@
      end
      end
      context 'when role and setup_for_company is set' do
      context 'when setup_for_company is set' do
      before do
      user.update!(setup_for_company: false)
      sign_in(user)
      ......@@ -46,15 +37,6 @@
      it { is_expected.to redirect_to(dashboard_projects_path) }
      end
      context 'when role is set and setup_for_company is not set' do
      before do
      user.update!(role: :software_developer)
      sign_in(user)
      end
      it { is_expected.to render_template(:show) }
      end
      context 'when 2FA is required from group' do
      before do
      user = create(:user, require_two_factor_authentication_from_group: true)
      ......
      ......@@ -332,7 +332,6 @@ def confirm_email
      click_button 'Register'
      expect(page).to have_current_path(users_sign_up_welcome_path), ignore_query: true
      visit new_project_path
      select 'Software Developer', from: 'user_role'
      click_button 'Get started!'
      ......@@ -341,7 +340,7 @@ def confirm_email
      expect(created_user.software_developer_role?).to be_truthy
      expect(created_user.setup_for_company).to be_nil
      expect(page).to have_current_path(new_project_path)
      expect(page).to have_current_path(dashboard_projects_path)
      end
      it_behaves_like 'Signup name validation', 'new_user_first_name', 127, 'First name'
      ......@@ -388,7 +387,7 @@ def confirm_email
      end
      end
      it 'redirects to step 2 of the signup process, sets the role and redirects back' do
      it 'allows visiting of a page after initial registration' do
      visit new_user_registration_path
      fill_in_signup_form
      ......@@ -397,15 +396,6 @@ def confirm_email
      visit new_project_path
      expect(page).to have_current_path(users_sign_up_welcome_path)
      select 'Software Developer', from: 'user_role'
      click_button 'Get started!'
      created_user = User.find_by_username(new_user.username)
      expect(created_user.software_developer_role?).to be_truthy
      expect(created_user.setup_for_company).to be_nil
      expect(page).to have_current_path(new_project_path)
      end
      ......
      0% Loading or .
      You are about to add 0 people to the discussion. Proceed with caution.
      Finish editing this message first!
      Please register or to comment