From 7bb84e64979edda8e76f077bd58aeb35857aec23 Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 6 May 2016 17:59:45 -0300 Subject: [PATCH] Change landing page when skipping confirmation email and add documentation --- app/controllers/registrations_controller.rb | 4 +- .../application_settings/_form.html.haml | 4 +- ...firmation_email_to_application_settings.rb | 2 +- doc/security/README.md | 1 + doc/security/user_email_confirmation.md | 7 +++ .../registrations_controller_spec.rb | 2 +- spec/features/signup_spec.rb | 45 ++++++++++++++----- 7 files changed, 48 insertions(+), 17 deletions(-) create mode 100644 doc/security/user_email_confirmation.md diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 352bff19383..26eb15f49e4 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -37,8 +37,8 @@ class RegistrationsController < Devise::RegistrationsController super end - def after_sign_up_path_for(_resource) - users_almost_there_path + def after_sign_up_path_for(user) + user.confirmed_at.present? ? dashboard_projects_path : users_almost_there_path end def after_inactive_sign_up_path_for(_resource) diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 289dda9a436..df286852b97 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -106,9 +106,9 @@ .form-group .col-sm-offset-2.col-sm-10 .checkbox - = f.label :skip_confirmation_email do + = f.label :send_user_confirmation_email do = f.check_box :send_user_confirmation_email - Send sign-up email confirmation + Send confirmation email on sign-up .form-group .col-sm-offset-2.col-sm-10 .checkbox diff --git a/db/migrate/20160421141709_add_send_confirmation_email_to_application_settings.rb b/db/migrate/20160421141709_add_send_confirmation_email_to_application_settings.rb index 0fef2a831e7..f92ef960509 100644 --- a/db/migrate/20160421141709_add_send_confirmation_email_to_application_settings.rb +++ b/db/migrate/20160421141709_add_send_confirmation_email_to_application_settings.rb @@ -3,7 +3,7 @@ class AddSendConfirmationEmailToApplicationSettings < ActiveRecord::Migration add_column :application_settings, :send_user_confirmation_email, :boolean, default: false #Sets confirmation email to true by default on existing installations. - ApplicationSetting.update_all(send_user_confirmation_email: true) + execute "UPDATE application_settings SET send_user_confirmation_email=true" end def down diff --git a/doc/security/README.md b/doc/security/README.md index 4cd0fdd4094..38706e48ec5 100644 --- a/doc/security/README.md +++ b/doc/security/README.md @@ -8,3 +8,4 @@ - [User File Uploads](user_file_uploads.md) - [How we manage the CRIME vulnerability](crime_vulnerability.md) - [Enforce Two-factor authentication](two_factor_authentication.md) +- [Send email confirmation on sign-up](user_email_confirmation.md) diff --git a/doc/security/user_email_confirmation.md b/doc/security/user_email_confirmation.md new file mode 100644 index 00000000000..4293944ae8b --- /dev/null +++ b/doc/security/user_email_confirmation.md @@ -0,0 +1,7 @@ +# User email confirmation at sign-up + +Gitlab admin can enable email confirmation on sign-up, if you want to confirm all +user emails before they are able to sign-in. + +In the Admin area under **Settings** (`/admin/application_settings`), go to section +**Sign-in Restrictions** and look for **Send confirmation email on sign-up** option. diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index 29f1847d9a1..df70a589a89 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -16,7 +16,7 @@ describe RegistrationsController do it 'logs user in directly' do post(:create, user_params) expect(ActionMailer::Base.deliveries.last).to be_nil - expect(subject.current_user).to be + expect(subject.current_user).to_not be_nil end end diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index c7840f26d8f..4229e82b443 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -2,22 +2,45 @@ require 'spec_helper' feature 'Signup', feature: true do describe 'signup with no errors' do - before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } - it 'creates the user account and sends a confirmation email' do - user = build(:user) + context "when sending confirmation email" do + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } - visit root_path + it 'creates the user account and sends a confirmation email' do + user = build(:user) - fill_in 'new_user_name', with: user.name - fill_in 'new_user_username', with: user.username - fill_in 'new_user_email', with: user.email - fill_in 'new_user_password', with: user.password - click_button "Sign up" + visit root_path + + fill_in 'new_user_name', with: user.name + fill_in 'new_user_username', with: user.username + fill_in 'new_user_email', with: user.email + fill_in 'new_user_password', with: user.password + click_button "Sign up" - expect(current_path).to eq users_almost_there_path - expect(page).to have_content("Please check your email to confirm your account") + expect(current_path).to eq users_almost_there_path + expect(page).to have_content("Please check your email to confirm your account") + end end + + context "when not sending confirmation email" do + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(false) } + + it 'creates the user account and goes to dashboard' do + user = build(:user) + + visit root_path + + fill_in 'new_user_name', with: user.name + fill_in 'new_user_username', with: user.username + fill_in 'new_user_email', with: user.email + fill_in 'new_user_password', with: user.password + click_button "Sign up" + + expect(current_path).to eq dashboard_projects_path + expect(page).to have_content("Welcome! You have signed up successfully.") + end + end + end describe 'signup with errors' do -- GitLab