Automatic merge of gitlab-org/gitlab-ce master

app/assets/stylesheets/framework/flash.scss

@@ -43,6 +43,7 @@
     @extend .alert;
     background-color: $orange-100;
     color: $orange-900;
+    cursor: default;
     margin: 0;
   }
 

app/controllers/application_controller.rb

@@ -12,6 +12,7 @@ class ApplicationController < ActionController::Base
   include EnforcesTwoFactorAuthentication
   include WithPerformanceBar
   include SessionlessAuthentication
+  include ConfirmEmailWarning
 
   before_action :authenticate_user!
   before_action :enforce_terms!, if: :should_enforce_terms?

app/controllers/concerns/confirm_email_warning.rb

+# frozen_string_literal: true
+
+module ConfirmEmailWarning
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :set_confirm_warning, if: -> { Feature.enabled?(:soft_email_confirmation) }
+  end
+
+  protected
+
+  def set_confirm_warning
+    return unless current_user
+    return if current_user.confirmed?
+    return if peek_request? || json_request? || !request.get?
+
+    email = current_user.unconfirmed_email || current_user.email
+
+    flash.now[:warning] = _("Please check your email (%{email}) to verify that you own this address. Didn't receive it? %{resend_link}. Wrong email address? %{update_link}.").html_safe % {
+      email: email,
+      resend_link: view_context.link_to(_('Resend it'), user_confirmation_path(user: { email: email }), method: :post),
+      update_link: view_context.link_to(_('Update it'), profile_path)
+    }
+  end
+end

app/controllers/confirmations_controller.rb

@@ -11,7 +11,7 @@ class ConfirmationsController < Devise::ConfirmationsController
   protected
 
   def after_resending_confirmation_instructions_path_for(resource)
-    users_almost_there_path
+    Feature.enabled?(:soft_email_confirmation) ? stored_location_for(resource) || dashboard_projects_path : users_almost_there_path
   end
 
   def after_confirmation_path_for(resource_name, resource)

app/controllers/registrations_controller.rb

@@ -69,12 +69,12 @@ class RegistrationsController < Devise::RegistrationsController
 
   def after_sign_up_path_for(user)
     Gitlab::AppLogger.info(user_created_message(confirmed: user.confirmed?))
-    user.confirmed? ? stored_location_for(user) || dashboard_projects_path : users_almost_there_path
+    confirmed_or_unconfirmed_access_allowed(user) ? stored_location_or_dashboard(user) : users_almost_there_path
   end
 
   def after_inactive_sign_up_path_for(resource)
     Gitlab::AppLogger.info(user_created_message)
-    users_almost_there_path
+    Feature.enabled?(:soft_email_confirmation) ? dashboard_projects_path : users_almost_there_path
   end
 
   private
@@ -135,6 +135,14 @@ class RegistrationsController < Devise::RegistrationsController
   def terms_accepted?
     Gitlab::Utils.to_boolean(params[:terms_opt_in])
   end
+
+  def confirmed_or_unconfirmed_access_allowed(user)
+    user.confirmed? || Feature.enabled?(:soft_email_confirmation)
+  end
+
+  def stored_location_or_dashboard(user)
+    stored_location_for(user) || dashboard_projects_path
+  end
 end
 
 RegistrationsController.prepend_if_ee('EE::RegistrationsController')

app/models/user.rb

@@ -1507,6 +1507,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

app/serializers/deployment_entity.rb

@@ -18,6 +18,7 @@ class DeploymentEntity < Grape::Entity
   end
 
   expose :created_at
+  expose :finished_at
   expose :tag
   expose :last?
   expose :user, using: UserEntity

app/serializers/deployment_serializer.rb

@@ -4,7 +4,7 @@ class DeploymentSerializer < BaseSerializer
   entity DeploymentEntity
 
   def represent_concise(resource, opts = {})
-    opts[:only] = [:iid, :id, :sha, :created_at, :tag, :last?, :id, ref: [:name]]
+    opts[:only] = [:iid, :id, :sha, :created_at, :finished_at, :tag, :last?, :id, ref: [:name]]
     represent(resource, opts)
   end
 end

changelogs/unreleased/56130-deployment-date.yml

+---
+title: Add finished_at to the internal API Deployment entity
+merge_request: 31808
+author:
+type: other

config/initializers/8_devise.rb

