summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/helpers/application_settings_helper.rb4
-rw-r--r--app/models/application_setting.rb19
-rw-r--r--app/models/application_setting/term.rb8
-rw-r--r--app/services/application_settings/update_service.rb15
-rw-r--r--app/views/admin/application_settings/_terms.html.haml6
-rw-r--r--app/views/admin/application_settings/show.html.haml11
-rw-r--r--doc/api/settings.md6
-rw-r--r--spec/factories/terms.rb5
-rw-r--r--spec/features/admin/admin_settings_spec.rb12
-rw-r--r--spec/models/application_setting/term_spec.rb15
-rw-r--r--spec/models/application_setting_spec.rb15
-rw-r--r--spec/requests/api/settings_spec.rb6
-rw-r--r--spec/services/application_settings/update_service_spec.rb57
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
View Lorry Controller Status