Commit 38335946 authored by Reuben Pereira's avatar Reuben Pereira

ContainerRepositoriesFinder now checks container registry visibility

Wherever container_registry_enabled is being checked, we need to
update it to check the actor's permissions as well.

This is because we are migrating towards using
container_registry_access_level which allows for different users to
have different visibility of the container registry.

This commit updates 2 locations where container_registry_enabled
is being checked.
- ContainerRepositoriesFinder
- PackagesHelper

Changelog: changed
parent 57e37611
...@@ -25,8 +25,6 @@ class ContainerRepositoriesFinder ...@@ -25,8 +25,6 @@ class ContainerRepositoriesFinder
end end
def project_repositories def project_repositories
return unless @subject.container_registry_enabled
@subject.container_repositories @subject.container_repositories
end end
......
...@@ -57,7 +57,7 @@ module PackagesHelper ...@@ -57,7 +57,7 @@ module PackagesHelper
def show_cleanup_policy_on_alert(project) def show_cleanup_policy_on_alert(project)
Gitlab.com? && Gitlab.com? &&
Gitlab.config.registry.enabled && Gitlab.config.registry.enabled &&
project.container_registry_enabled && project.feature_available?(:container_registry, current_user) &&
!Gitlab::CurrentSettings.container_expiration_policies_enable_historic_entries && !Gitlab::CurrentSettings.container_expiration_policies_enable_historic_entries &&
Feature.enabled?(:container_expiration_policies_historic_entry, project) && Feature.enabled?(:container_expiration_policies_historic_entry, project) &&
project.container_expiration_policy.nil? && project.container_expiration_policy.nil? &&
......
...@@ -7,12 +7,14 @@ RSpec.describe ContainerRepositoriesFinder do ...@@ -7,12 +7,14 @@ RSpec.describe ContainerRepositoriesFinder do
let_it_be(:guest) { create(:user) } let_it_be(:guest) { create(:user) }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, group: group) } let_it_be(:project) { create(:project, :public, group: group) }
let_it_be(:project_repository) { create(:container_repository, name: 'my_image', project: project) } let_it_be(:project_repository) { create(:container_repository, name: 'my_image', project: project) }
let(:params) { {} } let(:params) { {} }
before do before do
project.project_feature.update!(container_registry_access_level: ProjectFeature::PRIVATE)
group.add_reporter(reporter) group.add_reporter(reporter)
project.add_reporter(reporter) project.add_reporter(reporter)
end end
...@@ -77,6 +79,14 @@ RSpec.describe ContainerRepositoriesFinder do ...@@ -77,6 +79,14 @@ RSpec.describe ContainerRepositoriesFinder do
it_behaves_like 'with name search' it_behaves_like 'with name search'
it_behaves_like 'with sorting' it_behaves_like 'with sorting'
context 'when project has container registry disabled' do
before do
project.project_feature.update!(container_registry_access_level: ProjectFeature::DISABLED)
end
it { is_expected.to match_array([other_repository]) }
end
end end
context 'when subject_type is project' do context 'when subject_type is project' do
...@@ -86,6 +96,14 @@ RSpec.describe ContainerRepositoriesFinder do ...@@ -86,6 +96,14 @@ RSpec.describe ContainerRepositoriesFinder do
it_behaves_like 'with name search' it_behaves_like 'with name search'
it_behaves_like 'with sorting' it_behaves_like 'with sorting'
context 'when project has container registry disabled' do
before do
project.project_feature.update!(container_registry_access_level: ProjectFeature::DISABLED)
end
it { is_expected.to be nil }
end
end end
context 'with invalid subject_type' do context 'with invalid subject_type' do
...@@ -96,9 +114,19 @@ RSpec.describe ContainerRepositoriesFinder do ...@@ -96,9 +114,19 @@ RSpec.describe ContainerRepositoriesFinder do
end end
context 'with unauthorized user' do context 'with unauthorized user' do
subject { described_class.new(user: guest, subject: group).execute } subject { described_class.new(user: guest, subject: subject_type).execute }
it { is_expected.to be nil } context 'when subject_type is group' do
let(:subject_type) { group }
it { is_expected.to be nil }
end
context 'when subject_type is project' do
let(:subject_type) { project }
it { is_expected.to be nil }
end
end end
end end
end end
...@@ -66,6 +66,7 @@ RSpec.describe PackagesHelper do ...@@ -66,6 +66,7 @@ RSpec.describe PackagesHelper do
end end
describe '#show_cleanup_policy_on_alert' do describe '#show_cleanup_policy_on_alert' do
let_it_be(:user) { create(:user) }
let_it_be_with_reload(:container_repository) { create(:container_repository) } let_it_be_with_reload(:container_repository) { create(:container_repository) }
subject { helper.show_cleanup_policy_on_alert(project.reload) } subject { helper.show_cleanup_policy_on_alert(project.reload) }
...@@ -203,9 +204,10 @@ RSpec.describe PackagesHelper do ...@@ -203,9 +204,10 @@ RSpec.describe PackagesHelper do
with_them do with_them do
before do before do
allow(helper).to receive(:current_user).and_return(user)
allow(Gitlab).to receive(:com?).and_return(com) allow(Gitlab).to receive(:com?).and_return(com)
stub_config(registry: { enabled: config_registry }) stub_config(registry: { enabled: config_registry })
allow(project).to receive(:container_registry_enabled).and_return(project_registry) allow(project).to receive(:feature_available?).with(:container_registry, user).and_return(project_registry)
stub_application_setting(container_expiration_policies_enable_historic_entries: historic_entries) stub_application_setting(container_expiration_policies_enable_historic_entries: historic_entries)
stub_feature_flags(container_expiration_policies_historic_entry: false) stub_feature_flags(container_expiration_policies_historic_entry: false)
stub_feature_flags(container_expiration_policies_historic_entry: project) if historic_entry stub_feature_flags(container_expiration_policies_historic_entry: project) if historic_entry
......
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