summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/concerns/confirm_email_warning.rb2
-rw-r--r--app/controllers/confirmations_controller.rb2
-rw-r--r--app/controllers/registrations_controller.rb26
-rw-r--r--app/models/user.rb7
-rw-r--r--changelogs/unreleased/security-soft-email-confirmation-revert.yml5
-rw-r--r--spec/controllers/concerns/confirm_email_warning_spec.rb4
-rw-r--r--spec/controllers/registrations_controller_spec.rb18
-rw-r--r--spec/features/invites_spec.rb51
-rw-r--r--spec/features/users/login_spec.rb1
-rw-r--r--spec/features/users/signup_spec.rb68
10 files changed, 145 insertions, 39 deletions
diff --git a/app/controllers/concerns/confirm_email_warning.rb b/app/controllers/concerns/confirm_email_warning.rb
index f1c0bcd491d..32e1a46e580 100644
--- a/app/controllers/concerns/confirm_email_warning.rb
+++ b/app/controllers/concerns/confirm_email_warning.rb
@@ -10,7 +10,7 @@ module ConfirmEmailWarning
protected
def show_confirm_warning?
- html_request? && request.get?
+ html_request? && request.get? && Feature.enabled?(:soft_email_confirmation)
end
def set_confirm_warning
diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb
index f99345fa99d..a27c4027380 100644
--- a/app/controllers/confirmations_controller.rb
+++ b/app/controllers/confirmations_controller.rb
@@ -11,6 +11,8 @@ class ConfirmationsController < Devise::ConfirmationsController
protected
def after_resending_confirmation_instructions_path_for(resource)
+ return users_almost_there_path unless Feature.enabled?(:soft_email_confirmation)
+
stored_location_for(resource) || dashboard_projects_path
end
diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb
index 1c6cbf72cfa..06751dfbec8 100644
--- a/app/controllers/registrations_controller.rb
+++ b/app/controllers/registrations_controller.rb
@@ -54,7 +54,7 @@ class RegistrationsController < Devise::RegistrationsController
def welcome
return redirect_to new_user_registration_path unless current_user
- return redirect_to stored_location_or_dashboard(current_user) if current_user.role.present? && !current_user.setup_for_company.nil?
+ return redirect_to path_for_signed_in_user(current_user) if current_user.role.present? && !current_user.setup_for_company.nil?
end
def update_registration
@@ -64,7 +64,7 @@ class RegistrationsController < Devise::RegistrationsController
if result[:status] == :success
track_experiment_event(:signup_flow, 'end') # We want this event to be tracked when the user is _in_ the experimental group
set_flash_message! :notice, :signed_up
- redirect_to stored_location_or_dashboard(current_user)
+ redirect_to path_for_signed_in_user(current_user)
else
render :welcome
end
@@ -111,14 +111,12 @@ class RegistrationsController < Devise::RegistrationsController
return users_sign_up_welcome_path if experiment_enabled?(:signup_flow)
- stored_location_or_dashboard(user)
+ path_for_signed_in_user(user)
end
def after_inactive_sign_up_path_for(resource)
- # With the current `allow_unconfirmed_access_for` Devise setting in config/initializers/8_devise.rb,
- # this method is never called. Leaving this here in case that value is set to 0.
Gitlab::AppLogger.info(user_created_message)
- users_almost_there_path
+ Feature.enabled?(:soft_email_confirmation) ? dashboard_projects_path : users_almost_there_path
end
private
@@ -180,8 +178,20 @@ class RegistrationsController < Devise::RegistrationsController
Gitlab::Utils.to_boolean(params[:terms_opt_in])
end
- def stored_location_or_dashboard(user)
- stored_location_for(user) || dashboard_projects_path
+ def path_for_signed_in_user(user)
+ if requires_confirmation?(user)
+ users_almost_there_path
+ else
+ stored_location_for(user) || dashboard_projects_path
+ end
+ end
+
+ def requires_confirmation?(user)
+ return false if user.confirmed?
+ return false if Feature.enabled?(:soft_email_confirmation)
+ return false if experiment_enabled?(:signup_flow)
+
+ true
end
def load_recaptcha
diff --git a/app/models/user.rb b/app/models/user.rb
index ec9bc7ae01e..3051df822d2 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1670,6 +1670,13 @@ class User < ApplicationRecord
super
end
+ # override from Devise::Confirmable
+ def confirmation_period_valid?
+ return false if Feature.disabled?(:soft_email_confirmation)
+
+ super
+ end
+
private
def default_private_profile_to_false
diff --git a/changelogs/unreleased/security-soft-email-confirmation-revert.yml b/changelogs/unreleased/security-soft-email-confirmation-revert.yml
new file mode 100644
index 00000000000..2f33bc4d212
--- /dev/null
+++ b/changelogs/unreleased/security-soft-email-confirmation-revert.yml
@@ -0,0 +1,5 @@
+---
+title: Do not enable soft email confirmation by default
+merge_request:
+author:
+type: security
diff --git a/spec/controllers/concerns/confirm_email_warning_spec.rb b/spec/controllers/concerns/confirm_email_warning_spec.rb
index 2aad380203b..93e3423261c 100644
--- a/spec/controllers/concerns/confirm_email_warning_spec.rb
+++ b/spec/controllers/concerns/confirm_email_warning_spec.rb
@@ -3,6 +3,10 @@
require 'spec_helper'
describe ConfirmEmailWarning do
+ before do
+ stub_feature_flags(soft_email_confirmation: true)
+ end
+
controller(ApplicationController) do
# `described_class` is not available in this context
include ConfirmEmailWarning
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index 8d79e505e5d..d7fe3e87056 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -79,29 +79,31 @@ describe RegistrationsController do
stub_application_setting(send_user_confirmation_email: true)
end
- context 'when a grace period is active for confirming the email address' do
+ context 'when soft email confirmation is not enabled' do
before do
- allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
+ stub_feature_flags(soft_email_confirmation: false)
+ allow(User).to receive(:allow_unconfirmed_access_for).and_return 0
end
- it 'sends a confirmation email and redirects to the dashboard' do
+ it 'does not authenticate the user and sends a confirmation email' do
post(:create, params: user_params)
expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email])
- expect(response).to redirect_to(dashboard_projects_path)
+ expect(subject.current_user).to be_nil
end
end
- context 'when no grace period is active for confirming the email address' do
+ context 'when soft email confirmation is enabled' do
before do
- allow(User).to receive(:allow_unconfirmed_access_for).and_return 0
+ stub_feature_flags(soft_email_confirmation: true)
+ allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
end
- it 'sends a confirmation email and redirects to the almost there page' do
+ it 'authenticates the user and sends a confirmation email' do
post(:create, params: user_params)
expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email])
- expect(response).to redirect_to(users_almost_there_path)
+ expect(response).to redirect_to(dashboard_projects_path)
end
end
end
diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb
index 7884a16c118..9cd01894575 100644
--- a/spec/features/invites_spec.rb
+++ b/spec/features/invites_spec.rb
@@ -135,7 +135,9 @@ describe 'Invites' do
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
@@ -153,6 +155,25 @@ describe 'Invites' do
context 'email confirmation enabled' do
let(:send_email_confirmation) { true }
+ context 'when soft email confirmation is not enabled' do
+ before do
+ allow(User).to receive(:allow_unconfirmed_access_for).and_return 0
+ end
+
+ 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(new_user)
+ fill_in_sign_in_form(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
+ end
+
context 'when soft email confirmation is enabled' do
before do
allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
@@ -164,7 +185,9 @@ describe 'Invites' do
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
end
@@ -180,14 +203,32 @@ describe 'Invites' do
context 'the user sign-up using a different email address' do
let(:invite_email) { build_stubbed(:user).email }
- before do
- allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
+ context 'when soft email confirmation is not enabled' do
+ before do
+ stub_feature_flags(soft_email_confirmation: false)
+ allow(User).to receive(:allow_unconfirmed_access_for).and_return 0
+ end
+
+ it 'signs up and redirects to the invitation page' do
+ fill_in_sign_up_form(new_user)
+ confirm_email(new_user)
+ fill_in_sign_in_form(new_user)
+
+ expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
+ end
end
- it 'signs up and redirects to the invitation page' do
- fill_in_sign_up_form(new_user)
+ context 'when soft email confirmation is enabled' do
+ before do
+ stub_feature_flags(soft_email_confirmation: true)
+ allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
+ end
- expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
+ 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
end
diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb
index 0bef61a4854..5a8db3c070d 100644
--- a/spec/features/users/login_spec.rb
+++ b/spec/features/users/login_spec.rb
@@ -797,6 +797,7 @@ describe 'Login' do
before do
stub_application_setting(send_user_confirmation_email: true)
+ stub_feature_flags(soft_email_confirmation: true)
allow(User).to receive(:allow_unconfirmed_access_for).and_return grace_period
end
diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb
index 8d5c0657fa5..daa987ea389 100644
--- a/spec/features/users/signup_spec.rb
+++ b/spec/features/users/signup_spec.rb
@@ -129,29 +129,63 @@ shared_examples 'Signup' do
stub_application_setting(send_user_confirmation_email: true)
end
- it 'creates the user account and sends a confirmation email' do
- visit new_user_registration_path
+ context 'when soft email confirmation is not enabled' do
+ before do
+ stub_feature_flags(soft_email_confirmation: false)
+ end
- fill_in 'new_user_username', with: new_user.username
- fill_in 'new_user_email', with: new_user.email
+ it 'creates the user account and sends a confirmation email' do
+ visit new_user_registration_path
- if Gitlab::Experimentation.enabled?(:signup_flow)
- fill_in 'new_user_first_name', with: new_user.first_name
- fill_in 'new_user_last_name', with: new_user.last_name
- else
- fill_in 'new_user_name', with: new_user.name
- fill_in 'new_user_email_confirmation', with: new_user.email
+ fill_in 'new_user_username', with: new_user.username
+ fill_in 'new_user_email', with: new_user.email
+
+ if Gitlab::Experimentation.enabled?(:signup_flow)
+ fill_in 'new_user_first_name', with: new_user.first_name
+ fill_in 'new_user_last_name', with: new_user.last_name
+ else
+ fill_in 'new_user_name', with: new_user.name
+ fill_in 'new_user_email_confirmation', with: new_user.email
+ end
+
+ fill_in 'new_user_password', with: new_user.password
+
+ expect { click_button 'Register' }.to change { User.count }.by(1)
+
+ expect(current_path).to eq users_almost_there_path
+ expect(page).to have_content('Please check your email to confirm your account')
end
+ end
- fill_in 'new_user_password', with: new_user.password
+ context 'when soft email confirmation is enabled' do
+ before do
+ stub_feature_flags(soft_email_confirmation: true)
+ end
- expect { click_button 'Register' }.to change { User.count }.by(1)
+ it 'creates the user account and sends a confirmation email' do
+ visit new_user_registration_path
- if Gitlab::Experimentation.enabled?(:signup_flow)
- expect(current_path).to eq users_sign_up_welcome_path
- else
- expect(current_path).to eq dashboard_projects_path
- expect(page).to have_content("Please check your email (#{new_user.email}) to verify that you own this address and unlock the power of CI/CD.")
+ fill_in 'new_user_username', with: new_user.username
+ fill_in 'new_user_email', with: new_user.email
+
+ if Gitlab::Experimentation.enabled?(:signup_flow)
+ fill_in 'new_user_first_name', with: new_user.first_name
+ fill_in 'new_user_last_name', with: new_user.last_name
+ else
+ fill_in 'new_user_name', with: new_user.name
+ fill_in 'new_user_email_confirmation', with: new_user.email
+ end
+
+ fill_in 'new_user_password', with: new_user.password
+
+ expect { click_button 'Register' }.to change { User.count }.by(1)
+
+ if Gitlab::Experimentation.enabled?(:signup_flow)
+ expect(current_path).to eq users_sign_up_welcome_path
+ else
+ expect(current_path).to eq dashboard_projects_path
+ expect(page).to have_content("Please check your email (#{new_user.email}) to verify that you own this address and unlock the power of CI/CD.")
+ end
end
end
end
View Lorry Controller Status