@@ -81,7 +81,7 @@ Devise.setup do |config|
   # You can use this to let your user access some features of your application
   # without confirming the account, but blocking it after a certain period
   # (ie 2 days).
-  # config.allow_unconfirmed_access_for = 2.days
+  config.allow_unconfirmed_access_for = 30.days
 
   # Defines which key will be used when confirming an account
   # config.confirmation_keys = [ :email ]

spec/controllers/concerns/confirm_email_warning_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ConfirmEmailWarning do
+  before do
+    stub_feature_flags(soft_email_confirmation: true)
+    allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
+  end
+
+  controller(ApplicationController) do
+    # `described_class` is not available in this context
+    include ConfirmEmailWarning # rubocop:disable RSpec/DescribedClass
+
+    def index
+      head :ok
+    end
+  end
+
+  RSpec::Matchers.define :set_confirm_warning_for do |email|
+    match do |response|
+      expect(response).to set_flash.now[:warning].to include("Please check your email (#{email}) to verify that you own this address.")
+    end
+  end
+
+  describe 'confirm email flash warning' do
+    context 'when not signed in' do
+      let(:user) { create(:user, confirmed_at: nil) }
+
+      before do
+        get :index
+      end
+
+      it { is_expected.not_to set_confirm_warning_for(user.email) }
+    end
+
+    context 'when signed in' do
+      before do
+        sign_in(user)
+      end
+
+      context 'with a confirmed user' do
+        let(:user) { create(:user) }
+
+        before do
+          get :index
+        end
+
+        it { is_expected.not_to set_confirm_warning_for(user.email) }
+      end
+
+      context 'with an unconfirmed user' do
+        let(:user) { create(:user, confirmed_at: nil) }
+
+        context 'when executing a peek request' do
+          before do
+            request.path = '/-/peek'
+            get :index
+          end
+
+          it { is_expected.not_to set_confirm_warning_for(user.email) }
+        end
+
+        context 'when executing a json request' do
+          before do
+            get :index, format: :json
+          end
+
+          it { is_expected.not_to set_confirm_warning_for(user.email) }
+        end
+
+        context 'when executing a post request' do
+          before do
+            post :index
+          end
+
+          it { is_expected.not_to set_confirm_warning_for(user.email) }
+        end
+
+        context 'when executing a get request' do
+          before do
+            get :index
+          end
+
+          context 'with an unconfirmed email address present' do
+            let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'unconfirmed@gitlab.com') }
+
+            it { is_expected.to set_confirm_warning_for(user.unconfirmed_email) }
+          end
+
+          context 'without an unconfirmed email address present' do
+            it { is_expected.to set_confirm_warning_for(user.email) }
+          end
+        end
+      end
+    end
+  end
+end

spec/controllers/registrations_controller_spec.rb

@@ -30,13 +30,36 @@ describe RegistrationsController do
       end
 
       context 'when send_user_confirmation_email is true' do
-        it 'does not authenticate user and sends confirmation email' do
+        before do
           stub_application_setting(send_user_confirmation_email: true)
+        end
+
+        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 '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(subject.current_user).to be_nil
+          end
+        end
 
-          post(:create, params: user_params)
+        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(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email])
-          expect(subject.current_user).to be_nil
+          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(dashboard_projects_path)
+          end
         end
       end
 

spec/features/invites_spec.rb

@@ -17,11 +17,10 @@ describe 'Invites' do
     group_invite.generate_invite_token!
   end
 
-  def confirm_email_and_sign_in(new_user)
+  def confirm_email(new_user)
     new_user_token = User.find_by_email(new_user.email).confirmation_token
 
     visit user_confirmation_path(confirmation_token: new_user_token)
-    fill_in_sign_in_form(new_user)
   end
 
   def fill_in_sign_up_form(new_user)
@@ -155,17 +154,41 @@ describe 'Invites' do
     context 'email confirmation enabled' do
       let(:send_email_confirmation) { true }
 
-      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_and_sign_in(new_user)
+      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
 
-        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)
+        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
 
-      it "doesn't accept invitations until the user confirm his email" do
+      context 'when soft email confirmation is enabled' do
+        before do
+          allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
+        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)
+
+          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
+
+      it "doesn't accept invitations until the user confirms his email" do
         fill_in_sign_up_form(new_user)
         sign_in(owner)
 
