Allow a user to accept/decline terms

When a user accepts, we store this in the agreements to keep track of
which terms they accepted. We also update the flag on the user.

14 files changed, 307 insertions, 12 deletions

diff --git a/app/assets/stylesheets/framework/terms.scss b/app/assets/stylesheets/framework/terms.scss
index 0e9bdba034c..c73274540ac 100644
--- a/app/assets/stylesheets/framework/terms.scss
+++ b/app/assets/stylesheets/framework/terms.scss
@@ -33,5 +33,13 @@
     .panel-content {
       padding: 0 $gl-padding;
     }
+
+    .footer-block {
+      margin: 0;
+
+      .btn {
+        margin-left: 5px;
+      }
+    }
   }
 }
diff --git a/app/controllers/users/terms_controller.rb b/app/controllers/users/terms_controller.rb
index 778e388ff5a..32507bdb7b1 100644
--- a/app/controllers/users/terms_controller.rb
+++ b/app/controllers/users/terms_controller.rb
@@ -2,18 +2,55 @@ module Users
   class TermsController < ApplicationController
     before_action :terms
 
-
     layout 'terms'
 
     def index
+      @redirect = redirect_path
+    end
+
+    def accept
+      agreement = Users::RespondToTermsService.new(current_user, viewed_term)
+                    .execute(accepted: true)
+
+      if agreement.persisted?
+        redirect_to redirect_path
+      else
+        flash[:alert] = agreement.errors.full_messages.join(', ')
+        redirect_to terms_path, redirect: redirect_path
+      end
+    end
+
+    def decline
+      agreement = Users::RespondToTermsService.new(current_user, viewed_term)
+                    .execute(accepted: false)
+
+      if agreement.persisted?
+        sign_out(current_user)
+        redirect_to root_path
+      else
+        flash[:alert] = agreement.errors.full_messages.join(', ')
+        redirect_to terms_path, redirect: redirect_path
+      end
     end
 
     private
 
+    def viewed_term
+      @viewed_term ||= ApplicationSetting::Term.find(params[:id])
+    end
+
     def terms
-      unless @terms = Gitlab::CurrentSettings.current_application_settings.latest_terms
-        redirect_to request.referer || root_path
+      unless @term = Gitlab::CurrentSettings.current_application_settings.latest_terms
+        redirect_to redirect_path
       end
     end
+
+    def redirect_path
+      referer = if request.referer && !request.referer.include?(terms_path)
+                  URI(request.referer).path
+                end
+
+      params[:redirect] || referer || root_path
+    end
   end
 end
diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb
index 517268175e6..e803cd3a8d8 100644
--- a/app/helpers/users_helper.rb
+++ b/app/helpers/users_helper.rb
@@ -38,13 +38,24 @@ module UsersHelper
   end
 
   def get_current_user_menu_items
-    items = [:help, :sign_out]
+    items = []
 
-    if can?(current_user, :read_user, current_user)
+    items << :sign_out if current_user
+
+    # TODO: Remove these conditions when the permissions are prevented in
+    # https://gitlab.com/gitlab-org/gitlab-ce/issues/45849
+    terms_not_enforced = !Gitlab::CurrentSettings
+                                .current_application_settings
+                                .enforce_terms?
+    required_terms_accepted = terms_not_enforced || current_user.terms_accepted?
+
+    items << :help if required_terms_accepted
+
+    if can?(current_user, :read_user, current_user) && required_terms_accepted
       items << :profile
     end
 
-    if can?(current_user, :update_user, current_user)
+    if can?(current_user, :update_user, current_user) && required_terms_accepted
       items << :settings
     end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 4a602ffbb05..a9cfd39f604 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -138,6 +138,8 @@ class User < ActiveRecord::Base
   has_many :custom_attributes, class_name: 'UserCustomAttribute'
   has_many :callouts, class_name: 'UserCallout'
   has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+  has_many :term_agreements
+  belongs_to :accepted_term, class_name: 'ApplicationSetting::Term'
 
   #
   # Validations
@@ -1187,6 +1189,10 @@ class User < ActiveRecord::Base
     max_member_access_for_group_ids([group_id])[group_id]
   end
 
+  def terms_accepted?
+    accepted_term_id.present?
+  end
+
   protected
 
   # override, from Devise::Validatable
