Store application wide terms

This allows admins to define terms in the application settings.

Every time the terms are adjusted, a new version is stored and becomes
the 'active' version. This allows tracking which specific version was
accepted by a user.

13 files changed, 174 insertions, 5 deletions

diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 3fbb32c5229..1bf98d550b0 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -248,7 +248,9 @@ module ApplicationSettingsHelper
       :user_default_external,
       :user_oauth_applications,
       :version_check_enabled,
-      :allow_local_requests_from_hooks_and_services
+      :allow_local_requests_from_hooks_and_services,
+      :enforce_terms,
+      :terms
     ]
   end
 end
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 862933bf127..a734cc7a26b 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -220,12 +220,15 @@ class ApplicationSetting < ActiveRecord::Base
     end
   end
 
+  validate :terms_exist, if: :enforce_terms?
+
   before_validation :ensure_uuid!
 
   before_save :ensure_runners_registration_token
   before_save :ensure_health_check_access_token
 
   after_commit do
+    reset_memoized_terms
     Rails.cache.write(CACHE_KEY, self)
   end
 
@@ -507,6 +510,16 @@ class ApplicationSetting < ActiveRecord::Base
     password_authentication_enabled_for_web? || password_authentication_enabled_for_git?
   end
 
+  delegate :terms, to: :latest_terms, allow_nil: true
+  def latest_terms
+    @latest_terms ||= Term.latest
+  end
+
+  def reset_memoized_terms
+    @latest_terms = nil
+    latest_terms
+  end
+
   private
 
   def ensure_uuid!
@@ -520,4 +533,10 @@ class ApplicationSetting < ActiveRecord::Base
     errors.add(:repository_storages, "can't include: #{invalid.join(", ")}") unless
       invalid.empty?
   end
+
+  def terms_exist
+    return unless enforce_terms?
+
+    errors.add(:terms, "You need to set terms to be enforced") unless terms.present?
+  end
 end
diff --git a/app/models/application_setting/term.rb b/app/models/application_setting/term.rb
index 1f3d20e2b75..e8ce0ccbb71 100644
--- a/app/models/application_setting/term.rb
+++ b/app/models/application_setting/term.rb
@@ -1,5 +1,13 @@
 class ApplicationSetting
   class Term < ActiveRecord::Base
+    include CacheMarkdownField
+
     validates :terms, presence: true
+
+    cache_markdown_field :terms
+
+    def self.latest
+      order(:id).last
+    end
   end
 end
diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb
index 61589a07250..d6d3a661dab 100644
--- a/app/services/application_settings/update_service.rb
+++ b/app/services/application_settings/update_service.rb
@@ -1,7 +1,22 @@
 module ApplicationSettings
   class UpdateService < ApplicationSettings::BaseService
     def execute
+      update_terms(@params.delete(:terms))
+
       @application_setting.update(@params)
     end
+
+    private
+
+    def update_terms(terms)
+      return unless terms.present?
+
+      # Avoid creating a new terms record if the text is exactly the same.
+      terms = terms.strip
+      return if terms == @application_setting.terms
+
+      ApplicationSetting::Term.create(terms: terms)
+      @application_setting.reset_memoized_terms
+    end
   end
 end
diff --git a/app/views/admin/application_settings/_terms.html.haml b/app/views/admin/application_settings/_terms.html.haml
index 39a3fb147bd..724246ab7e7 100644
--- a/app/views/admin/application_settings/_terms.html.haml
+++ b/app/views/admin/application_settings/_terms.html.haml
@@ -9,7 +9,7 @@
             = f.check_box :enforce_terms
             = _("Require all users to accept Terms of Service when they access GitLab.")
           .help-block
-            When enabled, users cannot use GitLab until the terms have been accepted.
+            = _("When enabled, users cannot use GitLab until the terms have been accepted.")
     .form-group
       .col-sm-12
         = f.label :terms do
