Commit bc299f54 authored by GitLab Bot's avatar GitLab Bot

Add latest changes from gitlab-org/security/gitlab@13-11-stable-ee

parent 2fad4108
...@@ -171,10 +171,11 @@ GEM ...@@ -171,10 +171,11 @@ GEM
capybara-screenshot (1.0.22) capybara-screenshot (1.0.22)
capybara (>= 1.0, < 4) capybara (>= 1.0, < 4)
launchy launchy
carrierwave (1.3.1) carrierwave (1.3.2)
activemodel (>= 4.0.0) activemodel (>= 4.0.0)
activesupport (>= 4.0.0) activesupport (>= 4.0.0)
mime-types (>= 1.16) mime-types (>= 1.16)
ssrf_filter (~> 1.0)
cbor (0.5.9.6) cbor (0.5.9.6)
character_set (1.4.0) character_set (1.4.0)
charlock_holmes (0.7.7) charlock_holmes (0.7.7)
...@@ -1205,6 +1206,7 @@ GEM ...@@ -1205,6 +1206,7 @@ GEM
sprockets (>= 3.0.0) sprockets (>= 3.0.0)
sqlite3 (1.3.13) sqlite3 (1.3.13)
sshkey (2.0.0) sshkey (2.0.0)
ssrf_filter (1.0.7)
stackprof (0.2.15) stackprof (0.2.15)
state_machines (0.5.0) state_machines (0.5.0)
state_machines-activemodel (0.8.0) state_machines-activemodel (0.8.0)
......
...@@ -7,11 +7,15 @@ ...@@ -7,11 +7,15 @@
module SessionlessAuthentication module SessionlessAuthentication
# This filter handles personal access tokens, atom requests with rss tokens, and static object tokens # This filter handles personal access tokens, atom requests with rss tokens, and static object tokens
def authenticate_sessionless_user!(request_format) def authenticate_sessionless_user!(request_format)
user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user(request_format) user = request_authenticator.find_sessionless_user(request_format)
sessionless_sign_in(user) if user sessionless_sign_in(user) if user
end end
def request_authenticator
@request_authenticator ||= Gitlab::Auth::RequestAuthenticator.new(request)
end
def sessionless_user? def sessionless_user?
current_user && !session.key?('warden.user.user.key') current_user && !session.key?('warden.user.user.key')
end end
......
...@@ -110,7 +110,13 @@ class GraphqlController < ApplicationController ...@@ -110,7 +110,13 @@ class GraphqlController < ApplicationController
end end
def context def context
@context ||= { current_user: current_user, is_sessionless_user: !!sessionless_user?, request: request } api_user = !!sessionless_user?
@context ||= {
current_user: current_user,
is_sessionless_user: api_user,
request: request,
scope_validator: ::Gitlab::Auth::ScopeValidator.new(api_user, request_authenticator)
}
end end
def build_variables(variable_info) def build_variables(variable_info)
......
...@@ -44,9 +44,18 @@ module Mutations ...@@ -44,9 +44,18 @@ module Mutations
end end
end end
def self.authorizes_object?
true
end
def self.authorized?(object, context) def self.authorized?(object, context)
# we never provide an object to mutations, but we do need to have a user. auth = ::Gitlab::Graphql::Authorize::ObjectAuthorization.new(:execute_graphql_mutation, :api)
context[:current_user].present? && !context[:current_user].blocked?
return true if auth.ok?(:global, context[:current_user],
scope_validator: context[:scope_validator])
# in our mutations we raise, rather than returning a null value.
raise_resource_not_available_error!
end end
# See: AuthorizeResource#authorized_resource? # See: AuthorizeResource#authorized_resource?
......
...@@ -23,6 +23,7 @@ class GlobalPolicy < BasePolicy ...@@ -23,6 +23,7 @@ class GlobalPolicy < BasePolicy
prevent :receive_notifications prevent :receive_notifications
prevent :use_quick_actions prevent :use_quick_actions
prevent :create_group prevent :create_group
prevent :execute_graphql_mutation
end end
rule { default }.policy do rule { default }.policy do
...@@ -32,6 +33,7 @@ class GlobalPolicy < BasePolicy ...@@ -32,6 +33,7 @@ class GlobalPolicy < BasePolicy
enable :receive_notifications enable :receive_notifications
enable :use_quick_actions enable :use_quick_actions
enable :use_slash_commands enable :use_slash_commands
enable :execute_graphql_mutation
end end
rule { inactive }.policy do rule { inactive }.policy do
...@@ -48,6 +50,8 @@ class GlobalPolicy < BasePolicy ...@@ -48,6 +50,8 @@ class GlobalPolicy < BasePolicy
prevent :use_slash_commands prevent :use_slash_commands
end end
rule { ~can?(:access_api) }.prevent :execute_graphql_mutation
rule { blocked | (internal & ~migration_bot & ~security_bot) }.policy do rule { blocked | (internal & ~migration_bot & ~security_bot) }.policy do
prevent :access_git prevent :access_git
end end
......
...@@ -8,7 +8,10 @@ module Auth ...@@ -8,7 +8,10 @@ module Auth
def execute(authentication_abilities:) def execute(authentication_abilities:)
return error('dependency proxy not enabled', 404) unless ::Gitlab.config.dependency_proxy.enabled return error('dependency proxy not enabled', 404) unless ::Gitlab.config.dependency_proxy.enabled
return error('access forbidden', 403) unless current_user
# Because app/controllers/concerns/dependency_proxy/auth.rb consumes this
# JWT only as `User.find`, we currently only allow User (not DeployToken, etc)
return error('access forbidden', 403) unless current_user.is_a?(User)
{ token: authorized_token.encoded } { token: authorized_token.encoded }
end end
......
---
title: Prevent tokens with only read_api scope from executing mutations
merge_request:
author:
type: security
---
title: Do not allow deploy tokens in the dependency proxy authentication service
merge_request:
author:
type: security
---
title: Bump Carrierwave gem to v1.3.2
merge_request:
author:
type: security
# frozen_string_literal: true
# Wrapper around a RequestAuthenticator to
# perform authorization of scopes. Access is limited to
# only those methods needed to validate that an API user
# has at least one permitted scope.
module Gitlab
module Auth
class ScopeValidator
def initialize(api_user, request_authenticator)
@api_user = api_user
@request_authenticator = request_authenticator
end
def valid_for?(permitted)
return true unless @api_user
return true if permitted.none?
scopes = permitted.map { |s| API::Scope.new(s) }
@request_authenticator.valid_access_token?(scopes: scopes)
end
end
end
end
...@@ -4,10 +4,11 @@ module Gitlab ...@@ -4,10 +4,11 @@ module Gitlab
module Graphql module Graphql
module Authorize module Authorize
class ObjectAuthorization class ObjectAuthorization
attr_reader :abilities attr_reader :abilities, :permitted_scopes
def initialize(abilities) def initialize(abilities, scopes = %i[api read_api])
@abilities = Array.wrap(abilities).flatten @abilities = Array.wrap(abilities).flatten
@permitted_scopes = Array.wrap(scopes)
end end
def none? def none?
...@@ -18,7 +19,13 @@ module Gitlab ...@@ -18,7 +19,13 @@ module Gitlab
abilities.present? abilities.present?
end end
def ok?(object, current_user) def ok?(object, current_user, scope_validator: nil)
scopes_ok?(scope_validator) && abilities_ok?(object, current_user)
end
private
def abilities_ok?(object, current_user)
return true if none? return true if none?
subject = object.try(:declarative_policy_subject) || object subject = object.try(:declarative_policy_subject) || object
...@@ -26,6 +33,12 @@ module Gitlab ...@@ -26,6 +33,12 @@ module Gitlab
Ability.allowed?(current_user, ability, subject) Ability.allowed?(current_user, ability, subject)
end end
end end
def scopes_ok?(validator)
return true unless validator.present?
validator.valid_for?(permitted_scopes)
end
end end
end end
end end
......
...@@ -31,6 +31,8 @@ RSpec.describe 'Adding a Note' do ...@@ -31,6 +31,8 @@ RSpec.describe 'Adding a Note' do
project.add_developer(current_user) project.add_developer(current_user)
end end
it_behaves_like 'a working GraphQL mutation'
it_behaves_like 'a Note mutation that creates a Note' it_behaves_like 'a Note mutation that creates a Note'
it_behaves_like 'a Note mutation when there are active record validation errors' it_behaves_like 'a Note mutation when there are active record validation errors'
......
...@@ -263,25 +263,21 @@ RSpec.describe JwtController do ...@@ -263,25 +263,21 @@ RSpec.describe JwtController do
let(:credential_user) { group_deploy_token.username } let(:credential_user) { group_deploy_token.username }
let(:credential_password) { group_deploy_token.token } let(:credential_password) { group_deploy_token.token }
it_behaves_like 'with valid credentials' it_behaves_like 'returning response status', :forbidden
end end
context 'with project deploy token' do context 'with project deploy token' do
let(:credential_user) { project_deploy_token.username } let(:credential_user) { project_deploy_token.username }
let(:credential_password) { project_deploy_token.token } let(:credential_password) { project_deploy_token.token }
it_behaves_like 'with valid credentials' it_behaves_like 'returning response status', :forbidden
end end
context 'with invalid credentials' do context 'with invalid credentials' do
let(:credential_user) { 'foo' } let(:credential_user) { 'foo' }
let(:credential_password) { 'bar' } let(:credential_password) { 'bar' }
it 'returns unauthorized' do it_behaves_like 'returning response status', :unauthorized
subject
expect(response).to have_gitlab_http_status(:unauthorized)
end
end end
end end
......
...@@ -13,28 +13,31 @@ RSpec.describe Auth::DependencyProxyAuthenticationService do ...@@ -13,28 +13,31 @@ RSpec.describe Auth::DependencyProxyAuthenticationService do
describe '#execute' do describe '#execute' do
subject { service.execute(authentication_abilities: nil) } subject { service.execute(authentication_abilities: nil) }
shared_examples 'returning' do |status:, message:|
it "returns #{message}", :aggregate_failures do
expect(subject[:http_status]).to eq(status)
expect(subject[:message]).to eq(message)
end
end
context 'dependency proxy is not enabled' do context 'dependency proxy is not enabled' do
before do before do
stub_config(dependency_proxy: { enabled: false }) stub_config(dependency_proxy: { enabled: false })
end end
it 'returns not found' do it_behaves_like 'returning', status: 404, message: 'dependency proxy not enabled'
result = subject
expect(result[:http_status]).to eq(404)
expect(result[:message]).to eq('dependency proxy not enabled')
end
end end
context 'without a user' do context 'without a user' do
let(:user) { nil } let(:user) { nil }
it 'returns forbidden' do it_behaves_like 'returning', status: 403, message: 'access forbidden'
result = subject end
context 'with a deploy token as user' do
let_it_be(:user) { create(:deploy_token) }
expect(result[:http_status]).to eq(403) it_behaves_like 'returning', status: 403, message: 'access forbidden'
expect(result[:message]).to eq('access forbidden')
end
end end
context 'with a user' do context 'with a user' do
......
...@@ -20,8 +20,9 @@ RSpec.describe Projects::DownloadService do ...@@ -20,8 +20,9 @@ RSpec.describe Projects::DownloadService do
context 'for URLs that are on the whitelist' do context 'for URLs that are on the whitelist' do
before do before do
stub_request(:get, 'http://mycompany.fogbugz.com/rails_sample.jpg').to_return(body: File.read(Rails.root + 'spec/fixtures/rails_sample.jpg')) # `ssrf_filter` resolves the hostname. See https://github.com/carrierwaveuploader/carrierwave/commit/91714adda998bc9e8decf5b1f5d260d808761304
stub_request(:get, 'http://mycompany.fogbugz.com/doc_sample.txt').to_return(body: File.read(Rails.root + 'spec/fixtures/doc_sample.txt')) stub_request(:get, %r{http://[\d\.]+/rails_sample.jpg}).to_return(body: File.read(Rails.root + 'spec/fixtures/rails_sample.jpg'))
stub_request(:get, %r{http://[\d\.]+/doc_sample.txt}).to_return(body: File.read(Rails.root + 'spec/fixtures/doc_sample.txt'))
end end
context 'an image file' do context 'an image file' do
......
...@@ -396,17 +396,21 @@ module GraphqlHelpers ...@@ -396,17 +396,21 @@ module GraphqlHelpers
post api('/', current_user, version: 'graphql'), params: { _json: queries }, headers: headers post api('/', current_user, version: 'graphql'), params: { _json: queries }, headers: headers
end end
def post_graphql(query, current_user: nil, variables: nil, headers: {}) def post_graphql(query, current_user: nil, variables: nil, headers: {}, token: {})
params = { query: query, variables: serialize_variables(variables) } params = { query: query, variables: serialize_variables(variables) }
post api('/', current_user, version: 'graphql'), params: params, headers: headers post api('/', current_user, version: 'graphql', **token), params: params, headers: headers
if graphql_errors # Errors are acceptable, but not this one: return unless graphql_errors
expect(graphql_errors).not_to include(a_hash_including('message' => 'Internal server error'))
end # Errors are acceptable, but not this one:
expect(graphql_errors).not_to include(a_hash_including('message' => 'Internal server error'))
end end
def post_graphql_mutation(mutation, current_user: nil) def post_graphql_mutation(mutation, current_user: nil, token: {})
post_graphql(mutation.query, current_user: current_user, variables: mutation.variables) post_graphql(mutation.query,
current_user: current_user,
variables: mutation.variables,
token: token)
end end
def post_graphql_mutation_with_uploads(mutation, current_user: nil) def post_graphql_mutation_with_uploads(mutation, current_user: nil)
......
...@@ -10,6 +10,52 @@ RSpec.shared_examples 'a working graphql query' do ...@@ -10,6 +10,52 @@ RSpec.shared_examples 'a working graphql query' do
end end
end end
RSpec.shared_examples 'a working GraphQL mutation' do
include GraphqlHelpers
before do
post_graphql_mutation(mutation, current_user: current_user, token: token)
end
shared_examples 'allows access to the mutation' do
let(:scopes) { ['api'] }
it_behaves_like 'a working graphql query' do
it 'returns data' do
expect(graphql_data.compact).not_to be_empty
end
end
end
shared_examples 'prevents access to the mutation' do
let(:scopes) { ['read_api'] }
it 'does not resolve the mutation' do
expect(graphql_data.compact).to be_empty
expect(graphql_errors).to be_present
end
end
context 'with a personal access token' do
let(:token) do
pat = create(:personal_access_token, user: current_user, scopes: scopes)
{ personal_access_token: pat }
end
it_behaves_like 'prevents access to the mutation'
it_behaves_like 'allows access to the mutation'
end
context 'with an OAuth token' do
let(:token) do
{ oauth_access_token: create(:oauth_access_token, resource_owner: current_user, scopes: scopes.join(' ')) }
end
it_behaves_like 'prevents access to the mutation'
it_behaves_like 'allows access to the mutation'
end
end
RSpec.shared_examples 'a mutation on an unauthorized resource' do RSpec.shared_examples 'a mutation on an unauthorized resource' do
it_behaves_like 'a mutation that returns top-level errors', it_behaves_like 'a mutation that returns top-level errors',
errors: [::Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR] errors: [::Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR]
......
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