+
e # rubocop:disable Lint/RescueException
attempt_restore_repositories(source_project)
- if e.class == Exception
+ if e.instance_of?(Exception)
raise StandardError, e.message
else
raise
diff --git a/app/services/releases/base_service.rb b/app/services/releases/base_service.rb
index 9dd0c9a007a..b4b493624e7 100644
--- a/app/services/releases/base_service.rb
+++ b/app/services/releases/base_service.rb
@@ -5,6 +5,8 @@ module Releases
include BaseServiceUtility
include Gitlab::Utils::StrongMemoize
+ ReleaseProtectedTagAccessError = Class.new(StandardError)
+
attr_accessor :project, :current_user, :params
def initialize(project, user = nil, params = {})
@@ -81,6 +83,15 @@ module Releases
release.execute_hooks(action)
end
+ def track_protected_tag_access_error!
+ unless ::Gitlab::UserAccess.new(current_user, container: project).can_create_tag?(tag_name)
+ Gitlab::ErrorTracking.log_exception(
+ ReleaseProtectedTagAccessError.new,
+ project_id: project.id,
+ user_id: current_user.id)
+ end
+ end
+
# overridden in EE
def project_group_id; end
end
diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb
index 1096e207e02..52d95872414 100644
--- a/app/services/releases/create_service.rb
+++ b/app/services/releases/create_service.rb
@@ -7,6 +7,8 @@ module Releases
return error('Release already exists', 409) if release
return error("Milestone(s) not found: #{inexistent_milestones.join(', ')}", 400) if inexistent_milestones.any?
+ track_protected_tag_access_error!
+
# should be found before the creation of new tag
# because tag creation can spawn new pipeline
# which won't have any data for evidence yet
diff --git a/app/services/releases/destroy_service.rb b/app/services/releases/destroy_service.rb
index 8abf9308689..36cf29c955d 100644
--- a/app/services/releases/destroy_service.rb
+++ b/app/services/releases/destroy_service.rb
@@ -6,6 +6,8 @@ module Releases
return error('Release does not exist', 404) unless release
return error('Access Denied', 403) unless allowed?
+ track_protected_tag_access_error!
+
if release.destroy
success(tag: existing_tag, release: release)
else
diff --git a/app/services/releases/update_service.rb b/app/services/releases/update_service.rb
index 4e78120ac05..eda4b7102c0 100644
--- a/app/services/releases/update_service.rb
+++ b/app/services/releases/update_service.rb
@@ -7,6 +7,8 @@ module Releases
return error
end
+ track_protected_tag_access_error!
+
if param_for_milestone_titles_provided?
previous_milestones = release.milestones.map(&:title)
params[:milestones] = milestones
diff --git a/app/uploaders/dependency_proxy/file_uploader.rb b/app/uploaders/dependency_proxy/file_uploader.rb
index c46539bafaa..5154f180454 100644
--- a/app/uploaders/dependency_proxy/file_uploader.rb
+++ b/app/uploaders/dependency_proxy/file_uploader.rb
@@ -24,7 +24,7 @@ class DependencyProxy::FileUploader < GitlabUploader
# so we must store the custom content type in object storage.
# This does not apply to DependencyProxy::Blob uploads.
def set_content_type(file)
- return unless model.class == DependencyProxy::Manifest
+ return unless model.instance_of?(DependencyProxy::Manifest)
file.content_type = model.content_type
end
diff --git a/config/feature_flags/development/cached_markdown_blob.yml b/config/feature_flags/development/allow_archive_as_web_access_format.yml
similarity index 54%
rename from config/feature_flags/development/cached_markdown_blob.yml
rename to config/feature_flags/development/allow_archive_as_web_access_format.yml
index fcca7f89537..8f5d8101316 100644
--- a/config/feature_flags/development/cached_markdown_blob.yml
+++ b/config/feature_flags/development/allow_archive_as_web_access_format.yml
@@ -1,8 +1,8 @@
---
-name: cached_markdown_blob
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44300
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/263406
-milestone: '13.5'
+name: allow_archive_as_web_access_format
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64471
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334944
+milestone: '14.1'
type: development
-group: group::source code
-default_enabled: true
+group: group::release
+default_enabled: false
diff --git a/doc/ci/variables/index.md b/doc/ci/variables/index.md
index eff726843b1..5a8cf340fd1 100644
--- a/doc/ci/variables/index.md
+++ b/doc/ci/variables/index.md
@@ -71,6 +71,10 @@ to execute scripts. Each shell has its own set of reserved variable names.
Make sure each variable is defined for the [scope you want to use it in](where_variables_can_be_used.md).
+By default, pipelines from forked projects can't access CI/CD variables in the parent project.
+If you [run a merge request pipeline in the parent project for a merge request from a fork](../pipelines/merge_request_pipelines.md#run-pipelines-in-the-parent-project-for-merge-requests-from-a-forked-project),
+all variables become available to the pipeline.
+
### Create a custom CI/CD variable in the `.gitlab-ci.yml` file
To create a custom variable in the [`.gitlab-ci.yml`](../yaml/index.md#variables) file,
@@ -303,6 +307,10 @@ The value of the variable must:
- The `~` character ([In GitLab 13.12](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61517) and later).
- Not match the name of an existing predefined or custom CI/CD variable.
+NOTE:
+Masking a CI/CD variable is not a guaranteed way to prevent malicious users from accessing
+variable values. To make variables more secure, you can [use external secrets](../secrets/index.md).
+
### Protect a CI/CD variable
You can protect a project, group or instance CI/CD variable so it is only passed
diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md
index 76453bb5aa4..a5345f3b52d 100644
--- a/doc/development/documentation/styleguide/index.md
+++ b/doc/development/documentation/styleguide/index.md
@@ -499,7 +499,7 @@ Follow these guidelines for punctuation:
| Do not use tabs for indentation. Use spaces instead. You can configure your code editor to output spaces instead of tabs when pressing the tab key. | --- |
| Use serial commas (_Oxford commas_) before the final _and_ or _or_ in a list of three or more items. (Tested in [`OxfordComma.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/OxfordComma.yml).) | _You can create new issues, merge requests, and milestones._ |
| Always add a space before and after dashes when using it in a sentence (for replacing a comma, for example). | _You should try this - or not._ |
-| Always use lowercase after a colon. | Linked issues: a way to create a relationship between issues._ |
+| When a colon is part of a sentence, always use lowercase after the colon. | _Linked issues: a way to create a relationship between issues._ |
diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb
index 416e36c7ccb..0796f23fbfe 100644
--- a/lib/gitlab/auth/auth_finders.rb
+++ b/lib/gitlab/auth/auth_finders.rb
@@ -89,9 +89,11 @@ module Gitlab
job.user
end
- # We only allow Private Access Tokens with `api` scope to be used by web
+ # We allow Private Access Tokens with `api` scope to be used by web
# requests on RSS feeds or ICS files for backwards compatibility.
# It is also used by GraphQL/API requests.
+ # And to allow accessing /archive programatically as it was a big pain point
+ # for users https://gitlab.com/gitlab-org/gitlab/-/issues/28978.
def find_user_from_web_access_token(request_format, scopes: [:api])
return unless access_token && valid_web_access_format?(request_format)
@@ -269,6 +271,8 @@ module Gitlab
ics_request?
when :api
api_request?
+ when :archive
+ archive_request? if Feature.enabled?(:allow_archive_as_web_access_format, default_enabled: :yaml)
end
end
diff --git a/lib/gitlab/background_migration/user_mentions/models/note.rb b/lib/gitlab/background_migration/user_mentions/models/note.rb
index 7da933c7b11..4026a91903f 100644
--- a/lib/gitlab/background_migration/user_mentions/models/note.rb
+++ b/lib/gitlab/background_migration/user_mentions/models/note.rb
@@ -21,7 +21,7 @@ module Gitlab
belongs_to :project, class_name: "::Gitlab::BackgroundMigration::UserMentions::Models::Project"
def for_personal_snippet?
- noteable && noteable.class.name == 'PersonalSnippet'
+ noteable && noteable.instance_of?(PersonalSnippet)
end
def for_project_noteable?
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index 35581952f4a..0ba23b8ffc7 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -449,7 +449,7 @@ module Gitlab
end
def alternate_viewer_class
- return unless viewer.class == DiffViewer::Renamed
+ return unless viewer.instance_of?(DiffViewer::Renamed)
find_renderable_viewer_class(RICH_VIEWERS) || (DiffViewer::Text if text?)
end
diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb
index a2215366bdc..d5ced2045f5 100644
--- a/lib/gitlab/git.rb
+++ b/lib/gitlab/git.rb
@@ -80,7 +80,7 @@ module Gitlab
def shas_eql?(sha1, sha2)
return true if sha1.nil? && sha2.nil?
return false if sha1.nil? || sha2.nil?
- return false unless sha1.class == sha2.class
+ return false unless sha1.instance_of?(sha2.class)
# If either of the shas is below the minimum length, we cannot be sure
# that they actually refer to the same commit because of hash collision.
diff --git a/lib/gitlab/import_export/relation_tree_restorer.rb b/lib/gitlab/import_export/relation_tree_restorer.rb
index 46b82240ef7..d5f924ae2bd 100644
--- a/lib/gitlab/import_export/relation_tree_restorer.rb
+++ b/lib/gitlab/import_export/relation_tree_restorer.rb
@@ -37,7 +37,7 @@ module Gitlab
ActiveRecord::Base.no_touching do
update_params!
- BulkInsertableAssociations.with_bulk_insert(enabled: @importable.class == ::Project) do
+ BulkInsertableAssociations.with_bulk_insert(enabled: @importable.instance_of?(::Project)) do
fix_ci_pipelines_not_sorted_on_legacy_project_json!
create_relations!
end
diff --git a/spec/frontend/diffs/components/collapsed_files_warning_spec.js b/spec/frontend/diffs/components/collapsed_files_warning_spec.js
index 77c2e19cb68..46caeb01132 100644
--- a/spec/frontend/diffs/components/collapsed_files_warning_spec.js
+++ b/spec/frontend/diffs/components/collapsed_files_warning_spec.js
@@ -1,10 +1,13 @@
import { shallowMount, mount, createLocalVue } from '@vue/test-utils';
+import { nextTick } from 'vue';
import Vuex from 'vuex';
import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue';
import { CENTERED_LIMITED_CONTAINER_CLASSES, EVT_EXPAND_ALL_FILES } from '~/diffs/constants';
import eventHub from '~/diffs/event_hub';
import createStore from '~/diffs/store/modules';
+import file from '../mock_data/diff_file';
+
const propsData = {
limited: true,
mergeable: true,
@@ -12,6 +15,13 @@ const propsData = {
};
const limitedClasses = CENTERED_LIMITED_CONTAINER_CLASSES.split(' ');
+async function files(store, count) {
+ const copies = Array(count).fill(file);
+ store.state.diffs.diffFiles.push(...copies);
+
+ return nextTick();
+}
+
describe('CollapsedFilesWarning', () => {
const localVue = createLocalVue();
let store;
@@ -42,48 +52,63 @@ describe('CollapsedFilesWarning', () => {
wrapper.destroy();
});
- it.each`
- limited | containerClasses
- ${true} | ${limitedClasses}
- ${false} | ${[]}
- `(
- 'has the correct container classes when limited is $limited',
- ({ limited, containerClasses }) => {
- createComponent({ limited });
+ describe('when there is more than one file', () => {
+ it.each`
+ limited | containerClasses
+ ${true} | ${limitedClasses}
+ ${false} | ${[]}
+ `(
+ 'has the correct container classes when limited is $limited',
+ async ({ limited, containerClasses }) => {
+ createComponent({ limited });
+ await files(store, 2);
- expect(wrapper.classes()).toEqual(['col-12'].concat(containerClasses));
- },
- );
+ expect(wrapper.classes()).toEqual(['col-12'].concat(containerClasses));
+ },
+ );
- it.each`
- present | dismissed
- ${false} | ${true}
- ${true} | ${false}
- `('toggles the alert when dismissed is $dismissed', ({ present, dismissed }) => {
- createComponent({ dismissed });
+ it.each`
+ present | dismissed
+ ${false} | ${true}
+ ${true} | ${false}
+ `('toggles the alert when dismissed is $dismissed', async ({ present, dismissed }) => {
+ createComponent({ dismissed });
+ await files(store, 2);
- expect(wrapper.find('[data-testid="root"]').exists()).toBe(present);
+ expect(wrapper.find('[data-testid="root"]').exists()).toBe(present);
+ });
+
+ it('dismisses the component when the alert "x" is clicked', async () => {
+ createComponent({}, { full: true });
+ await files(store, 2);
+
+ expect(wrapper.find('[data-testid="root"]').exists()).toBe(true);
+
+ getAlertCloseButton().element.click();
+
+ await wrapper.vm.$nextTick();
+
+ expect(wrapper.find('[data-testid="root"]').exists()).toBe(false);
+ });
+
+ it(`emits the \`${EVT_EXPAND_ALL_FILES}\` event when the alert action button is clicked`, async () => {
+ createComponent({}, { full: true });
+ await files(store, 2);
+
+ jest.spyOn(eventHub, '$emit');
+
+ getAlertActionButton().vm.$emit('click');
+
+ expect(eventHub.$emit).toHaveBeenCalledWith(EVT_EXPAND_ALL_FILES);
+ });
});
- it('dismisses the component when the alert "x" is clicked', async () => {
- createComponent({}, { full: true });
+ describe('when there is a single file', () => {
+ it('should not display', async () => {
+ createComponent();
+ await files(store, 1);
- expect(wrapper.find('[data-testid="root"]').exists()).toBe(true);
-
- getAlertCloseButton().element.click();
-
- await wrapper.vm.$nextTick();
-
- expect(wrapper.find('[data-testid="root"]').exists()).toBe(false);
- });
-
- it(`emits the \`${EVT_EXPAND_ALL_FILES}\` event when the alert action button is clicked`, () => {
- createComponent({}, { full: true });
-
- jest.spyOn(eventHub, '$emit');
-
- getAlertActionButton().vm.$emit('click');
-
- expect(eventHub.$emit).toHaveBeenCalledWith(EVT_EXPAND_ALL_FILES);
+ expect(wrapper.find('[data-testid="root"]').exists()).toBe(false);
+ });
});
});
diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb
index 7475ed2796f..14200733c19 100644
--- a/spec/lib/gitlab/auth/auth_finders_spec.rb
+++ b/spec/lib/gitlab/auth/auth_finders_spec.rb
@@ -460,7 +460,7 @@ RSpec.describe Gitlab::Auth::AuthFinders do
expect { find_user_from_access_token }.to raise_error(Gitlab::Auth::UnauthorizedError)
end
- context 'no feed or API requests' do
+ context 'no feed, API or archive requests' do
it 'returns nil if the request is not RSS' do
expect(find_user_from_web_access_token(:rss)).to be_nil
end
@@ -472,6 +472,10 @@ RSpec.describe Gitlab::Auth::AuthFinders do
it 'returns nil if the request is not API' do
expect(find_user_from_web_access_token(:api)).to be_nil
end
+
+ it 'returns nil if the request is not ARCHIVE' do
+ expect(find_user_from_web_access_token(:archive)).to be_nil
+ end
end
it 'returns the user for RSS requests' do
@@ -486,6 +490,24 @@ RSpec.describe Gitlab::Auth::AuthFinders do
expect(find_user_from_web_access_token(:ics)).to eq(user)
end
+ it 'returns the user for ARCHIVE requests' do
+ set_header('SCRIPT_NAME', '/-/archive/main.zip')
+
+ expect(find_user_from_web_access_token(:archive)).to eq(user)
+ end
+
+ context 'when allow_archive_as_web_access_format feature flag is disabled' do
+ before do
+ stub_feature_flags(allow_archive_as_web_access_format: false)
+ end
+
+ it 'returns nil for ARCHIVE requests' do
+ set_header('SCRIPT_NAME', '/-/archive/main.zip')
+
+ expect(find_user_from_web_access_token(:archive)).to be_nil
+ end
+ end
+
context 'for API requests' do
it 'returns the user' do
set_header('SCRIPT_NAME', '/api/endpoint')
diff --git a/spec/models/blob_viewer/markup_spec.rb b/spec/models/blob_viewer/markup_spec.rb
index 13b040d62d0..dae1b79dda2 100644
--- a/spec/models/blob_viewer/markup_spec.rb
+++ b/spec/models/blob_viewer/markup_spec.rb
@@ -24,15 +24,5 @@ RSpec.describe BlobViewer::Markup do
expect(subject.banzai_render_context.keys).to include(:rendered)
end
end
-
- context 'when cached_markdown_blob feature flag is disabled' do
- before do
- stub_feature_flags(cached_markdown_blob: false)
- end
-
- it 'does not set cache_key key' do
- expect(subject.banzai_render_context.keys).not_to include(:cache_key)
- end
- end
end
end
diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb
index 90b9742b139..581fef20b62 100644
--- a/spec/requests/api/services_spec.rb
+++ b/spec/requests/api/services_spec.rb
@@ -76,7 +76,7 @@ RSpec.describe API::Services do
required_attributes = service_attrs_list.select do |attr|
service_klass.validators_on(attr).any? do |v|
- v.class == ActiveRecord::Validations::PresenceValidator &&
+ v.instance_of?(ActiveRecord::Validations::PresenceValidator) &&
# exclude presence validators with conditional since those are not really required
![:if, :unless].any? { |cond| v.options.include?(cond) }
end
diff --git a/spec/services/releases/create_service_spec.rb b/spec/services/releases/create_service_spec.rb
index 7287825a0be..bf28fde3d90 100644
--- a/spec/services/releases/create_service_spec.rb
+++ b/spec/services/releases/create_service_spec.rb
@@ -44,6 +44,21 @@ RSpec.describe Releases::CreateService do
it_behaves_like 'a successful release creation'
+ context 'when tag is protected and user does not have access to it' do
+ let!(:protected_tag) { create(:protected_tag, :no_one_can_create, name: '*', project: project) }
+
+ it 'track the error event' do
+ stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
+
+ expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+ kind_of(described_class::ReleaseProtectedTagAccessError),
+ project_id: project.id,
+ user_id: user.id)
+
+ service.execute
+ end
+ end
+
context 'when the tag does not exist' do
let(:tag_name) { 'non-exist-tag' }
diff --git a/spec/services/releases/destroy_service_spec.rb b/spec/services/releases/destroy_service_spec.rb
index bc5bff0b31d..38cdcef3825 100644
--- a/spec/services/releases/destroy_service_spec.rb
+++ b/spec/services/releases/destroy_service_spec.rb
@@ -28,6 +28,21 @@ RSpec.describe Releases::DestroyService do
it 'returns the destroyed object' do
is_expected.to include(status: :success, release: release)
end
+
+ context 'when tag is protected and user does not have access to it' do
+ let!(:protected_tag) { create(:protected_tag, :no_one_can_create, name: '*', project: project) }
+
+ it 'track the error event' do
+ stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
+
+ expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+ kind_of(described_class::ReleaseProtectedTagAccessError),
+ project_id: project.id,
+ user_id: user.id)
+
+ service.execute
+ end
+ end
end
context 'when tag does not exist in the repository' do
diff --git a/spec/services/releases/update_service_spec.rb b/spec/services/releases/update_service_spec.rb
index 932a7fab5ec..96b562a8071 100644
--- a/spec/services/releases/update_service_spec.rb
+++ b/spec/services/releases/update_service_spec.rb
@@ -38,6 +38,21 @@ RSpec.describe Releases::UpdateService do
service.execute
end
+ context 'when tag is protected and user does not have access to it' do
+ let!(:protected_tag) { create(:protected_tag, :no_one_can_create, name: '*', project: project) }
+
+ it 'track the error event' do
+ stub_feature_flags(evalute_protected_tag_for_release_permissions: false)
+
+ expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+ kind_of(described_class::ReleaseProtectedTagAccessError),
+ project_id: project.id,
+ user_id: user.id)
+
+ service.execute
+ end
+ end
+
context 'when the tag does not exists' do
let(:tag_name) { 'foobar' }
diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb
index 4515b96c79e..e48c8125d84 100644
--- a/spec/support/helpers/cycle_analytics_helpers.rb
+++ b/spec/support/helpers/cycle_analytics_helpers.rb
@@ -12,9 +12,7 @@ module CycleAnalyticsHelpers
page.all('.gl-path-button').collect(&:text).map {|name_with_median| name_with_median.split("\n")[0] }
end
- def add_custom_stage_to_form
- page.find_button(s_('CreateValueStreamForm|Add another stage')).click
-
+ def fill_in_custom_stage_fields
index = page.all('[data-testid="value-stream-stage-fields"]').length
last_stage = page.all('[data-testid="value-stream-stage-fields"]').last
@@ -25,6 +23,12 @@ module CycleAnalyticsHelpers
end
end
+ def add_custom_stage_to_form
+ page.find_button(s_('CreateValueStreamForm|Add another stage')).click
+
+ fill_in_custom_stage_fields
+ end
+
def save_value_stream(custom_value_stream_name)
fill_in 'create-value-stream-name', with: custom_value_stream_name
diff --git a/spec/support/shared_examples/lib/gitlab/import_export/relation_factory_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/import_export/relation_factory_shared_examples.rb
index 33061f17bde..3c5c65f0690 100644
--- a/spec/support/shared_examples/lib/gitlab/import_export/relation_factory_shared_examples.rb
+++ b/spec/support/shared_examples/lib/gitlab/import_export/relation_factory_shared_examples.rb
@@ -12,7 +12,7 @@ RSpec.shared_examples 'Notes user references' do
'id' => 111,
'access_level' => 30,
'source_id' => 1,
- 'source_type' => importable.class.name == 'Project' ? 'Project' : 'Namespace',
+ 'source_type' => importable.instance_of?(Project) ? 'Project' : 'Namespace',
'user_id' => 3,
'notification_level' => 3,
'created_at' => '2016-11-18T09:29:42.634Z',