diff --git a/CHANGELOG b/CHANGELOG index 4c31381855eaa2d12586a6497f6a92d1ae9f3f65..e1252d4b947c8b5b4cb1c46976c497e3ca8b3504 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 8.8.0 (unreleased) - Fix error when using link to uploads in global snippets - Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen) - Use a case-insensitive comparison in sanitizing URI schemes + - Toggle sign-up confirmation emails in application settings - Project#open_branches has been cleaned up and no longer loads entire records into memory. - Escape HTML in commit titles in system note messages - Improve multiple branch push performance by memoizing permission checking diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 8c973f0e4a882e9c0d782032984d8d4238b0ffde..ff7a5cad2fb9d3b0baf66e5d10722bd212cc25f5 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -106,6 +106,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :email_author_in_body, :repository_checks_enabled, :metrics_packet_size, + :send_user_confirmation_email, restricted_visibility_levels: [], import_sources: [], disabled_oauth_sign_in_sources: [] diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 352bff1938335734ba410866ad4089119be275b2..26eb15f49e41c88ce5ec11939cee650040188fe6 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/models/application_setting.rb b/app/models/application_setting.rb index 1a10768655fb33bd8e1f5539146e491e500906e5..f5079f92444ced909ec9de1c14ec9a747d2b4575 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -120,7 +120,8 @@ class ApplicationSetting < ActiveRecord::Base recaptcha_enabled: false, akismet_enabled: false, repository_checks_enabled: true, - disabled_oauth_sign_in_sources: [] + disabled_oauth_sign_in_sources: [], + send_user_confirmation_email: false ) end diff --git a/app/models/user.rb b/app/models/user.rb index 489bff3fa4a907963c2efe8ebad5ee5da12324e8..368a3f3cfbae5a6d9dffd8f265bf88272b13a110 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -112,6 +112,7 @@ class User < ActiveRecord::Base before_save :ensure_external_user_rights after_save :ensure_namespace_correct after_initialize :set_projects_limit + before_create :check_confirmation_email after_create :post_create_hook after_destroy :post_destroy_hook @@ -307,6 +308,10 @@ class User < ActiveRecord::Base @reset_token end + def check_confirmation_email + skip_confirmation! unless current_application_settings.send_user_confirmation_email + end + def recently_sent_password_reset? reset_password_sent_at.present? && reset_password_sent_at >= 1.minute.ago end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index f7c799c968f5e64b29df22b9ed1bb3f86745cf3d..df286852b97df2f650ce39fc804da6908f94b4a3 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -103,6 +103,12 @@ = f.label :signup_enabled do = f.check_box :signup_enabled Sign-up enabled + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :send_user_confirmation_email do + = f.check_box :send_user_confirmation_email + Send confirmation email on sign-up .form-group .col-sm-offset-2.col-sm-10 .checkbox diff --git a/db/migrate/20160516174813_add_send_user_confirmation_email_to_application_settings.rb b/db/migrate/20160516174813_add_send_user_confirmation_email_to_application_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..c34e7ba540951aabcc12826936501d8b4c4b559d --- /dev/null +++ b/db/migrate/20160516174813_add_send_user_confirmation_email_to_application_settings.rb @@ -0,0 +1,12 @@ +class AddSendUserConfirmationEmailToApplicationSettings < ActiveRecord::Migration + def up + add_column :application_settings, :send_user_confirmation_email, :boolean, default: false + + #Sets confirmation email to true by default on existing installations. + execute "UPDATE application_settings SET send_user_confirmation_email=true" + end + + def down + remove_column :application_settings, :send_user_confirmation_email + end +end diff --git a/doc/security/README.md b/doc/security/README.md index 4cd0fdd409443e6889a8232fc28cdd7603c61393..38706e48ec5713c7cbb4b45daafb26f8c0bd131b 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 0000000000000000000000000000000000000000..4293944ae8b75e83d0b0d92d934e49963d581faa --- /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 new file mode 100644 index 0000000000000000000000000000000000000000..df70a589a89cb7a7b620652d3720a0252b38410d --- /dev/null +++ b/spec/controllers/registrations_controller_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe RegistrationsController do + describe '#create' do + around(:each) do |example| + perform_enqueued_jobs do + example.run + end + end + + let(:user_params) { { user: { name: "new_user", username: "new_username", email: "new@user.com", password: "Any_password" } } } + + context 'when sending email confirmation' do + before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(false) } + + it 'logs user in directly' do + post(:create, user_params) + expect(ActionMailer::Base.deliveries.last).to be_nil + expect(subject.current_user).to_not be_nil + end + end + + context 'when not sending email confirmation' do + before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } + + it 'does not authenticate user and sends confirmation email' do + post(:create, user_params) + expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) + expect(subject.current_user).to be_nil + end + end + end +end diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index 58aabd913ebcb150780c11a7c8f9a906fd22b524..4229e82b4438c5c1aea00153079cf3ef32a5b613 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -2,20 +2,45 @@ require 'spec_helper' feature 'Signup', feature: true do describe 'signup with no errors' do - it 'creates the user account and sends a confirmation email' do - user = build(:user) - visit root_path + context "when sending confirmation email" do + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } - 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" + it 'creates the user account and sends a confirmation email' 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 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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 10e7e693571825376ca65860cbcc52432c03b7d9..9581990666b1a95d2bd8920c7e21b25855a4283d 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -141,6 +141,7 @@ describe User, models: true do end describe '#confirm' do + before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'test@gitlab.com') } it 'returns unconfirmed' do