diff options
author | Kamil TrzciĆski <ayufan@ayufan.eu> | 2018-04-10 07:31:30 +0000 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2018-04-10 19:03:07 -0500 |
commit | 44b89668371dff6b32b7c077f6dddba6fd13d52f (patch) | |
tree | 5feb5899d1dd333f89d08ba8a0b9d9eb9edc6b72 | |
parent | bff933f0f1bfc83bbfbec40c7bd4181c88c175da (diff) | |
download | gitlab-ce-44b89668371dff6b32b7c077f6dddba6fd13d52f.tar.gz |
Merge branch 'deploy-tokens-container-registry-specs' into 'master'
Verify that deploy token has valid access when pulling container registry image
Closes #45148
See merge request gitlab-org/gitlab-ce!18260
5 files changed, 165 insertions, 12 deletions
diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb index fe726b156d4..979e9232fda 100644 --- a/app/models/deploy_token.rb +++ b/app/models/deploy_token.rb @@ -34,7 +34,7 @@ class DeployToken < ActiveRecord::Base end def has_access_to?(requested_project) - project == requested_project + active? && project == requested_project end # This is temporal. Currently we limit DeployToken diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 8f050072f74..f28cddb2af3 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -149,7 +149,8 @@ module Auth def deploy_token_can_pull?(requested_project) has_authentication_ability?(:read_container_image) && current_user.is_a?(DeployToken) && - current_user.has_access_to?(requested_project) + current_user.has_access_to?(requested_project) && + current_user.read_registry? end ## diff --git a/changelogs/unreleased/deploy-tokens-container-registry-specs.yml b/changelogs/unreleased/deploy-tokens-container-registry-specs.yml new file mode 100644 index 00000000000..d86f955c966 --- /dev/null +++ b/changelogs/unreleased/deploy-tokens-container-registry-specs.yml @@ -0,0 +1,5 @@ +--- +title: Verify that deploy token has valid access when pulling container registry image +merge_request: 18260 +author: +type: fixed diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 5a15c23def4..780b200e837 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -78,19 +78,30 @@ describe DeployToken do describe '#has_access_to?' do let(:project) { create(:project) } - subject(:deploy_token) { create(:deploy_token, projects: [project]) } + subject { deploy_token.has_access_to?(project) } - context 'when the deploy token has access to the project' do - it 'should return true' do - expect(deploy_token.has_access_to?(project)).to be_truthy - end + context 'when deploy token is active and related to project' do + let(:deploy_token) { create(:deploy_token, projects: [project]) } + + it { is_expected.to be_truthy } end - context 'when the deploy token does not have access to the project' do - it 'should return false' do - another_project = create(:project) - expect(deploy_token.has_access_to?(another_project)).to be_falsy - end + context 'when deploy token is active but not related to project' do + let(:deploy_token) { create(:deploy_token) } + + it { is_expected.to be_falsy } + end + + context 'when deploy token is revoked and related to project' do + let(:deploy_token) { create(:deploy_token, :revoked, projects: [project]) } + + it { is_expected.to be_falsy } + end + + context 'when deploy token is revoked and not related to the project' do + let(:deploy_token) { create(:deploy_token, :revoked) } + + it { is_expected.to be_falsy } end end diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 290eeae828e..da8e660c16b 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -585,4 +585,140 @@ describe Auth::ContainerRegistryAuthenticationService do it_behaves_like 'not a container repository factory' end end + + context 'for deploy tokens' do + let(:current_params) do + { scope: "repository:#{project.full_path}:pull" } + end + + context 'when deploy token has read_registry as a scope' do + let(:current_user) { create(:deploy_token, projects: [project]) } + + context 'for public project' do + let(:project) { create(:project, :public) } + + context 'when pulling' do + it_behaves_like 'a pullable' + end + + context 'when pushing' do + let(:current_params) do + { scope: "repository:#{project.full_path}:push" } + end + + it_behaves_like 'an inaccessible' + end + end + + context 'for internal project' do + let(:project) { create(:project, :internal) } + + context 'when pulling' do + it_behaves_like 'a pullable' + end + + context 'when pushing' do + let(:current_params) do + { scope: "repository:#{project.full_path}:push" } + end + + it_behaves_like 'an inaccessible' + end + end + + context 'for private project' do + let(:project) { create(:project, :private) } + + context 'when pulling' do + it_behaves_like 'a pullable' + end + + context 'when pushing' do + let(:current_params) do + { scope: "repository:#{project.full_path}:push" } + end + + it_behaves_like 'an inaccessible' + end + end + end + + context 'when deploy token does not have read_registry scope' do + let(:current_user) { create(:deploy_token, projects: [project], read_registry: false) } + + context 'for public project' do + let(:project) { create(:project, :public) } + + context 'when pulling' do + it_behaves_like 'a pullable' + end + end + + context 'for internal project' do + let(:project) { create(:project, :internal) } + + context 'when pulling' do + it_behaves_like 'an inaccessible' + end + end + + context 'for private project' do + let(:project) { create(:project, :internal) } + + context 'when pulling' do + it_behaves_like 'an inaccessible' + end + end + end + + context 'when deploy token is not related to the project' do + let(:current_user) { create(:deploy_token, read_registry: false) } + + context 'for public project' do + let(:project) { create(:project, :public) } + + context 'when pulling' do + it_behaves_like 'a pullable' + end + end + + context 'for internal project' do + let(:project) { create(:project, :internal) } + + context 'when pulling' do + it_behaves_like 'an inaccessible' + end + end + + context 'for private project' do + let(:project) { create(:project, :internal) } + + context 'when pulling' do + it_behaves_like 'an inaccessible' + end + end + end + + context 'when deploy token has been revoked' do + let(:current_user) { create(:deploy_token, :revoked, projects: [project]) } + + context 'for public project' do + let(:project) { create(:project, :public) } + + it_behaves_like 'a pullable' + end + + context 'for internal project' do + let(:project) { create(:project, :internal) } + + it_behaves_like 'an inaccessible' + end + + context 'for private project' do + let(:project) { create(:project, :internal) } + + it_behaves_like 'an inaccessible' + end + end + end end |