From 47414496d427785d86832bcaca617233f904a2e0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 1 Mar 2023 18:28:24 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-9-stable-ee --- .../work_items/components/item_title.vue | 6 ++ .../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 +- .../work_items/components/item_title_spec.js | 24 +++++++- spec/lib/banzai/filter/kroki_filter_spec.rb | 7 +++ spec/requests/api/tags_spec.rb | 64 ++++++++++++++++++++++ 13 files changed, 217 insertions(+), 38 deletions(-) diff --git a/app/assets/javascripts/work_items/components/item_title.vue b/app/assets/javascripts/work_items/components/item_title.vue index 6aa3c54705c..1c0fed2dde9 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/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/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/lib/banzai/filter/kroki_filter_spec.rb b/spec/lib/banzai/filter/kroki_filter_spec.rb index a528c5835b2..1cd11161439 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, feature_category: :team_planning 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