diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-05-04 15:17:25 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-05-04 15:17:25 +0000 |
commit | 7603beffc916d06039cac63b223d8e6234b5d666 (patch) | |
tree | b8b52e683a2c5829a97d7a802fae383369ab9eb5 | |
parent | a1ce521c995cc7943583e6c42e13666e1edd93ac (diff) | |
parent | 39916fdfeddfd75279d13fa976fdb07f3b9b0e26 (diff) | |
download | gitlab-ce-7603beffc916d06039cac63b223d8e6234b5d666.tar.gz |
Merge branch 'bvl-enforce-terms' into 'master'
Enforce application wide terms
Closes #44798
See merge request gitlab-org/gitlab-ce!18570
61 files changed, 1586 insertions, 101 deletions
diff --git a/app/assets/javascripts/issuable_form.js b/app/assets/javascripts/issuable_form.js index bb8b3d91e40..90d4e19e90b 100644 --- a/app/assets/javascripts/issuable_form.js +++ b/app/assets/javascripts/issuable_form.js @@ -30,7 +30,7 @@ export default class IssuableForm { } this.initAutosave(); - this.form.on('submit', this.handleSubmit); + this.form.on('submit:success', this.handleSubmit); this.form.on('click', '.btn-cancel', this.resetAutosave); this.initWip(); diff --git a/app/assets/stylesheets/framework.scss b/app/assets/stylesheets/framework.scss index 360dcb6afef..9bd35183d8a 100644 --- a/app/assets/stylesheets/framework.scss +++ b/app/assets/stylesheets/framework.scss @@ -61,3 +61,4 @@ @import 'framework/stacked_progress_bar'; @import 'framework/ci_variable_list'; @import 'framework/feature_highlight'; +@import 'framework/terms'; diff --git a/app/assets/stylesheets/framework/terms.scss b/app/assets/stylesheets/framework/terms.scss new file mode 100644 index 00000000000..dadfaf1c3f9 --- /dev/null +++ b/app/assets/stylesheets/framework/terms.scss @@ -0,0 +1,55 @@ +.terms { + .alert-wrapper { + min-height: $header-height + $gl-padding; + } + + .content { + padding-top: $gl-padding; + } + + .panel { + .panel-heading { + display: -webkit-flex; + display: flex; + align-items: center; + justify-content: space-between; + + .title { + display: flex; + align-items: center; + + .logo-text { + width: 55px; + height: 24px; + display: flex; + flex-direction: column; + justify-content: center; + } + } + + .navbar-collapse { + padding-right: 0; + } + + .nav li a { + color: $theme-gray-700; + } + } + + .panel-content { + padding: $gl-padding; + + *:first-child { + margin-top: 0; + } + + *:last-child { + margin-bottom: 0; + } + } + + .footer-block { + margin: 0; + } + } +} diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 8ad13a82f89..2caffec66ac 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -13,12 +13,14 @@ class ApplicationController < ActionController::Base before_action :authenticate_sessionless_user! before_action :authenticate_user! + before_action :enforce_terms!, if: -> { Gitlab::CurrentSettings.current_application_settings.enforce_terms }, + unless: :peek_request? before_action :validate_user_service_ticket! before_action :check_password_expiration before_action :ldap_security_check before_action :sentry_context before_action :default_headers - before_action :add_gon_variables, unless: -> { request.path.start_with?('/-/peek') } + before_action :add_gon_variables, unless: :peek_request? before_action :configure_permitted_parameters, if: :devise_controller? before_action :require_email, unless: :devise_controller? @@ -269,6 +271,27 @@ class ApplicationController < ActionController::Base end end + def enforce_terms! + return unless current_user + return if current_user.terms_accepted? + + if sessionless_user? + render_403 + else + # Redirect to the destination if the request is a get. + # Redirect to the source if it was a post, so the user can re-submit after + # accepting the terms. + redirect_path = if request.get? + request.fullpath + else + URI(request.referer).path if request.referer + end + + flash[:notice] = _("Please accept the Terms of Service before continuing.") + redirect_to terms_path(redirect: redirect_path), status: :found + end + end + def import_sources_enabled? !Gitlab::CurrentSettings.import_sources.empty? end @@ -342,4 +365,12 @@ class ApplicationController < ActionController::Base # Per https://tools.ietf.org/html/rfc5987, headers need to be ISO-8859-1, not UTF-8 response.headers['Page-Title'] = URI.escape(page_title('GitLab')) end + + def sessionless_user? + current_user && !session.keys.include?('warden.user.user.key') + end + + def peek_request? + request.path.start_with?('/-/peek') + end end diff --git a/app/controllers/concerns/continue_params.rb b/app/controllers/concerns/continue_params.rb index eb3a623acdd..8b7355974df 100644 --- a/app/controllers/concerns/continue_params.rb +++ b/app/controllers/concerns/continue_params.rb @@ -1,4 +1,5 @@ module ContinueParams + include InternalRedirect extend ActiveSupport::Concern def continue_params @@ -6,8 +7,7 @@ module ContinueParams return nil unless continue_params continue_params = continue_params.permit(:to, :notice, :notice_now) - return unless continue_params[:to] && continue_params[:to].start_with?('/') - return if continue_params[:to].start_with?('//') + continue_params[:to] = safe_redirect_path(continue_params[:to]) continue_params end diff --git a/app/controllers/concerns/internal_redirect.rb b/app/controllers/concerns/internal_redirect.rb new file mode 100644 index 00000000000..7409b2e89a5 --- /dev/null +++ b/app/controllers/concerns/internal_redirect.rb @@ -0,0 +1,35 @@ +module InternalRedirect + extend ActiveSupport::Concern + + def safe_redirect_path(path) + return unless path + # Verify that the string starts with a `/` but not a double `/`. + return unless path =~ %r{^/\w.*$} + + uri = URI(path) + # Ignore anything path of the redirect except for the path, querystring and, + # fragment, forcing the redirect within the same host. + full_path_for_uri(uri) + rescue URI::InvalidURIError + nil + end + + def safe_redirect_path_for_url(url) + return unless url + + uri = URI(url) + safe_redirect_path(full_path_for_uri(uri)) if host_allowed?(uri) + rescue URI::InvalidURIError + nil + end + + def host_allowed?(uri) + uri.host == request.host && + uri.port == request.port + end + + def full_path_for_uri(uri) + path_with_query = [uri.path, uri.query].compact.join('?') + [path_with_query, uri.fragment].compact.join("#") + end +end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f3a4aa849c7..1a339f76d26 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,4 +1,5 @@ class SessionsController < Devise::SessionsController + include InternalRedirect include AuthenticatesWithTwoFactor include Devise::Controllers::Rememberable include Recaptcha::ClientHelper @@ -102,18 +103,12 @@ class SessionsController < Devise::SessionsController # we should never redirect to '/users/sign_in' after signing in successfully. return true if redirect_uri.path == new_user_session_path - redirect_to = redirect_uri.to_s if redirect_allowed_to?(redirect_uri) + redirect_to = redirect_uri.to_s if host_allowed?(redirect_uri) @redirect_to = redirect_to store_location_for(:redirect, redirect_to) end - # Overridden in EE - def redirect_allowed_to?(uri) - uri.host == Gitlab.config.gitlab.host && - uri.port == Gitlab.config.gitlab.port - end - def two_factor_enabled? find_user&.two_factor_enabled? end diff --git a/app/controllers/users/terms_controller.rb b/app/controllers/users/terms_controller.rb new file mode 100644 index 00000000000..95c5c3432d5 --- /dev/null +++ b/app/controllers/users/terms_controller.rb @@ -0,0 +1,66 @@ +module Users + class TermsController < ApplicationController + include InternalRedirect + + skip_before_action :enforce_terms! + 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 @term = Gitlab::CurrentSettings.current_application_settings.latest_terms + redirect_to redirect_path + end + end + + def redirect_path + redirect_to_path = safe_redirect_path(params[:redirect]) || safe_redirect_path_for_url(request.referer) + + if redirect_to_path && + excluded_redirect_paths.none? { |excluded| redirect_to_path.include?(excluded) } + redirect_to_path + else + root_path + end + end + + def excluded_redirect_paths + [terms_path, new_user_session_path] + end + end +end 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/helpers/users_helper.rb b/app/helpers/users_helper.rb index 01af68088df..e803cd3a8d8 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -23,9 +23,42 @@ module UsersHelper profile_tabs.include?(tab) end + def current_user_menu_items + @current_user_menu_items ||= get_current_user_menu_items + end + + def current_user_menu?(item) + current_user_menu_items.include?(item) + end + private def get_profile_tabs [:activity, :groups, :contributed, :projects, :snippets] end + + def get_current_user_menu_items + items = [] + + 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) && required_terms_accepted + items << :settings + end + + items + 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 new file mode 100644 index 00000000000..e8ce0ccbb71 --- /dev/null +++ b/app/models/application_setting/term.rb @@ -0,0 +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/models/term_agreement.rb b/app/models/term_agreement.rb new file mode 100644 index 00000000000..8458a231bbd --- /dev/null +++ b/app/models/term_agreement.rb @@ -0,0 +1,6 @@ +class TermAgreement < ActiveRecord::Base + belongs_to :term, class_name: 'ApplicationSetting::Term' + belongs_to :user + + validates :user, :term, presence: true +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..f03bf748c76 --- /dev/null +++ b/app/policies/application_setting/term_policy.rb @@ -0,0 +1,28 @@ +class ApplicationSetting + class TermPolicy < BasePolicy + include Gitlab::Utils::StrongMemoize + + condition(:current_terms, scope: :subject) do + Gitlab::CurrentSettings.current_application_settings.latest_terms == @subject + end + + condition(:terms_accepted, score: 1) do + agreement&.accepted + end + + rule { ~anonymous & 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/policies/user_policy.rb b/app/policies/user_policy.rb index 0905ddd9b38..ee219f0a0d0 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -8,6 +8,8 @@ class UserPolicy < BasePolicy rule { ~restricted_public_level }.enable :read_user rule { ~anonymous }.enable :read_user - rule { user_is_self | admin }.enable :destroy_user - rule { subject_ghost }.prevent :destroy_user + rule { ~subject_ghost & (user_is_self | admin) }.policy do + enable :destroy_user + enable :update_user + 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/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/admin/application_settings/_terms.html.haml b/app/views/admin/application_settings/_terms.html.haml new file mode 100644 index 00000000000..724246ab7e7 --- /dev/null +++ b/app/views/admin/application_settings/_terms.html.haml @@ -0,0 +1,22 @@ += form_for @application_setting, url: admin_application_settings_path, html: { class: 'form-horizontal fieldset-form' } do |f| + = form_errors(@application_setting) + + %fieldset + .form-group + .col-sm-12 + .checkbox + = f.label :enforce_terms do + = 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.") + .form-group + .col-sm-12 + = f.label :terms do + = _("Terms of Service Agreement") + .col-sm-12 + = f.text_area :terms, class: 'form-control', rows: 8 + .help-block + = _("Markdown enabled") + + = 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..3c00e3c8fc4 100644 --- a/app/views/admin/application_settings/show.html.haml +++ b/app/views/admin/application_settings/show.html.haml @@ -8,7 +8,7 @@ %h4 = _('Visibility and access controls') %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + = expanded ? _('Collapse') : _('Expand') %p = _('Set default and restrict visibility levels. Configure import sources and git access protocol.') .settings-content @@ -19,7 +19,7 @@ %h4 = _('Account and limit settings') %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + = expanded ? _('Collapse') : _('Expand') %p = _('Session expiration, projects limit and attachment size.') .settings-content @@ -30,7 +30,7 @@ %h4 = _('Sign-up restrictions') %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + = expanded ? _('Collapse') : _('Expand') %p = _('Configure the way a user creates a new account.') .settings-content @@ -41,18 +41,29 @@ %h4 = _('Sign-in restrictions') %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + = expanded ? _('Collapse') : _('Expand') %p = _('Set requirements for a user to sign-in. Enable mandatory two-factor authentication.') .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.btn-default.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 = _('Help page') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Help page text and support page url.') .settings-content @@ -62,8 +73,8 @@ .settings-header %h4 = _('Pages') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Size and domain settings for static websites') .settings-content @@ -73,8 +84,8 @@ .settings-header %h4 = _('Continuous Integration and Deployment') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Auto DevOps, runners and job artifacts') .settings-content @@ -84,8 +95,8 @@ .settings-header %h4 = _('Metrics - Influx') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Enable and configure InfluxDB metrics.') .settings-content @@ -95,8 +106,8 @@ .settings-header %h4 = _('Metrics - Prometheus') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Enable and configure Prometheus metrics.') .settings-content @@ -106,8 +117,8 @@ .settings-header %h4 = _('Profiling - Performance bar') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Enable the Performance Bar for a given group.') = link_to icon('question-circle'), help_page_path('administration/monitoring/performance/performance_bar') @@ -118,8 +129,8 @@ .settings-header %h4 = _('Background jobs') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Configure Sidekiq job throttling.') .settings-content @@ -129,8 +140,8 @@ .settings-header %h4 = _('Spam and Anti-bot Protection') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Enable reCAPTCHA or Akismet and set IP limits.') .settings-content @@ -140,8 +151,8 @@ .settings-header %h4 = _('Abuse reports') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Set notification email for abuse reports.') .settings-content @@ -151,8 +162,8 @@ .settings-header %h4 = _('Error Reporting and Logging') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Enable Sentry for error reporting and logging.') .settings-content @@ -162,8 +173,8 @@ .settings-header %h4 = _('Repository storage') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Configure storage path and circuit breaker settings.') .settings-content @@ -173,8 +184,8 @@ .settings-header %h4 = _('Repository maintenance') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Configure automatic git checks and housekeeping on repositories.') .settings-content @@ -185,8 +196,8 @@ .settings-header %h4 = _('Container Registry') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Various container registry settings.') .settings-content @@ -197,8 +208,8 @@ .settings-header %h4 = _('Koding') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Online IDE integration settings.') .settings-content @@ -208,8 +219,8 @@ .settings-header %h4 = _('PlantUML') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Allow rendering of PlantUML diagrams in Asciidoc documents.') .settings-content @@ -219,8 +230,8 @@ .settings-header#usage-statistics %h4 = _('Usage statistics') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Enable or disable version check and usage ping.') .settings-content @@ -230,8 +241,8 @@ .settings-header %h4 = _('Email') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Various email settings.') .settings-content @@ -241,8 +252,8 @@ .settings-header %h4 = _('Gitaly') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Configure Gitaly timeouts.') .settings-content @@ -252,8 +263,8 @@ .settings-header %h4 = _('Web terminal') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Set max session time for web terminal.') .settings-content @@ -263,8 +274,8 @@ .settings-header %h4 = _('Real-time features') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Change this value to influence how frequently the GitLab UI polls for updates.') .settings-content @@ -274,8 +285,8 @@ .settings-header %h4 = _('Performance optimization') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Various settings that affect GitLab performance.') .settings-content @@ -285,8 +296,8 @@ .settings-header %h4 = _('User and IP Rate Limits') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Configure limits for web and API requests.') .settings-content @@ -296,8 +307,8 @@ .settings-header %h4 = _('Outbound requests') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? 'Collapse' : 'Expand' + %button.btn.btn-default.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') %p = _('Allow requests to the local network from hooks and services.') .settings-content diff --git a/app/views/layouts/_flash.html.haml b/app/views/layouts/_flash.html.haml index 05ddd0ec733..8bd5708d490 100644 --- a/app/views/layouts/_flash.html.haml +++ b/app/views/layouts/_flash.html.haml @@ -1,8 +1,10 @@ +- extra_flash_class = local_assigns.fetch(:extra_flash_class, nil) + .flash-container.flash-container-page -# We currently only support `alert`, `notice`, `success` - flash.each do |key, value| -# Don't show a flash message if the message is nil - if value %div{ class: "flash-#{key}" } - %div{ class: (container_class) } + %div{ class: "#{container_class} #{extra_flash_class}" } %span= value diff --git a/app/views/layouts/header/_current_user_dropdown.html.haml b/app/views/layouts/header/_current_user_dropdown.html.haml new file mode 100644 index 00000000000..24b6c490a5a --- /dev/null +++ b/app/views/layouts/header/_current_user_dropdown.html.haml @@ -0,0 +1,22 @@ +- return unless current_user + +%ul + %li.current-user + .user-name.bold + = current_user.name + = current_user.to_reference + %li.divider + - if current_user_menu?(:profile) + %li + = link_to s_("CurrentUser|Profile"), current_user, class: 'profile-link', data: { user: current_user.username } + - if current_user_menu?(:settings) + %li + = link_to s_("CurrentUser|Settings"), profile_path + - if current_user_menu?(:help) + %li + = link_to _("Help"), help_path + - if current_user_menu?(:help) || current_user_menu?(:settings) || current_user_menu?(:profile) + %li.divider + - if current_user_menu?(:sign_out) + %li + = link_to _("Sign out"), destroy_user_session_path, class: "sign-out-link" diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index e6238c0dddb..dc121812406 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -53,22 +53,7 @@ = image_tag avatar_icon_for_user(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar" = sprite_icon('angle-down', css_class: 'caret-down') .dropdown-menu-nav.dropdown-menu-align-right - %ul - %li.current-user - .user-name.bold - = current_user.name - @#{current_user.username} - %li.divider - %li - = link_to "Profile", current_user, class: 'profile-link', data: { user: current_user.username } - %li - = link_to "Settings", profile_path - - if current_user - %li - = link_to "Help", help_path - %li.divider - %li - = link_to "Sign out", destroy_user_session_path, class: "sign-out-link" + = render 'layouts/header/current_user_dropdown' - if header_link?(:admin_impersonation) %li.impersonation = link_to admin_impersonation_path, class: 'impersonation-btn', method: :delete, title: "Stop impersonation", aria: { label: 'Stop impersonation' }, data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do diff --git a/app/views/layouts/terms.html.haml b/app/views/layouts/terms.html.haml new file mode 100644 index 00000000000..a30d6e2688c --- /dev/null +++ b/app/views/layouts/terms.html.haml @@ -0,0 +1,34 @@ +!!! 5 +- @hide_breadcrumbs = true +%html{ lang: I18n.locale, class: page_class } + = render "layouts/head" + + %body{ data: { page: body_data_page } } + .layout-page.terms{ class: page_class } + .content-wrapper.prepend-top-0 + .mobile-overlay + .alert-wrapper + = render "layouts/broadcast" + = render 'layouts/header/read_only_banner' + = render "layouts/flash", extra_flash_class: 'limit-container-width' + + %div{ class: "#{container_class} limit-container-width" } + .content{ id: "content-body" } + .panel.panel-default + .panel-heading + .title + = brand_header_logo + - logo_text = brand_header_logo_type + - if logo_text.present? + %span.logo-text.hidden-xs.prepend-left-8 + = logo_text + - if header_link?(:user_dropdown) + .navbar-collapse.collapse + %ul.nav.navbar-nav + %li.header-user.dropdown + = link_to current_user, class: user_dropdown_class, data: { toggle: "dropdown" } do + = image_tag avatar_icon_for_user(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar" + = sprite_icon('angle-down', css_class: 'caret-down') + .dropdown-menu-nav.dropdown-menu-align-right + = render 'layouts/header/current_user_dropdown' + = yield diff --git a/app/views/users/terms/index.html.haml b/app/views/users/terms/index.html.haml new file mode 100644 index 00000000000..c5406696bdd --- /dev/null +++ b/app/views/users/terms/index.html.haml @@ -0,0 +1,13 @@ +- redirect_params = { redirect: @redirect } if @redirect + +.panel-content.rendered-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/changelogs/unreleased/bvl-enforce-terms.yml b/changelogs/unreleased/bvl-enforce-terms.yml new file mode 100644 index 00000000000..1bb1ecdf623 --- /dev/null +++ b/changelogs/unreleased/bvl-enforce-terms.yml @@ -0,0 +1,5 @@ +--- +title: Allow admins to enforce accepting Terms of Service on an instance +merge_request: 18570 +author: +type: added diff --git a/config/routes/user.rb b/config/routes/user.rb index f8677693fab..bc7df5e7584 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -27,6 +27,13 @@ devise_scope :user do get '/users/almost_there' => 'confirmations#almost_there' end +scope '-/users', module: :users do + resources :terms, only: [:index] do + post :accept, on: :member + post :decline, on: :member + end +end + scope(constraints: { username: Gitlab::PathRegex.root_namespace_route_regex }) do scope(path: 'users/:username', as: :user, diff --git a/db/migrate/20180424090541_add_enforce_terms_to_application_settings.rb b/db/migrate/20180424090541_add_enforce_terms_to_application_settings.rb new file mode 100644 index 00000000000..306cd737771 --- /dev/null +++ b/db/migrate/20180424090541_add_enforce_terms_to_application_settings.rb @@ -0,0 +1,9 @@ +class AddEnforceTermsToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :application_settings, :enforce_terms, :boolean, default: false + end +end diff --git a/db/migrate/20180424134533_create_application_setting_terms.rb b/db/migrate/20180424134533_create_application_setting_terms.rb new file mode 100644 index 00000000000..f29335cfc51 --- /dev/null +++ b/db/migrate/20180424134533_create_application_setting_terms.rb @@ -0,0 +1,13 @@ +class CreateApplicationSettingTerms < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :application_setting_terms do |t| + t.integer :cached_markdown_version + t.text :terms, null: false + t.text :terms_html + end + end +end diff --git a/db/migrate/20180425075446_create_term_agreements.rb b/db/migrate/20180425075446_create_term_agreements.rb new file mode 100644 index 00000000000..22a9d7b574d --- /dev/null +++ b/db/migrate/20180425075446_create_term_agreements.rb @@ -0,0 +1,28 @@ +class CreateTermAgreements < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + create_table :term_agreements do |t| + t.references :term, index: true, null: false + t.foreign_key :application_setting_terms, column: :term_id + t.references :user, index: true, null: false, foreign_key: { on_delete: :cascade } + t.boolean :accepted, default: false, null: false + + t.timestamps_with_timezone null: false + end + + add_index :term_agreements, [:user_id, :term_id], + unique: true, + name: 'term_agreements_unique_index' + end + + def down + remove_index :term_agreements, name: 'term_agreements_unique_index' + + drop_table :term_agreements + end +end diff --git a/db/migrate/20180426102016_add_accepted_term_to_users.rb b/db/migrate/20180426102016_add_accepted_term_to_users.rb new file mode 100644 index 00000000000..3d446f66214 --- /dev/null +++ b/db/migrate/20180426102016_add_accepted_term_to_users.rb @@ -0,0 +1,23 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddAcceptedTermToUsers < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + change_table :users do |t| + t.references :accepted_term, + null: true + end + add_concurrent_foreign_key :users, :application_setting_terms, column: :accepted_term_id + end + + def down + remove_foreign_key :users, column: :accepted_term_id + remove_column :users, :accepted_term_id + end +end diff --git a/db/schema.rb b/db/schema.rb index a37e6edc8d1..277b14ef7ed 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -40,6 +40,12 @@ ActiveRecord::Schema.define(version: 20180503150427) do t.text "new_project_guidelines_html" end + create_table "application_setting_terms", force: :cascade do |t| + t.integer "cached_markdown_version" + t.text "terms", null: false + t.text "terms_html" + end + create_table "application_settings", force: :cascade do |t| t.integer "default_projects_limit" t.boolean "signup_enabled" @@ -158,6 +164,7 @@ ActiveRecord::Schema.define(version: 20180503150427) do t.string "auto_devops_domain" t.boolean "pages_domain_verification_enabled", default: true, null: false t.boolean "allow_local_requests_from_hooks_and_services", default: false, null: false + t.boolean "enforce_terms", default: false end create_table "audit_events", force: :cascade do |t| @@ -1817,6 +1824,18 @@ ActiveRecord::Schema.define(version: 20180503150427) do add_index "tags", ["name"], name: "index_tags_on_name", unique: true, using: :btree + create_table "term_agreements", force: :cascade do |t| + t.integer "term_id", null: false + t.integer "user_id", null: false + t.boolean "accepted", default: false, null: false + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false + end + + add_index "term_agreements", ["term_id"], name: "index_term_agreements_on_term_id", using: :btree + add_index "term_agreements", ["user_id", "term_id"], name: "term_agreements_unique_index", unique: true, using: :btree + add_index "term_agreements", ["user_id"], name: "index_term_agreements_on_user_id", using: :btree + create_table "timelogs", force: :cascade do |t| t.integer "time_spent", null: false t.integer "user_id" @@ -2005,6 +2024,7 @@ ActiveRecord::Schema.define(version: 20180503150427) do t.string "preferred_language" t.string "rss_token" t.integer "theme_id", limit: 2 + t.integer "accepted_term_id" end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree @@ -2205,6 +2225,8 @@ ActiveRecord::Schema.define(version: 20180503150427) do add_foreign_key "snippets", "projects", name: "fk_be41fd4bb7", on_delete: :cascade add_foreign_key "subscriptions", "projects", on_delete: :cascade add_foreign_key "system_note_metadata", "notes", name: "fk_d83a918cb1", on_delete: :cascade + add_foreign_key "term_agreements", "application_setting_terms", column: "term_id" + add_foreign_key "term_agreements", "users", on_delete: :cascade add_foreign_key "timelogs", "issues", name: "fk_timelogs_issues_issue_id", on_delete: :cascade add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade add_foreign_key "todos", "notes", name: "fk_91d1f47b13", on_delete: :cascade @@ -2218,6 +2240,7 @@ ActiveRecord::Schema.define(version: 20180503150427) do add_foreign_key "user_interacted_projects", "projects", name: "fk_722ceba4f7", on_delete: :cascade add_foreign_key "user_interacted_projects", "users", name: "fk_0894651f08", on_delete: :cascade add_foreign_key "user_synced_attributes_metadata", "users", on_delete: :cascade + add_foreign_key "users", "application_setting_terms", column: "accepted_term_id", name: "fk_789cd90b35", on_delete: :cascade add_foreign_key "users_star_projects", "projects", name: "fk_22cd27ddfc", on_delete: :cascade add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade add_foreign_key "web_hooks", "projects", name: "fk_0c8ca6d9d1", on_delete: :cascade diff --git a/doc/administration/index.md b/doc/administration/index.md index b472ca5b4d8..5551a04959c 100644 --- a/doc/administration/index.md +++ b/doc/administration/index.md @@ -40,6 +40,7 @@ Learn how to install, configure, update, and maintain your GitLab instance. [source installations](../install/installation.md#installation-from-source). - [Environment variables](environment_variables.md): Supported environment variables that can be used to override their defaults values in order to configure GitLab. - [Plugins](plugins.md): With custom plugins, GitLab administrators can introduce custom integrations without modifying GitLab's source code. +- [Enforcing Terms of Service](../user/admin_area/settings/terms.md) #### Customizing GitLab's appearance 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/doc/user/admin_area/settings/img/enforce_terms.png b/doc/user/admin_area/settings/img/enforce_terms.png Binary files differnew file mode 100755 index 00000000000..e5f0a2683b5 --- /dev/null +++ b/doc/user/admin_area/settings/img/enforce_terms.png diff --git a/doc/user/admin_area/settings/img/respond_to_terms.png b/doc/user/admin_area/settings/img/respond_to_terms.png Binary files differnew file mode 100755 index 00000000000..d0d086c3498 --- /dev/null +++ b/doc/user/admin_area/settings/img/respond_to_terms.png diff --git a/doc/user/admin_area/settings/terms.md b/doc/user/admin_area/settings/terms.md new file mode 100644 index 00000000000..8e1fb982aba --- /dev/null +++ b/doc/user/admin_area/settings/terms.md @@ -0,0 +1,38 @@ +# Enforce accepting Terms of Service + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18570) +> in [GitLab Core](https://about.gitlab.com/pricing/) 10.8 + +## Configuration + +When it is required for all users of the GitLab instance to accept the +Terms of Service, this can be configured by an admin on the settings +page: + +![Enable enforcing Terms of Service](img/enforce_terms.png). + +The terms itself can be entered using Markdown. For each update to the +terms, a new version is stored. When a user accepts or declines the +terms, GitLab will keep track of which version they accepted or +declined. + +When an admin enables this feature, they will automattically be +directed to the page to accept the terms themselves. After they +accept, they will be directed back to the settings page. + +## Accepting terms + +When this feature was enabled, the users that have not accepted the +terms of service will be presented with a screen where they can either +accept or decline the terms. + +![Respond to terms](img/respond_to_terms.png) + +When the user accepts the terms, they will be directed to where they +were going. After a sign-in or sign-up this will most likely be the +dashboard. + +When the user was already logged in when the feature was turned on, +they will be asked to accept the terms on their next interaction. + +When a user declines the terms, they will be signed out. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 17917b1176f..728c3605131 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8,8 +8,8 @@ msgid "" msgstr "" "Project-Id-Version: gitlab 1.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2018-04-24 13:19+0000\n" -"PO-Revision-Date: 2018-04-24 13:19+0000\n" +"POT-Creation-Date: 2018-05-02 22:28+0200\n" +"PO-Revision-Date: 2018-05-02 22:28+0200\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" "Language: \n" @@ -84,6 +84,9 @@ msgstr "" msgid "%{openOrClose} %{noteable}" msgstr "" +msgid "%{percent}%% complete" +msgstr "" + msgid "%{storage_name}: failed storage access attempt on host:" msgid_plural "%{storage_name}: %{failed_attempts} failed storage access attempts:" msgstr[0] "" @@ -92,6 +95,9 @@ msgstr[1] "" msgid "%{text} is available" msgstr "" +msgid "%{title} changes" +msgstr "" + msgid "(checkout the %{link} for information on how to install it)." msgstr "" @@ -101,6 +107,41 @@ msgstr "" msgid "- show less" msgstr "" +msgid "1 %{type} addition" +msgid_plural "%d %{type} additions" +msgstr[0] "" +msgstr[1] "" + +msgid "1 %{type} modification" +msgid_plural "%d %{type} modifications" +msgstr[0] "" +msgstr[1] "" + +msgid "1 closed issue" +msgid_plural "%d closed issues" +msgstr[0] "" +msgstr[1] "" + +msgid "1 closed merge request" +msgid_plural "%d closed merge requests" +msgstr[0] "" +msgstr[1] "" + +msgid "1 merged merge request" +msgid_plural "%d merged merge requests" +msgstr[0] "" +msgstr[1] "" + +msgid "1 open issue" +msgid_plural "%d open issues" +msgstr[0] "" +msgstr[1] "" + +msgid "1 open merge request" +msgid_plural "%d open merge requests" +msgstr[0] "" +msgstr[1] "" + msgid "1 pipeline" msgid_plural "%d pipelines" msgstr[0] "" @@ -136,6 +177,9 @@ msgstr "" msgid "Abuse reports" msgstr "" +msgid "Accept terms" +msgstr "" + msgid "Access Tokens" msgstr "" @@ -367,6 +411,9 @@ msgstr "" msgid "Assignee" msgstr "" +msgid "Assignee(s)" +msgstr "" + msgid "Attach a file by drag & drop or %{upload_link}" msgstr "" @@ -669,9 +716,39 @@ msgstr "" msgid "CI/CD configuration" msgstr "" +msgid "CICD|An explicit %{ci_file} needs to be specified before you can begin using Continuous Integration and Delivery." +msgstr "" + +msgid "CICD|Auto DevOps (Beta)" +msgstr "" + +msgid "CICD|Auto DevOps will automatically build, test, and deploy your application based on a predefined Continuous Integration and Delivery configuration." +msgstr "" + +msgid "CICD|Disable Auto DevOps" +msgstr "" + +msgid "CICD|Enable Auto DevOps" +msgstr "" + +msgid "CICD|Follow the instance default to either have Auto DevOps enabled or disabled when there is no project specific %{ci_file}." +msgstr "" + +msgid "CICD|Instance default (%{state})" +msgstr "" + msgid "CICD|Jobs" msgstr "" +msgid "CICD|Learn more about Auto DevOps" +msgstr "" + +msgid "CICD|The Auto DevOps pipeline configuration will be used when there is no %{ci_file} in the project." +msgstr "" + +msgid "CICD|You need to specify a domain if you want to use Auto Review Apps and Auto Deploy stages." +msgstr "" + msgid "Cancel" msgstr "" @@ -825,6 +902,9 @@ msgstr "" msgid "CircuitBreakerApiLink|circuitbreaker api" msgstr "" +msgid "Clear search input" +msgstr "" + msgid "Click any <strong>project name</strong> in the project list below to navigate to the project milestone." msgstr "" @@ -1128,6 +1208,12 @@ msgstr "" msgid "ClusterIntegration|properly configured" msgstr "" +msgid "Collapse" +msgstr "" + +msgid "Collapse sidebar" +msgstr "" + msgid "Comment and resolve discussion" msgstr "" @@ -1411,16 +1497,16 @@ msgstr "" msgid "CreateTokenToCloneLink|create a personal access token" msgstr "" -msgid "Creates a new branch from %{branchName}" +msgid "Cron Timezone" msgstr "" -msgid "Creates a new branch from %{branchName} and re-directs to create a new merge request" +msgid "Cron syntax" msgstr "" -msgid "Cron Timezone" +msgid "CurrentUser|Profile" msgstr "" -msgid "Cron syntax" +msgid "CurrentUser|Settings" msgstr "" msgid "Custom notification events" @@ -1465,6 +1551,9 @@ msgstr "" msgid "December" msgstr "" +msgid "Decline and sign out" +msgstr "" + msgid "Define a custom pattern with cron syntax" msgstr "" @@ -1563,12 +1652,18 @@ msgstr "" msgid "Directory name" msgstr "" +msgid "Discard changes" +msgstr "" + msgid "Discard draft" msgstr "" msgid "Dismiss Cycle Analytics introduction box" msgstr "" +msgid "Domain" +msgstr "" + msgid "Don't show again" msgstr "" @@ -1734,6 +1829,9 @@ msgstr "" msgid "Error updating todo status." msgstr "" +msgid "Estimated" +msgstr "" + msgid "EventFilterBy|Filter by all" msgstr "" @@ -1761,6 +1859,12 @@ msgstr "" msgid "Every week (Sundays at 4:00am)" msgstr "" +msgid "Expand" +msgstr "" + +msgid "Expand sidebar" +msgstr "" + msgid "Explore projects" msgstr "" @@ -1797,9 +1901,6 @@ msgstr "" msgid "Fields on this page are now uneditable, you can configure" msgstr "" -msgid "File name" -msgstr "" - msgid "Files" msgstr "" @@ -1901,6 +2002,9 @@ msgstr "" msgid "Got it!" msgstr "" +msgid "Group ID" +msgstr "" + msgid "GroupSettings|Prevent sharing a project within %{group} with other groups" msgstr "" @@ -2023,6 +2127,9 @@ msgstr "" msgid "Import repository" msgstr "" +msgid "Include a Terms of Service agreement that all users must accept." +msgstr "" + msgid "Install Runner on Kubernetes" msgstr "" @@ -2199,6 +2306,9 @@ msgstr "" msgid "Loading the GitLab IDE..." msgstr "" +msgid "Loading..." +msgstr "" + msgid "Lock" msgstr "" @@ -2229,7 +2339,10 @@ msgstr "" msgid "March" msgstr "" -msgid "Mark done" +msgid "Mark todo as done" +msgstr "" + +msgid "Markdown enabled" msgstr "" msgid "Maximum git storage failures" @@ -2244,6 +2357,9 @@ msgstr "" msgid "Members" msgstr "" +msgid "Merge Request:" +msgstr "" + msgid "Merge Requests" msgstr "" @@ -2253,6 +2369,9 @@ msgstr "" msgid "Merge request" msgstr "" +msgid "Merge requests" +msgstr "" + msgid "Merge requests are a place to propose changes you've made to a project and discuss those changes with others" msgstr "" @@ -2313,6 +2432,9 @@ msgstr "" msgid "Move issue" msgstr "" +msgid "Name" +msgstr "" + msgid "Name new label" msgstr "" @@ -2387,6 +2509,9 @@ msgstr "" msgid "No file chosen" msgstr "" +msgid "No files found." +msgstr "" + msgid "No labels created yet." msgstr "" @@ -2675,12 +2800,27 @@ msgstr "" msgid "Pipelines|This project is not currently set up to run pipelines." msgstr "" +msgid "Pipeline|Existing branch name, tag" +msgstr "" + msgid "Pipeline|Retry pipeline" msgstr "" msgid "Pipeline|Retry pipeline #%{pipelineId}?" msgstr "" +msgid "Pipeline|Run Pipeline" +msgstr "" + +msgid "Pipeline|Run on" +msgstr "" + +msgid "Pipeline|Run pipeline" +msgstr "" + +msgid "Pipeline|Search branches" +msgstr "" + msgid "Pipeline|Stop pipeline" msgstr "" @@ -2714,6 +2854,9 @@ msgstr "" msgid "Please <a href=%{link_to_billing} target=\"_blank\" rel=\"noopener noreferrer\">enable billing for one of your projects to be able to create a Kubernetes cluster</a>, then try again." msgstr "" +msgid "Please accept the Terms of Service before continuing." +msgstr "" + msgid "Please select at least one filter to see results" msgstr "" @@ -2798,6 +2941,9 @@ msgstr "" msgid "Programming languages used in this repository" msgstr "" +msgid "Progress" +msgstr "" + msgid "Project '%{project_name}' is in the process of being deleted." msgstr "" @@ -3041,6 +3187,9 @@ msgstr "" msgid "Request Access" msgstr "" +msgid "Require all users to accept Terms of Service when they access GitLab." +msgstr "" + msgid "Reset git storage health information" msgstr "" @@ -3053,6 +3202,9 @@ msgstr "" msgid "Resolve discussion" msgstr "" +msgid "Retry" +msgstr "" + msgid "Retry this job" msgstr "" @@ -3115,6 +3267,9 @@ msgstr "" msgid "Search branches and tags" msgstr "" +msgid "Search files" +msgstr "" + msgid "Search milestones" msgstr "" @@ -3219,6 +3374,9 @@ msgid_plural "Showing %d events" msgstr[0] "" msgstr[1] "" +msgid "Sign out" +msgstr "" + msgid "Sign-in restrictions" msgstr "" @@ -3360,6 +3518,18 @@ msgstr "" msgid "Specify the following URL during the Runner setup:" msgstr "" +msgid "Stage all" +msgstr "" + +msgid "Stage changes" +msgstr "" + +msgid "Staged" +msgstr "" + +msgid "Staged %{type}" +msgstr "" + msgid "StarProject|Star" msgstr "" @@ -3410,6 +3580,9 @@ msgstr[1] "" msgid "Tags" msgstr "" +msgid "Tags:" +msgstr "" + msgid "TagsPage|Browse commits" msgstr "" @@ -3488,6 +3661,12 @@ msgstr "" msgid "Team" msgstr "" +msgid "Terms of Service" +msgstr "" + +msgid "Terms of Service Agreement" +msgstr "" + msgid "The Issue Tracker is the place to add things that need to be improved or solved in a project" msgstr "" @@ -3665,6 +3844,12 @@ msgstr "" msgid "Time between merge request creation and merge/close" msgstr "" +msgid "Time remaining" +msgstr "" + +msgid "Time spent" +msgstr "" + msgid "Time tracking" msgstr "" @@ -3837,6 +4022,9 @@ msgstr "" msgid "Todo" msgstr "" +msgid "Toggle Sidebar" +msgstr "" + msgid "Toggle sidebar" msgstr "" @@ -3861,6 +4049,12 @@ msgstr "" msgid "Trigger this manual action" msgstr "" +msgid "Try again" +msgstr "" + +msgid "Unable to load the diff. %{button_try_again}" +msgstr "" + msgid "Unlock" msgstr "" @@ -3870,6 +4064,21 @@ msgstr "" msgid "Unresolve discussion" msgstr "" +msgid "Unstage all" +msgstr "" + +msgid "Unstage changes" +msgstr "" + +msgid "Unstaged" +msgstr "" + +msgid "Unstaged %{type}" +msgstr "" + +msgid "Unstaged and staged %{type}" +msgstr "" + msgid "Unstar" msgstr "" @@ -3978,6 +4187,9 @@ msgstr "" msgid "Web terminal" msgstr "" +msgid "When enabled, users cannot use GitLab until the terms have been accepted." +msgstr "" + msgid "Wiki" msgstr "" @@ -4200,6 +4412,9 @@ msgstr "" msgid "Your projects" msgstr "" +msgid "ago" +msgstr "" + msgid "among other things" msgstr "" @@ -4223,6 +4438,12 @@ msgstr[1] "" msgid "deploy token" msgstr "" +msgid "disabled" +msgstr "" + +msgid "enabled" +msgstr "" + msgid "estimateCommand|%{slash_command} will update the estimated time with the latest command." msgstr "" @@ -4422,6 +4643,9 @@ msgstr "" msgid "personal access token" msgstr "" +msgid "remaining" +msgstr "" + msgid "remove due date" msgstr "" diff --git a/qa/qa/page/menu/main.rb b/qa/qa/page/menu/main.rb index df93a5fa2d2..d3562effaab 100644 --- a/qa/qa/page/menu/main.rb +++ b/qa/qa/page/menu/main.rb @@ -2,12 +2,15 @@ module QA module Page module Menu class Main < Page::Base + view 'app/views/layouts/header/_current_user_dropdown.html.haml' do + element :user_sign_out_link, 'link_to _("Sign out")' + element :settings_link, 'link_to s_("CurrentUser|Settings")' + end + view 'app/views/layouts/header/_default.html.haml' do element :navbar element :user_avatar element :user_menu, '.dropdown-menu-nav' - element :user_sign_out_link, 'link_to "Sign out"' - element :settings_link, 'link_to "Settings"' end view 'app/views/layouts/nav/_dashboard.html.haml' do diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index fe95d1ef9cd..f0caac40afd 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe ApplicationController do + include TermsHelper + let(:user) { create(:user) } describe '#check_password_expiration' do @@ -406,4 +408,65 @@ describe ApplicationController do end end end + + context 'terms' do + controller(described_class) do + def index + render text: 'authenticated' + end + end + + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + sign_in user + end + + it 'does not query more when terms are enforced' do + control = ActiveRecord::QueryRecorder.new { get :index } + + enforce_terms + + expect { get :index }.not_to exceed_query_limit(control) + end + + context 'when terms are enforced' do + before do + enforce_terms + end + + it 'redirects if the user did not accept the terms' do + get :index + + expect(response).to have_gitlab_http_status(302) + end + + it 'does not redirect when the user accepted terms' do + accept_terms(user) + + get :index + + expect(response).to have_gitlab_http_status(200) + end + + context 'for sessionless users' do + before do + sign_out user + end + + it 'renders a 403 when the sessionless user did not accept the terms' do + get :index, rss_token: user.rss_token, format: :atom + + expect(response).to have_gitlab_http_status(403) + end + + it 'renders a 200 when the sessionless user accepted the terms' do + accept_terms(user) + + get :index, rss_token: user.rss_token, format: :atom + + expect(response).to have_gitlab_http_status(200) + end + end + end + end end diff --git a/spec/controllers/concerns/continue_params_spec.rb b/spec/controllers/concerns/continue_params_spec.rb new file mode 100644 index 00000000000..e2f683ae393 --- /dev/null +++ b/spec/controllers/concerns/continue_params_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' + +describe ContinueParams do + let(:controller_class) do + Class.new(ActionController::Base) do + include ContinueParams + + def request + @request ||= Struct.new(:host, :port).new('test.host', 80) + end + end + end + subject(:controller) { controller_class.new } + + def strong_continue_params(params) + ActionController::Parameters.new(continue: params) + end + + it 'cleans up any params that are not allowed' do + allow(controller).to receive(:params) do + strong_continue_params(to: '/hello', + notice: 'world', + notice_now: '!', + something: 'else') + end + + expect(controller.continue_params.keys).to contain_exactly(*%w(to notice notice_now)) + end + + it 'does not allow cross host redirection' do + allow(controller).to receive(:params) do + strong_continue_params(to: '//example.com') + end + + expect(controller.continue_params[:to]).to be_nil + end + + it 'allows redirecting to a path with querystring' do + allow(controller).to receive(:params) do + strong_continue_params(to: '/hello/world?query=string') + end + + expect(controller.continue_params[:to]).to eq('/hello/world?query=string') + end +end diff --git a/spec/controllers/concerns/internal_redirect_spec.rb b/spec/controllers/concerns/internal_redirect_spec.rb new file mode 100644 index 00000000000..a0ee13b2352 --- /dev/null +++ b/spec/controllers/concerns/internal_redirect_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe InternalRedirect do + let(:controller_class) do + Class.new do + include InternalRedirect + + def request + @request ||= Struct.new(:host, :port).new('test.host', 80) + end + end + end + subject(:controller) { controller_class.new } + + describe '#safe_redirect_path' do + it 'is `nil` for invalid uris' do + expect(controller.safe_redirect_path('Hello world')).to be_nil + end + + it 'is `nil` for paths trying to include a host' do + expect(controller.safe_redirect_path('//example.com/hello/world')).to be_nil + end + + it 'returns the path if it is valid' do + expect(controller.safe_redirect_path('/hello/world')).to eq('/hello/world') + end + + it 'returns the path with querystring if it is valid' do + expect(controller.safe_redirect_path('/hello/world?hello=world#L123')) + .to eq('/hello/world?hello=world#L123') + end + end + + describe '#safe_redirect_path_for_url' do + it 'is `nil` for invalid urls' do + expect(controller.safe_redirect_path_for_url('Hello world')).to be_nil + end + + it 'is `nil` for urls from a with a different host' do + expect(controller.safe_redirect_path_for_url('http://example.com/hello/world')).to be_nil + end + + it 'is `nil` for urls from a with a different port' do + expect(controller.safe_redirect_path_for_url('http://test.host:3000/hello/world')).to be_nil + end + + it 'returns the path if the url is on the same host' do + expect(controller.safe_redirect_path_for_url('http://test.host/hello/world')).to eq('/hello/world') + end + + it 'returns the path including querystring if the url is on the same host' do + expect(controller.safe_redirect_path_for_url('http://test.host/hello/world?hello=world#L123')) + .to eq('/hello/world?hello=world#L123') + end + end + + describe '#host_allowed?' do + it 'allows uris with the same host and port' do + expect(controller.host_allowed?(URI('http://test.host/test'))).to be(true) + end + + it 'rejects uris with other host and port' do + expect(controller.host_allowed?(URI('http://example.com/test'))).to be(false) + end + end +end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 55bd4352bd3..555b186fe31 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -265,7 +265,7 @@ describe SessionsController do it 'redirects correctly for referer on same host with params' do search_path = '/search?search=seed_project' allow(controller.request).to receive(:referer) - .and_return('http://%{host}%{path}' % { host: Gitlab.config.gitlab.host, path: search_path }) + .and_return('http://%{host}%{path}' % { host: 'test.host', path: search_path }) get(:new, redirect_to_referer: :yes) diff --git a/spec/controllers/users/terms_controller_spec.rb b/spec/controllers/users/terms_controller_spec.rb new file mode 100644 index 00000000000..a744463413c --- /dev/null +++ b/spec/controllers/users/terms_controller_spec.rb @@ -0,0 +1,81 @@ +require 'spec_helper' + +describe Users::TermsController do + let(:user) { create(:user) } + let(:term) { create(:term) } + + before do + sign_in user + end + + describe 'GET #index' do + it 'redirects when no terms exist' do + get :index + + expect(response).to have_gitlab_http_status(:redirect) + end + + it 'shows terms when they exist' do + 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 + + it 'redirects to the referer when no redirect specified' do + request.env["HTTP_REFERER"] = groups_url + + post :accept, id: term.id + + expect(response).to redirect_to(groups_path) + end + + context 'redirecting to another domain' do + it 'is prevented when passing a redirect param' do + post :accept, id: term.id, redirect: '//example.com/random/path' + + expect(response).to redirect_to(root_path) + end + + it 'is prevented when redirecting to the referer' do + request.env["HTTP_REFERER"] = 'http://example.com/and/a/path' + + post :accept, id: term.id + + expect(response).to redirect_to(root_path) + end + 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/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..f2f9b734c39 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -2,10 +2,13 @@ require 'spec_helper' feature 'Admin updates settings' do include StubENV + include TermsHelper + + let(:admin) { create(:admin) } before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - sign_in(create(:admin)) + sign_in(admin) visit admin_application_settings_path end @@ -85,6 +88,22 @@ feature 'Admin updates settings' do expect(page).to have_content "Application settings saved successfully" end + scenario 'Terms of Service' do + # Already have the admin accept terms, so they don't need to accept in this spec. + _existing_terms = create(:term) + accept_terms(admin) + + 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/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 9e10bfb2adc..94a2b289e64 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' feature 'Login' do + include TermsHelper + scenario 'Successful user signin invalidates password reset token' do user = create(:user) @@ -399,4 +401,41 @@ feature 'Login' do expect(page).to have_selector('.tab-pane.active', count: 1) end end + + context 'when terms are enforced' do + let(:user) { create(:user) } + + before do + enforce_terms + end + + it 'asks to accept the terms on first login' do + visit new_user_session_path + + fill_in 'user_login', with: user.email + fill_in 'user_password', with: '12345678' + + click_button 'Sign in' + + expect_to_be_on_terms_page + + click_button 'Accept terms' + + expect(current_path).to eq(root_path) + expect(page).not_to have_content('You are already signed in.') + end + + it 'does not ask for terms when the user already accepted them' do + accept_terms(user) + + visit new_user_session_path + + fill_in 'user_login', with: user.email + fill_in 'user_password', with: '12345678' + + click_button 'Sign in' + + expect(current_path).to eq(root_path) + end + end end diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb index 5d539f0ccbe..b5bd5c505f2 100644 --- a/spec/features/users/signup_spec.rb +++ b/spec/features/users/signup_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe 'Signup' do + include TermsHelper + let(:new_user) { build_stubbed(:user) } describe 'username validation', :js do @@ -132,4 +134,27 @@ describe 'Signup' do expect(page.body).not_to match(/#{new_user.password}/) end end + + context 'when terms are enforced' do + before do + enforce_terms + end + + it 'asks the user to accept terms before going to the dashboard' 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 + click_button "Register" + + expect_to_be_on_terms_page + + click_button 'Accept terms' + + expect(current_path).to eq dashboard_projects_path + end + end end diff --git a/spec/features/users/terms_spec.rb b/spec/features/users/terms_spec.rb new file mode 100644 index 00000000000..bf6b5fa3d6a --- /dev/null +++ b/spec/features/users/terms_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +describe 'Users > Terms' do + include TermsHelper + + let(:user) { create(:user) } + let!(:term) { create(:term, terms: 'By accepting, you promise to be nice!') } + + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + sign_in(user) + 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 + + context 'terms were enforced while session is active', :js do + let(:project) { create(:project) } + + before do + project.add_developer(user) + end + + it 'redirects to terms and back to where the user was going' do + visit project_path(project) + + enforce_terms + + within('.nav-sidebar') do + click_link 'Issues' + end + + expect_to_be_on_terms_page + + click_button('Accept terms') + + expect(current_path).to eq(project_issues_path(project)) + end + + it 'redirects back to the page the user was trying to save' do + visit new_project_issue_path(project) + + fill_in :issue_title, with: 'Hello world, a new issue' + fill_in :issue_description, with: "We don't want to lose what the user typed" + + enforce_terms + + click_button 'Submit issue' + + expect(current_path).to eq(terms_path) + + click_button('Accept terms') + + expect(current_path).to eq(new_project_issue_path(project)) + expect(find_field('issue_title').value).to eq('Hello world, a new issue') + expect(find_field('issue_description').value).to eq("We don't want to lose what the user typed") + end + end +end diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb index 6332217b920..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 @@ -27,4 +29,39 @@ describe UsersHelper do expect(tabs).to include(:activity, :groups, :contributed, :projects, :snippets) end end + + describe '#current_user_menu_items' do + subject(:items) { helper.current_user_menu_items } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?).and_return(false) + end + + it 'includes all default items' do + expect(items).to include(:help, :sign_out) + end + + it 'includes the profile tab if the user can read themself' do + expect(helper).to receive(:can?).with(user, :read_user, user) { true } + + expect(items).to include(:profile) + end + + it 'includes the settings tab if the user can update themself' do + expect(helper).to receive(:can?).with(user, :read_user, user) { true } + + 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/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/models/term_agreement_spec.rb b/spec/models/term_agreement_spec.rb new file mode 100644 index 00000000000..a59bf119692 --- /dev/null +++ b/spec/models/term_agreement_spec.rb @@ -0,0 +1,8 @@ +require 'spec_helper' + +describe TermAgreement do + describe 'validations' do + it { is_expected.to validate_presence_of(:term) } + it { is_expected.to validate_presence_of(:user) } + 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/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index 5b8cf2e6ab5..ec26810e371 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe GlobalPolicy do + include TermsHelper + let(:current_user) { create(:user) } let(:user) { create(:user) } diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index 6593a6ca3b9..a7a77abc3ee 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -10,28 +10,36 @@ describe UserPolicy do it { is_expected.to be_allowed(:read_user) } end - describe "destroying a user" do + shared_examples 'changing a user' do |ability| context "when a regular user tries to destroy another regular user" do - it { is_expected.not_to be_allowed(:destroy_user) } + it { is_expected.not_to be_allowed(ability) } end context "when a regular user tries to destroy themselves" do let(:current_user) { user } - it { is_expected.to be_allowed(:destroy_user) } + it { is_expected.to be_allowed(ability) } end context "when an admin user tries to destroy a regular user" do let(:current_user) { create(:user, :admin) } - it { is_expected.to be_allowed(:destroy_user) } + it { is_expected.to be_allowed(ability) } end context "when an admin user tries to destroy a ghost user" do let(:current_user) { create(:user, :admin) } let(:user) { create(:user, :ghost) } - it { is_expected.not_to be_allowed(:destroy_user) } + it { is_expected.not_to be_allowed(ability) } end end + + describe "destroying a user" do + it_behaves_like 'changing a user', :destroy_user + end + + describe "updating a user" do + it_behaves_like 'changing a user', :update_user + end end 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 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 diff --git a/spec/support/helpers/terms_helper.rb b/spec/support/helpers/terms_helper.rb new file mode 100644 index 00000000000..a00ec14138b --- /dev/null +++ b/spec/support/helpers/terms_helper.rb @@ -0,0 +1,19 @@ +module TermsHelper + def enforce_terms + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + settings = Gitlab::CurrentSettings.current_application_settings + ApplicationSettings::UpdateService.new( + settings, nil, terms: 'These are the terms', enforce_terms: true + ).execute + end + + def accept_terms(user) + terms = Gitlab::CurrentSettings.current_application_settings.latest_terms + Users::RespondToTermsService.new(user, terms).execute(accepted: true) + end + + def expect_to_be_on_terms_page + expect(current_path).to eq terms_path + expect(page).to have_content('Please accept the Terms of Service before continuing.') + end +end |