Commit a7090d00 authored by James Lopez's avatar James Lopez

Merge branch '10-8-stable-prepare-rc13' into '10-8-stable'

Prepare 10.8 RC13 release

See merge request gitlab-org/gitlab-ce!19033
parents 09c4c065 8236be82
Pipeline #22252837 passed with stages
in 35 minutes and 46 seconds
......@@ -42,22 +42,11 @@ module UsersHelper
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?
return items if current_user&.required_terms_not_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 << :help
items << :profile if can?(current_user, :read_user, current_user)
items << :settings if can?(current_user, :update_user, current_user)
items
end
......
......@@ -1193,6 +1193,11 @@ class User < ActiveRecord::Base
accepted_term_id.present?
end
def required_terms_not_accepted?
Gitlab::CurrentSettings.current_application_settings.enforce_terms? &&
!terms_accepted?
end
protected
# override, from Devise::Validatable
......
class GlobalPolicy < BasePolicy
desc "User is blocked"
with_options scope: :user, score: 0
condition(:blocked) { @user.blocked? }
condition(:blocked) { @user&.blocked? }
desc "User is an internal user"
with_options scope: :user, score: 0
condition(:internal) { @user.internal? }
condition(:internal) { @user&.internal? }
desc "User's access has been locked"
with_options scope: :user, score: 0
condition(:access_locked) { @user.access_locked? }
condition(:access_locked) { @user&.access_locked? }
condition(:can_create_fork, scope: :user) { @user.manageable_namespaces.any? { |namespace| @user.can?(:create_projects, namespace) } }
condition(:can_create_fork, scope: :user) { @user && @user.manageable_namespaces.any? { |namespace| @user.can?(:create_projects, namespace) } }
condition(:required_terms_not_accepted, scope: :user, score: 0) do
@user&.required_terms_not_accepted?
end
rule { anonymous }.policy do
prevent :log_in
prevent :access_api
prevent :access_git
prevent :receive_notifications
prevent :use_quick_actions
prevent :create_group
......@@ -38,6 +40,11 @@ class GlobalPolicy < BasePolicy
prevent :use_quick_actions
end
rule { required_terms_not_accepted }.policy do
prevent :access_api
prevent :access_git
end
rule { can_create_group }.policy do
enable :create_group
end
......
---
title: Block access to the API & git for users that did not accept enforced Terms
of Service
merge_request: 18816
author:
type: other
......@@ -45,7 +45,9 @@ module API
user = find_user_from_sources
return unless user
forbidden!('User is blocked') unless Gitlab::UserAccess.new(user).allowed? && user.can?(:access_api)
unless api_access_allowed?(user)
forbidden!(api_access_denied_message(user))
end
user
end
......@@ -72,6 +74,14 @@ module API
end
end
end
def api_access_allowed?(user)
Gitlab::UserAccess.new(user).allowed? && user.can?(:access_api)
end
def api_access_denied_message(user)
Gitlab::Auth::UserAccessDeniedReason.new(user).rejection_message
end
end
module ClassMethods
......
module Gitlab
module Auth
class UserAccessDeniedReason
def initialize(user)
@user = user
end
def rejection_message
case rejection_type
when :internal
'This action cannot be performed by internal users'
when :terms_not_accepted
'You must accept the Terms of Service in order to perform this action. '\
'Please access GitLab from a web browser to accept these terms.'
else
'Your account has been blocked.'
end
end
private
def rejection_type
if @user.internal?
:internal
elsif @user.required_terms_not_accepted?
:terms_not_accepted
else
:blocked
end
end
end
end
end
module Gitlab
class BuildAccess < UserAccess
attr_accessor :user, :project
# This bypasses the `can?(:access_git)`-check we normally do in `UserAccess`
# for CI. That way if a user was able to trigger a pipeline, then the
# build is allowed to clone the project.
def can_access_git?
true
end
end
end
......@@ -2,8 +2,6 @@
# class return an instance of `GitlabAccessStatus`
module Gitlab
class GitAccess
include Gitlab::Utils::StrongMemoize
UnauthorizedError = Class.new(StandardError)
NotFoundError = Class.new(StandardError)
ProjectCreationError = Class.new(StandardError)
......@@ -17,7 +15,6 @@ module Gitlab
deploy_key_upload: 'This deploy key does not have write access to this project.',
no_repo: 'A repository for this project does not exist yet.',
project_not_found: 'The project you were looking for could not be found.',
account_blocked: 'Your account has been blocked.',
command_not_allowed: "The command you're trying to execute is not allowed.",
upload_pack_disabled_over_http: 'Pulling over HTTP is not allowed.',
receive_pack_disabled_over_http: 'Pushing over HTTP is not allowed.',
......@@ -108,8 +105,11 @@ module Gitlab
end
def check_active_user!
if user && !user_access.allowed?
raise UnauthorizedError, ERROR_MESSAGES[:account_blocked]
return unless user
unless user_access.allowed?
message = Gitlab::Auth::UserAccessDeniedReason.new(user).rejection_message
raise UnauthorizedError, message
end
end
......@@ -340,6 +340,8 @@ module Gitlab
def user_access
@user_access ||= if ci?
CiAccess.new
elsif user && request_from_ci_build?
BuildAccess.new(user, project: project)
else
UserAccess.new(user, project: project)
end
......
require 'spec_helper'
describe Gitlab::Auth::UserAccessDeniedReason do
include TermsHelper
let(:user) { build(:user) }
let(:reason) { described_class.new(user) }
describe '#rejection_message' do
subject { reason.rejection_message }
context 'when a user is blocked' do
before do
user.block!
end
it { is_expected.to match /blocked/ }
end
context 'a user did not accept the enforced terms' do
before do
enforce_terms
end
it { is_expected.to match /You must accept the Terms of Service/ }
end
context 'when the user is internal' do
let(:user) { User.ghost }
it { is_expected.to match /This action cannot be performed by internal users/ }
end
end
end
require 'spec_helper'
describe Gitlab::BuildAccess do
let(:user) { create(:user) }
let(:project) { create(:project) }
describe '#can_do_action' do
subject { described_class.new(user, project: project).can_do_action?(:download_code) }
context 'when the user can do an action on the project but cannot access git' do
before do
user.block!
project.add_developer(user)
end
it { is_expected.to be(true) }
end
context 'when the user cannot do an action on the project' do
it { is_expected.to be(false) }
end
end
end
require 'spec_helper'
describe Gitlab::GitAccess do
set(:user) { create(:user) }
include TermsHelper
let(:user) { create(:user) }
let(:actor) { user }
let(:project) { create(:project, :repository) }
......@@ -1040,6 +1042,96 @@ describe Gitlab::GitAccess do
end
end
context 'terms are enforced' do
before do
enforce_terms
end
shared_examples 'access after accepting terms' do
let(:actions) do
[-> { pull_access_check },
-> { push_access_check }]
end
it 'blocks access when the user did not accept terms', :aggregate_failures do
actions.each do |action|
expect { action.call }.to raise_unauthorized(/You must accept the Terms of Service in order to perform this action/)
end
end
it 'allows access when the user accepted the terms', :aggregate_failures do
accept_terms(user)
actions.each do |action|
expect { action.call }.not_to raise_error
end
end
end
describe 'as an anonymous user to a public project' do
let(:actor) { nil }
let(:project) { create(:project, :public, :repository) }
it { expect { pull_access_check }.not_to raise_error }
end
describe 'as a guest to a public project' do
let(:project) { create(:project, :public, :repository) }
it_behaves_like 'access after accepting terms' do
let(:actions) { [-> { pull_access_check }] }
end
end
describe 'as a reporter to the project' do
before do
project.add_reporter(user)
end
it_behaves_like 'access after accepting terms' do
let(:actions) { [-> { pull_access_check }] }
end
end
describe 'as a developer of the project' do
before do
project.add_developer(user)
end
it_behaves_like 'access after accepting terms'
end
describe 'as a master of the project' do
before do
project.add_master(user)
end
it_behaves_like 'access after accepting terms'
end
describe 'as an owner of the project' do
let(:project) { create(:project, :repository, namespace: user.namespace) }
it_behaves_like 'access after accepting terms'
end
describe 'when a ci build clones the project' do
let(:protocol) { 'http' }
let(:authentication_abilities) { [:build_download_code] }
let(:auth_result_type) { :build }
before do
project.add_developer(user)
end
it "doesn't block http pull" do
aggregate_failures do
expect { pull_access_check }.not_to raise_error
end
end
end
end
private
def raise_unauthorized(message)
......
......@@ -2,6 +2,7 @@ require 'spec_helper'
describe User do
include ProjectForksHelper
include TermsHelper
describe 'modules' do
subject { described_class }
......@@ -2728,4 +2729,30 @@ describe User do
.to change { RedirectRoute.where(path: 'foo').count }.by(-1)
end
end
describe '#required_terms_not_accepted?' do
let(:user) { build(:user) }
subject { user.required_terms_not_accepted? }
context "when terms are not enforced" do
it { is_expected.to be_falsy }
end
context "when terms are enforced and accepted by the user" do
before do
enforce_terms
accept_terms(user)
end
it { is_expected.to be_falsy }
end
context "when terms are enforced but the user has not accepted" do
before do
enforce_terms
end
it { is_expected.to be_truthy }
end
end
end
......@@ -90,4 +90,94 @@ describe GlobalPolicy do
it { is_expected.to be_allowed(:update_custom_attribute) }
end
end
shared_examples 'access allowed when terms accepted' do |ability|
it { is_expected.not_to be_allowed(ability) }
it "allows #{ability} when the user accepted the terms" do
accept_terms(current_user)
is_expected.to be_allowed(ability)
end
end
describe 'API access' do
context 'regular user' do
it { is_expected.to be_allowed(:access_api) }
end
context 'admin' do
let(:current_user) { create(:admin) }
it { is_expected.to be_allowed(:access_api) }
end
context 'anonymous' do
let(:current_user) { nil }
it { is_expected.to be_allowed(:access_api) }
end
context 'when terms are enforced' do
before do
enforce_terms
end
context 'regular user' do
it_behaves_like 'access allowed when terms accepted', :access_api
end
context 'admin' do
let(:current_user) { create(:admin) }
it_behaves_like 'access allowed when terms accepted', :access_api
end
context 'anonymous' do
let(:current_user) { nil }
it { is_expected.to be_allowed(:access_api) }
end
end
end
describe 'git access' do
describe 'regular user' do
it { is_expected.to be_allowed(:access_git) }
end
describe 'admin' do
let(:current_user) { create(:admin) }
it { is_expected.to be_allowed(:access_git) }
end
describe 'anonymous' do
let(:current_user) { nil }
it { is_expected.to be_allowed(:access_git) }
end
context 'when terms are enforced' do
before do
enforce_terms
end
context 'regular user' do
it_behaves_like 'access allowed when terms accepted', :access_git
end
context 'admin' do
let(:current_user) { create(:admin) }
it_behaves_like 'access allowed when terms accepted', :access_git
end
context 'anonymous' do
let(:current_user) { nil }
it { is_expected.to be_allowed(:access_git) }
end
end
end
end
......@@ -6,6 +6,7 @@ describe API::Helpers do
include API::APIGuard::HelperMethods
include described_class
include SentryHelper
include TermsHelper
let(:user) { create(:user) }
let(:admin) { create(:admin) }
......@@ -163,6 +164,23 @@ describe API::Helpers do
expect { current_user }.to raise_error /403/
end
context 'when terms are enforced' do
before do
enforce_terms
env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
end
it 'returns a 403 when a user has not accepted the terms' do
expect { current_user }.to raise_error /You must accept the Terms of Service/
end
it 'sets the current user when the user accepted the terms' do
accept_terms(user)
expect(current_user).to eq(user)
end
end
it "sets current_user" do
env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
expect(current_user).to eq(user)
......
require "spec_helper"
describe 'Git HTTP requests' do
include TermsHelper
include GitHttpHelpers
include WorkhorseHelpers
include UserActivitiesHelpers
......@@ -824,4 +825,56 @@ describe 'Git HTTP requests' do
end
end
end
context 'when terms are enforced' do
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:path) { "#{project.full_path}.git" }
let(:env) { { user: user.username, password: user.password } }
before do
project.add_master(user)
enforce_terms
end
it 'blocks git access when the user did not accept terms', :aggregate_failures do
clone_get(path, env) do |response|
expect(response).to have_gitlab_http_status(:forbidden)
end
download(path, env) do |response|
expect(response).to have_gitlab_http_status(:forbidden)
end
upload(path, env) do |response|
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'when the user accepted the terms' do
before do
accept_terms(user)
end
it 'allows clones' do
clone_get(path, env) do |response|
expect(response).to have_gitlab_http_status(:ok)
end
end
it_behaves_like 'pulls are allowed'
it_behaves_like 'pushes are allowed'
end
context 'from CI' do
let(:build) { create(:ci_build, :running) }
let(:env) { { user: 'gitlab-ci-token', password: build.token } }
before do
build.update!(user: user, project: project)
end
it_behaves_like 'pulls are allowed'
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment