Commit 7e2d8998 authored by GitLab Bot's avatar GitLab Bot

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

parent a12a8608
......@@ -4,7 +4,11 @@ entry.
## 13.2.5 (2020-08-17)
- No changes.
### Security (2 changes)
- Stop deploy token being mis-used as user in ProjectPolicy and GroupPolicy.
- Project access is checked during deploy token authentication.
## 13.2.4 (2020-08-11)
......
......@@ -166,6 +166,7 @@ class GroupPolicy < BasePolicy
def access_level
return GroupMember::NO_ACCESS if @user.nil?
return GroupMember::NO_ACCESS unless user_is_user?
@access_level ||= lookup_access_level!
end
......@@ -173,6 +174,12 @@ class GroupPolicy < BasePolicy
def lookup_access_level!
@subject.max_member_access_for_user(@user)
end
private
def user_is_user?
user.is_a?(User)
end
end
GroupPolicy.prepend_if_ee('EE::GroupPolicy')
......@@ -603,8 +603,13 @@ class ProjectPolicy < BasePolicy
private
def user_is_user?
user.is_a?(User)
end
def team_member?
return false if @user.nil?
return false unless user_is_user?
greedy_load_subject = false
......@@ -632,6 +637,7 @@ class ProjectPolicy < BasePolicy
# rubocop: disable CodeReuse/ActiveRecord
def project_group_member?
return false if @user.nil?
return false unless user_is_user?
project.group &&
(
......@@ -643,6 +649,7 @@ class ProjectPolicy < BasePolicy
def team_access_level
return -1 if @user.nil?
return -1 unless user_is_user?
lookup_access_level!
end
......
......@@ -218,6 +218,9 @@ module Gitlab
return unless token && login
return if login != token.username
# Registry access (with jwt) does not have access to project
return if project && !token.has_access_to?(project)
scopes = abilities_for_scopes(token.scopes)
if valid_scoped_token?(token, all_available_scopes)
......
......@@ -541,7 +541,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
it 'fails if token is not related to project' do
another_deploy_token = create(:deploy_token)
expect(gl_auth.find_for_git_client(login, another_deploy_token.token, project: project, ip: 'ip'))
expect(gl_auth.find_for_git_client(another_deploy_token.username, another_deploy_token.token, project: project, ip: 'ip'))
.to eq(auth_failure)
end
......@@ -566,6 +566,13 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
expect(subject).to eq(auth_success)
end
it 'fails if token is not related to group' do
another_deploy_token = create(:deploy_token, :group, read_repository: true)
expect(gl_auth.find_for_git_client(another_deploy_token.username, another_deploy_token.token, project: project_with_group, ip: 'ip'))
.to eq(auth_failure)
end
end
context 'when the deploy token has read_registry as a scope' do
......
......@@ -63,6 +63,24 @@ RSpec.describe GroupPolicy do
end
end
shared_examples 'deploy token does not get confused with user' do
before do
deploy_token.update!(id: user_id)
end
let(:deploy_token) { create(:deploy_token) }
let(:current_user) { deploy_token }
it do
expect_disallowed(*read_group_permissions)
expect_disallowed(*guest_permissions)
expect_disallowed(*reporter_permissions)
expect_disallowed(*developer_permissions)
expect_disallowed(*maintainer_permissions)
expect_disallowed(*owner_permissions)
end
end
context 'guests' do
let(:current_user) { guest }
......@@ -74,6 +92,10 @@ RSpec.describe GroupPolicy do
expect_disallowed(*maintainer_permissions)
expect_disallowed(*owner_permissions)
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { guest.id }
end
end
context 'reporter' do
......@@ -87,6 +109,10 @@ RSpec.describe GroupPolicy do
expect_disallowed(*maintainer_permissions)
expect_disallowed(*owner_permissions)
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { reporter.id }
end
end
context 'developer' do
......@@ -100,6 +126,10 @@ RSpec.describe GroupPolicy do
expect_disallowed(*maintainer_permissions)
expect_disallowed(*owner_permissions)
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { developer.id }
end
end
context 'maintainer' do
......@@ -136,6 +166,10 @@ RSpec.describe GroupPolicy do
expect_disallowed(*owner_permissions)
end
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { maintainer.id }
end
end
context 'owner' do
......@@ -149,6 +183,10 @@ RSpec.describe GroupPolicy do
expect_allowed(*maintainer_permissions)
expect_allowed(*owner_permissions)
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { owner.id }
end
end
context 'admin' do
......@@ -166,6 +204,14 @@ RSpec.describe GroupPolicy do
context 'with admin mode', :enable_admin_mode do
specify { expect_allowed(*admin_permissions) }
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { admin.id }
context 'with admin mode', :enable_admin_mode do
it { expect_disallowed(*admin_permissions) }
end
end
end
describe 'private nested group use the highest access level from the group and inherited permissions' do
......
......@@ -193,6 +193,24 @@ RSpec.describe API::MavenPackages do
it_behaves_like 'downloads with a job token'
it_behaves_like 'downloads with a deploy token'
it 'does not allow download by a unauthorized deploy token with same id as a user with access' do
unauthorized_deploy_token = create(:deploy_token, read_package_registry: true, write_package_registry: true)
another_user = create(:user)
project.add_developer(another_user)
# We force the id of the deploy token and the user to be the same
unauthorized_deploy_token.update!(id: another_user.id)
download_file(
package_file.file_name,
{},
Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => unauthorized_deploy_token.token
)
expect(response).to have_gitlab_http_status(:forbidden)
end
end
context 'project name is different from a package name' do
......@@ -451,6 +469,20 @@ RSpec.describe API::MavenPackages do
expect(response).to have_gitlab_http_status(:ok)
end
it 'rejects requests by a unauthorized deploy token with same id as a user with access' do
unauthorized_deploy_token = create(:deploy_token, read_package_registry: true, write_package_registry: true)
another_user = create(:user)
project.add_developer(another_user)
# We force the id of the deploy token and the user to be the same
unauthorized_deploy_token.update!(id: another_user.id)
authorize_upload({}, headers.merge(Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => unauthorized_deploy_token.token))
expect(response).to have_gitlab_http_status(:forbidden)
end
def authorize_upload(params = {}, request_headers = headers)
put api("/projects/#{project.id}/packages/maven/com/example/my-app/#{version}/maven-metadata.xml/authorize"), params: params, headers: request_headers
end
......@@ -538,6 +570,20 @@ RSpec.describe API::MavenPackages do
expect(response).to have_gitlab_http_status(:ok)
end
it 'rejects uploads by a unauthorized deploy token with same id as a user with access' do
unauthorized_deploy_token = create(:deploy_token, read_package_registry: true, write_package_registry: true)
another_user = create(:user)
project.add_developer(another_user)
# We force the id of the deploy token and the user to be the same
unauthorized_deploy_token.update!(id: another_user.id)
upload_file(params, headers.merge(Gitlab::Auth::AuthFinders::DEPLOY_TOKEN_HEADER => unauthorized_deploy_token.token))
expect(response).to have_gitlab_http_status(:forbidden)
end
context 'version is not correct' do
let(:version) { '$%123' }
......
......@@ -547,12 +547,6 @@ RSpec.describe 'Git LFS API and storage' do
project.lfs_objects << lfs_object
end
context 'when Deploy Token is valid' do
let(:deploy_token) { create(:deploy_token, projects: [project]) }
it_behaves_like 'an authorized request', renew_authorization: false
end
context 'when Deploy Token is not valid' do
let(:deploy_token) { create(:deploy_token, projects: [project], read_repository: false) }
......@@ -562,7 +556,14 @@ RSpec.describe 'Git LFS API and storage' do
context 'when Deploy Token is not related to the project' do
let(:deploy_token) { create(:deploy_token, projects: [other_project]) }
it_behaves_like 'LFS http 404 response'
it_behaves_like 'LFS http 401 response'
end
# TODO: We should fix this test case that causes flakyness by alternating the result of the above test cases.
context 'when Deploy Token is valid' do
let(:deploy_token) { create(:deploy_token, projects: [project]) }
it_behaves_like 'an authorized request', renew_authorization: false
end
end
......
......@@ -97,6 +97,28 @@ RSpec.shared_examples 'project policies as anonymous' do
end
end
RSpec.shared_examples 'deploy token does not get confused with user' do
before do
deploy_token.update!(id: user_id)
# Project with public builds are available to all
project.update!(public_builds: false)
end
let(:deploy_token) { create(:deploy_token) }
subject { described_class.new(deploy_token, project) }
it do
expect_disallowed(*guest_permissions)
expect_disallowed(*reporter_permissions)
expect_disallowed(*team_member_reporter_permissions)
expect_disallowed(*developer_permissions)
expect_disallowed(*maintainer_permissions)
expect_disallowed(*owner_permissions)
end
end
RSpec.shared_examples 'project policies as guest' do
subject { described_class.new(guest, project) }
......@@ -115,6 +137,10 @@ RSpec.shared_examples 'project policies as guest' do
expect_disallowed(*owner_permissions)
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { guest.id }
end
it_behaves_like 'archived project policies' do
let(:regular_abilities) { guest_permissions }
end
......@@ -128,7 +154,7 @@ RSpec.shared_examples 'project policies as guest' do
context 'when public builds disabled' do
before do
project.update(public_builds: false)
project.update!(public_builds: false)
end
it do
......@@ -139,7 +165,7 @@ RSpec.shared_examples 'project policies as guest' do
context 'when builds are disabled' do
before do
project.project_feature.update(builds_access_level: ProjectFeature::DISABLED)
project.project_feature.update!(builds_access_level: ProjectFeature::DISABLED)
end
it do
......@@ -165,6 +191,10 @@ RSpec.shared_examples 'project policies as reporter' do
expect_disallowed(*owner_permissions)
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { reporter.id }
end
it_behaves_like 'archived project policies' do
let(:regular_abilities) { reporter_permissions }
end
......@@ -186,6 +216,10 @@ RSpec.shared_examples 'project policies as developer' do
expect_disallowed(*owner_permissions)
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { developer.id }
end
it_behaves_like 'archived project policies' do
let(:regular_abilities) { developer_permissions }
end
......@@ -207,6 +241,10 @@ RSpec.shared_examples 'project policies as maintainer' do
expect_disallowed(*owner_permissions)
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { maintainer.id }
end
it_behaves_like 'archived project policies' do
let(:regular_abilities) { maintainer_permissions }
end
......@@ -228,6 +266,10 @@ RSpec.shared_examples 'project policies as owner' do
expect_allowed(*owner_permissions)
end
it_behaves_like 'deploy token does not get confused with user' do
let(:user_id) { owner.id }
end
it_behaves_like 'archived project policies' do
let(:regular_abilities) { owner_permissions }
end
......@@ -249,6 +291,28 @@ RSpec.shared_examples 'project policies as admin with admin mode' do
expect_allowed(*owner_permissions)
end
context 'deploy token does not get confused with user' do
before do
allow(deploy_token).to receive(:id).and_return(admin.id)
# Project with public builds are available to all
project.update!(public_builds: false)
end
let(:deploy_token) { create(:deploy_token) }
subject { described_class.new(deploy_token, project) }
it do
expect_disallowed(*guest_permissions)
expect_disallowed(*reporter_permissions)
expect_disallowed(*team_member_reporter_permissions)
expect_disallowed(*developer_permissions)
expect_disallowed(*maintainer_permissions)
expect_disallowed(*owner_permissions)
end
end
it_behaves_like 'archived project policies' do
let(:regular_abilities) { owner_permissions }
end
......@@ -268,5 +332,20 @@ RSpec.shared_examples 'project policies as admin without admin mode' do
subject { described_class.new(admin, project) }
it { is_expected.to be_banned }
context 'deploy token does not get confused with user' do
before do
allow(deploy_token).to receive(:id).and_return(admin.id)
# Project with public builds are available to all
project.update!(public_builds: false)
end
let(:deploy_token) { create(:deploy_token) }
subject { described_class.new(deploy_token, project) }
it { is_expected.to be_banned }
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