Merge branch 'issue_14684' into 'master'

Toggle email signup confirmation in admin settings

Implements toggling verification email #14684

See merge request !3862

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

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: []

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)

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

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

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

db/migrate/20160516174813_add_send_user_confirmation_email_to_application_settings.rb

+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

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)

doc/security/user_email_confirmation.md

+# 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.

spec/controllers/registrations_controller_spec.rb

+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

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
-      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")
+    context "when sending confirmation email" 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)
+
+        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")
+      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

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