From 165c4fb3de28c1b03b1d3a5e05b8962b73eea228 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 1 Mar 2023 18:28:50 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-8-stable-ee --- .../oauth/jira_dvcs/authorizations_controller.rb | 13 +++++ app/finders/notes_finder.rb | 2 +- lib/api/entities/tag.rb | 8 ++- lib/api/tags.rb | 10 +++- lib/banzai/filter/kroki_filter.rb | 6 +- .../jira_dvcs/authorizations_controller_spec.rb | 40 ++++++++++---- spec/finders/notes_finder_spec.rb | 47 +++++++++++++--- spec/finders/snippets_finder_spec.rb | 25 ++++++--- spec/fixtures/api/schemas/public_api/v4/tag.json | 3 +- spec/lib/banzai/filter/kroki_filter_spec.rb | 7 +++ spec/requests/api/tags_spec.rb | 64 ++++++++++++++++++++++ 11 files changed, 189 insertions(+), 36 deletions(-) diff --git a/app/controllers/oauth/jira_dvcs/authorizations_controller.rb b/app/controllers/oauth/jira_dvcs/authorizations_controller.rb index 613999f4ca7..03921761f45 100644 --- a/app/controllers/oauth/jira_dvcs/authorizations_controller.rb +++ b/app/controllers/oauth/jira_dvcs/authorizations_controller.rb @@ -8,6 +8,8 @@ class Oauth::JiraDvcs::AuthorizationsController < ApplicationController skip_before_action :authenticate_user! skip_before_action :verify_authenticity_token + before_action :validate_redirect_uri, only: :new + feature_category :integrations # 1. Rewire Jira OAuth initial request to our stablished OAuth authorization URL. @@ -56,4 +58,15 @@ class Oauth::JiraDvcs::AuthorizationsController < ApplicationController def normalize_scope(scope) scope == 'repo' ? 'api' : scope end + + def validate_redirect_uri + client = Doorkeeper::OAuth::Client.find(params[:client_id]) + return render_404 unless client + + return true if Doorkeeper::OAuth::Helpers::URIChecker.valid_for_authorization?( + params['redirect_uri'], client.redirect_uri + ) + + render_403 + end end diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 7890502cf0e..c542ffbce7e 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -117,7 +117,7 @@ class NotesFinder when "snippet", "project_snippet" SnippetsFinder.new(@current_user, project: @project).execute # rubocop: disable CodeReuse/Finder when "personal_snippet" - PersonalSnippet.all + SnippetsFinder.new(@current_user, only_personal: true).execute # rubocop: disable CodeReuse/Finder else raise "invalid target_type '#{noteable_type}'" end diff --git a/lib/api/entities/tag.rb b/lib/api/entities/tag.rb index 713bae64d5c..5047258dd97 100644 --- a/lib/api/entities/tag.rb +++ b/lib/api/entities/tag.rb @@ -3,6 +3,8 @@ module API module Entities class Tag < Grape::Entity + include RequestAwareEntity + expose :name, documentation: { type: 'string', example: 'v1.0.0' } expose :message, documentation: { type: 'string', example: 'Release v1.0.0' } expose :target, documentation: { type: 'string', example: '2695effb5807a22ff3d138d593fd856244e155e7' } @@ -12,7 +14,7 @@ module API end # rubocop: disable CodeReuse/ActiveRecord - expose :release, using: Entities::TagRelease do |repo_tag, options| + expose :release, using: Entities::TagRelease, if: ->(*) { can_read_release? } do |repo_tag, options| options[:project].releases.find_by(tag: repo_tag.name) end # rubocop: enable CodeReuse/ActiveRecord @@ -20,6 +22,10 @@ module API expose :protected, documentation: { type: 'boolean', example: true } do |repo_tag, options| ::ProtectedTag.protected?(options[:project], repo_tag.name) end + + def can_read_release? + can?(options[:current_user], :read_release, options[:project]) + end end end end diff --git a/lib/api/tags.rb b/lib/api/tags.rb index 4ddf22c726f..f918fb997bf 100644 --- a/lib/api/tags.rb +++ b/lib/api/tags.rb @@ -45,7 +45,13 @@ module API paginated_tags = Gitlab::Pagination::GitalyKeysetPager.new(self, user_project).paginate(tags_finder) - present_cached paginated_tags, with: Entities::Tag, project: user_project, cache_context: -> (_tag) { user_project.cache_key } + present_cached paginated_tags, + with: Entities::Tag, + project: user_project, + current_user: current_user, + cache_context: -> (_tag) do + [user_project.cache_key, can?(current_user, :read_release, user_project)].join(':') + end rescue Gitlab::Git::InvalidPageToken => e unprocessable_entity!(e.message) @@ -68,7 +74,7 @@ module API tag = user_project.repository.find_tag(params[:tag_name]) not_found!('Tag') unless tag - present tag, with: Entities::Tag, project: user_project + present tag, with: Entities::Tag, project: user_project, current_user: current_user end desc 'Create a new repository tag' do diff --git a/lib/banzai/filter/kroki_filter.rb b/lib/banzai/filter/kroki_filter.rb index 26f42c6b194..2b9e2a22c11 100644 --- a/lib/banzai/filter/kroki_filter.rb +++ b/lib/banzai/filter/kroki_filter.rb @@ -9,6 +9,8 @@ module Banzai # HTML that replaces all diagrams supported by Kroki with the corresponding img tags. # If the source content is large then the hidden attribute is added to the img tag. class KrokiFilter < HTML::Pipeline::Filter + include ActionView::Helpers::TagHelper + MAX_CHARACTER_LIMIT = 2000 def call @@ -27,9 +29,11 @@ module Banzai diagram_format = "svg" doc.xpath(xpath).each do |node| diagram_type = node.parent['lang'] || node['lang'] + next unless diagram_selectors.include?(diagram_type) + diagram_src = node.content image_src = create_image_src(diagram_type, diagram_format, diagram_src) - img_tag = Nokogiri::HTML::DocumentFragment.parse(%()) + img_tag = Nokogiri::HTML::DocumentFragment.parse(content_tag(:img, nil, src: image_src)) img_tag = img_tag.children.first next if img_tag.nil? diff --git a/spec/controllers/oauth/jira_dvcs/authorizations_controller_spec.rb b/spec/controllers/oauth/jira_dvcs/authorizations_controller_spec.rb index 496ef7859f9..3d271a22f27 100644 --- a/spec/controllers/oauth/jira_dvcs/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/jira_dvcs/authorizations_controller_spec.rb @@ -2,24 +2,40 @@ require 'spec_helper' -RSpec.describe Oauth::JiraDvcs::AuthorizationsController do +RSpec.describe Oauth::JiraDvcs::AuthorizationsController, feature_category: :integrations do + let_it_be(:application) { create(:oauth_application, redirect_uri: 'https://example.com/callback') } + describe 'GET new' do it 'redirects to OAuth authorization with correct params' do - get :new, params: { client_id: 'client-123', scope: 'foo', redirect_uri: 'http://example.com/' } + get :new, params: { client_id: application.uid, scope: 'foo', redirect_uri: 'https://example.com/callback' } - expect(response).to redirect_to(oauth_authorization_url(client_id: 'client-123', - response_type: 'code', - scope: 'foo', - redirect_uri: oauth_jira_dvcs_callback_url)) + expect(response).to redirect_to(oauth_authorization_url( + client_id: application.uid, + response_type: 'code', + scope: 'foo', + redirect_uri: oauth_jira_dvcs_callback_url)) end it 'replaces the GitHub "repo" scope with "api"' do - get :new, params: { client_id: 'client-123', scope: 'repo', redirect_uri: 'http://example.com/' } + get :new, params: { client_id: application.uid, scope: 'repo', redirect_uri: 'https://example.com/callback' } + + expect(response).to redirect_to(oauth_authorization_url( + client_id: application.uid, + response_type: 'code', + scope: 'api', + redirect_uri: oauth_jira_dvcs_callback_url)) + end + + it 'returns 404 with an invalid client' do + get :new, params: { client_id: 'client-123', scope: 'foo', redirect_uri: 'https://example.com/callback' } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 403 with an incorrect redirect_uri' do + get :new, params: { client_id: application.uid, scope: 'foo', redirect_uri: 'http://unsafe-website.com/callback' } - expect(response).to redirect_to(oauth_authorization_url(client_id: 'client-123', - response_type: 'code', - scope: 'api', - redirect_uri: oauth_jira_dvcs_callback_url)) + expect(response).to have_gitlab_http_status(:forbidden) end end @@ -47,7 +63,7 @@ RSpec.describe Oauth::JiraDvcs::AuthorizationsController do double(status: :ok, body: { 'access_token' => 'fake-123', 'scope' => 'foo', 'token_type' => 'bar' }) end - post :access_token, params: { code: 'code-123', client_id: 'client-123', client_secret: 'secret-123' } + post :access_token, params: { code: 'code-123', client_id: application.uid, client_secret: 'secret-123' } expect(response.body).to eq('access_token=fake-123&scope=foo&token_type=bar') end diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb index 61be90b267a..792a14e3064 100644 --- a/spec/finders/notes_finder_spec.rb +++ b/spec/finders/notes_finder_spec.rb @@ -148,15 +148,6 @@ RSpec.describe NotesFinder do expect(notes.count).to eq(1) end - it 'finds notes on personal snippets' do - note = create(:note_on_personal_snippet) - params = { project: project, target_type: 'personal_snippet', target_id: note.noteable_id } - - notes = described_class.new(user, params).execute - - expect(notes.count).to eq(1) - end - it 'raises an exception for an invalid target_type' do params[:target_type] = 'invalid' expect { described_class.new(user, params).execute }.to raise_error("invalid target_type '#{params[:target_type]}'") @@ -190,6 +181,44 @@ RSpec.describe NotesFinder do expect { described_class.new(user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) end end + + context 'when targeting personal_snippet' do + using RSpec::Parameterized::TableSyntax + + let(:author) { create(:user) } + let(:user) { create(:user, email: 'foo@baz.com') } + let(:admin) { create(:admin) } + + where(:snippet_visibility, :current_user, :access) do + Snippet::PRIVATE | ref(:author) | true + Snippet::PRIVATE | ref(:admin) | true + Snippet::PRIVATE | ref(:user) | false + Snippet::PUBLIC | ref(:author) | true + Snippet::PUBLIC | ref(:user) | true + end + + with_them do + let(:personal_snippet) { create(:personal_snippet, author: author, visibility_level: snippet_visibility) } + let(:note) { create(:note, noteable: personal_snippet) } + let(:params) { { project: project, target_type: 'personal_snippet', target_id: note.noteable.id } } + + subject(:notes) do + described_class.new(current_user, params).execute + end + + before do + allow(admin).to receive(:can_read_all_resources?).and_return(true) + end + + it 'returns the proper access' do + if access + expect(notes.count).to eq(1) + else + expect { notes }.to raise_error(::ActiveRecord::RecordNotFound) + end + end + end + end end context 'for explicit target' do diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb index 9c9a04a4df5..48880ec2c1f 100644 --- a/spec/finders/snippets_finder_spec.rb +++ b/spec/finders/snippets_finder_spec.rb @@ -231,22 +231,31 @@ RSpec.describe SnippetsFinder do context 'filter by snippet type' do context 'when filtering by only_personal snippet', :enable_admin_mode do - it 'returns only personal snippet' do + let!(:admin_private_personal_snippet) { create(:personal_snippet, :private, author: admin) } + let(:user_without_snippets) { create :user } + + it 'returns all personal snippets for the admin' do snippets = described_class.new(admin, only_personal: true).execute + expect(snippets).to contain_exactly(admin_private_personal_snippet, + private_personal_snippet, + internal_personal_snippet, + public_personal_snippet) + end + + it 'returns only personal snippets visible by user' do + snippets = described_class.new(user, only_personal: true).execute + expect(snippets).to contain_exactly(private_personal_snippet, internal_personal_snippet, public_personal_snippet) end - end - context 'when filtering by only_project snippet', :enable_admin_mode do - it 'returns only project snippet' do - snippets = described_class.new(admin, only_project: true).execute + it 'returns only internal or public personal snippets for user without snippets' do + snippets = described_class.new(user_without_snippets, only_personal: true).execute - expect(snippets).to contain_exactly(private_project_snippet, - internal_project_snippet, - public_project_snippet) + expect(snippets).to contain_exactly(internal_personal_snippet, + public_personal_snippet) end end end diff --git a/spec/fixtures/api/schemas/public_api/v4/tag.json b/spec/fixtures/api/schemas/public_api/v4/tag.json index bb0190955f0..71e6c0ec035 100644 --- a/spec/fixtures/api/schemas/public_api/v4/tag.json +++ b/spec/fixtures/api/schemas/public_api/v4/tag.json @@ -3,8 +3,7 @@ "required" : [ "name", "message", - "commit", - "release" + "commit" ], "properties" : { "name": { "type": "string" }, diff --git a/spec/lib/banzai/filter/kroki_filter_spec.rb b/spec/lib/banzai/filter/kroki_filter_spec.rb index 3f4f3aafdd6..34e79ed485d 100644 --- a/spec/lib/banzai/filter/kroki_filter_spec.rb +++ b/spec/lib/banzai/filter/kroki_filter_spec.rb @@ -54,4 +54,11 @@ RSpec.describe Banzai::Filter::KrokiFilter do expect(doc.to_s).to start_with '
xss
)) + + expect(doc.to_s).to eq %(
xss
) + end end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index b02c7135b7b..ab5e04246e8 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -111,6 +111,22 @@ RSpec.describe API::Tags, feature_category: :source_code_management do let(:project) { create(:project, :public, :repository) } it_behaves_like 'repository tags' + + context 'and releases are private' do + before do + create(:release, project: project, tag: tag_name) + project.project_feature.update!(releases_access_level: ProjectFeature::PRIVATE) + end + + it 'returns the repository tags without release information' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/tags') + expect(response).to include_pagination_headers + expect(json_response.map { |r| r.has_key?('release') }).to all(be_falsey) + end + end end context 'when unauthenticated', 'and project is private' do @@ -251,6 +267,21 @@ RSpec.describe API::Tags, feature_category: :source_code_management do it_behaves_like "cache expired" end + + context 'when user is not allowed to :read_release' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + project.project_feature.update!(releases_access_level: ProjectFeature::PRIVATE) + + get api(route, user) # Cache as a user allowed to :read_release + end + + it "isn't cached" do + expect(API::Entities::Tag).to receive(:represent).exactly(3).times + + get api(route, nil) + end + end end context 'when gitaly is unavailable' do @@ -302,6 +333,21 @@ RSpec.describe API::Tags, feature_category: :source_code_management do let(:project) { create(:project, :public, :repository) } it_behaves_like 'repository tag' + + context 'and releases are private' do + before do + create(:release, project: project, tag: tag_name) + project.project_feature.update!(releases_access_level: ProjectFeature::PRIVATE) + end + + it 'returns the repository tags without release information' do + get api(route, current_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/tag') + expect(json_response.has_key?('release')).to be_falsey + end + end end context 'when unauthenticated', 'and project is private' do @@ -328,6 +374,24 @@ RSpec.describe API::Tags, feature_category: :source_code_management do let(:request) { get api(route, guest) } end end + + context 'with releases' do + let(:description) { 'Awesome release!' } + + before do + create(:release, project: project, tag: tag_name, description: description) + end + + it 'returns release information' do + get api(route, user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('public_api/v4/tag') + + expect(json_response['message']).to eq(tag_message) + expect(json_response.dig('release', 'description')).to eq(description) + end + end end describe 'POST /projects/:id/repository/tags' do -- cgit v1.2.1 From cf599b3cb9210c48820e7d88c4393303aa28826e Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 1 Mar 2023 18:29:25 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-8-stable-ee --- .../work_items/components/item_title.vue | 6 +++ app/models/integration.rb | 2 +- .../resource_access_tokens/create_service.rb | 2 +- doc/development/integrations/index.md | 9 ++++ .../work_items/components/item_title_spec.js | 24 +++++++++- spec/models/integration_spec.rb | 9 +++- .../resource_access_tokens/create_service_spec.rb | 51 +++++++++++----------- 7 files changed, 72 insertions(+), 31 deletions(-) diff --git a/app/assets/javascripts/work_items/components/item_title.vue b/app/assets/javascripts/work_items/components/item_title.vue index b2c8b7ae1db..3d49bb9cf10 100644 --- a/app/assets/javascripts/work_items/components/item_title.vue +++ b/app/assets/javascripts/work_items/components/item_title.vue @@ -29,6 +29,11 @@ export default { handleSubmit() { this.$refs.titleEl.blur(); }, + handlePaste(e) { + e.preventDefault(); + const text = e.clipboardData.getData('text'); + this.$refs.titleEl.innerText = text; + }, }, }; @@ -48,6 +53,7 @@ export default { :contenteditable="!disabled" class="gl-px-4 gl-py-3 gl-ml-n4 gl-border gl-border-white gl-rounded-base gl-display-block" :class="{ 'gl-hover-border-gray-200 gl-pseudo-placeholder': !disabled }" + @paste="handlePaste" @blur="handleBlur" @keyup="handleInput" @keydown.enter.exact="handleSubmit" diff --git a/app/models/integration.rb b/app/models/integration.rb index 54eeab10360..4e5c90bffa1 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -510,7 +510,7 @@ class Integration < ApplicationRecord end def api_field_names - fields.reject { _1[:type] == 'password' }.pluck(:name) + fields.reject { _1[:type] == 'password' || _1[:name] == 'webhook' }.pluck(:name) end def form_fields diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index c6948536053..f6fe23b4555 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -125,7 +125,7 @@ module ResourceAccessTokens def do_not_allow_owner_access_level_for_project_bot?(access_level) resource.is_a?(Project) && - access_level == Gitlab::Access::OWNER && + access_level.to_i == Gitlab::Access::OWNER && !current_user.can?(:manage_owners, resource) end end diff --git a/doc/development/integrations/index.md b/doc/development/integrations/index.md index 9fd8fb7eb61..ceb64ba2bb7 100644 --- a/doc/development/integrations/index.md +++ b/doc/development/integrations/index.md @@ -249,6 +249,15 @@ To expose the integration in the [REST API](../../api/integrations.md): You can also refer to our [REST API style guide](../api_styleguide.md). +Sensitive fields are not exposed over the API. Sensitive fields are those fields that contain any of the following in their name: + +- `key` +- `passphrase` +- `password` +- `secret` +- `token` +- `webhook` + #### GraphQL API Integrations use the `Types::Projects::ServiceType` type by default, diff --git a/spec/frontend/work_items/components/item_title_spec.js b/spec/frontend/work_items/components/item_title_spec.js index 13e04ef6671..6361f8dafc4 100644 --- a/spec/frontend/work_items/components/item_title_spec.js +++ b/spec/frontend/work_items/components/item_title_spec.js @@ -1,8 +1,7 @@ import { shallowMount } from '@vue/test-utils'; +import { escape } from 'lodash'; import ItemTitle from '~/work_items/components/item_title.vue'; -jest.mock('lodash/escape', () => jest.fn((fn) => fn)); - const createComponent = ({ title = 'Sample title', disabled = false } = {}) => shallowMount(ItemTitle, { propsData: { @@ -51,4 +50,25 @@ describe('ItemTitle', () => { expect(wrapper.emitted(eventName)).toBeDefined(); }); + + it('renders only the text content from clipboard', async () => { + const htmlContent = 'bold text'; + const buildClipboardData = (data = {}) => ({ + clipboardData: { + getData(mimeType) { + return data[mimeType]; + }, + types: Object.keys(data), + }, + }); + + findInputEl().trigger( + 'paste', + buildClipboardData({ + html: htmlContent, + text: htmlContent, + }), + ); + expect(findInputEl().element.innerHTML).toBe(escape(htmlContent)); + }); }); diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 9b3250e3c08..3c6f9ad7fea 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integration do +RSpec.describe Integration, feature_category: :integrations do using RSpec::Parameterized::TableSyntax let_it_be(:group) { create(:group) } @@ -854,6 +854,7 @@ RSpec.describe Integration do { name: 'api_key', type: 'password' }, { name: 'password', type: 'password' }, { name: 'password_field', type: 'password' }, + { name: 'webhook' }, { name: 'some_safe_field' }, { name: 'safe_field' }, { name: 'url' }, @@ -881,6 +882,7 @@ RSpec.describe Integration do field :api_key, type: 'password' field :password, type: 'password' field :password_field, type: 'password' + field :webhook field :some_safe_field field :safe_field field :url @@ -1090,6 +1092,8 @@ RSpec.describe Integration do field :bar, type: 'password' field :password + field :webhook + field :with_help, help: -> { 'help' } field :select, type: 'select' field :boolean, type: 'checkbox' @@ -1140,7 +1144,7 @@ RSpec.describe Integration do it 'registers fields in the fields list' do expect(integration.fields.pluck(:name)).to match_array %w[ - foo foo_p foo_dt bar password with_help select boolean + foo foo_p foo_dt bar password with_help select boolean webhook ] expect(integration.api_field_names).to match_array %w[ @@ -1155,6 +1159,7 @@ RSpec.describe Integration do have_attributes(name: 'foo_dt', type: 'text'), have_attributes(name: 'bar', type: 'password'), have_attributes(name: 'password', type: 'password'), + have_attributes(name: 'webhook', type: 'text'), have_attributes(name: 'with_help', help: 'help'), have_attributes(name: 'select', type: 'select'), have_attributes(name: 'boolean', type: 'checkbox') diff --git a/spec/services/resource_access_tokens/create_service_spec.rb b/spec/services/resource_access_tokens/create_service_spec.rb index 442232920f9..a8c8d41ca09 100644 --- a/spec/services/resource_access_tokens/create_service_spec.rb +++ b/spec/services/resource_access_tokens/create_service_spec.rb @@ -27,6 +27,13 @@ RSpec.describe ResourceAccessTokens::CreateService do end end + shared_examples 'correct error message' do + it 'returns correct error message' do + expect(subject.error?).to be true + expect(subject.errors).to include(error_message) + end + end + shared_examples 'allows creation of bot with valid params' do it { expect { subject }.to change { User.count }.by(1) } @@ -200,16 +207,11 @@ RSpec.describe ResourceAccessTokens::CreateService do end context 'when invalid scope is passed' do + let(:error_message) { 'Scopes can only contain available scopes' } let_it_be(:params) { { scopes: [:invalid_scope] } } it_behaves_like 'token creation fails' - - it 'returns the scope error message' do - response = subject - - expect(response.error?).to be true - expect(response.errors).to include("Scopes can only contain available scopes") - end + it_behaves_like 'correct error message' end end @@ -217,6 +219,7 @@ RSpec.describe ResourceAccessTokens::CreateService do let_it_be(:bot_user) { create(:user, :project_bot) } let(:unpersisted_member) { build(:project_member, source: resource, user: bot_user) } + let(:error_message) { 'Could not provision maintainer access to project access token' } before do allow_next_instance_of(ResourceAccessTokens::CreateService) do |service| @@ -226,13 +229,7 @@ RSpec.describe ResourceAccessTokens::CreateService do end it_behaves_like 'token creation fails' - - it 'returns the provisioning error message' do - response = subject - - expect(response.error?).to be true - expect(response.errors).to include("Could not provision maintainer access to project access token") - end + it_behaves_like 'correct error message' end end @@ -246,14 +243,10 @@ RSpec.describe ResourceAccessTokens::CreateService do end shared_examples 'when user does not have permission to create a resource bot' do - it_behaves_like 'token creation fails' - - it 'returns the permission error message' do - response = subject + let(:error_message) { "User does not have permission to create #{resource_type} access token" } - expect(response.error?).to be true - expect(response.errors).to include("User does not have permission to create #{resource_type} access token") - end + it_behaves_like 'token creation fails' + it_behaves_like 'correct error message' end context 'when resource is a project' do @@ -273,11 +266,19 @@ RSpec.describe ResourceAccessTokens::CreateService do let_it_be(:params) { { access_level: Gitlab::Access::OWNER } } context 'when the executor is a MAINTAINER' do - it 'does not add the bot user with the specified access level in the resource' do - response = subject + let(:error_message) { 'Could not provision owner access to project access token' } - expect(response.error?).to be true - expect(response.errors).to include('Could not provision owner access to project access token') + context 'with OWNER access_level, in integer format' do + it_behaves_like 'token creation fails' + it_behaves_like 'correct error message' + end + + context 'with OWNER access_level, in string format' do + let(:error_message) { 'Could not provision owner access to project access token' } + let_it_be(:params) { { access_level: Gitlab::Access::OWNER.to_s } } + + it_behaves_like 'token creation fails' + it_behaves_like 'correct error message' end end -- cgit v1.2.1 From 0fcbe48468f0e566929599dda36b2dedd72e5708 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 1 Mar 2023 18:33:31 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-8-stable-ee --- app/models/integration.rb | 2 +- .../resource_access_tokens/create_service.rb | 2 +- doc/development/integrations/index.md | 9 ---- spec/models/integration_spec.rb | 9 +--- .../resource_access_tokens/create_service_spec.rb | 51 +++++++++++----------- 5 files changed, 29 insertions(+), 44 deletions(-) diff --git a/app/models/integration.rb b/app/models/integration.rb index 4e5c90bffa1..54eeab10360 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -510,7 +510,7 @@ class Integration < ApplicationRecord end def api_field_names - fields.reject { _1[:type] == 'password' || _1[:name] == 'webhook' }.pluck(:name) + fields.reject { _1[:type] == 'password' }.pluck(:name) end def form_fields diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index f6fe23b4555..c6948536053 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -125,7 +125,7 @@ module ResourceAccessTokens def do_not_allow_owner_access_level_for_project_bot?(access_level) resource.is_a?(Project) && - access_level.to_i == Gitlab::Access::OWNER && + access_level == Gitlab::Access::OWNER && !current_user.can?(:manage_owners, resource) end end diff --git a/doc/development/integrations/index.md b/doc/development/integrations/index.md index ceb64ba2bb7..9fd8fb7eb61 100644 --- a/doc/development/integrations/index.md +++ b/doc/development/integrations/index.md @@ -249,15 +249,6 @@ To expose the integration in the [REST API](../../api/integrations.md): You can also refer to our [REST API style guide](../api_styleguide.md). -Sensitive fields are not exposed over the API. Sensitive fields are those fields that contain any of the following in their name: - -- `key` -- `passphrase` -- `password` -- `secret` -- `token` -- `webhook` - #### GraphQL API Integrations use the `Types::Projects::ServiceType` type by default, diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 3c6f9ad7fea..9b3250e3c08 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integration, feature_category: :integrations do +RSpec.describe Integration do using RSpec::Parameterized::TableSyntax let_it_be(:group) { create(:group) } @@ -854,7 +854,6 @@ RSpec.describe Integration, feature_category: :integrations do { name: 'api_key', type: 'password' }, { name: 'password', type: 'password' }, { name: 'password_field', type: 'password' }, - { name: 'webhook' }, { name: 'some_safe_field' }, { name: 'safe_field' }, { name: 'url' }, @@ -882,7 +881,6 @@ RSpec.describe Integration, feature_category: :integrations do field :api_key, type: 'password' field :password, type: 'password' field :password_field, type: 'password' - field :webhook field :some_safe_field field :safe_field field :url @@ -1092,8 +1090,6 @@ RSpec.describe Integration, feature_category: :integrations do field :bar, type: 'password' field :password - field :webhook - field :with_help, help: -> { 'help' } field :select, type: 'select' field :boolean, type: 'checkbox' @@ -1144,7 +1140,7 @@ RSpec.describe Integration, feature_category: :integrations do it 'registers fields in the fields list' do expect(integration.fields.pluck(:name)).to match_array %w[ - foo foo_p foo_dt bar password with_help select boolean webhook + foo foo_p foo_dt bar password with_help select boolean ] expect(integration.api_field_names).to match_array %w[ @@ -1159,7 +1155,6 @@ RSpec.describe Integration, feature_category: :integrations do have_attributes(name: 'foo_dt', type: 'text'), have_attributes(name: 'bar', type: 'password'), have_attributes(name: 'password', type: 'password'), - have_attributes(name: 'webhook', type: 'text'), have_attributes(name: 'with_help', help: 'help'), have_attributes(name: 'select', type: 'select'), have_attributes(name: 'boolean', type: 'checkbox') diff --git a/spec/services/resource_access_tokens/create_service_spec.rb b/spec/services/resource_access_tokens/create_service_spec.rb index a8c8d41ca09..442232920f9 100644 --- a/spec/services/resource_access_tokens/create_service_spec.rb +++ b/spec/services/resource_access_tokens/create_service_spec.rb @@ -27,13 +27,6 @@ RSpec.describe ResourceAccessTokens::CreateService do end end - shared_examples 'correct error message' do - it 'returns correct error message' do - expect(subject.error?).to be true - expect(subject.errors).to include(error_message) - end - end - shared_examples 'allows creation of bot with valid params' do it { expect { subject }.to change { User.count }.by(1) } @@ -207,11 +200,16 @@ RSpec.describe ResourceAccessTokens::CreateService do end context 'when invalid scope is passed' do - let(:error_message) { 'Scopes can only contain available scopes' } let_it_be(:params) { { scopes: [:invalid_scope] } } it_behaves_like 'token creation fails' - it_behaves_like 'correct error message' + + it 'returns the scope error message' do + response = subject + + expect(response.error?).to be true + expect(response.errors).to include("Scopes can only contain available scopes") + end end end @@ -219,7 +217,6 @@ RSpec.describe ResourceAccessTokens::CreateService do let_it_be(:bot_user) { create(:user, :project_bot) } let(:unpersisted_member) { build(:project_member, source: resource, user: bot_user) } - let(:error_message) { 'Could not provision maintainer access to project access token' } before do allow_next_instance_of(ResourceAccessTokens::CreateService) do |service| @@ -229,7 +226,13 @@ RSpec.describe ResourceAccessTokens::CreateService do end it_behaves_like 'token creation fails' - it_behaves_like 'correct error message' + + it 'returns the provisioning error message' do + response = subject + + expect(response.error?).to be true + expect(response.errors).to include("Could not provision maintainer access to project access token") + end end end @@ -243,10 +246,14 @@ RSpec.describe ResourceAccessTokens::CreateService do end shared_examples 'when user does not have permission to create a resource bot' do - let(:error_message) { "User does not have permission to create #{resource_type} access token" } - it_behaves_like 'token creation fails' - it_behaves_like 'correct error message' + + it 'returns the permission error message' do + response = subject + + expect(response.error?).to be true + expect(response.errors).to include("User does not have permission to create #{resource_type} access token") + end end context 'when resource is a project' do @@ -266,19 +273,11 @@ RSpec.describe ResourceAccessTokens::CreateService do let_it_be(:params) { { access_level: Gitlab::Access::OWNER } } context 'when the executor is a MAINTAINER' do - let(:error_message) { 'Could not provision owner access to project access token' } - - context 'with OWNER access_level, in integer format' do - it_behaves_like 'token creation fails' - it_behaves_like 'correct error message' - end - - context 'with OWNER access_level, in string format' do - let(:error_message) { 'Could not provision owner access to project access token' } - let_it_be(:params) { { access_level: Gitlab::Access::OWNER.to_s } } + it 'does not add the bot user with the specified access level in the resource' do + response = subject - it_behaves_like 'token creation fails' - it_behaves_like 'correct error message' + expect(response.error?).to be true + expect(response.errors).to include('Could not provision owner access to project access token') end end -- cgit v1.2.1 From 8d2da107095dc8d7d23db88f169306cdc9a869e1 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 1 Mar 2023 18:38:07 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-8-stable-ee --- app/models/integration.rb | 2 +- app/models/integrations/datadog.rb | 1 + app/models/integrations/prometheus.rb | 31 ++++++++++-- app/services/groups/transfer_service.rb | 2 + .../resource_access_tokens/create_service.rb | 2 +- .../initializers/rest-client-hostname_override.rb | 2 +- doc/development/integrations/index.md | 9 ++++ lib/api/commits.rb | 8 +++- lib/gitlab/http_connection_adapter.rb | 2 +- locale/gitlab.pot | 3 ++ .../gitlab/ci/config/external/file/remote_spec.rb | 2 +- spec/lib/gitlab/fogbugz_import/importer_spec.rb | 4 +- spec/lib/gitlab/http_connection_adapter_spec.rb | 6 +-- .../import_export/remote_stream_upload_spec.rb | 10 ++-- .../prometheus/queries/validate_query_spec.rb | 5 +- spec/models/integration_spec.rb | 9 +++- spec/models/integrations/datadog_spec.rb | 2 +- spec/models/integrations/prometheus_spec.rb | 55 +++++++++++++++++++++- spec/requests/api/commits_spec.rb | 26 +++++++++- .../resource_access_tokens/create_service_spec.rb | 51 ++++++++++---------- .../integrations/integrations_shared_context.rb | 1 + .../uses_gitlab_url_blocker_shared_examples.rb | 12 ++--- 22 files changed, 183 insertions(+), 62 deletions(-) diff --git a/app/models/integration.rb b/app/models/integration.rb index 54eeab10360..4e5c90bffa1 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -510,7 +510,7 @@ class Integration < ApplicationRecord end def api_field_names - fields.reject { _1[:type] == 'password' }.pluck(:name) + fields.reject { _1[:type] == 'password' || _1[:name] == 'webhook' }.pluck(:name) end def form_fields diff --git a/app/models/integrations/datadog.rb b/app/models/integrations/datadog.rb index 80eecc14d0f..3b3c7d8f2cd 100644 --- a/app/models/integrations/datadog.rb +++ b/app/models/integrations/datadog.rb @@ -15,6 +15,7 @@ module Integrations TAG_KEY_VALUE_RE = %r{\A [\w-]+ : .*\S.* \z}x.freeze field :datadog_site, + exposes_secrets: true, placeholder: DEFAULT_DOMAIN, help: -> do ERB::Util.html_escape( diff --git a/app/models/integrations/prometheus.rb b/app/models/integrations/prometheus.rb index 142f466018b..2f0995e9ab0 100644 --- a/app/models/integrations/prometheus.rb +++ b/app/models/integrations/prometheus.rb @@ -3,6 +3,7 @@ module Integrations class Prometheus < BaseMonitoring include PrometheusAdapter + include Gitlab::Utils::StrongMemoize field :manual_configuration, type: 'checkbox', @@ -81,7 +82,7 @@ module Integrations allow_local_requests: allow_local_api_url? ) - if behind_iap? + if behind_iap? && iap_client # Adds the Authorization header options[:headers] = iap_client.apply({}) end @@ -106,6 +107,22 @@ module Integrations should_return_client? end + alias_method :google_iap_service_account_json_raw, :google_iap_service_account_json + private :google_iap_service_account_json_raw + + MASKED_VALUE = '*' * 8 + + def google_iap_service_account_json + json = google_iap_service_account_json_raw + return json unless json.present? + + Gitlab::Json.parse(json) + .then { |hash| hash.transform_values { MASKED_VALUE } } + .then { |hash| Gitlab::Json.generate(hash) } + rescue Gitlab::Json.parser_error + json + end + private delegate :allow_local_requests_from_web_hooks_and_services?, to: :current_settings, private: true @@ -155,17 +172,21 @@ module Integrations end def clean_google_iap_service_account - return unless google_iap_service_account_json + json = google_iap_service_account_json_raw + return unless json.present? - google_iap_service_account_json - .then { |json| Gitlab::Json.parse(json) } - .except('token_credential_uri') + Gitlab::Json.parse(json).except('token_credential_uri') + rescue Gitlab::Json.parser_error + {} end def iap_client @iap_client ||= Google::Auth::Credentials .new(clean_google_iap_service_account, target_audience: google_iap_audience_client_id) .client + rescue StandardError + nil end + strong_memoize_attr :iap_client end end diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index 0a9705181ba..7e9fd9dad54 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -51,6 +51,7 @@ module Groups publish_event(old_root_ancestor_id) end + # Overridden in EE def ensure_allowed_transfer raise_transfer_error(:group_is_already_root) if group_is_already_root? raise_transfer_error(:same_parent_as_current) if same_parent? @@ -208,6 +209,7 @@ module Groups raise TransferError, localized_error_messages[message] end + # Overridden in EE def localized_error_messages { database_not_supported: s_('TransferGroup|Database is not supported.'), diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index c6948536053..f6fe23b4555 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -125,7 +125,7 @@ module ResourceAccessTokens def do_not_allow_owner_access_level_for_project_bot?(access_level) resource.is_a?(Project) && - access_level == Gitlab::Access::OWNER && + access_level.to_i == Gitlab::Access::OWNER && !current_user.can?(:manage_owners, resource) end end diff --git a/config/initializers/rest-client-hostname_override.rb b/config/initializers/rest-client-hostname_override.rb index 2fb3b9fc27d..b647fe9cac8 100644 --- a/config/initializers/rest-client-hostname_override.rb +++ b/config/initializers/rest-client-hostname_override.rb @@ -14,7 +14,7 @@ module RestClient self.hostname_override = hostname_override rescue Gitlab::UrlBlocker::BlockedUrlError => e - raise ArgumentError, "URL '#{uri}' is blocked: #{e.message}" + raise ArgumentError, "URL is blocked: #{e.message}" end # Gitlab::UrlBlocker returns a Addressable::URI which we need to coerce diff --git a/doc/development/integrations/index.md b/doc/development/integrations/index.md index 9fd8fb7eb61..ceb64ba2bb7 100644 --- a/doc/development/integrations/index.md +++ b/doc/development/integrations/index.md @@ -249,6 +249,15 @@ To expose the integration in the [REST API](../../api/integrations.md): You can also refer to our [REST API style guide](../api_styleguide.md). +Sensitive fields are not exposed over the API. Sensitive fields are those fields that contain any of the following in their name: + +- `key` +- `passphrase` +- `password` +- `secret` +- `token` +- `webhook` + #### GraphQL API Integrations use the `Types::Projects::ServiceType` type by default, diff --git a/lib/api/commits.rb b/lib/api/commits.rb index ad2bbf90917..fa99a1a2bc8 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -86,11 +86,15 @@ module API get ':id/repository/commits', urgency: :low do not_found! 'Repository' unless user_project.repository_exists? + page = params[:page] > 0 ? params[:page] : 1 + per_page = params[:per_page] > 0 ? params[:per_page] : Kaminari.config.default_per_page + limit = [per_page, Kaminari.config.max_per_page].min + offset = (page - 1) * limit + path = params[:path] before = params[:until] after = params[:since] ref = params[:ref_name].presence || user_project.default_branch unless params[:all] - offset = (params[:page] - 1) * params[:per_page] all = params[:all] with_stats = params[:with_stats] first_parent = params[:first_parent] @@ -98,7 +102,7 @@ module API commits = user_project.repository.commits(ref, path: path, - limit: params[:per_page], + limit: limit, offset: offset, before: before, after: after, diff --git a/lib/gitlab/http_connection_adapter.rb b/lib/gitlab/http_connection_adapter.rb index 3ef60be67a9..c6f9f2df299 100644 --- a/lib/gitlab/http_connection_adapter.rb +++ b/lib/gitlab/http_connection_adapter.rb @@ -47,7 +47,7 @@ module Gitlab dns_rebind_protection: dns_rebind_protection?, schemes: %w[http https]) rescue Gitlab::UrlBlocker::BlockedUrlError => e - raise Gitlab::HTTP::BlockedUrlError, "URL '#{url}' is blocked: #{e.message}" + raise Gitlab::HTTP::BlockedUrlError, "URL is blocked: #{e.message}" end def allow_local_requests? diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d1b86c8ad1a..ae06cb3ae7a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -44346,6 +44346,9 @@ msgstr "" msgid "TransferGroup|Group is already associated to the parent group." msgstr "" +msgid "TransferGroup|SAML Provider or SCIM Token is configured for this group." +msgstr "" + msgid "TransferGroup|The parent group already has a subgroup or a project with the same path." msgstr "" diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index 8d93cdcf378..5ef0fec1e9d 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -184,7 +184,7 @@ RSpec.describe Gitlab::Ci::Config::External::File::Remote do let(:location) { 'http://127.0.0.1/some/path/to/config.yaml' } it 'includes details about blocked URL' do - expect(subject).to eq "Remote file could not be fetched because URL '#{location}' " \ + expect(subject).to eq "Remote file could not be fetched because URL " \ 'is blocked: Requests to localhost are not allowed!' end end diff --git a/spec/lib/gitlab/fogbugz_import/importer_spec.rb b/spec/lib/gitlab/fogbugz_import/importer_spec.rb index 9b58b772d1a..a4246809725 100644 --- a/spec/lib/gitlab/fogbugz_import/importer_spec.rb +++ b/spec/lib/gitlab/fogbugz_import/importer_spec.rb @@ -72,7 +72,7 @@ RSpec.describe Gitlab::FogbugzImport::Importer do expect { subject.execute } .to raise_error( ::Gitlab::HTTP::BlockedUrlError, - "URL 'https://localhost:3000/api.asp' is blocked: Requests to localhost are not allowed" + "URL is blocked: Requests to localhost are not allowed" ) end end @@ -84,7 +84,7 @@ RSpec.describe Gitlab::FogbugzImport::Importer do expect { subject.execute } .to raise_error( ::Gitlab::HTTP::BlockedUrlError, - "URL 'http://192.168.0.1/api.asp' is blocked: Requests to the local network are not allowed" + "URL is blocked: Requests to the local network are not allowed" ) end end diff --git a/spec/lib/gitlab/http_connection_adapter_spec.rb b/spec/lib/gitlab/http_connection_adapter_spec.rb index 5e2c6be8993..dbf0252da46 100644 --- a/spec/lib/gitlab/http_connection_adapter_spec.rb +++ b/spec/lib/gitlab/http_connection_adapter_spec.rb @@ -44,7 +44,7 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do it 'raises error' do expect { subject }.to raise_error( Gitlab::HTTP::BlockedUrlError, - "URL 'http://172.16.0.0/12' is blocked: Requests to the local network are not allowed" + "URL is blocked: Requests to the local network are not allowed" ) end @@ -67,7 +67,7 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do it 'raises error' do expect { subject }.to raise_error( Gitlab::HTTP::BlockedUrlError, - "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed" + "URL is blocked: Requests to localhost are not allowed" ) end @@ -131,7 +131,7 @@ RSpec.describe Gitlab::HTTPConnectionAdapter do it 'raises error' do expect { subject }.to raise_error( Gitlab::HTTP::BlockedUrlError, - "URL 'ssh://example.org' is blocked: Only allowed schemes are http, https" + "URL is blocked: Only allowed schemes are http, https" ) end end diff --git a/spec/lib/gitlab/import_export/remote_stream_upload_spec.rb b/spec/lib/gitlab/import_export/remote_stream_upload_spec.rb index b1bc6b7eeaf..3d9d6e1b96b 100644 --- a/spec/lib/gitlab/import_export/remote_stream_upload_spec.rb +++ b/spec/lib/gitlab/import_export/remote_stream_upload_spec.rb @@ -88,7 +88,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do it 'raises error' do expect { subject.execute }.to raise_error( Gitlab::HTTP::BlockedUrlError, - "URL 'http://127.0.0.1/file.txt' is blocked: Requests to localhost are not allowed" + "URL is blocked: Requests to localhost are not allowed" ) end @@ -114,7 +114,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do it 'raises error' do expect { subject.execute }.to raise_error( Gitlab::HTTP::BlockedUrlError, - "URL 'http://172.16.0.0/file.txt' is blocked: Requests to the local network are not allowed" + "URL is blocked: Requests to the local network are not allowed" ) end @@ -142,7 +142,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do expect { subject.execute }.to raise_error( Gitlab::HTTP::BlockedUrlError, - "URL 'http://127.0.0.1/file.txt' is blocked: Requests to localhost are not allowed" + "URL is blocked: Requests to localhost are not allowed" ) end @@ -168,7 +168,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do it 'raises error' do expect { subject.execute }.to raise_error( Gitlab::HTTP::BlockedUrlError, - "URL 'http://172.16.0.0/file.txt' is blocked: Requests to the local network are not allowed" + "URL is blocked: Requests to the local network are not allowed" ) end @@ -192,7 +192,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do expect { subject.execute }.to raise_error( Gitlab::HTTP::BlockedUrlError, - "URL 'http://example.com/file.txt' is blocked: Requests to localhost are not allowed" + "URL is blocked: Requests to localhost are not allowed" ) end end diff --git a/spec/lib/gitlab/prometheus/queries/validate_query_spec.rb b/spec/lib/gitlab/prometheus/queries/validate_query_spec.rb index e3706a4b106..f09fa3548f8 100644 --- a/spec/lib/gitlab/prometheus/queries/validate_query_spec.rb +++ b/spec/lib/gitlab/prometheus/queries/validate_query_spec.rb @@ -43,10 +43,7 @@ RSpec.describe Gitlab::Prometheus::Queries::ValidateQuery do context 'Gitlab::HTTP::BlockedUrlError' do let(:api_url) { 'http://192.168.1.1' } - let(:message) do - "URL 'http://192.168.1.1/api/v1/query?query=avg%28metric%29&time=#{Time.now.to_f}'" \ - " is blocked: Requests to the local network are not allowed" - end + let(:message) { "URL is blocked: Requests to the local network are not allowed" } before do stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 9b3250e3c08..3c6f9ad7fea 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integration do +RSpec.describe Integration, feature_category: :integrations do using RSpec::Parameterized::TableSyntax let_it_be(:group) { create(:group) } @@ -854,6 +854,7 @@ RSpec.describe Integration do { name: 'api_key', type: 'password' }, { name: 'password', type: 'password' }, { name: 'password_field', type: 'password' }, + { name: 'webhook' }, { name: 'some_safe_field' }, { name: 'safe_field' }, { name: 'url' }, @@ -881,6 +882,7 @@ RSpec.describe Integration do field :api_key, type: 'password' field :password, type: 'password' field :password_field, type: 'password' + field :webhook field :some_safe_field field :safe_field field :url @@ -1090,6 +1092,8 @@ RSpec.describe Integration do field :bar, type: 'password' field :password + field :webhook + field :with_help, help: -> { 'help' } field :select, type: 'select' field :boolean, type: 'checkbox' @@ -1140,7 +1144,7 @@ RSpec.describe Integration do it 'registers fields in the fields list' do expect(integration.fields.pluck(:name)).to match_array %w[ - foo foo_p foo_dt bar password with_help select boolean + foo foo_p foo_dt bar password with_help select boolean webhook ] expect(integration.api_field_names).to match_array %w[ @@ -1155,6 +1159,7 @@ RSpec.describe Integration do have_attributes(name: 'foo_dt', type: 'text'), have_attributes(name: 'bar', type: 'password'), have_attributes(name: 'password', type: 'password'), + have_attributes(name: 'webhook', type: 'text'), have_attributes(name: 'with_help', help: 'help'), have_attributes(name: 'select', type: 'select'), have_attributes(name: 'boolean', type: 'checkbox') diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb index 65ecd9bee83..2d1e23b103f 100644 --- a/spec/models/integrations/datadog_spec.rb +++ b/spec/models/integrations/datadog_spec.rb @@ -3,7 +3,7 @@ require 'securerandom' require 'spec_helper' -RSpec.describe Integrations::Datadog do +RSpec.describe Integrations::Datadog, feature_category: :integrations do let_it_be(:project) { create(:project) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } let_it_be(:build) { create(:ci_build, pipeline: pipeline) } diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index 3c3850854b3..aa248abd3bb 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -239,6 +239,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, context 'behind IAP' do let(:manual_configuration) { true } + let(:google_iap_service_account_json) { Gitlab::Json.generate(google_iap_service_account) } let(:google_iap_service_account) do { @@ -259,7 +260,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, end def stub_iap_request - integration.google_iap_service_account_json = Gitlab::Json.generate(google_iap_service_account) + integration.google_iap_service_account_json = google_iap_service_account_json integration.google_iap_audience_client_id = 'IAP_CLIENT_ID.apps.googleusercontent.com' stub_request(:post, 'https://oauth2.googleapis.com/token') @@ -278,6 +279,17 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, expect(integration.prometheus_client.send(:options)[:headers]).to eq(authorization: "Bearer FOO") end + context 'with invalid IAP JSON' do + let(:google_iap_service_account_json) { 'invalid json' } + + it 'does not include authorization header' do + stub_iap_request + + expect(integration.prometheus_client).not_to be_nil + expect(integration.prometheus_client.send(:options)).not_to have_key(:headers) + end + end + context 'when passed with token_credential_uri', issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/284819' do let(:malicious_host) { 'http://example.com' } @@ -477,4 +489,45 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, end end end + + describe '#google_iap_service_account_json' do + subject(:iap_details) { integration.google_iap_service_account_json } + + before do + integration.google_iap_service_account_json = value + end + + context 'with valid JSON' do + let(:masked_value) { described_class::MASKED_VALUE } + let(:json) { Gitlab::Json.parse(iap_details) } + + let(:value) do + Gitlab::Json.generate({ + type: 'service_account', + private_key: 'SECRET', + foo: 'secret', + nested: { + key: 'value' + } + }) + end + + it 'masks all JSON values', issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/384580' do + expect(json).to eq( + 'type' => masked_value, + 'private_key' => masked_value, + 'foo' => masked_value, + 'nested' => masked_value + ) + end + end + + context 'with invalid JSON' do + where(:value) { [nil, '', ' ', 'invalid json'] } + + with_them do + it { is_expected.to eq(value) } + end + end + end end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 3932abd20cc..bcc27a80cf8 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -249,6 +249,18 @@ RSpec.describe API::Commits, feature_category: :source_code_management do end end + context 'when per_page is over 100' do + let(:per_page) { 101 } + + it 'returns 100 commits (maximum)' do + expect(Gitlab::Git::Commit).to receive(:where).with( + hash_including(ref: ref_name, limit: 100, offset: 0) + ) + + request + end + end + context 'when pagination params are invalid' do let_it_be(:project) { create(:project, :repository) } @@ -279,7 +291,7 @@ RSpec.describe API::Commits, feature_category: :source_code_management do where(:page, :per_page, :error_message, :status) do 0 | nil | nil | :success - -10 | nil | nil | :internal_server_error + -10 | nil | nil | :success 'a' | nil | 'page is invalid' | :bad_request nil | 0 | 'per_page has a value not allowed' | :bad_request nil | -1 | nil | :success @@ -297,6 +309,18 @@ RSpec.describe API::Commits, feature_category: :source_code_management do end end end + + context 'when per_page is below 0' do + let(:per_page) { -100 } + + it 'returns 20 commits (default)' do + expect(Gitlab::Git::Commit).to receive(:where).with( + hash_including(ref: ref_name, limit: 20, offset: 0) + ) + + request + end + end end end end diff --git a/spec/services/resource_access_tokens/create_service_spec.rb b/spec/services/resource_access_tokens/create_service_spec.rb index 442232920f9..a8c8d41ca09 100644 --- a/spec/services/resource_access_tokens/create_service_spec.rb +++ b/spec/services/resource_access_tokens/create_service_spec.rb @@ -27,6 +27,13 @@ RSpec.describe ResourceAccessTokens::CreateService do end end + shared_examples 'correct error message' do + it 'returns correct error message' do + expect(subject.error?).to be true + expect(subject.errors).to include(error_message) + end + end + shared_examples 'allows creation of bot with valid params' do it { expect { subject }.to change { User.count }.by(1) } @@ -200,16 +207,11 @@ RSpec.describe ResourceAccessTokens::CreateService do end context 'when invalid scope is passed' do + let(:error_message) { 'Scopes can only contain available scopes' } let_it_be(:params) { { scopes: [:invalid_scope] } } it_behaves_like 'token creation fails' - - it 'returns the scope error message' do - response = subject - - expect(response.error?).to be true - expect(response.errors).to include("Scopes can only contain available scopes") - end + it_behaves_like 'correct error message' end end @@ -217,6 +219,7 @@ RSpec.describe ResourceAccessTokens::CreateService do let_it_be(:bot_user) { create(:user, :project_bot) } let(:unpersisted_member) { build(:project_member, source: resource, user: bot_user) } + let(:error_message) { 'Could not provision maintainer access to project access token' } before do allow_next_instance_of(ResourceAccessTokens::CreateService) do |service| @@ -226,13 +229,7 @@ RSpec.describe ResourceAccessTokens::CreateService do end it_behaves_like 'token creation fails' - - it 'returns the provisioning error message' do - response = subject - - expect(response.error?).to be true - expect(response.errors).to include("Could not provision maintainer access to project access token") - end + it_behaves_like 'correct error message' end end @@ -246,14 +243,10 @@ RSpec.describe ResourceAccessTokens::CreateService do end shared_examples 'when user does not have permission to create a resource bot' do - it_behaves_like 'token creation fails' - - it 'returns the permission error message' do - response = subject + let(:error_message) { "User does not have permission to create #{resource_type} access token" } - expect(response.error?).to be true - expect(response.errors).to include("User does not have permission to create #{resource_type} access token") - end + it_behaves_like 'token creation fails' + it_behaves_like 'correct error message' end context 'when resource is a project' do @@ -273,11 +266,19 @@ RSpec.describe ResourceAccessTokens::CreateService do let_it_be(:params) { { access_level: Gitlab::Access::OWNER } } context 'when the executor is a MAINTAINER' do - it 'does not add the bot user with the specified access level in the resource' do - response = subject + let(:error_message) { 'Could not provision owner access to project access token' } - expect(response.error?).to be true - expect(response.errors).to include('Could not provision owner access to project access token') + context 'with OWNER access_level, in integer format' do + it_behaves_like 'token creation fails' + it_behaves_like 'correct error message' + end + + context 'with OWNER access_level, in string format' do + let(:error_message) { 'Could not provision owner access to project access token' } + let_it_be(:params) { { access_level: Gitlab::Access::OWNER.to_s } } + + it_behaves_like 'token creation fails' + it_behaves_like 'correct error message' end end diff --git a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb index bf5158c9a92..2c92ef64815 100644 --- a/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb +++ b/spec/support/shared_contexts/features/integrations/integrations_shared_context.rb @@ -93,6 +93,7 @@ Integration.available_integration_names.each do |integration| def initialize_integration(integration, attrs = {}) record = project.find_or_initialize_integration(integration) + record.reset_updated_properties if integration == 'datadog' record.attributes = attrs record.properties = integration_attrs record.save! diff --git a/spec/support/shared_examples/initializers/uses_gitlab_url_blocker_shared_examples.rb b/spec/support/shared_examples/initializers/uses_gitlab_url_blocker_shared_examples.rb index 553e9f10b0d..cef76bd4356 100644 --- a/spec/support/shared_examples/initializers/uses_gitlab_url_blocker_shared_examples.rb +++ b/spec/support/shared_examples/initializers/uses_gitlab_url_blocker_shared_examples.rb @@ -33,7 +33,7 @@ RSpec.shared_examples 'a request using Gitlab::UrlBlocker' do expect { make_request('https://example.com') } .to raise_error(url_blocked_error_class, - "URL 'https://example.com' is blocked: Requests to the local network are not allowed") + "URL is blocked: Requests to the local network are not allowed") end it 'raises error when it is a request that resolves to a localhost address' do @@ -41,19 +41,19 @@ RSpec.shared_examples 'a request using Gitlab::UrlBlocker' do expect { make_request('https://example.com') } .to raise_error(url_blocked_error_class, - "URL 'https://example.com' is blocked: Requests to localhost are not allowed") + "URL is blocked: Requests to localhost are not allowed") end it 'raises error when it is a request to local address' do expect { make_request('http://172.16.0.0') } .to raise_error(url_blocked_error_class, - "URL 'http://172.16.0.0' is blocked: Requests to the local network are not allowed") + "URL is blocked: Requests to the local network are not allowed") end it 'raises error when it is a request to localhost address' do expect { make_request('http://127.0.0.1') } .to raise_error(url_blocked_error_class, - "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed") + "URL is blocked: Requests to localhost are not allowed") end end @@ -69,13 +69,13 @@ RSpec.shared_examples 'a request using Gitlab::UrlBlocker' do it 'raises error when it is a request to local address' do expect { make_request('https://172.16.0.0:8080') } .to raise_error(url_blocked_error_class, - "URL 'https://172.16.0.0:8080' is blocked: Requests to the local network are not allowed") + "URL is blocked: Requests to the local network are not allowed") end it 'raises error when it is a request to localhost address' do expect { make_request('https://127.0.0.1:8080') } .to raise_error(url_blocked_error_class, - "URL 'https://127.0.0.1:8080' is blocked: Requests to localhost are not allowed") + "URL is blocked: Requests to localhost are not allowed") end end -- cgit v1.2.1 From 7104900200c8801d5c5a30957a5d63eb7371344f Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 2 Mar 2023 00:18:48 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-8-stable-ee --- app/models/ci/build.rb | 10 +- .../projects/artifacts_controller_spec.rb | 4 +- spec/controllers/projects/jobs_controller_spec.rb | 46 ++------ spec/features/projects/jobs/permissions_spec.rb | 125 +++++---------------- spec/models/ci/build_spec.rb | 116 +++++++++---------- spec/requests/api/ci/jobs_spec.rb | 40 +++---- 6 files changed, 120 insertions(+), 221 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 0139b025d98..d58ebbcaa32 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -72,6 +72,7 @@ module Ci delegate :trigger_short_token, to: :trigger_request, allow_nil: true delegate :ensure_persistent_ref, to: :pipeline delegate :enable_debug_trace!, to: :metadata + delegate :debug_trace_enabled?, to: :metadata serialize :options # rubocop:disable Cop/ActiveRecordSerialize serialize :yaml_variables, Gitlab::Serializer::Ci::Variables # rubocop:disable Cop/ActiveRecordSerialize @@ -1059,11 +1060,10 @@ module Ci end def debug_mode? - # TODO: Have `debug_mode?` check against data on sent back from runner - # to capture all the ways that variables can be set. - # See (https://gitlab.com/gitlab-org/gitlab/-/issues/290955) - variables['CI_DEBUG_TRACE']&.value&.casecmp('true') == 0 || - variables['CI_DEBUG_SERVICES']&.value&.casecmp('true') == 0 + # perform the check on both sides in case the runner version is old + debug_trace_enabled? || + Gitlab::Utils.to_boolean(variables['CI_DEBUG_SERVICES']&.value, default: false) || + Gitlab::Utils.to_boolean(variables['CI_DEBUG_TRACE']&.value, default: false) end def drop_with_exit_code!(failure_reason, exit_code) diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index 3d12926c07a..7c542e02b61 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -229,7 +229,9 @@ RSpec.describe Projects::ArtifactsController do let(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) } before do - create(:ci_job_variable, key: 'CI_DEBUG_TRACE', value: 'true', job: job) + allow_next_found_instance_of(Ci::Build) do |build| + allow(build).to receive(:debug_mode?).and_return(true) + end end context 'when the user does not have update_build permissions' do diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 3dc89365530..21757f4cf75 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -629,40 +629,12 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do expect(json_response['lines'].count).to be_positive end - context 'when CI_DEBUG_TRACE enabled' do - let!(:variable) { create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: 'true') } - - context 'with proper permissions on a project' do - let(:user) { developer } - - before do - sign_in(user) - end - - it 'returns response ok' do - get_trace - - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'without proper permissions for debug logging' do - let(:user) { guest } - - before do - sign_in(user) - end - - it 'returns response forbidden' do - get_trace - - expect(response).to have_gitlab_http_status(:forbidden) + context 'when debug_mode? is enabled' do + before do + allow_next_found_instance_of(Ci::Build) do |build| + allow(build).to receive(:debug_mode?).and_return(true) end end - end - - context 'when CI_DEBUG_SERVICES enabled' do - let!(:variable) { create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: 'true') } context 'with proper permissions on a project' do let(:user) { developer } @@ -1219,10 +1191,10 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do context 'when CI_DEBUG_TRACE and/or CI_DEBUG_SERVICES are enabled' do using RSpec::Parameterized::TableSyntax where(:ci_debug_trace, :ci_debug_services) do - 'true' | 'true' - 'true' | 'false' - 'false' | 'true' - 'false' | 'false' + true | true + true | false + false | true + false | false end with_them do @@ -1255,7 +1227,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do it 'returns response forbidden if dev mode enabled' do response = subject - if ci_debug_trace == 'true' || ci_debug_services == 'true' + if ci_debug_trace || ci_debug_services expect(response).to have_gitlab_http_status(:forbidden) else expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/features/projects/jobs/permissions_spec.rb b/spec/features/projects/jobs/permissions_spec.rb index c3c0043a6ef..dce86c9f0a4 100644 --- a/spec/features/projects/jobs/permissions_spec.rb +++ b/spec/features/projects/jobs/permissions_spec.rb @@ -149,109 +149,44 @@ RSpec.describe 'Project Jobs Permissions', feature_category: :projects do end end - context 'with CI_DEBUG_TRACE' do - let_it_be(:ci_instance_variable) { create(:ci_instance_variable, key: 'CI_DEBUG_TRACE') } - - describe 'trace endpoint' do - let_it_be(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) } - - where(:public_builds, :user_project_role, :ci_debug_trace, :expected_status_code) do - true | 'developer' | true | 200 - true | 'guest' | true | 403 - true | 'developer' | false | 200 - true | 'guest' | false | 200 - false | 'developer' | true | 200 - false | 'guest' | true | 403 - false | 'developer' | false | 200 - false | 'guest' | false | 403 - end - - with_them do - before do - ci_instance_variable.update!(value: ci_debug_trace) - project.update!(public_builds: public_builds) - project.add_role(user, user_project_role) - end - - it 'renders trace to authorized users' do - visit trace_project_job_path(project, job) - - expect(status_code).to eq(expected_status_code) - end - end + describe 'debug_mode' do + let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) } + + where(:public_builds, :user_project_role, :debug_mode, :expected_status_code, :expected_msg) do + true | 'developer' | true | 200 | '' + true | 'guest' | true | 403 | 'You must have developer or higher permissions' + true | nil | true | 404 | 'Page Not Found Make sure the address is correct' + true | 'developer' | false | 200 | '' + true | 'guest' | false | 200 | '' + true | nil | false | 404 | 'Page Not Found Make sure the address is correct' + false | 'developer' | true | 200 | '' + false | 'guest' | true | 403 | 'You must have developer or higher permissions' + false | nil | true | 404 | 'Page Not Found Make sure the address is correct' + false | 'developer' | false | 200 | '' + false | 'guest' | false | 403 | 'The current user is not authorized to access the job log' + false | nil | false | 404 | 'Page Not Found Make sure the address is correct' end - describe 'raw page' do - let_it_be(:job) { create(:ci_build, :running, :coverage, :trace_artifact, pipeline: pipeline) } - - where(:public_builds, :user_project_role, :ci_debug_trace, :expected_status_code, :expected_msg) do - true | 'developer' | true | 200 | nil - true | 'guest' | true | 403 | 'You must have developer or higher permissions' - true | 'developer' | false | 200 | nil - true | 'guest' | false | 200 | nil - false | 'developer' | true | 200 | nil - false | 'guest' | true | 403 | 'You must have developer or higher permissions' - false | 'developer' | false | 200 | nil - false | 'guest' | false | 403 | 'The current user is not authorized to access the job log' - end - - with_them do - before do - ci_instance_variable.update!(value: ci_debug_trace) - project.update!(public_builds: public_builds) - project.add_role(user, user_project_role) - end - - it 'renders raw trace to authorized users' do - visit raw_project_job_path(project, job) - - expect(status_code).to eq(expected_status_code) - expect(page).to have_content(expected_msg) + with_them do + before do + project.update!(public_builds: public_builds) + user_project_role && project.add_role(user, user_project_role) + allow_next_found_instance_of(Ci::Build) do |build| + allow(build).to receive(:debug_mode?).and_return(debug_mode) end end - end - end - - context 'with CI_DEBUG_SERVICES' do - let_it_be(:ci_instance_variable) { create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES') } - - describe 'trace endpoint and raw page' do - let_it_be(:job) { create(:ci_build, :running, :coverage, :trace_artifact, pipeline: pipeline) } - - where(:public_builds, :user_project_role, :ci_debug_services, :expected_status_code, :expected_msg) do - true | 'developer' | true | 200 | nil - true | 'guest' | true | 403 | 'You must have developer or higher permissions' - true | nil | true | 404 | 'Page Not Found Make sure the address is correct' - true | 'developer' | false | 200 | nil - true | 'guest' | false | 200 | nil - true | nil | false | 404 | 'Page Not Found Make sure the address is correct' - false | 'developer' | true | 200 | nil - false | 'guest' | true | 403 | 'You must have developer or higher permissions' - false | nil | true | 404 | 'Page Not Found Make sure the address is correct' - false | 'developer' | false | 200 | nil - false | 'guest' | false | 403 | 'The current user is not authorized to access the job log' - false | nil | false | 404 | 'Page Not Found Make sure the address is correct' - end - with_them do - before do - ci_instance_variable.update!(value: ci_debug_services) - project.update!(public_builds: public_builds) - user_project_role && project.add_role(user, user_project_role) - end - - it 'renders trace to authorized users' do - visit trace_project_job_path(project, job) + it 'renders trace to authorized users' do + visit trace_project_job_path(project, job) - expect(status_code).to eq(expected_status_code) - end + expect(status_code).to eq(expected_status_code) + end - it 'renders raw trace to authorized users' do - visit raw_project_job_path(project, job) + it 'renders raw trace to authorized users' do + visit raw_project_job_path(project, job) - expect(status_code).to eq(expected_status_code) - expect(page).to have_content(expected_msg) - end + expect(status_code).to eq(expected_status_code) + expect(page).to have_content(expected_msg) end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index dd1fbd7d0d5..a4d4cb87688 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -5109,52 +5109,42 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration do subject { build.debug_mode? } context 'when CI_DEBUG_TRACE=true is in variables' do - context 'when in instance variables' do - before do - create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: 'true') + ['true', 1, 'y'].each do |value| + it 'reflects instance variables' do + create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: value) + + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'reflects group variables' do + create(:ci_group_variable, key: 'CI_DEBUG_TRACE', value: value, group: project.group) - context 'when in group variables' do - before do - create(:ci_group_variable, key: 'CI_DEBUG_TRACE', value: 'true', group: project.group) + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'reflects pipeline variables' do + create(:ci_pipeline_variable, key: 'CI_DEBUG_TRACE', value: value, pipeline: pipeline) - context 'when in pipeline variables' do - before do - create(:ci_pipeline_variable, key: 'CI_DEBUG_TRACE', value: 'true', pipeline: pipeline) + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'reflects project variables' do + create(:ci_variable, key: 'CI_DEBUG_TRACE', value: value, project: project) - context 'when in project variables' do - before do - create(:ci_variable, key: 'CI_DEBUG_TRACE', value: 'true', project: project) + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'reflects job variables' do + create(:ci_job_variable, key: 'CI_DEBUG_TRACE', value: value, job: build) - context 'when in job variables' do - before do - create(:ci_job_variable, key: 'CI_DEBUG_TRACE', value: 'true', job: build) + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'when in yaml variables' do + build.update!(yaml_variables: [{ key: :CI_DEBUG_TRACE, value: value.to_s }]) - context 'when in yaml variables' do - before do - build.update!(yaml_variables: [{ key: :CI_DEBUG_TRACE, value: 'true' }]) + is_expected.to eq true end - - it { is_expected.to eq true } end end @@ -5163,58 +5153,64 @@ RSpec.describe Ci::Build, feature_category: :continuous_integration do end context 'when CI_DEBUG_SERVICES=true is in variables' do - context 'when in instance variables' do - before do - create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: 'true') + ['true', 1, 'y'].each do |value| + it 'reflects instance variables' do + create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: value) + + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'reflects group variables' do + create(:ci_group_variable, key: 'CI_DEBUG_SERVICES', value: value, group: project.group) - context 'when in group variables' do - before do - create(:ci_group_variable, key: 'CI_DEBUG_SERVICES', value: 'true', group: project.group) + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'reflects pipeline variables' do + create(:ci_pipeline_variable, key: 'CI_DEBUG_SERVICES', value: value, pipeline: pipeline) - context 'when in pipeline variables' do - before do - create(:ci_pipeline_variable, key: 'CI_DEBUG_SERVICES', value: 'true', pipeline: pipeline) + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'reflects project variables' do + create(:ci_variable, key: 'CI_DEBUG_SERVICES', value: value, project: project) - context 'when in project variables' do - before do - create(:ci_variable, key: 'CI_DEBUG_SERVICES', value: 'true', project: project) + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'reflects job variables' do + create(:ci_job_variable, key: 'CI_DEBUG_SERVICES', value: value, job: build) - context 'when in job variables' do - before do - create(:ci_job_variable, key: 'CI_DEBUG_SERVICES', value: 'true', job: build) + is_expected.to eq true end - it { is_expected.to eq true } - end + it 'when in yaml variables' do + build.update!(yaml_variables: [{ key: :CI_DEBUG_SERVICES, value: value.to_s }]) - context 'when in yaml variables' do - before do - build.update!(yaml_variables: [{ key: :CI_DEBUG_SERVICES, value: 'true' }]) + is_expected.to eq true end - - it { is_expected.to eq true } end end context 'when CI_DEBUG_SERVICES is not in variables' do it { is_expected.to eq false } end + + context 'when metadata has debug_trace_enabled true' do + before do + build.metadata.update!(debug_trace_enabled: true) + end + + it { is_expected.to eq true } + end + + context 'when metadata has debug_trace_enabled false' do + before do + build.metadata.update!(debug_trace_enabled: false) + end + + it { is_expected.to eq false } + end end describe '#drop_with_exit_code!' do diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 875bfc5b94f..f8a71792ad5 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -752,11 +752,7 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do end end - context 'when ci_debug_trace is set to true' do - before_all do - create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: true) - end - + shared_examples_for "additional access criteria" do where(:public_builds, :user_project_role, :expected_status) do true | 'developer' | :ok true | 'guest' | :forbidden @@ -778,30 +774,28 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do end end - context 'when ci_debug_services is set to true' do - before_all do - create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: true) + describe 'when metadata debug_trace_enabled is set to true' do + before do + job.metadata.update!(debug_trace_enabled: true) end - where(:public_builds, :user_project_role, :expected_status) do - true | 'developer' | :ok - true | 'guest' | :forbidden - false | 'developer' | :ok - false | 'guest' | :forbidden - end + it_behaves_like "additional access criteria" + end - with_them do - before do - project.update!(public_builds: public_builds) - project.add_role(user, user_project_role) + context 'when ci_debug_trace is set to true' do + before_all do + create(:ci_instance_variable, key: 'CI_DEBUG_TRACE', value: true) + end - get api("/projects/#{project.id}/jobs/#{job.id}/trace", api_user) - end + it_behaves_like "additional access criteria" + end - it 'renders successfully to authorized users' do - expect(response).to have_gitlab_http_status(expected_status) - end + context 'when ci_debug_services is set to true' do + before_all do + create(:ci_instance_variable, key: 'CI_DEBUG_SERVICES', value: true) end + + it_behaves_like "additional access criteria" end end -- cgit v1.2.1 From 05fdaf8aa373b617150e67d05efe43bab64165df Mon Sep 17 00:00:00 2001 From: GitLab Release Tools Bot Date: Thu, 2 Mar 2023 09:49:28 +0000 Subject: Update VERSION files [ci skip] --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 9ad4f49b15d..da467831eb0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -15.8.3 \ No newline at end of file +15.8.4 \ No newline at end of file -- cgit v1.2.1 From 54f2a13176e4d174500a39302d29895d7e729d38 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 2 Mar 2023 09:53:28 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-8-stable-ee --- CHANGELOG.md | 17 +++++++++++++++++ GITALY_SERVER_VERSION | 2 +- GITLAB_PAGES_VERSION | 2 +- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da0dd48b608..6071648bf66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 15.8.4 (2023-03-02) + +### Security (12 changes) + +- [Using builds metadata to determine debug_mode](gitlab-org/security/gitlab@169fdb3222a9701b5818ef7c00f8f292dc60495d) ([merge request](gitlab-org/security/gitlab!3035)) +- [Fix pagination limits for Commits API](gitlab-org/security/gitlab@3d58c0fef6429d1030d1dfce1ca523ef33a0054b) ([merge request](gitlab-org/security/gitlab!3072)) +- [Mask Google IAP account details in Prometheus integration](gitlab-org/security/gitlab@96426e4c799e9bf5e90e5e57b2e54235831819a3) ([merge request](gitlab-org/security/gitlab!3082)) +- [Stop Group Transfer Service if SAML Provider or SCIM token is present](gitlab-org/security/gitlab@9496a2ed22f73bf83e56b1ff502fefcfe777ad07) ([merge request](gitlab-org/security/gitlab!3097)) +- [Protect Datadog API key by changing Datadog site](gitlab-org/security/gitlab@c6804e50cb60fc4747ea573306eec17eb0dd25f9) ([merge request](gitlab-org/security/gitlab!3094)) +- [Protect integrations' sensitive information exposed via API](gitlab-org/security/gitlab@a408475163272b926e65b1cf56c9efde09eac8dd) ([merge request](gitlab-org/security/gitlab!3088)) +- [Disallow maintainer to create an owner access token](gitlab-org/security/gitlab@d184909f6ab9123a6131c5c37452ace5c4bc8d3d) ([merge request](gitlab-org/security/gitlab!3091)) +- [Paste only text content in work items title](gitlab-org/security/gitlab@d8c48ade46fd75ab62731fced05cdfa2451bcdfa) ([merge request](gitlab-org/security/gitlab!3075)) +- [Jira DVCS OAuth Open Redirect Vulnerability](gitlab-org/security/gitlab@91ee37eeaaae8cc6d923f6b4b28ce0d7914342dd) ([merge request](gitlab-org/security/gitlab!3063)) +- [Block private personal snippet from unauthorized users](gitlab-org/security/gitlab@d687866d69cbdf25a3ca7185974c02402345015d) ([merge request](gitlab-org/security/gitlab!3030)) +- [Verify Kroki diagram type](gitlab-org/security/gitlab@4ec26a4479e73233d0f77bc5a5e764d506c29faf) ([merge request](gitlab-org/security/gitlab!3055)) +- [Check read_release permission before showing releases in Tags API](gitlab-org/security/gitlab@32bf21efc32fcb6a3803993959b50d8a9cd07d25) ([merge request](gitlab-org/security/gitlab!3057)) + ## 15.8.3 (2023-02-15) ### Fixed (3 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 9ad4f49b15d..da467831eb0 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -15.8.3 \ No newline at end of file +15.8.4 \ No newline at end of file diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 9ad4f49b15d..da467831eb0 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -15.8.3 \ No newline at end of file +15.8.4 \ No newline at end of file -- cgit v1.2.1