@@ -17,6 +17,6 @@
       .col-sm-12
         = f.text_area :terms, class: 'form-control', rows: 8
         .help-block
-          Markdown enabled
+          = _("Markdown enabled")
 
-  = f.submit 'Save changes', class: "btn btn-success"
+  = f.submit _("Save changes"), class: "btn btn-success"
diff --git a/app/views/admin/application_settings/show.html.haml b/app/views/admin/application_settings/show.html.haml
index caaa93aa1e2..8cb5bba8f63 100644
--- a/app/views/admin/application_settings/show.html.haml
+++ b/app/views/admin/application_settings/show.html.haml
@@ -47,6 +47,17 @@
   .settings-content
     = render 'signin'
 
+%section.settings.as-terms.no-animate#js-terms-settings{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = _('Terms of Service')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      = _('Include a Terms of Service agreement that all users must accept.')
+  .settings-content
+    = render 'terms'
+
 %section.settings.as-help-page.no-animate#js-help-settings{ class: ('expanded' if expanded) }
   .settings-header
     %h4
diff --git a/doc/api/settings.md b/doc/api/settings.md
index 0b5b1f0c134..e06b1bfb6df 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -53,6 +53,8 @@ Example response:
    "dsa_key_restriction": 0,
    "ecdsa_key_restriction": 0,
    "ed25519_key_restriction": 0,
+   "enforce_terms": true,
+   "terms": "Hello world!",
 }
 ```
 
@@ -153,6 +155,8 @@ PUT /application/settings
 | `user_default_external`                  | boolean          | no                                            | Newly registered users will by default be external                                                                                                                                                                                                                                                                                                                                                                                                        |
 | `user_oauth_applications`                | boolean          | no                                            | Allow users to register any application to use GitLab as an OAuth provider                                                                                                                                                                                                                                                                                                                                                                                |
 | `version_check_enabled`                  | boolean          | no                                            | Let GitLab inform you when an update is available.                                                                                                                                                                                                                                                                                                                                                                                                        |
+| `enforce_terms`                          | boolean          | no                                            | Enforce application ToS to all users                                                                                                                                                                                                                                                                                                                                                                                                         |
+| `terms`                                  | text             | yes (if `enforce_terms` is true)              | Markdown content for the ToS                                                                                                                                                                                                                                                                                                                                                                                                                 |
 
 ```bash
 curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/application/settings?signup_enabled=false&default_project_visibility=internal
@@ -195,5 +199,7 @@ Example response:
   "dsa_key_restriction": 0,
   "ecdsa_key_restriction": 0,
   "ed25519_key_restriction": 0,