diff --git a/app/policies/application_setting/term_policy.rb b/app/policies/application_setting/term_policy.rb
new file mode 100644
index 00000000000..648932ddba9
--- /dev/null
+++ b/app/policies/application_setting/term_policy.rb
@@ -0,0 +1,30 @@
+class ApplicationSetting
+  class TermPolicy < BasePolicy
+    include Gitlab::Utils::StrongMemoize
+
+    condition(:logged_in, scope: :user) { @user }
+
+    condition(:current_terms, scope: :subject) do
+      Gitlab::CurrentSettings.current_application_settings.latest_terms == @subject
+    end
+
+    condition(:terms_accepted, scope: :user, score: 1) do
+      agreement&.accepted
+    end
+
+    rule { logged_in & current_terms }.policy do
+      enable :accept_terms
+      enable :decline_terms
+    end
+
+    rule { terms_accepted }.prevent :accept_terms
+
+    def agreement
+      strong_memoize(:agreement) do
+        next nil if @user.nil? || @subject.nil?
+
+        @user.term_agreements.find_by(term: @subject)
+      end
+    end
+  end
+end
diff --git a/app/services/users/respond_to_terms_service.rb b/app/services/users/respond_to_terms_service.rb
new file mode 100644
index 00000000000..06d660186cf
--- /dev/null
+++ b/app/services/users/respond_to_terms_service.rb
@@ -0,0 +1,24 @@
+module Users
+  class RespondToTermsService
+    def initialize(user, term)
+      @user, @term = user, term
+    end
+
+    def execute(accepted:)
+      agreement = @user.term_agreements.find_or_initialize_by(term: @term)
+      agreement.accepted = accepted
+
+      if agreement.save
+        store_accepted_term(accepted)
+      end
+
+      agreement
+    end
+
+    private
+
+    def store_accepted_term(accepted)
+      @user.update_column(:accepted_term_id, accepted ? @term.id : nil)
+    end
+  end
+end
diff --git a/app/views/layouts/terms.html.haml b/app/views/layouts/terms.html.haml
index 09c4567a362..b04dbc595a5 100644
--- a/app/views/layouts/terms.html.haml
+++ b/app/views/layouts/terms.html.haml
@@ -3,11 +3,18 @@
 %html{ lang: I18n.locale, class: page_class }
   = render "layouts/head"
 
-  %body{ class: "#{user_application_theme} #{@body_class}", data: { page: body_data_page } }
+  %body{ data: { page: body_data_page } }
     = render 'peek/bar'
     .layout-page.terms
       .content-wrapper
-        %div{ class: "#{(container_class unless @no_container)} #{@content_class}" }
+        .mobile-overlay
+        .alert-wrapper
+          = render "layouts/broadcast"
+          = render 'layouts/header/read_only_banner'
+          = yield :flash_message
+          = render "layouts/flash"
+
+        %div{ class: "#{container_class}" }
           .content{ id: "content-body" }
             .panel.panel-default
               .panel-heading
diff --git a/app/views/users/terms/index.html.haml b/app/views/users/terms/index.html.haml
index 49fdab84069..614dab57331 100644
--- a/app/views/users/terms/index.html.haml
+++ b/app/views/users/terms/index.html.haml
@@ -1,2 +1,12 @@
+- redirect_params = { redirect: @redirect } if @redirect
 .panel-content.rendered-terms
-  = markdown_field(@terms, :terms)
+  = markdown_field(@term, :terms)
+.row-content-block.footer-block.clearfix
+  - if can?(current_user, :accept_terms, @term)
+    .pull-right
+      = button_to accept_term_path(@term, redirect_params), class: 'btn btn-success prepend-left-8' do
+        = _('Accept terms')
+  - if can?(current_user, :decline_terms, @term)
+    .pull-right
+      = button_to decline_term_path(@term, redirect_params), class: 'btn btn-default prepend-left-8' do
+        =  _('Decline and sign out')
diff --git a/spec/controllers/users/terms_controller_spec.rb b/spec/controllers/users/terms_controller_spec.rb
index 74d17748093..50e818a4520 100644
--- a/spec/controllers/users/terms_controller_spec.rb
+++ b/spec/controllers/users/terms_controller_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe Users::TermsController do
   let(:user) { create(:user) }
+  let(:term) { create(:term) }
 
   before do
     sign_in user
@@ -15,9 +16,42 @@ describe Users::TermsController do
     end
 
     it 'shows terms when they exist' do
-      create(:term)
+      term
 
       expect(response).to have_gitlab_http_status(:success)
     end
   end
+
+  describe 'POST #accept' do
+    it 'saves that the user accepted the terms' do
+      post :accept, id: term.id
+
+      agreement = user.term_agreements.find_by(term: term)
+
+      expect(agreement.accepted).to eq(true)
+    end
+
+    it 'redirects to a path when specified' do
+      post :accept, id: term.id, redirect: groups_path
+
+      expect(response).to redirect_to(groups_path)
+    end
+  end
+
+  describe 'POST #decline' do
+    it 'stores that the user declined the terms' do
+      post :decline, id: term.id
+
+      agreement = user.term_agreements.find_by(term: term)
+
+      expect(agreement.accepted).to eq(false)
+    end
+
+    it 'signs out the user' do
+      post :decline, id: term.id
+
+      expect(response).to redirect_to(root_path)
+      expect(assigns(:current_user)).to be_nil
+    end
+  end
 end
