summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2023-03-01 18:28:24 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2023-03-01 18:28:24 +0000
commit47414496d427785d86832bcaca617233f904a2e0 (patch)
tree55c0e9671c5f513654fabdfc6dea1982528a5f9e
parent6b75388b67c35271bc18f2dbd41a72accd927808 (diff)
downloadgitlab-ce-47414496d427785d86832bcaca617233f904a2e0.tar.gz
Add latest changes from gitlab-org/security/gitlab@15-9-stable-ee
-rw-r--r--app/assets/javascripts/work_items/components/item_title.vue6
-rw-r--r--app/controllers/oauth/jira_dvcs/authorizations_controller.rb13
-rw-r--r--app/finders/notes_finder.rb2
-rw-r--r--lib/api/entities/tag.rb8
-rw-r--r--lib/api/tags.rb10
-rw-r--r--lib/banzai/filter/kroki_filter.rb6
-rw-r--r--spec/controllers/oauth/jira_dvcs/authorizations_controller_spec.rb40
-rw-r--r--spec/finders/notes_finder_spec.rb47
-rw-r--r--spec/finders/snippets_finder_spec.rb25
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/tag.json3
-rw-r--r--spec/frontend/work_items/components/item_title_spec.js24
-rw-r--r--spec/lib/banzai/filter/kroki_filter_spec.rb7
-rw-r--r--spec/requests/api/tags_spec.rb64
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;
+ },
},
};
</script>
@@ -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 src="#{image_src}" />))
+ 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 = '<strong>bold text</strong>';
+ 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 '<img src="http://localhost:8000/nomnoml/svg/eNqLDsgsSixJrUmtTHXOL80rsVLwzCupKUrMTNHQtC7IzMlJTE_V0KyJyVNQiE5KTSxKidXVjS5ILCrKL4lFFrSyi07LL81RyM0vLckAysRGjxo8avCowaMGjxo8avCowaMGU8lgAE7mIdc=" hidden="" class="js-render-kroki" data-diagram="nomnoml" data-diagram-src="data:text/plain;base64,W1BpcmF0ZXxleWVDb3VudDog'
end
+
+ it 'verifies diagram type to avoid possible XSS' do
+ stub_application_setting(kroki_enabled: true, kroki_url: "http://localhost:8000")
+ doc = filter(%(<a><pre lang='f/" onerror=alert(1) onload=alert(1) '><code lang="wavedrom">xss</code></pre></a>))
+
+ expect(doc.to_s).to eq %(<a><pre lang='f/" onerror=alert(1) onload=alert(1) '><code lang="wavedrom">xss</code></pre></a>)
+ 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