+  "enforce_terms": true,
+  "terms": "Hello world!",
 }
 ```
diff --git a/spec/factories/terms.rb b/spec/factories/terms.rb
new file mode 100644
index 00000000000..5ffca365a5f
--- /dev/null
+++ b/spec/factories/terms.rb
@@ -0,0 +1,5 @@
+FactoryBot.define do
+  factory :term, class: ApplicationSetting::Term do
+    terms "Lorem ipsum dolor sit amet, consectetur adipiscing elit."
+  end
+end
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index 7853d2952ea..b74643bac55 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -85,6 +85,18 @@ feature 'Admin updates settings' do
     expect(page).to have_content "Application settings saved successfully"
   end
 
+  scenario 'Terms of Service' do
+    page.within('.as-terms') do
+      check 'Require all users to accept Terms of Service when they access GitLab.'
+      fill_in 'Terms of Service Agreement', with: 'Be nice!'
+      click_button 'Save changes'
+    end
+
+    expect(Gitlab::CurrentSettings.enforce_terms).to be(true)
+    expect(Gitlab::CurrentSettings.terms).to eq 'Be nice!'
+    expect(page).to have_content 'Application settings saved successfully'
+  end
+
   scenario 'Modify oauth providers' do
     expect(Gitlab::CurrentSettings.disabled_oauth_sign_in_sources).to be_empty
 
diff --git a/spec/models/application_setting/term_spec.rb b/spec/models/application_setting/term_spec.rb
new file mode 100644
index 00000000000..1eddf3c56ff
--- /dev/null
+++ b/spec/models/application_setting/term_spec.rb
@@ -0,0 +1,15 @@
+require 'spec_helper'
+
+describe ApplicationSetting::Term do
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:terms) }
+  end
+
+  describe '.latest' do
+    it 'finds the latest terms' do
+      terms = create(:term)
+
+      expect(described_class.latest).to eq(terms)
+    end
+  end
+end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index ae2d34750a7..10d6109cae7 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -301,6 +301,21 @@ describe ApplicationSetting do
         expect(subject).to be_invalid
       end
     end
+
+    describe 'enforcing terms' do
+      it 'requires the terms to present when enforcing users to accept' do
+        subject.enforce_terms = true
+
+        expect(subject).to be_invalid
+      end
+
+      it 'is valid when terms are created' do
+        create(:term)
+        subject.enforce_terms = true
+
+        expect(subject).to be_valid
+      end
+    end
   end
 
   describe '.current' do
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 015d4b9a491..8b22d1e72f3 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -54,7 +54,9 @@ describe API::Settings, 'Settings' do
           dsa_key_restriction: 2048,
           ecdsa_key_restriction: 384,
           ed25519_key_restriction: 256,
-          circuitbreaker_check_interval: 2
+          circuitbreaker_check_interval: 2,
+          enforce_terms: true,
+          terms: 'Hello world!'
 
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
@@ -76,6 +78,8 @@ describe API::Settings, 'Settings' do
         expect(json_response['ecdsa_key_restriction']).to eq(384)
         expect(json_response['ed25519_key_restriction']).to eq(256)
         expect(json_response['circuitbreaker_check_interval']).to eq(2)
+        expect(json_response['enforce_terms']).to be(true)
+        expect(json_response['terms']).to eq('Hello world!')
       end
     end
 
diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb
new file mode 100644
index 00000000000..fb07ecc6ae8
--- /dev/null
+++ b/spec/services/application_settings/update_service_spec.rb
@@ -0,0 +1,57 @@
+require 'spec_helper'
+
+describe ApplicationSettings::UpdateService do
+  let(:application_settings) { Gitlab::CurrentSettings.current_application_settings }
+  let(:admin) { create(:user, :admin) }
+  let(:params) { {} }
+
+  subject { described_class.new(application_settings, admin, params) }
+
+  before do
+    # So the caching behaves like it would in production
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+  end
+
+  describe 'updating terms' do
+    context 'when the passed terms are blank' do
+      let(:params) { { terms: ''  } }
+
+      it 'does not create terms' do
+        expect { subject.execute }.not_to change { ApplicationSetting::Term.count }
+      end
+    end
+
+    context 'when passing terms' do
+      let(:params) { { terms: 'Be nice!  '  } }
+
+      it 'creates the terms' do
+        expect { subject.execute }.to change { ApplicationSetting::Term.count }.by(1)
+      end
+
+      it 'does not create terms if they are the same as the existing ones' do
+        create(:term, terms: 'Be nice!')
+
+        expect { subject.execute }.not_to change { ApplicationSetting::Term.count }
+      end
+
+      it 'updates terms if they already existed' do
+        create(:term, terms: 'Other terms')
+
+        subject.execute
+
+        expect(application_settings.terms).to eq('Be nice!')
+      end
+
+      it 'Only queries once when the terms are changed' do
+        create(:term, terms: 'Other terms')
+        expect(application_settings.terms).to eq('Other terms')
+
+        subject.execute
+
+        expect(application_settings.terms).to eq('Be nice!')
+        expect { 2.times { application_settings.terms } }
+          .not_to exceed_query_limit(0)
+      end
+    end
+  end
+end