Commit 556f4b72 authored by Bob Van Landuyt's avatar Bob Van Landuyt
Browse files

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.
parent a2d01ad290c0
......@@ -248,7 +248,9 @@ def visible_attributes
: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
......@@ -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 @@ def password_authentication_enabled?
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 @@ def check_repository_storages
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
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
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
......@@ -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"
......@@ -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
......
......@@ -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!",
}
```
FactoryBot.define do
factory :term, class: ApplicationSetting::Term do
terms "Lorem ipsum dolor sit amet, consectetur adipiscing elit."
end
end
......@@ -85,6 +85,18 @@
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
......
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
......@@ -301,6 +301,21 @@ def expect_invalid
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
......
......@@ -54,7 +54,9 @@
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 @@
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
......
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
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment