Commit 7603beff authored by Douwe Maan's avatar Douwe Maan

Merge branch 'bvl-enforce-terms' into 'master'

Enforce application wide terms

Closes #44798

See merge request gitlab-org/gitlab-ce!18570
parents a1ce521c 39916fdf
......@@ -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();
......
......@@ -61,3 +61,4 @@
@import 'framework/stacked_progress_bar';
@import 'framework/ci_variable_list';
@import 'framework/feature_highlight';
@import 'framework/terms';
.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;
}
}
}
......@@ -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
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
......
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
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
......
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
......@@ -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
......@@ -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
......@@ -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
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
class TermAgreement < ActiveRecord::Base
belongs_to :term, class_name: 'ApplicationSetting::Term'
belongs_to :user
validates :user, :term, presence: true
end
......@@ -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
......
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
......@@ -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
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
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
= 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"
- 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
- 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"
......@@ -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
......
!!! 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
- 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')
---
title: Allow admins to enforce accepting Terms of Service on an instance
merge_request: 18570
author:
type: added
......@@ -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,
......
class AddEnforceTermsToApplicationSettings < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :application_settings, :enforce_terms, :boolean, default: false
end
end
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
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
# 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
......@@ -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
......
......@@ -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
......
......@@ -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!",
}
```
# 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.
This diff is collapsed.
......@@ -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
......
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
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
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
......@@ -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)
......
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
FactoryBot.define do
factory :term_agreement do
term
user
end
end
FactoryBot.define do
factory :term, class: ApplicationSetting::Term do
terms "Lorem ipsum dolor sit amet, consectetur adipiscing elit."
end
end
......@@ -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
......
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
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
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
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
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 @@ 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
......
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
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
require 'spec_helper'
describe GlobalPolicy do
include TermsHelper
let(:current_user) { create(:user) }
let(:user) { create(:user) }
......
......@@ -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
......@@ -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
......
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
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
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
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