diff --git a/spec/factories/term_agreements.rb b/spec/factories/term_agreements.rb
new file mode 100644
index 00000000000..557599e663d
--- /dev/null
+++ b/spec/factories/term_agreements.rb
@@ -0,0 +1,6 @@
+FactoryBot.define do
+  factory :term_agreement do
+    term
+    user
+  end
+end
diff --git a/spec/features/users/terms_spec.rb b/spec/features/users/terms_spec.rb
index 34e759bc56a..a33f0937fab 100644
--- a/spec/features/users/terms_spec.rb
+++ b/spec/features/users/terms_spec.rb
@@ -5,12 +5,35 @@ describe 'Users > Terms' do
   let!(:term) { create(:term, terms: 'By accepting, you promise to be nice!') }
 
   before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
     sign_in(user)
-
-    visit terms_path
   end
 
   it 'shows the terms' do
+    visit terms_path
+
     expect(page).to have_content('By accepting, you promise to be nice!')
   end
+
+  context 'declining the terms' do
+    it 'returns the user to the app' do
+      visit terms_path
+
+      click_button 'Decline and sign out'
+
+      expect(page).not_to have_content(term.terms)
+      expect(user.reload.terms_accepted?).to be(false)
+    end
+  end
+
+  context 'accepting the terms' do
+    it 'returns the user to the app' do
+      visit terms_path
+
+      click_button 'Accept terms'
+
+      expect(page).not_to have_content(term.terms)
+      expect(user.reload.terms_accepted?).to be(true)
+    end
+  end
 end
diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb
index 9f67277801f..b18c045848f 100644
--- a/spec/helpers/users_helper_spec.rb
+++ b/spec/helpers/users_helper_spec.rb
@@ -1,6 +1,8 @@
 require 'rails_helper'
 
 describe UsersHelper do
+  include TermsHelper
+
   let(:user) { create(:user) }
 
   describe '#user_link' do
@@ -51,5 +53,15 @@ describe UsersHelper do
 
       expect(items).to include(:profile)
     end
+
+    context 'when terms are enforced' do
+      before do
+        enforce_terms
+      end
+
+      it 'hides the profile and the settings tab' do
+        expect(items).not_to include(:settings, :profile, :help)
+      end
+    end
   end
 end
diff --git a/spec/policies/application_setting/term_policy_spec.rb b/spec/policies/application_setting/term_policy_spec.rb
new file mode 100644
index 00000000000..93b5ebf5f72
--- /dev/null
+++ b/spec/policies/application_setting/term_policy_spec.rb
@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+describe ApplicationSetting::TermPolicy do
+  include TermsHelper
+
+  set(:term) { create(:term) }
+  let(:user) { create(:user) }
+
+  subject(:policy) { described_class.new(user, term) }
+
+  before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+  end
+
+  it 'has the correct permissions', :aggregate_failures do
+    is_expected.to be_allowed(:accept_terms)
+    is_expected.to be_allowed(:decline_terms)
+  end
+
+  context 'for anonymous users' do
+    let(:user) { nil }
+
+    it 'has the correct permissions', :aggregate_failures do
+      is_expected.to be_disallowed(:accept_terms)
+      is_expected.to be_disallowed(:decline_terms)
+    end
+  end
+
+  context 'when the terms are not current' do
+    before do
+      create(:term)
+    end
+
+    it 'has the correct permissions', :aggregate_failures do
+      is_expected.to be_disallowed(:accept_terms)
+      is_expected.to be_disallowed(:decline_terms)
+    end
+  end
+
+  context 'when the user already accepted the terms' do
+    before do
+      accept_terms(user)
+    end
+
+    it 'has the correct permissions', :aggregate_failures do
+      is_expected.to be_disallowed(:accept_terms)
+      is_expected.to be_allowed(:decline_terms)
+    end
+  end
+end
diff --git a/spec/services/users/respond_to_terms_service_spec.rb b/spec/services/users/respond_to_terms_service_spec.rb
new file mode 100644
index 00000000000..fb08dd10b87
--- /dev/null
+++ b/spec/services/users/respond_to_terms_service_spec.rb
@@ -0,0 +1,37 @@
+require 'spec_helper'
+
+describe Users::RespondToTermsService do
+  let(:user) { create(:user) }
+  let(:term) { create(:term) }
+
+  subject(:service) { described_class.new(user, term) }
+
+  describe '#execute' do
+    it 'creates a new agreement if it did not exist' do
+      expect { service.execute(accepted: true) }
+        .to change { user.term_agreements.size }.by(1)
+    end
+
+    it 'updates an agreement if it existed' do
+      agreement = create(:term_agreement, user: user, term: term, accepted: true)
+
+      service.execute(accepted: true)
+
+      expect(agreement.reload.accepted).to be_truthy
+    end
+
+    it 'adds the accepted terms to the user' do
+      service.execute(accepted: true)
+
+      expect(user.reload.accepted_term).to eq(term)
+    end
+
+    it 'removes accepted terms when declining' do
+      user.update!(accepted_term: term)
+
+      service.execute(accepted: false)
+
+      expect(user.reload.accepted_term).to be_nil
+    end
+  end
+end