@@ -176,11 +199,32 @@ describe 'Invites' do
       context 'the user sign-up using a different email address' do
         let(:invite_email) { build_stubbed(:user).email }
 
-        it 'signs up and redirects to the invitation page' do
-          fill_in_sign_up_form(new_user)
-          confirm_email_and_sign_in(new_user)
+        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
 
-          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)
+            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
+
+        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
+
+          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

spec/features/users/login_spec.rb

@@ -781,4 +781,39 @@ describe 'Login' do
       end
     end
   end
+
+  context 'when sending confirmation email and not yet confirmed' do
+    let!(:user) { create(:user, confirmed_at: nil) }
+    let(:grace_period) { 2.days }
+
+    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
+
+    it 'allows login and shows a flash warning to confirm the email address' do
+      expect(authentication_metrics).to increment(:user_authenticated_counter)
+
+      gitlab_sign_in(user)
+
+      expect(current_path).to eq root_path
+      expect(page).to have_content("Please check your email (#{user.email}) to verify that you own this address.")
+    end
+
+    context "when not having confirmed within Devise's allow_unconfirmed_access_for time" do
+      it 'does not allow login and shows a flash alert to confirm the email address' do
+        travel_to((grace_period + 1.day).from_now) do
+          expect(authentication_metrics)
+            .to increment(:user_unauthenticated_counter)
+            .and increment(:user_session_destroyed_counter).twice
+
+          gitlab_sign_in(user)
+
+          expect(current_path).to eq new_user_session_path
+          expect(page).to have_content('You have to confirm your email address before continuing.')
+        end
+      end
+    end
+  end
 end

spec/features/users/signup_spec.rb

@@ -166,24 +166,51 @@ describe 'Signup' do
   end
 
   context 'with no errors' do
-    context "when sending confirmation email" do
+    context 'when sending confirmation email' do
       before do
         stub_application_setting(send_user_confirmation_email: true)
       end
 
-      it 'creates the user account and sends a confirmation email' do
-        visit root_path
+      context 'when soft email confirmation is not enabled' do
+        before do
+          stub_feature_flags(soft_email_confirmation: false)
+        end
 
-        fill_in 'new_user_name',                with: new_user.name
-        fill_in 'new_user_username',            with: new_user.username
-        fill_in 'new_user_email',               with: new_user.email
-        fill_in 'new_user_email_confirmation',  with: new_user.email
-        fill_in 'new_user_password',            with: new_user.password
+        it 'creates the user account and sends a confirmation email' do
+          visit root_path
+
+          fill_in 'new_user_name',                with: new_user.name
+          fill_in 'new_user_username',            with: new_user.username
+          fill_in 'new_user_email',               with: new_user.email
+          fill_in 'new_user_email_confirmation',  with: new_user.email
+          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
+
+      context 'when soft email confirmation is enabled' do
+        before do
+          stub_feature_flags(soft_email_confirmation: true)
+        end
+
+        it 'creates the user account and sends a confirmation email' do
+          visit root_path
+
+          fill_in 'new_user_name',                with: new_user.name
+          fill_in 'new_user_username',            with: new_user.username
+          fill_in 'new_user_email',               with: new_user.email
+          fill_in 'new_user_email_confirmation',  with: new_user.email
+          fill_in 'new_user_password',            with: new_user.password
 
-        expect { click_button 'Register' }.to change { User.count }.by(1)
+          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")
+          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.")
+        end
       end
     end
 

spec/fixtures/api/schemas/deployment.json

@@ -3,6 +3,7 @@
   "required": [
     "sha",
     "created_at",
+    "finished_at",
     "iid",
     "tag",
     "last?",
@@ -11,6 +12,7 @@
   ],
   "properties": {
     "created_at": { "type": "string" },
+    "finished_at": { "type": ["string", "null"] },
     "id": { "type": "integer" },
     "iid": { "type": "integer" },
     "last?": { "type": "boolean" },

spec/serializers/deployment_entity_spec.rb

@@ -32,6 +32,10 @@ describe DeploymentEntity do
     expect(subject).to include(:created_at)
   end
 
+  it 'exposes finished_at' do
+    expect(subject).to include(:finished_at)
+  end
+
   context 'when the pipeline has another manual action' do
     let(:other_build) { create(:ci_build, :manual, name: 'another deploy', pipeline: pipeline) }
     let!(:other_deployment) { create(:deployment, deployable: other_build) }