From 351586876ffc217f757032d477f871da4ece3b77 Mon Sep 17 00:00:00 2001 From: David Planella Date: Sun, 16 Dec 2018 17:50:43 +0100 Subject: Fix leading whitespace in translatable string --- app/views/import/bitbucket_server/status.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/import/bitbucket_server/status.html.haml b/app/views/import/bitbucket_server/status.html.haml index ef69197e453..c4e27efc76f 100644 --- a/app/views/import/bitbucket_server/status.html.haml +++ b/app/views/import/bitbucket_server/status.html.haml @@ -29,7 +29,7 @@ %tr %th= _('From Bitbucket Server') %th= _('To GitLab') - %th= _(' Status') + %th= _('Status') %tbody - @already_added_projects.each do |project| %tr{ id: "project_#{project.id}", class: "#{project_status_css_class(project.import_status)}" } -- cgit v1.2.1 From 38e55d6b96e2bee43e7773182679fbcfe6f41ff3 Mon Sep 17 00:00:00 2001 From: David Planella Date: Sun, 16 Dec 2018 17:59:02 +0100 Subject: Update .pot file with changed strings --- locale/gitlab.pot | 3 --- 1 file changed, 3 deletions(-) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0584d2006f7..902cf366dc0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16,9 +16,6 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n" -msgid " Status" -msgstr "" - msgid "%d addition" msgid_plural "%d additions" msgstr[0] "" -- cgit v1.2.1 From 0f2dff6264256531d314a8ac7f701f13bcfc7020 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Tue, 23 Apr 2019 20:57:24 +1200 Subject: Exclude fields from note import --- lib/gitlab/import_export/import_export.yml | 3 +++ spec/lib/gitlab/import_export/project.json | 4 ++++ .../import_export/project_tree_restorer_spec.rb | 24 ++++++++++++++++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index ce268793128..ab0c9a32d42 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -141,6 +141,9 @@ excluded_attributes: - :external_diff_size issues: - :milestone_id + notes: + - :note_html + - :cached_markdown_version merge_requests: - :milestone_id - :ref_fetched diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 4a7accc4c52..24d376999fc 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -158,6 +158,8 @@ { "id": 351, "note": "Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi.", + "note_html": "

dodgy html

", + "cached_markdown_version": 434343, "noteable_type": "Issue", "author_id": 26, "created_at": "2016-06-14T15:02:47.770Z", @@ -182,6 +184,8 @@ "note": "Est reprehenderit quas aut aspernatur autem recusandae voluptatem.", "noteable_type": "Issue", "author_id": 25, + "note_html": "

dodgy html

", + "cached_markdown_version": 121212, "created_at": "2016-06-14T15:02:47.795Z", "updated_at": "2016-06-14T15:02:47.795Z", "project_id": 5, diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 6084dc96410..952844c5d6f 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -7,8 +7,8 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do # Using an admin for import, so we can check assignment of existing members @user = create(:admin) @existing_members = [ - create(:user, username: 'bernard_willms'), - create(:user, username: 'saul_will') + create(:user, username: 'bernard_willms'), + create(:user, username: 'saul_will') ] RSpec::Mocks.with_temporary_scope do @@ -21,6 +21,8 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch).with('feature', 'DCBA') allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch) + allow_any_instance_of(Project).to receive(:latest_cached_markdown_version).and_return(434343) + allow_any_instance_of(Note).to receive(:latest_cached_markdown_version).and_return(434343) project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project) @@ -58,6 +60,24 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(Milestone.find_by_description('test milestone').issues.count).to eq(2) end + context 'when importing a project with cached_markdown_version and note_html' do + let!(:issue) { Issue.find_by(description: 'Aliquam enim illo et possimus.') } + let(:note1) { issue.notes.select {|n| n.note.match(/Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi/)}.first } + let(:note2) { issue.notes.select {|n| n.note.match(/Est reprehenderit quas aut aspernatur autem recusandae voluptatem/)}.first } + + it 'does not import the note_html' do + expect(note1.note_html).to match(/Et hic est id similique et non nesciunt voluptate/) + end + + it 'does not set the old cached_markdown_version' do + expect(note2.cached_markdown_version).not_to eq(121212) + end + + it 'does not import the note_html' do + expect(note2.note_html).to match(/Est reprehenderit quas aut aspernatur autem recusandae voluptatem/) + end + end + it 'creates a valid pipeline note' do expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty end -- cgit v1.2.1 From d5bbc33aa58f506614629351a0192d6443c660b5 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Tue, 23 Apr 2019 21:46:26 +1200 Subject: Add changelog entry --- .../unreleased/security-58856-persistent-xss-in-note-objects.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/security-58856-persistent-xss-in-note-objects.yml diff --git a/changelogs/unreleased/security-58856-persistent-xss-in-note-objects.yml b/changelogs/unreleased/security-58856-persistent-xss-in-note-objects.yml new file mode 100644 index 00000000000..d9ad5af256a --- /dev/null +++ b/changelogs/unreleased/security-58856-persistent-xss-in-note-objects.yml @@ -0,0 +1,5 @@ +--- +title: Prevent XSS injection in note imports +merge_request: +author: +type: security -- cgit v1.2.1 From b8c3e60ea23cd8c6f32ed438fd3c155add47a8e1 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Tue, 23 Apr 2019 23:30:32 +1200 Subject: Re-stub stubbed method calls --- spec/lib/gitlab/import_export/project_tree_restorer_spec.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 952844c5d6f..8853a922e47 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -24,12 +24,12 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do allow_any_instance_of(Project).to receive(:latest_cached_markdown_version).and_return(434343) allow_any_instance_of(Note).to receive(:latest_cached_markdown_version).and_return(434343) - project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project) + @project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project) expect(Gitlab::ImportExport::RelationFactory).to receive(:create).with(hash_including(excluded_keys: ['whatever'])).and_call_original.at_least(:once) - allow(project_tree_restorer).to receive(:excluded_keys_for_relation).and_return(['whatever']) + allow(@project_tree_restorer).to receive(:excluded_keys_for_relation).and_return(['whatever']) - @restored_project_json = project_tree_restorer.restore + @restored_project_json = @project_tree_restorer.restore end end @@ -61,6 +61,12 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end context 'when importing a project with cached_markdown_version and note_html' do + before do + expect(Gitlab::ImportExport::RelationFactory).not_to receive(:create).with(hash_including(excluded_keys: ['whatever'])) + expect(Gitlab::ImportExport::RelationFactory).to receive(:create).with(hash_including(excluded_keys: %w(note_html, cached_markdown_version))).and_call_original.at_least(:once) + allow(@project_tree_restorer).to receive(:excluded_keys_for_relation).and_return(%w(note_html, cached_markdown_version)) + end + let!(:issue) { Issue.find_by(description: 'Aliquam enim illo et possimus.') } let(:note1) { issue.notes.select {|n| n.note.match(/Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi/)}.first } let(:note2) { issue.notes.select {|n| n.note.match(/Est reprehenderit quas aut aspernatur autem recusandae voluptatem/)}.first } -- cgit v1.2.1 From 7e6befc05de87ba44115bbae0274c65727f9c2b9 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Wed, 24 Apr 2019 14:31:20 +1200 Subject: Add disallowed fields to AttributeCleaner --- lib/gitlab/import_export/attribute_cleaner.rb | 14 +++++++++++++- lib/gitlab/import_export/import_export.yml | 3 --- spec/lib/gitlab/import_export/attribute_cleaner_spec.rb | 6 +++++- .../lib/gitlab/import_export/project_tree_restorer_spec.rb | 12 +++--------- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 93b37b7bc5f..1faa3c1614f 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -24,7 +24,19 @@ module Gitlab private def prohibited_key?(key) - key.end_with?('_id') && !ALLOWED_REFERENCES.include?(key) + return false if allowed_reference?(key) + + return true if 'cached_markdown_version'.equal?(key) + + prohibited_suffices = %w(_id _html) + prohibited_suffices.each do |suffix| + return true if key.end_with?(suffix) + end + false + end + + def allowed_reference?(key) + ALLOWED_REFERENCES.include?(key) end def excluded_key?(key) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index ab0c9a32d42..ce268793128 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -141,9 +141,6 @@ excluded_attributes: - :external_diff_size issues: - :milestone_id - notes: - - :note_html - - :cached_markdown_version merge_requests: - :milestone_id - :ref_fetched diff --git a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb index 536cc359d39..99669285d5b 100644 --- a/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb +++ b/spec/lib/gitlab/import_export/attribute_cleaner_spec.rb @@ -18,7 +18,11 @@ describe Gitlab::ImportExport::AttributeCleaner do 'notid' => 99, 'import_source' => 'whatever', 'import_type' => 'whatever', - 'non_existent_attr' => 'whatever' + 'non_existent_attr' => 'whatever', + 'some_html' => '

dodgy html

', + 'legit_html' => '

legit html

', + '_html' => '

perfectly ordinary html

', + 'cached_markdown_version' => 12345 } end diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 8853a922e47..7fd0ea539c2 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -10,6 +10,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do create(:user, username: 'bernard_willms'), create(:user, username: 'saul_will') ] + @markdown_classes = [AbuseReport, Appearance, ApplicationSetting, BroadcastMessage, Issue, Label, MergeRequest, Milestone, Namespace, Project, Release, ResourceLabelEvent, Snippet, UserStatus] RSpec::Mocks.with_temporary_scope do @project = create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project') @@ -21,8 +22,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch).with('feature', 'DCBA') allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch) - allow_any_instance_of(Project).to receive(:latest_cached_markdown_version).and_return(434343) - allow_any_instance_of(Note).to receive(:latest_cached_markdown_version).and_return(434343) + @markdown_classes.each {|klass| allow_any_instance_of(klass).to receive(:latest_cached_markdown_version).and_return(434343)} @project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project) @@ -61,18 +61,12 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end context 'when importing a project with cached_markdown_version and note_html' do - before do - expect(Gitlab::ImportExport::RelationFactory).not_to receive(:create).with(hash_including(excluded_keys: ['whatever'])) - expect(Gitlab::ImportExport::RelationFactory).to receive(:create).with(hash_including(excluded_keys: %w(note_html, cached_markdown_version))).and_call_original.at_least(:once) - allow(@project_tree_restorer).to receive(:excluded_keys_for_relation).and_return(%w(note_html, cached_markdown_version)) - end - let!(:issue) { Issue.find_by(description: 'Aliquam enim illo et possimus.') } let(:note1) { issue.notes.select {|n| n.note.match(/Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi/)}.first } let(:note2) { issue.notes.select {|n| n.note.match(/Est reprehenderit quas aut aspernatur autem recusandae voluptatem/)}.first } it 'does not import the note_html' do - expect(note1.note_html).to match(/Et hic est id similique et non nesciunt voluptate/) + expect(note1.note_html).to match(/Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi/) end it 'does not set the old cached_markdown_version' do -- cgit v1.2.1 From 8eae788fd4248db1fcab2062195e542baf04c936 Mon Sep 17 00:00:00 2001 From: Charlie Ablett Date: Wed, 24 Apr 2019 21:17:53 +0000 Subject: Use English instead of Latin --- lib/gitlab/import_export/attribute_cleaner.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 1faa3c1614f..2d64ea77060 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -28,8 +28,8 @@ module Gitlab return true if 'cached_markdown_version'.equal?(key) - prohibited_suffices = %w(_id _html) - prohibited_suffices.each do |suffix| + prohibited_suffixes = %w(_id _html) + prohibited_suffixes.each do |suffix| return true if key.end_with?(suffix) end false -- cgit v1.2.1 From 4bd331a568cff64f4097fdf0cc4e45727750f740 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Fri, 26 Apr 2019 09:40:00 +1200 Subject: Tighten up prohibited_key method --- lib/gitlab/import_export/attribute_cleaner.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 2d64ea77060..f476905b6db 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -29,10 +29,9 @@ module Gitlab return true if 'cached_markdown_version'.equal?(key) prohibited_suffixes = %w(_id _html) - prohibited_suffixes.each do |suffix| - return true if key.end_with?(suffix) + prohibited_suffixes.any? do |suffix| + true if key.end_with?(suffix) end - false end def allowed_reference?(key) -- cgit v1.2.1 From b240012c4f474fc9ac24afb3286fbc967cde86d4 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Mon, 29 Apr 2019 21:31:16 +1200 Subject: Further clarify `attribute_cleaner` --- lib/gitlab/import_export/attribute_cleaner.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index f476905b6db..537cb36c89b 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -4,6 +4,7 @@ module Gitlab module ImportExport class AttributeCleaner ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + ['group_id'] + PROHIBITED_SUFFIXES = %w(_id _html).freeze def self.clean(*args) new(*args).clean @@ -17,24 +18,17 @@ module Gitlab def clean @relation_hash.reject do |key, _value| - prohibited_key?(key) || !@relation_class.attribute_method?(key) || excluded_key?(key) + (prohibited_key?(key) && !permitted_key?(key)) || !@relation_class.attribute_method?(key) || excluded_key?(key) end.except('id') end private def prohibited_key?(key) - return false if allowed_reference?(key) - - return true if 'cached_markdown_version'.equal?(key) - - prohibited_suffixes = %w(_id _html) - prohibited_suffixes.any? do |suffix| - true if key.end_with?(suffix) - end + 'cached_markdown_version' == key || PROHIBITED_SUFFIXES.any? {|suffix| key.end_with?(suffix)} end - def allowed_reference?(key) + def permitted_key?(key) ALLOWED_REFERENCES.include?(key) end -- cgit v1.2.1 From 1cbdc5326c779999350481b0aae78364837f3bb6 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Tue, 30 Apr 2019 11:25:09 +1200 Subject: Refactor `attribute_cleaner` for readability --- lib/gitlab/import_export/attribute_cleaner.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 537cb36c89b..7f67f63f26b 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -18,13 +18,15 @@ module Gitlab def clean @relation_hash.reject do |key, _value| - (prohibited_key?(key) && !permitted_key?(key)) || !@relation_class.attribute_method?(key) || excluded_key?(key) + prohibited_key?(key) || !@relation_class.attribute_method?(key) || excluded_key?(key) end.except('id') end private def prohibited_key?(key) + return false if permitted_key?(key) + 'cached_markdown_version' == key || PROHIBITED_SUFFIXES.any? {|suffix| key.end_with?(suffix)} end -- cgit v1.2.1 From 3d0fc7fe2e9a5e9f3a24d3eed875044c196e5050 Mon Sep 17 00:00:00 2001 From: Ash McKenzie Date: Tue, 30 Apr 2019 17:43:01 +1200 Subject: Ensure Issue & MR note_html cannot be imported --- spec/lib/gitlab/import_export/project.json | 6 +++-- .../import_export/project_tree_restorer_spec.rb | 26 ++++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 24d376999fc..43afc067e08 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -158,8 +158,8 @@ { "id": 351, "note": "Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi.", - "note_html": "

dodgy html

", - "cached_markdown_version": 434343, + "note_html": "

something else entirely

", + "cached_markdown_version": 917504, "noteable_type": "Issue", "author_id": 26, "created_at": "2016-06-14T15:02:47.770Z", @@ -2367,6 +2367,8 @@ { "id": 671, "note": "Sit voluptatibus eveniet architecto quidem.", + "note_html": "

something else entirely

", + "cached_markdown_version": 917504, "noteable_type": "MergeRequest", "author_id": 26, "created_at": "2016-06-14T15:02:56.632Z", diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 7fd0ea539c2..056a1d52d78 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -10,7 +10,6 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do create(:user, username: 'bernard_willms'), create(:user, username: 'saul_will') ] - @markdown_classes = [AbuseReport, Appearance, ApplicationSetting, BroadcastMessage, Issue, Label, MergeRequest, Milestone, Namespace, Project, Release, ResourceLabelEvent, Snippet, UserStatus] RSpec::Mocks.with_temporary_scope do @project = create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project') @@ -22,7 +21,6 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch).with('feature', 'DCBA') allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch) - @markdown_classes.each {|klass| allow_any_instance_of(klass).to receive(:latest_cached_markdown_version).and_return(434343)} @project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project) @@ -61,20 +59,20 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end context 'when importing a project with cached_markdown_version and note_html' do - let!(:issue) { Issue.find_by(description: 'Aliquam enim illo et possimus.') } - let(:note1) { issue.notes.select {|n| n.note.match(/Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi/)}.first } - let(:note2) { issue.notes.select {|n| n.note.match(/Est reprehenderit quas aut aspernatur autem recusandae voluptatem/)}.first } - - it 'does not import the note_html' do - expect(note1.note_html).to match(/Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi/) - end - - it 'does not set the old cached_markdown_version' do - expect(note2.cached_markdown_version).not_to eq(121212) + context 'for an Issue' do + it 'does not import note_html' do + note_content = 'Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi' + issue_note = Issue.find_by(description: 'Aliquam enim illo et possimus.').notes.select { |n| n.note.match(/#{note_content}/)}.first + expect(issue_note.note_html).to match(/#{note_content}/) + end end - it 'does not import the note_html' do - expect(note2.note_html).to match(/Est reprehenderit quas aut aspernatur autem recusandae voluptatem/) + context 'for a Merge Request' do + it 'does not import note_html' do + note_content = 'Sit voluptatibus eveniet architecto quidem' + merge_request_note = MergeRequest.find_by(title: 'MR1').notes.select { |n| n.note.match(/#{note_content}/)}.first + expect(merge_request_note.note_html).to match(/#{note_content}/) + end end end -- cgit v1.2.1 From f2bc55d76f278e492902bec99534600d589177b7 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Tue, 30 Apr 2019 20:57:52 +1200 Subject: Remove accidental regressions --- spec/lib/gitlab/import_export/project_tree_restorer_spec.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 056a1d52d78..9aafa41feee 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -7,8 +7,8 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do # Using an admin for import, so we can check assignment of existing members @user = create(:admin) @existing_members = [ - create(:user, username: 'bernard_willms'), - create(:user, username: 'saul_will') + create(:user, username: 'bernard_willms'), + create(:user, username: 'saul_will') ] RSpec::Mocks.with_temporary_scope do @@ -22,12 +22,12 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch).with('feature', 'DCBA') allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch) - @project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project) + project_tree_restorer = described_class.new(user: @user, shared: @shared, project: @project) expect(Gitlab::ImportExport::RelationFactory).to receive(:create).with(hash_including(excluded_keys: ['whatever'])).and_call_original.at_least(:once) - allow(@project_tree_restorer).to receive(:excluded_keys_for_relation).and_return(['whatever']) + allow(project_tree_restorer).to receive(:excluded_keys_for_relation).and_return(['whatever']) - @restored_project_json = @project_tree_restorer.restore + @restored_project_json = project_tree_restorer.restore end end @@ -76,6 +76,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do end end + it 'creates a valid pipeline note' do expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty end -- cgit v1.2.1 From 4b46b530829cc3dd82c2620a76fbe637ca9009c0 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Wed, 1 May 2019 10:38:41 +1200 Subject: Add `html` to sensitive words --- lib/gitlab/import_export/attribute_cleaner.rb | 2 +- spec/features/projects/import_export/export_file_spec.rb | 2 +- spec/lib/gitlab/import_export/project_tree_restorer_spec.rb | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 7f67f63f26b..7bdef2b6cdb 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -4,7 +4,7 @@ module Gitlab module ImportExport class AttributeCleaner ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + ['group_id'] - PROHIBITED_SUFFIXES = %w(_id _html).freeze + PROHIBITED_SUFFIXES = %w[_id _html].freeze def self.clean(*args) new(*args).clean diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb index f76f9ba7577..9d74a96ab3d 100644 --- a/spec/features/projects/import_export/export_file_spec.rb +++ b/spec/features/projects/import_export/export_file_spec.rb @@ -12,7 +12,7 @@ describe 'Import/Export - project export integration test', :js do let(:export_path) { "#{Dir.tmpdir}/import_file_spec" } let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys } - let(:sensitive_words) { %w[pass secret token key encrypted] } + let(:sensitive_words) { %w[pass secret token key encrypted html] } let(:safe_list) do { token: [ProjectHook, Ci::Trigger, CommitStatus], diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index 9aafa41feee..9d2b69ea798 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -63,6 +63,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do it 'does not import note_html' do note_content = 'Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi' issue_note = Issue.find_by(description: 'Aliquam enim illo et possimus.').notes.select { |n| n.note.match(/#{note_content}/)}.first + expect(issue_note.note_html).to match(/#{note_content}/) end end @@ -71,12 +72,12 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do it 'does not import note_html' do note_content = 'Sit voluptatibus eveniet architecto quidem' merge_request_note = MergeRequest.find_by(title: 'MR1').notes.select { |n| n.note.match(/#{note_content}/)}.first + expect(merge_request_note.note_html).to match(/#{note_content}/) end end end - it 'creates a valid pipeline note' do expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty end -- cgit v1.2.1 From 0aff6238f777155750c567ab409d0bce60536526 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Wed, 1 May 2019 12:15:29 +1200 Subject: Change `prohibited_key` to use regexes --- lib/gitlab/import_export/attribute_cleaner.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 7bdef2b6cdb..c28a1674018 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -4,7 +4,7 @@ module Gitlab module ImportExport class AttributeCleaner ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + ['group_id'] - PROHIBITED_SUFFIXES = %w[_id _html].freeze + PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_html\Z/).freeze def self.clean(*args) new(*args).clean @@ -25,9 +25,7 @@ module Gitlab private def prohibited_key?(key) - return false if permitted_key?(key) - - 'cached_markdown_version' == key || PROHIBITED_SUFFIXES.any? {|suffix| key.end_with?(suffix)} + key =~ PROHIBITED_REFERENCES && !permitted_key?(key) end def permitted_key?(key) -- cgit v1.2.1 From d8bddb16624f34600069bb5d3540960b25176381 Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Wed, 10 Apr 2019 11:39:45 +0800 Subject: Validate MR branch names Prevents refspec as branch name, which would bypass branch protection when used in conjunction with rebase. HEAD seems to be a special case with lots of occurrence, so it is considered valid for now. Another special case is `refs/head/*`, which can be imported. --- app/models/merge_request.rb | 12 +++ changelogs/unreleased/security-60039.yml | 5 ++ lib/gitlab/git_ref_validator.rb | 23 +++++- spec/features/issuables/issuable_list_spec.rb | 2 +- spec/lib/gitlab/bitbucket_import/importer_spec.rb | 1 + spec/lib/gitlab/git_ref_validator_spec.rb | 92 ++++++++++++++++------- spec/models/merge_request_spec.rb | 36 +++++++++ 7 files changed, 141 insertions(+), 30 deletions(-) create mode 100644 changelogs/unreleased/security-60039.yml diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 458c57c1dc6..368772a5cf4 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -620,6 +620,8 @@ class MergeRequest < ApplicationRecord return end + [:source_branch, :target_branch].each { |attr| validate_branch_name(attr) } + if opened? similar_mrs = target_project .merge_requests @@ -640,6 +642,16 @@ class MergeRequest < ApplicationRecord end end + def validate_branch_name(attr) + return unless changes_include?(attr) + + branch = read_attribute(attr) + + return unless branch + + errors.add(attr) unless Gitlab::GitRefValidator.validate_merge_request_branch(branch) + end + def validate_target_project return true if target_project.merge_requests_enabled? diff --git a/changelogs/unreleased/security-60039.yml b/changelogs/unreleased/security-60039.yml new file mode 100644 index 00000000000..5edbf32ec97 --- /dev/null +++ b/changelogs/unreleased/security-60039.yml @@ -0,0 +1,5 @@ +--- +title: Prevent invalid branch for merge request +merge_request: +author: +type: security diff --git a/lib/gitlab/git_ref_validator.rb b/lib/gitlab/git_ref_validator.rb index 3f13ebeb9d0..dfff6823689 100644 --- a/lib/gitlab/git_ref_validator.rb +++ b/lib/gitlab/git_ref_validator.rb @@ -5,12 +5,15 @@ module Gitlab module GitRefValidator extend self + + EXPANDED_PREFIXES = %w[refs/heads/ refs/remotes/].freeze + DISALLOWED_PREFIXES = %w[-].freeze + # Validates a given name against the git reference specification # # Returns true for a valid reference name, false otherwise def validate(ref_name) - not_allowed_prefixes = %w(refs/heads/ refs/remotes/ -) - return false if ref_name.start_with?(*not_allowed_prefixes) + return false if ref_name.start_with?(*(EXPANDED_PREFIXES + DISALLOWED_PREFIXES)) return false if ref_name == 'HEAD' begin @@ -19,5 +22,21 @@ module Gitlab return false end end + + def validate_merge_request_branch(ref_name) + return false if ref_name.start_with?(*DISALLOWED_PREFIXES) + + expanded_name = if ref_name.start_with?(*EXPANDED_PREFIXES) + ref_name + else + "refs/heads/#{ref_name}" + end + + begin + Rugged::Reference.valid_name?(expanded_name) + rescue ArgumentError + return false + end + end end end diff --git a/spec/features/issuables/issuable_list_spec.rb b/spec/features/issuables/issuable_list_spec.rb index 7b6e9cd66b2..225b858742d 100644 --- a/spec/features/issuables/issuable_list_spec.rb +++ b/spec/features/issuables/issuable_list_spec.rb @@ -76,7 +76,7 @@ describe 'issuable list' do create(:issue, project: project, author: user) else create(:merge_request, source_project: project, source_branch: generate(:branch)) - source_branch = FFaker::Name.name + source_branch = FFaker::Lorem.characters(8) pipeline = create(:ci_empty_pipeline, project: project, ref: source_branch, status: %w(running failed success).sample, sha: 'any') create(:merge_request, title: FFaker::Lorem.sentence, source_project: project, source_branch: source_branch, head_pipeline: pipeline) end diff --git a/spec/lib/gitlab/bitbucket_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_import/importer_spec.rb index e1a2bae5fe8..fbe87aaefa1 100644 --- a/spec/lib/gitlab/bitbucket_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_import/importer_spec.rb @@ -5,6 +5,7 @@ describe Gitlab::BitbucketImport::Importer do before do stub_omniauth_provider('bitbucket') + stub_feature_flags(stricter_mr_branch_name: false) end let(:statuses) do diff --git a/spec/lib/gitlab/git_ref_validator_spec.rb b/spec/lib/gitlab/git_ref_validator_spec.rb index 3ab04a1c46d..6fc41cd64f9 100644 --- a/spec/lib/gitlab/git_ref_validator_spec.rb +++ b/spec/lib/gitlab/git_ref_validator_spec.rb @@ -1,31 +1,69 @@ require 'spec_helper' describe Gitlab::GitRefValidator do - it { expect(described_class.validate('feature/new')).to be_truthy } - it { expect(described_class.validate('implement_@all')).to be_truthy } - it { expect(described_class.validate('my_new_feature')).to be_truthy } - it { expect(described_class.validate('my-branch')).to be_truthy } - it { expect(described_class.validate('#1')).to be_truthy } - it { expect(described_class.validate('feature/refs/heads/foo')).to be_truthy } - it { expect(described_class.validate('feature/~new/')).to be_falsey } - it { expect(described_class.validate('feature/^new/')).to be_falsey } - it { expect(described_class.validate('feature/:new/')).to be_falsey } - it { expect(described_class.validate('feature/?new/')).to be_falsey } - it { expect(described_class.validate('feature/*new/')).to be_falsey } - it { expect(described_class.validate('feature/[new/')).to be_falsey } - it { expect(described_class.validate('feature/new/')).to be_falsey } - it { expect(described_class.validate('feature/new.')).to be_falsey } - it { expect(described_class.validate('feature\@{')).to be_falsey } - it { expect(described_class.validate('feature\new')).to be_falsey } - it { expect(described_class.validate('feature//new')).to be_falsey } - it { expect(described_class.validate('feature new')).to be_falsey } - it { expect(described_class.validate('refs/heads/')).to be_falsey } - it { expect(described_class.validate('refs/remotes/')).to be_falsey } - it { expect(described_class.validate('refs/heads/feature')).to be_falsey } - it { expect(described_class.validate('refs/remotes/origin')).to be_falsey } - it { expect(described_class.validate('-')).to be_falsey } - it { expect(described_class.validate('-branch')).to be_falsey } - it { expect(described_class.validate('.tag')).to be_falsey } - it { expect(described_class.validate('my branch')).to be_falsey } - it { expect(described_class.validate("\xA0\u0000\xB0")).to be_falsey } + using RSpec::Parameterized::TableSyntax + + context '.validate' do + it { expect(described_class.validate('feature/new')).to be_truthy } + it { expect(described_class.validate('implement_@all')).to be_truthy } + it { expect(described_class.validate('my_new_feature')).to be_truthy } + it { expect(described_class.validate('my-branch')).to be_truthy } + it { expect(described_class.validate('#1')).to be_truthy } + it { expect(described_class.validate('feature/refs/heads/foo')).to be_truthy } + it { expect(described_class.validate('feature/~new/')).to be_falsey } + it { expect(described_class.validate('feature/^new/')).to be_falsey } + it { expect(described_class.validate('feature/:new/')).to be_falsey } + it { expect(described_class.validate('feature/?new/')).to be_falsey } + it { expect(described_class.validate('feature/*new/')).to be_falsey } + it { expect(described_class.validate('feature/[new/')).to be_falsey } + it { expect(described_class.validate('feature/new/')).to be_falsey } + it { expect(described_class.validate('feature/new.')).to be_falsey } + it { expect(described_class.validate('feature\@{')).to be_falsey } + it { expect(described_class.validate('feature\new')).to be_falsey } + it { expect(described_class.validate('feature//new')).to be_falsey } + it { expect(described_class.validate('feature new')).to be_falsey } + it { expect(described_class.validate('refs/heads/')).to be_falsey } + it { expect(described_class.validate('refs/remotes/')).to be_falsey } + it { expect(described_class.validate('refs/heads/feature')).to be_falsey } + it { expect(described_class.validate('refs/remotes/origin')).to be_falsey } + it { expect(described_class.validate('-')).to be_falsey } + it { expect(described_class.validate('-branch')).to be_falsey } + it { expect(described_class.validate('+foo:bar')).to be_falsey } + it { expect(described_class.validate('foo:bar')).to be_falsey } + it { expect(described_class.validate('.tag')).to be_falsey } + it { expect(described_class.validate('my branch')).to be_falsey } + it { expect(described_class.validate("\xA0\u0000\xB0")).to be_falsey } + end + + context '.validate_merge_request_branch' do + it { expect(described_class.validate_merge_request_branch('HEAD')).to be_truthy } + it { expect(described_class.validate_merge_request_branch('feature/new')).to be_truthy } + it { expect(described_class.validate_merge_request_branch('implement_@all')).to be_truthy } + it { expect(described_class.validate_merge_request_branch('my_new_feature')).to be_truthy } + it { expect(described_class.validate_merge_request_branch('my-branch')).to be_truthy } + it { expect(described_class.validate_merge_request_branch('#1')).to be_truthy } + it { expect(described_class.validate_merge_request_branch('feature/refs/heads/foo')).to be_truthy } + it { expect(described_class.validate_merge_request_branch('feature/~new/')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature/^new/')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature/:new/')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature/?new/')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature/*new/')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature/[new/')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature/new/')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature/new.')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature\@{')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature\new')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature//new')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('feature new')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('refs/heads/master')).to be_truthy } + it { expect(described_class.validate_merge_request_branch('refs/heads/')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('refs/remotes/')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('-')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('-branch')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('+foo:bar')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('foo:bar')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('.tag')).to be_falsey } + it { expect(described_class.validate_merge_request_branch('my branch')).to be_falsey } + it { expect(described_class.validate_merge_request_branch("\xA0\u0000\xB0")).to be_falsey } + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 6f34ef9c1bc..2b78e1e361e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -150,6 +150,42 @@ describe MergeRequest do end end + context 'for branch' do + before do + stub_feature_flags(stricter_mr_branch_name: false) + end + + using RSpec::Parameterized::TableSyntax + + where(:branch_name, :valid) do + 'foo' | true + 'foo:bar' | false + '+foo:bar' | false + 'foo bar' | false + '-foo' | false + 'HEAD' | true + 'refs/heads/master' | true + end + + with_them do + it "validates source_branch" do + subject = build(:merge_request, source_branch: branch_name, target_branch: 'master') + + subject.valid? + + expect(subject.errors.added?(:source_branch)).to eq(!valid) + end + + it "validates target_branch" do + subject = build(:merge_request, source_branch: 'master', target_branch: branch_name) + + subject.valid? + + expect(subject.errors.added?(:target_branch)).to eq(!valid) + end + end + end + context 'for forks' do let(:project) { create(:project) } let(:fork1) { fork_project(project) } -- cgit v1.2.1 From 76469b508234c0528cbb5afb22e2dd776c11d56b Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Mon, 22 Apr 2019 14:57:08 +0900 Subject: Use full ref when creating MR pipeline in specs Continuation of 426488b7218e85ce69868ae4628801af2322b74a --- spec/services/ci/create_pipeline_service_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 866d709d446..0f5554fa577 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -923,7 +923,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end @@ -954,7 +954,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end @@ -983,7 +983,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end -- cgit v1.2.1 From c7e8f5c613754a7221d6b2f0b0e154b75c55dd84 Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Fri, 3 May 2019 03:01:57 +0800 Subject: Refactor spec to not use truthy or falsey --- spec/lib/gitlab/git_ref_validator_spec.rb | 116 +++++++++++++++--------------- 1 file changed, 58 insertions(+), 58 deletions(-) diff --git a/spec/lib/gitlab/git_ref_validator_spec.rb b/spec/lib/gitlab/git_ref_validator_spec.rb index 6fc41cd64f9..b63389af29f 100644 --- a/spec/lib/gitlab/git_ref_validator_spec.rb +++ b/spec/lib/gitlab/git_ref_validator_spec.rb @@ -4,66 +4,66 @@ describe Gitlab::GitRefValidator do using RSpec::Parameterized::TableSyntax context '.validate' do - it { expect(described_class.validate('feature/new')).to be_truthy } - it { expect(described_class.validate('implement_@all')).to be_truthy } - it { expect(described_class.validate('my_new_feature')).to be_truthy } - it { expect(described_class.validate('my-branch')).to be_truthy } - it { expect(described_class.validate('#1')).to be_truthy } - it { expect(described_class.validate('feature/refs/heads/foo')).to be_truthy } - it { expect(described_class.validate('feature/~new/')).to be_falsey } - it { expect(described_class.validate('feature/^new/')).to be_falsey } - it { expect(described_class.validate('feature/:new/')).to be_falsey } - it { expect(described_class.validate('feature/?new/')).to be_falsey } - it { expect(described_class.validate('feature/*new/')).to be_falsey } - it { expect(described_class.validate('feature/[new/')).to be_falsey } - it { expect(described_class.validate('feature/new/')).to be_falsey } - it { expect(described_class.validate('feature/new.')).to be_falsey } - it { expect(described_class.validate('feature\@{')).to be_falsey } - it { expect(described_class.validate('feature\new')).to be_falsey } - it { expect(described_class.validate('feature//new')).to be_falsey } - it { expect(described_class.validate('feature new')).to be_falsey } - it { expect(described_class.validate('refs/heads/')).to be_falsey } - it { expect(described_class.validate('refs/remotes/')).to be_falsey } - it { expect(described_class.validate('refs/heads/feature')).to be_falsey } - it { expect(described_class.validate('refs/remotes/origin')).to be_falsey } - it { expect(described_class.validate('-')).to be_falsey } - it { expect(described_class.validate('-branch')).to be_falsey } - it { expect(described_class.validate('+foo:bar')).to be_falsey } - it { expect(described_class.validate('foo:bar')).to be_falsey } - it { expect(described_class.validate('.tag')).to be_falsey } - it { expect(described_class.validate('my branch')).to be_falsey } - it { expect(described_class.validate("\xA0\u0000\xB0")).to be_falsey } + it { expect(described_class.validate('feature/new')).to be true } + it { expect(described_class.validate('implement_@all')).to be true } + it { expect(described_class.validate('my_new_feature')).to be true } + it { expect(described_class.validate('my-branch')).to be true } + it { expect(described_class.validate('#1')).to be true } + it { expect(described_class.validate('feature/refs/heads/foo')).to be true } + it { expect(described_class.validate('feature/~new/')).to be false } + it { expect(described_class.validate('feature/^new/')).to be false } + it { expect(described_class.validate('feature/:new/')).to be false } + it { expect(described_class.validate('feature/?new/')).to be false } + it { expect(described_class.validate('feature/*new/')).to be false } + it { expect(described_class.validate('feature/[new/')).to be false } + it { expect(described_class.validate('feature/new/')).to be false } + it { expect(described_class.validate('feature/new.')).to be false } + it { expect(described_class.validate('feature\@{')).to be false } + it { expect(described_class.validate('feature\new')).to be false } + it { expect(described_class.validate('feature//new')).to be false } + it { expect(described_class.validate('feature new')).to be false } + it { expect(described_class.validate('refs/heads/')).to be false } + it { expect(described_class.validate('refs/remotes/')).to be false } + it { expect(described_class.validate('refs/heads/feature')).to be false } + it { expect(described_class.validate('refs/remotes/origin')).to be false } + it { expect(described_class.validate('-')).to be false } + it { expect(described_class.validate('-branch')).to be false } + it { expect(described_class.validate('+foo:bar')).to be false } + it { expect(described_class.validate('foo:bar')).to be false } + it { expect(described_class.validate('.tag')).to be false } + it { expect(described_class.validate('my branch')).to be false } + it { expect(described_class.validate("\xA0\u0000\xB0")).to be false } end context '.validate_merge_request_branch' do - it { expect(described_class.validate_merge_request_branch('HEAD')).to be_truthy } - it { expect(described_class.validate_merge_request_branch('feature/new')).to be_truthy } - it { expect(described_class.validate_merge_request_branch('implement_@all')).to be_truthy } - it { expect(described_class.validate_merge_request_branch('my_new_feature')).to be_truthy } - it { expect(described_class.validate_merge_request_branch('my-branch')).to be_truthy } - it { expect(described_class.validate_merge_request_branch('#1')).to be_truthy } - it { expect(described_class.validate_merge_request_branch('feature/refs/heads/foo')).to be_truthy } - it { expect(described_class.validate_merge_request_branch('feature/~new/')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature/^new/')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature/:new/')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature/?new/')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature/*new/')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature/[new/')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature/new/')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature/new.')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature\@{')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature\new')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature//new')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('feature new')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('refs/heads/master')).to be_truthy } - it { expect(described_class.validate_merge_request_branch('refs/heads/')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('refs/remotes/')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('-')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('-branch')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('+foo:bar')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('foo:bar')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('.tag')).to be_falsey } - it { expect(described_class.validate_merge_request_branch('my branch')).to be_falsey } - it { expect(described_class.validate_merge_request_branch("\xA0\u0000\xB0")).to be_falsey } + it { expect(described_class.validate_merge_request_branch('HEAD')).to be true } + it { expect(described_class.validate_merge_request_branch('feature/new')).to be true } + it { expect(described_class.validate_merge_request_branch('implement_@all')).to be true } + it { expect(described_class.validate_merge_request_branch('my_new_feature')).to be true } + it { expect(described_class.validate_merge_request_branch('my-branch')).to be true } + it { expect(described_class.validate_merge_request_branch('#1')).to be true } + it { expect(described_class.validate_merge_request_branch('feature/refs/heads/foo')).to be true } + it { expect(described_class.validate_merge_request_branch('feature/~new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/^new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/:new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/?new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/*new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/[new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/new/')).to be false } + it { expect(described_class.validate_merge_request_branch('feature/new.')).to be false } + it { expect(described_class.validate_merge_request_branch('feature\@{')).to be false } + it { expect(described_class.validate_merge_request_branch('feature\new')).to be false } + it { expect(described_class.validate_merge_request_branch('feature//new')).to be false } + it { expect(described_class.validate_merge_request_branch('feature new')).to be false } + it { expect(described_class.validate_merge_request_branch('refs/heads/master')).to be true } + it { expect(described_class.validate_merge_request_branch('refs/heads/')).to be false } + it { expect(described_class.validate_merge_request_branch('refs/remotes/')).to be false } + it { expect(described_class.validate_merge_request_branch('-')).to be false } + it { expect(described_class.validate_merge_request_branch('-branch')).to be false } + it { expect(described_class.validate_merge_request_branch('+foo:bar')).to be false } + it { expect(described_class.validate_merge_request_branch('foo:bar')).to be false } + it { expect(described_class.validate_merge_request_branch('.tag')).to be false } + it { expect(described_class.validate_merge_request_branch('my branch')).to be false } + it { expect(described_class.validate_merge_request_branch("\xA0\u0000\xB0")).to be false } end end -- cgit v1.2.1 From 7c60c4be652ff81656663f71352e7b4856939bcd Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Thu, 2 May 2019 20:39:41 +0000 Subject: Add AWS ELB note about not supporting web sockets --- doc/administration/integration/terminal.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/administration/integration/terminal.md b/doc/administration/integration/terminal.md index 2596e3fe68b..c34858cd0db 100644 --- a/doc/administration/integration/terminal.md +++ b/doc/administration/integration/terminal.md @@ -43,6 +43,11 @@ detail below. ## Enabling and disabling terminal support +NOTE: **Note:** AWS Elastic Load Balancers (ELBs) do not support web sockets. +AWS Application Load Balancers (ALBs) must be used if you want web terminals +to work. See [AWS Elastic Load Balancing Product Comparison](https://aws.amazon.com/elasticloadbalancing/features/#compare) +for more information. + As web terminals use WebSockets, every HTTP/HTTPS reverse proxy in front of Workhorse needs to be configured to pass the `Connection` and `Upgrade` headers through to the next one in the chain. If you installed GitLab using Omnibus, or -- cgit v1.2.1 From b0fbf001dab134b6638411f0be209bc0d1460519 Mon Sep 17 00:00:00 2001 From: Patrick Derichs Date: Fri, 3 May 2019 15:09:20 +0200 Subject: Fix url redaction for issue links Add changelog entry Add missing href to all redactor specs and removed href assignment Remove obsolete spec If original_content is given, it should be used for link content --- ...ity-fix-project-existence-disclosure-master.yml | 5 ++++ lib/banzai/redactor.rb | 7 +++-- spec/lib/banzai/redactor_spec.rb | 32 ++++++++++++---------- 3 files changed, 28 insertions(+), 16 deletions(-) create mode 100644 changelogs/unreleased/security-fix-project-existence-disclosure-master.yml diff --git a/changelogs/unreleased/security-fix-project-existence-disclosure-master.yml b/changelogs/unreleased/security-fix-project-existence-disclosure-master.yml new file mode 100644 index 00000000000..084439c71d9 --- /dev/null +++ b/changelogs/unreleased/security-fix-project-existence-disclosure-master.yml @@ -0,0 +1,5 @@ +--- +title: Fix url redaction for issue links +merge_request: +author: +type: security diff --git a/lib/banzai/redactor.rb b/lib/banzai/redactor.rb index 7db5f5e1f7d..c2da7fec7cc 100644 --- a/lib/banzai/redactor.rb +++ b/lib/banzai/redactor.rb @@ -70,8 +70,11 @@ module Banzai # Build the raw tag just with a link as href and content if # it's originally a link pattern. We shouldn't return a plain text href. original_link = - if link_reference == 'true' && href = original_content - %(#{href}) + if link_reference == 'true' + href = node.attr('href') + content = original_content + + %(#{content}) end # The reference should be replaced by the original link's content, diff --git a/spec/lib/banzai/redactor_spec.rb b/spec/lib/banzai/redactor_spec.rb index aaeec953e4b..718649e0e10 100644 --- a/spec/lib/banzai/redactor_spec.rb +++ b/spec/lib/banzai/redactor_spec.rb @@ -13,10 +13,10 @@ describe Banzai::Redactor do it 'redacts an array of documents' do doc1 = Nokogiri::HTML - .fragment('foo') + .fragment('foo') doc2 = Nokogiri::HTML - .fragment('bar') + .fragment('bar') redacted_data = redactor.redact([doc1, doc2]) @@ -27,7 +27,7 @@ describe Banzai::Redactor do end it 'replaces redacted reference with inner HTML' do - doc = Nokogiri::HTML.fragment("foo") + doc = Nokogiri::HTML.fragment("foo") redactor.redact([doc]) expect(doc.to_html).to eq('foo') end @@ -35,20 +35,24 @@ describe Banzai::Redactor do context 'when data-original attribute provided' do let(:original_content) { 'foo' } it 'replaces redacted reference with original content' do - doc = Nokogiri::HTML.fragment("bar") + doc = Nokogiri::HTML.fragment("bar") redactor.redact([doc]) expect(doc.to_html).to eq(original_content) end - end - - it 'returns tag with original href if it is originally a link reference' do - href = 'http://localhost:3000' - doc = Nokogiri::HTML - .fragment("#{href}") - redactor.redact([doc]) + it 'does not replace redacted reference with original content if href is given' do + html = "Marge" + doc = Nokogiri::HTML.fragment(html) + redactor.redact([doc]) + expect(doc.to_html).to eq('Marge') + end - expect(doc.to_html).to eq('http://localhost:3000') + it 'uses the original content as the link content if given' do + html = "Marge" + doc = Nokogiri::HTML.fragment(html) + redactor.redact([doc]) + expect(doc.to_html).to eq('Homer') + end end end @@ -61,7 +65,7 @@ describe Banzai::Redactor do end it 'redacts an issue attached' do - doc = Nokogiri::HTML.fragment("foo") + doc = Nokogiri::HTML.fragment("foo") redactor.redact([doc]) @@ -69,7 +73,7 @@ describe Banzai::Redactor do end it 'redacts an external issue' do - doc = Nokogiri::HTML.fragment("foo") + doc = Nokogiri::HTML.fragment("foo") redactor.redact([doc]) -- cgit v1.2.1 From 8c7623e2f09ebaf7f9a0c018b38b84d861b2bd65 Mon Sep 17 00:00:00 2001 From: Ben Bodenmiller Date: Sat, 11 May 2019 21:59:39 +0000 Subject: list omnibus grafana installation details --- .../monitoring/performance/grafana_configuration.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/administration/monitoring/performance/grafana_configuration.md b/doc/administration/monitoring/performance/grafana_configuration.md index ab43ec2cc4f..187fb2f73a1 100644 --- a/doc/administration/monitoring/performance/grafana_configuration.md +++ b/doc/administration/monitoring/performance/grafana_configuration.md @@ -3,7 +3,7 @@ [Grafana](http://grafana.org/) is a tool that allows you to visualize time series metrics through graphs and dashboards. It supports several backend data stores, including InfluxDB. GitLab writes performance data to InfluxDB -and Grafana will allow you to query InfluxDB to display useful graphs. +and Grafana will allow you to query to display useful graphs. For the easiest installation and configuration, install Grafana on the same server as InfluxDB. For larger installations, you may want to split out these @@ -11,11 +11,13 @@ services. ## Installation -Grafana supplies package repositories (Yum/Apt) for easy installation. +[GitLab Omnibus can help you install Grafana (recommended)](https://docs.gitlab.com/omnibus/settings/grafana.html) +or Grafana supplies package repositories (Yum/Apt) for easy installation. See [Grafana installation documentation](http://docs.grafana.org/installation/) for detailed steps. -> **Note**: Before starting Grafana for the first time, set the admin user +NOTE: **Note:** +Before starting Grafana for the first time, set the admin user and password in `/etc/grafana/grafana.ini`. Otherwise, the default password will be `admin`. -- cgit v1.2.1 From b6424b378d3fd79a78c597f1c3d630ab2245f460 Mon Sep 17 00:00:00 2001 From: Patrick Derichs Date: Tue, 14 May 2019 13:16:30 +0200 Subject: Fix confidential issue label disclosure on milestone view Add changelog entry Method should be public Use milestonish method Use render data to filter labels Add specs for label visibility on milestone --- app/controllers/concerns/milestone_actions.rb | 8 ++++- ...-confidential-issue-label-visibility-master.yml | 5 ++++ .../projects/milestones_controller_spec.rb | 34 ++++++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/security-fix-confidential-issue-label-visibility-master.yml diff --git a/app/controllers/concerns/milestone_actions.rb b/app/controllers/concerns/milestone_actions.rb index cfff154c3dd..8b8b7db72f8 100644 --- a/app/controllers/concerns/milestone_actions.rb +++ b/app/controllers/concerns/milestone_actions.rb @@ -26,16 +26,22 @@ module MilestoneActions end end + # rubocop:disable Gitlab/ModuleWithInstanceVariables def labels respond_to do |format| format.html { redirect_to milestone_redirect_path } format.json do + milestone_labels = @milestone.issue_labels_visible_by_user(current_user) + render json: tabs_json("shared/milestones/_labels_tab", { - labels: @milestone.labels.map { |label| label.present(issuable_subject: @milestone.parent) } # rubocop:disable Gitlab/ModuleWithInstanceVariables + labels: milestone_labels.map do |label| + label.present(issuable_subject: @milestone.parent) + end }) end end end + # rubocop:enable Gitlab/ModuleWithInstanceVariables private diff --git a/changelogs/unreleased/security-fix-confidential-issue-label-visibility-master.yml b/changelogs/unreleased/security-fix-confidential-issue-label-visibility-master.yml new file mode 100644 index 00000000000..adfd8e1298f --- /dev/null +++ b/changelogs/unreleased/security-fix-confidential-issue-label-visibility-master.yml @@ -0,0 +1,5 @@ +--- +title: Fix confidential issue label disclosure on milestone view +merge_request: +author: +type: security diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index f8470a94f98..767cee7d54a 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -175,6 +175,40 @@ describe Projects::MilestonesController do end end + describe '#labels' do + render_views + + context 'as json' do + let!(:guest) { create(:user, username: 'guest1') } + let!(:group) { create(:group, :public) } + let!(:project) { create(:project, :public, group: group) } + let!(:label) { create(:label, title: 'test_label_on_private_issue', project: project) } + let!(:confidential_issue) { create(:labeled_issue, confidential: true, project: project, milestone: milestone, labels: [label]) } + + it 'does not render labels of private issues if user has no access' do + sign_in(guest) + + get :labels, params: { namespace_id: group.id, project_id: project.id, id: milestone.iid }, format: :json + + expect(response).to have_gitlab_http_status(200) + expect(response.content_type).to eq 'application/json' + + expect(json_response['html']).not_to include(label.title) + end + + it 'does render labels of private issues if user has access' do + sign_in(user) + + get :labels, params: { namespace_id: group.id, project_id: project.id, id: milestone.iid }, format: :json + + expect(response).to have_gitlab_http_status(200) + expect(response.content_type).to eq 'application/json' + + expect(json_response['html']).to include(label.title) + end + end + end + context 'promotion succeeds' do before do group.add_developer(user) -- cgit v1.2.1 From 1be66c4a098290d72cb19b4c844a9bee4eff630b Mon Sep 17 00:00:00 2001 From: Alexandru Croitor Date: Thu, 9 May 2019 16:09:04 +0300 Subject: Hide issue title on unsubscribe for anonymous users --- app/helpers/notifications_helper.rb | 4 + app/views/sent_notifications/unsubscribe.html.haml | 2 +- .../security-unsubscribing-from-issue.yml | 5 + .../sent_notifications_controller_spec.rb | 109 +++++++++++++++++++-- 4 files changed, 109 insertions(+), 11 deletions(-) create mode 100644 changelogs/unreleased/security-unsubscribing-from-issue.yml diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index a7ce7667916..11b9cf22142 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -100,4 +100,8 @@ module NotificationsHelper css_class: "icon notifications-icon js-notifications-icon" ) end + + def show_unsubscribe_title?(noteable) + can?(current_user, "read_#{noteable.to_ability_name}".to_sym, noteable) + end end diff --git a/app/views/sent_notifications/unsubscribe.html.haml b/app/views/sent_notifications/unsubscribe.html.haml index ca392e1adfc..22fcfcda297 100644 --- a/app/views/sent_notifications/unsubscribe.html.haml +++ b/app/views/sent_notifications/unsubscribe.html.haml @@ -1,6 +1,6 @@ - noteable = @sent_notification.noteable - noteable_type = @sent_notification.noteable_type.titleize.downcase -- noteable_text = %(#{noteable.title} (#{noteable.to_reference})) +- noteable_text = show_unsubscribe_title?(noteable) ? %(#{noteable.title} (#{noteable.to_reference})) : %(#{noteable.to_reference}) - page_title _("Unsubscribe"), noteable_text, noteable_type.pluralize, @sent_notification.project.full_name %h3.page-title diff --git a/changelogs/unreleased/security-unsubscribing-from-issue.yml b/changelogs/unreleased/security-unsubscribing-from-issue.yml new file mode 100644 index 00000000000..3a33a457c69 --- /dev/null +++ b/changelogs/unreleased/security-unsubscribing-from-issue.yml @@ -0,0 +1,5 @@ +--- +title: Hide confidential issue title on unsubscribe for anonymous users +merge_request: +author: +type: security diff --git a/spec/controllers/sent_notifications_controller_spec.rb b/spec/controllers/sent_notifications_controller_spec.rb index 2b9df71aa3a..89857a9d21b 100644 --- a/spec/controllers/sent_notifications_controller_spec.rb +++ b/spec/controllers/sent_notifications_controller_spec.rb @@ -4,15 +4,31 @@ require 'rails_helper' describe SentNotificationsController do let(:user) { create(:user) } - let(:project) { create(:project) } - let(:sent_notification) { create(:sent_notification, project: project, noteable: issue, recipient: user) } + let(:project) { create(:project, :public) } + let(:private_project) { create(:project, :private) } + let(:sent_notification) { create(:sent_notification, project: target_project, noteable: noteable, recipient: user) } let(:issue) do - create(:issue, project: project, author: user) do |issue| - issue.subscriptions.create(user: user, project: project, subscribed: true) + create(:issue, project: target_project) do |issue| + issue.subscriptions.create(user: user, project: target_project, subscribed: true) end end + let(:confidential_issue) do + create(:issue, project: target_project, confidential: true) do |issue| + issue.subscriptions.create(user: user, project: target_project, subscribed: true) + end + end + + let(:merge_request) do + create(:merge_request, source_project: target_project, target_project: target_project) do |mr| + mr.subscriptions.create(user: user, project: target_project, subscribed: true) + end + end + + let(:noteable) { issue } + let(:target_project) { project } + describe 'GET unsubscribe' do context 'when the user is not logged in' do context 'when the force param is passed' do @@ -34,20 +50,93 @@ describe SentNotificationsController do end context 'when the force param is not passed' do + render_views + before do get(:unsubscribe, params: { id: sent_notification.reply_key }) end - it 'does not unsubscribe the user' do - expect(issue.subscribed?(user, project)).to be_truthy + shared_examples 'unsubscribing as anonymous' do + it 'does not unsubscribe the user' do + expect(noteable.subscribed?(user, target_project)).to be_truthy + end + + it 'does not set the flash message' do + expect(controller).not_to set_flash[:notice] + end + + it 'renders unsubscribe page' do + expect(response.status).to eq(200) + expect(response).to render_template :unsubscribe + end end - it 'does not set the flash message' do - expect(controller).not_to set_flash[:notice] + context 'when project is public' do + context 'when unsubscribing from issue' do + let(:noteable) { issue } + + it 'shows issue title' do + expect(response.body).to include(issue.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from confidential issue' do + let(:noteable) { confidential_issue } + + it 'does not show issue title' do + expect(response.body).not_to include(confidential_issue.title) + expect(response.body).to include(confidential_issue.to_reference) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from merge request' do + let(:noteable) { merge_request } + + it 'shows merge request title' do + expect(response.body).to include(merge_request.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end end - it 'redirects to the login page' do - expect(response).to render_template :unsubscribe + context 'when project is not public' do + let(:target_project) { private_project } + + context 'when unsubscribing from issue' do + let(:noteable) { issue } + + it 'shows issue title' do + expect(response.body).not_to include(issue.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from confidential issue' do + let(:noteable) { confidential_issue } + + it 'does not show issue title' do + expect(response.body).not_to include(confidential_issue.title) + expect(response.body).to include(confidential_issue.to_reference) + end + + it_behaves_like 'unsubscribing as anonymous' + end + + context 'when unsubscribing from merge request' do + let(:noteable) { merge_request } + + it 'shows merge request title' do + expect(response.body).not_to include(merge_request.title) + end + + it_behaves_like 'unsubscribing as anonymous' + end end end end -- cgit v1.2.1 From b70b43d07ec27c6410e4a8d7ad417662a8823f8f Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 20 May 2019 11:08:31 -0300 Subject: Resolve: Milestones leaked via search API Fix milestone titles being leaked using search API when users cannot read milestones --- app/models/project.rb | 12 ++++++ .../security-fix_milestones_search_api_leak.yml | 5 +++ lib/gitlab/project_search_results.rb | 6 +++ lib/gitlab/search_results.rb | 28 +++++++++++-- spec/lib/gitlab/search_results_spec.rb | 24 +++++++++++ spec/models/project_spec.rb | 17 ++++++++ spec/requests/api/search_spec.rb | 46 ++++++++++++++++++++-- 7 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 changelogs/unreleased/security-fix_milestones_search_api_leak.yml diff --git a/app/models/project.rb b/app/models/project.rb index ab4da61dcf8..4ca14d1c2ac 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -406,6 +406,7 @@ class Project < ApplicationRecord scope :with_builds_enabled, -> { with_feature_enabled(:builds) } scope :with_issues_enabled, -> { with_feature_enabled(:issues) } scope :with_issues_available_for_user, ->(current_user) { with_feature_available_for_user(:issues, current_user) } + scope :with_merge_requests_available_for_user, ->(current_user) { with_feature_available_for_user(:merge_requests, current_user) } scope :with_merge_requests_enabled, -> { with_feature_enabled(:merge_requests) } scope :with_remote_mirrors, -> { joins(:remote_mirrors).where(remote_mirrors: { enabled: true }).distinct } @@ -596,6 +597,17 @@ class Project < ApplicationRecord def group_ids joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id) end + + # Returns ids of projects with milestones available for given user + # + # Used on queries to find milestones which user can see + # For example: Milestone.where(project_id: ids_with_milestone_available_for(user)) + def ids_with_milestone_available_for(user) + with_issues_enabled = with_issues_available_for_user(user).select(:id) + with_merge_requests_enabled = with_merge_requests_available_for_user(user).select(:id) + + from_union([with_issues_enabled, with_merge_requests_enabled]).select(:id) + end end def all_pipelines diff --git a/changelogs/unreleased/security-fix_milestones_search_api_leak.yml b/changelogs/unreleased/security-fix_milestones_search_api_leak.yml new file mode 100644 index 00000000000..5691550b602 --- /dev/null +++ b/changelogs/unreleased/security-fix_milestones_search_api_leak.yml @@ -0,0 +1,5 @@ +--- +title: 'Resolve: Milestones leaked via search API' +merge_request: +author: +type: security diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 78337518988..0f3b97e2317 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -138,6 +138,12 @@ module Gitlab project end + def filter_milestones_by_project(milestones) + return Milestone.none unless Ability.allowed?(@current_user, :read_milestone, @project) + + milestones.where(project_id: project.id) # rubocop: disable CodeReuse/ActiveRecord + end + def repository_project_ref @repository_project_ref ||= repository_ref || project.default_branch end diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index 4a097a00101..7c1e6b1baff 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -103,9 +103,11 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def milestones - milestones = Milestone.where(project_id: project_ids_relation) - milestones = milestones.search(query) - milestones.reorder('milestones.updated_at DESC') + milestones = Milestone.search(query) + + milestones = filter_milestones_by_project(milestones) + + milestones.reorder('updated_at DESC') end # rubocop: enable CodeReuse/ActiveRecord @@ -123,6 +125,26 @@ module Gitlab 'projects' end + # Filter milestones by authorized projects. + # For performance reasons project_id is being plucked + # to be used on a smaller query. + # + # rubocop: disable CodeReuse/ActiveRecord + def filter_milestones_by_project(milestones) + project_ids = + milestones.where(project_id: project_ids_relation) + .select(:project_id).distinct + .pluck(:project_id) + + return Milestone.none if project_ids.nil? + + authorized_project_ids_relation = + Project.where(id: project_ids).ids_with_milestone_available_for(current_user) + + milestones.where(project_id: authorized_project_ids_relation) + end + # rubocop: enable CodeReuse/ActiveRecord + # rubocop: disable CodeReuse/ActiveRecord def project_ids_relation limit_projects.select(:id).reorder(nil) diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index 312aa3be490..3d27156b356 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -256,4 +256,28 @@ describe Gitlab::SearchResults do expect(results.objects('merge_requests')).not_to include merge_request end + + context 'milestones' do + it 'returns correct set of milestones' do + private_project_1 = create(:project, :private) + private_project_2 = create(:project, :private) + internal_project = create(:project, :internal) + public_project_1 = create(:project, :public) + public_project_2 = create(:project, :public, :issues_disabled, :merge_requests_disabled) + private_project_1.add_developer(user) + # milestones that should not be visible + create(:milestone, project: private_project_2, title: 'Private project without access milestone') + create(:milestone, project: public_project_2, title: 'Public project with milestones disabled milestone') + # milestones that should be visible + milestone_1 = create(:milestone, project: private_project_1, title: 'Private project with access milestone', state: 'closed') + milestone_2 = create(:milestone, project: internal_project, title: 'Internal project milestone') + milestone_3 = create(:milestone, project: public_project_1, title: 'Public project with milestones enabled milestone') + # Global search scope takes user authorized projects, internal projects and public projects. + limit_projects = ProjectsFinder.new(current_user: user).execute + + milestones = described_class.new(user, limit_projects, 'milestone').objects('milestones') + + expect(milestones).to match_array([milestone_1, milestone_2, milestone_3]) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 425096d7e80..ba2411d0b95 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3170,6 +3170,23 @@ describe Project do end end + describe '.ids_with_milestone_available_for' do + let!(:user) { create(:user) } + + it 'returns project ids with milestones available for user' do + project_1 = create(:project, :public, :merge_requests_disabled, :issues_disabled) + project_2 = create(:project, :public, :merge_requests_disabled) + project_3 = create(:project, :public, :issues_disabled) + project_4 = create(:project, :public) + project_4.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE ) + + project_ids = described_class.ids_with_milestone_available_for(user).pluck(:id) + + expect(project_ids).to include(project_2.id, project_3.id) + expect(project_ids).not_to include(project_1.id, project_4.id) + end + end + describe '.with_feature_available_for_user' do let(:user) { create(:user) } let(:feature) { MergeRequest } diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 49672591b3b..42e0efa10b7 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -70,11 +70,30 @@ describe API::Search do context 'for milestones scope' do before do create(:milestone, project: project, title: 'awesome milestone') + end + + context 'when user can read project milestones' do + before do + get api('/search', user), params: { scope: 'milestones', search: 'awesome' } + end - get api('/search', user), params: { scope: 'milestones', search: 'awesome' } + it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' end - it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + context 'when user cannot read project milestones' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'returns empty array' do + get api('/search', user), params: { scope: 'milestones', search: 'awesome' } + + milestones = JSON.parse(response.body) + + expect(milestones).to be_empty + end + end end context 'for users scope' do @@ -318,11 +337,30 @@ describe API::Search do context 'for milestones scope' do before do create(:milestone, project: project, title: 'awesome milestone') + end + + context 'when user can read milestones' do + before do + get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } + end - get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } + it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' end - it_behaves_like 'response is correct', schema: 'public_api/v4/milestones' + context 'when user cannot read project milestones' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'returns empty array' do + get api("/projects/#{project.id}/search", user), params: { scope: 'milestones', search: 'awesome' } + + milestones = JSON.parse(response.body) + + expect(milestones).to be_empty + end + end end context 'for users scope' do -- cgit v1.2.1 From 9eeedfccbcbaa98265597a964cdf895f27fcf68b Mon Sep 17 00:00:00 2001 From: Ryan Cobb Date: Mon, 20 May 2019 13:36:59 -0600 Subject: Adds ruby and unicorn instrumentation This adds ruby and unicorn instrumentation. This was originally intended in 11.11 but due to performance concerns it was reverted. This new commit foregoes the sys-proctable gem was causing performance issues previously. --- .../monitoring/prometheus/gitlab_metrics.md | 13 ++++++--- lib/gitlab/metrics/samplers/ruby_sampler.rb | 31 +++++++++++++++------ lib/gitlab/metrics/samplers/unicorn_sampler.rb | 32 ++++++++++++++++------ lib/gitlab/metrics/system.rb | 26 ++++++++++++++++++ .../gitlab/metrics/samplers/ruby_sampler_spec.rb | 30 ++++++++++++++++++-- .../metrics/samplers/unicorn_sampler_spec.rb | 25 +++++++++++++---- spec/lib/gitlab/metrics/system_spec.rb | 24 ++++++++++++++++ 7 files changed, 152 insertions(+), 29 deletions(-) diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index c243dd9edbb..48bd709a2b7 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -43,10 +43,11 @@ The following metrics are available: | redis_ping_latency_seconds | Gauge | 9.4 | Round trip time of the redis ping | | user_session_logins_total | Counter | 9.4 | Counter of how many users have logged in | | upload_file_does_not_exist | Counter | 10.7 in EE, 11.5 in CE | Number of times an upload record could not find its file | -| failed_login_captcha_total | Gauge | 11.0 | Counter of failed CAPTCHA attempts during login | -| successful_login_captcha_total | Gauge | 11.0 | Counter of successful CAPTCHA attempts during login | -| unicorn_active_connections | Gauge | 11.0 | The number of active Unicorn connections (workers) | -| unicorn_queued_connections | Gauge | 11.0 | The number of queued Unicorn connections | +| failed_login_captcha_total | Gauge | 11.0 | Counter of failed CAPTCHA attempts during login | +| successful_login_captcha_total | Gauge | 11.0 | Counter of successful CAPTCHA attempts during login | +| unicorn_active_connections | Gauge | 11.0 | The number of active Unicorn connections (workers) | +| unicorn_queued_connections | Gauge | 11.0 | The number of queued Unicorn connections | +| unicorn_workers | Gauge | 11.11 | The number of Unicorn workers | ## Sidekiq Metrics available for Geo **[PREMIUM]** @@ -100,6 +101,10 @@ Some basic Ruby runtime metrics are available: | ruby_file_descriptors | Gauge | 11.1 | File descriptors per process | | ruby_memory_bytes | Gauge | 11.1 | Memory usage by process | | ruby_sampler_duration_seconds_total | Counter | 11.1 | Time spent collecting stats | +| ruby_process_cpu_seconds_total | Gauge | 11.11 | Total amount of CPU time per process | +| ruby_process_max_fds | Gauge | 11.11 | Maximum number of open file descriptors per process | +| ruby_process_resident_memory_bytes | Gauge | 11.11 | Memory usage by process, measured in bytes | +| ruby_process_start_time_seconds | Gauge | 11.11 | The elapsed time between system boot and the process started, measured in seconds | [GC.stat]: https://ruby-doc.org/core-2.3.0/GC.html#method-c-stat diff --git a/lib/gitlab/metrics/samplers/ruby_sampler.rb b/lib/gitlab/metrics/samplers/ruby_sampler.rb index 18a69321905..4d9c43f37e7 100644 --- a/lib/gitlab/metrics/samplers/ruby_sampler.rb +++ b/lib/gitlab/metrics/samplers/ruby_sampler.rb @@ -23,25 +23,32 @@ module Gitlab end def init_metrics - metrics = {} - metrics[:sampler_duration] = ::Gitlab::Metrics.counter(with_prefix(:sampler, :duration_seconds_total), 'Sampler time', labels) - metrics[:total_time] = ::Gitlab::Metrics.counter(with_prefix(:gc, :duration_seconds_total), 'Total GC time', labels) + metrics = { + file_descriptors: ::Gitlab::Metrics.gauge(with_prefix(:file, :descriptors), 'File descriptors used', labels, :livesum), + memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:memory, :bytes), 'Memory used', labels, :livesum), + process_cpu_seconds_total: ::Gitlab::Metrics.gauge(with_prefix(:process, :cpu_seconds_total), 'Process CPU seconds total'), + process_max_fds: ::Gitlab::Metrics.gauge(with_prefix(:process, :max_fds), 'Process max fds'), + process_resident_memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:process, :resident_memory_bytes), 'Memory used', labels, :livesum), + process_start_time_seconds: ::Gitlab::Metrics.gauge(with_prefix(:process, :start_time_seconds), 'Process start time seconds'), + sampler_duration: ::Gitlab::Metrics.counter(with_prefix(:sampler, :duration_seconds_total), 'Sampler time', labels), + total_time: ::Gitlab::Metrics.counter(with_prefix(:gc, :duration_seconds_total), 'Total GC time', labels) + } + GC.stat.keys.each do |key| metrics[key] = ::Gitlab::Metrics.gauge(with_prefix(:gc_stat, key), to_doc_string(key), labels, :livesum) end - metrics[:memory_usage] = ::Gitlab::Metrics.gauge(with_prefix(:memory, :bytes), 'Memory used', labels, :livesum) - metrics[:file_descriptors] = ::Gitlab::Metrics.gauge(with_prefix(:file, :descriptors), 'File descriptors used', labels, :livesum) - metrics end def sample start_time = System.monotonic_time - metrics[:memory_usage].set(labels.merge(worker_label), System.memory_usage) metrics[:file_descriptors].set(labels.merge(worker_label), System.file_descriptor_count) - + metrics[:process_cpu_seconds_total].set(labels.merge(worker_label), ::Gitlab::Metrics::System.cpu_time) + metrics[:process_max_fds].set(labels.merge(worker_label), ::Gitlab::Metrics::System.max_open_file_descriptors) + metrics[:process_start_time_seconds].set(labels.merge(worker_label), ::Gitlab::Metrics::System.process_start_time) + set_memory_usage_metrics sample_gc metrics[:sampler_duration].increment(labels, System.monotonic_time - start_time) @@ -61,6 +68,14 @@ module Gitlab metrics[:total_time].increment(labels, GC::Profiler.total_time) end + def set_memory_usage_metrics + memory_usage = System.memory_usage + memory_labels = labels.merge(worker_label) + + metrics[:memory_bytes].set(memory_labels, memory_usage) + metrics[:process_resident_memory_bytes].set(memory_labels, memory_usage) + end + def worker_label return {} unless defined?(Unicorn::Worker) diff --git a/lib/gitlab/metrics/samplers/unicorn_sampler.rb b/lib/gitlab/metrics/samplers/unicorn_sampler.rb index bec64e864b3..1b7a4c79080 100644 --- a/lib/gitlab/metrics/samplers/unicorn_sampler.rb +++ b/lib/gitlab/metrics/samplers/unicorn_sampler.rb @@ -8,12 +8,16 @@ module Gitlab super(interval) end - def unicorn_active_connections - @unicorn_active_connections ||= ::Gitlab::Metrics.gauge(:unicorn_active_connections, 'Unicorn active connections', {}, :max) + def metrics + @metrics ||= init_metrics end - def unicorn_queued_connections - @unicorn_queued_connections ||= ::Gitlab::Metrics.gauge(:unicorn_queued_connections, 'Unicorn queued connections', {}, :max) + def init_metrics + { + unicorn_active_connections: ::Gitlab::Metrics.gauge(:unicorn_active_connections, 'Unicorn active connections', {}, :max), + unicorn_queued_connections: ::Gitlab::Metrics.gauge(:unicorn_queued_connections, 'Unicorn queued connections', {}, :max), + unicorn_workers: ::Gitlab::Metrics.gauge(:unicorn_workers, 'Unicorn workers') + } end def enabled? @@ -23,14 +27,13 @@ module Gitlab def sample Raindrops::Linux.tcp_listener_stats(tcp_listeners).each do |addr, stats| - unicorn_active_connections.set({ socket_type: 'tcp', socket_address: addr }, stats.active) - unicorn_queued_connections.set({ socket_type: 'tcp', socket_address: addr }, stats.queued) + set_unicorn_connection_metrics('tcp', addr, stats) end - Raindrops::Linux.unix_listener_stats(unix_listeners).each do |addr, stats| - unicorn_active_connections.set({ socket_type: 'unix', socket_address: addr }, stats.active) - unicorn_queued_connections.set({ socket_type: 'unix', socket_address: addr }, stats.queued) + set_unicorn_connection_metrics('unix', addr, stats) end + + metrics[:unicorn_workers].set({}, unicorn_workers_count) end private @@ -39,6 +42,13 @@ module Gitlab @tcp_listeners ||= Unicorn.listener_names.grep(%r{\A[^/]+:\d+\z}) end + def set_unicorn_connection_metrics(type, addr, stats) + labels = { socket_type: type, socket_address: addr } + + metrics[:unicorn_active_connections].set(labels, stats.active) + metrics[:unicorn_queued_connections].set(labels, stats.queued) + end + def unix_listeners @unix_listeners ||= Unicorn.listener_names - tcp_listeners end @@ -46,6 +56,10 @@ module Gitlab def unicorn_with_listeners? defined?(Unicorn) && Unicorn.listener_names.any? end + + def unicorn_workers_count + `pgrep -f '[u]nicorn_rails worker.+ #{Rails.root.to_s}'`.split.count + end end end end diff --git a/lib/gitlab/metrics/system.rb b/lib/gitlab/metrics/system.rb index 426496855e3..d3cbd200584 100644 --- a/lib/gitlab/metrics/system.rb +++ b/lib/gitlab/metrics/system.rb @@ -23,6 +23,20 @@ module Gitlab def self.file_descriptor_count Dir.glob('/proc/self/fd/*').length end + + def self.max_open_file_descriptors + match = File.read('/proc/self/limits').match(/Max open files\s*(\d+)/) + + return unless match && match[1] + + match[1].to_i + end + + def self.process_start_time + fields = File.read('/proc/self/stat').split + + ( fields[21].to_i || 0 ) / clk_tck + end else def self.memory_usage 0.0 @@ -31,6 +45,14 @@ module Gitlab def self.file_descriptor_count 0 end + + def self.max_open_file_descriptors + 0 + end + + def self.process_start_time + 0 + end end # THREAD_CPUTIME is not supported on OS X @@ -59,6 +81,10 @@ module Gitlab def self.monotonic_time Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) end + + def self.clk_tck + @clk_tck ||= `genconf CLK_TCK`.to_i + end end end end diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 7972ff253fe..aaf8c9fa2a0 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -10,17 +10,20 @@ describe Gitlab::Metrics::Samplers::RubySampler do describe '#sample' do it 'samples various statistics' do - expect(Gitlab::Metrics::System).to receive(:memory_usage) + expect(Gitlab::Metrics::System).to receive(:cpu_time) expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) + expect(Gitlab::Metrics::System).to receive(:memory_usage) + expect(Gitlab::Metrics::System).to receive(:process_start_time) + expect(Gitlab::Metrics::System).to receive(:max_open_file_descriptors) expect(sampler).to receive(:sample_gc) sampler.sample end - it 'adds a metric containing the memory usage' do + it 'adds a metric containing the process resident memory bytes' do expect(Gitlab::Metrics::System).to receive(:memory_usage).and_return(9000) - expect(sampler.metrics[:memory_usage]).to receive(:set).with({}, 9000) + expect(sampler.metrics[:process_resident_memory_bytes]).to receive(:set).with({}, 9000) sampler.sample end @@ -34,6 +37,27 @@ describe Gitlab::Metrics::Samplers::RubySampler do sampler.sample end + it 'adds a metric containing the process total cpu time' do + expect(Gitlab::Metrics::System).to receive(:cpu_time).and_return(0.51) + expect(sampler.metrics[:process_cpu_seconds_total]).to receive(:set).with({}, 0.51) + + sampler.sample + end + + it 'adds a metric containing the process start time' do + expect(Gitlab::Metrics::System).to receive(:process_start_time).and_return(12345) + expect(sampler.metrics[:process_start_time_seconds]).to receive(:set).with({}, 12345) + + sampler.sample + end + + it 'adds a metric containing the process max file descriptors' do + expect(Gitlab::Metrics::System).to receive(:max_open_file_descriptors).and_return(1024) + expect(sampler.metrics[:process_max_fds]).to receive(:set).with({}, 1024) + + sampler.sample + end + it 'clears any GC profiles' do expect(GC::Profiler).to receive(:clear) diff --git a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb index 4b03f3c2532..090e456644f 100644 --- a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb @@ -39,8 +39,8 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do it 'updates metrics type unix and with addr' do labels = { socket_type: 'unix', socket_address: socket_address } - expect(subject).to receive_message_chain(:unicorn_active_connections, :set).with(labels, 'active') - expect(subject).to receive_message_chain(:unicorn_queued_connections, :set).with(labels, 'queued') + expect(subject.metrics[:unicorn_active_connections]).to receive(:set).with(labels, 'active') + expect(subject.metrics[:unicorn_queued_connections]).to receive(:set).with(labels, 'queued') subject.sample end @@ -50,7 +50,6 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do context 'unicorn listens on tcp sockets' do let(:tcp_socket_address) { '0.0.0.0:8080' } let(:tcp_sockets) { [tcp_socket_address] } - before do allow(unicorn).to receive(:listener_names).and_return(tcp_sockets) end @@ -71,13 +70,29 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do it 'updates metrics type unix and with addr' do labels = { socket_type: 'tcp', socket_address: tcp_socket_address } - expect(subject).to receive_message_chain(:unicorn_active_connections, :set).with(labels, 'active') - expect(subject).to receive_message_chain(:unicorn_queued_connections, :set).with(labels, 'queued') + expect(subject.metrics[:unicorn_active_connections]).to receive(:set).with(labels, 'active') + expect(subject.metrics[:unicorn_queued_connections]).to receive(:set).with(labels, 'queued') subject.sample end end end + + context 'additional metrics' do + let(:unicorn_workers) { 2 } + + before do + allow(unicorn).to receive(:listener_names).and_return([""]) + allow(::Gitlab::Metrics::System).to receive(:cpu_time).and_return(3.14) + allow(subject).to receive(:unicorn_workers_count).and_return(unicorn_workers) + end + + it "sets additional metrics" do + expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, unicorn_workers) + + subject.sample + end + end end describe '#start' do diff --git a/spec/lib/gitlab/metrics/system_spec.rb b/spec/lib/gitlab/metrics/system_spec.rb index 14afcdf5daa..b0603d96eb2 100644 --- a/spec/lib/gitlab/metrics/system_spec.rb +++ b/spec/lib/gitlab/metrics/system_spec.rb @@ -13,6 +13,18 @@ describe Gitlab::Metrics::System do expect(described_class.file_descriptor_count).to be > 0 end end + + describe '.max_open_file_descriptors' do + it 'returns the max allowed open file descriptors' do + expect(described_class.max_open_file_descriptors).to be > 0 + end + end + + describe '.process_start_time' do + it 'returns the process start time' do + expect(described_class.process_start_time).to be > 0 + end + end else describe '.memory_usage' do it 'returns 0.0' do @@ -25,6 +37,18 @@ describe Gitlab::Metrics::System do expect(described_class.file_descriptor_count).to eq(0) end end + + describe '.max_open_file_descriptors' do + it 'returns 0' do + expect(described_class.max_open_file_descriptors).to eq(0) + end + end + + describe 'process_start_time' do + it 'returns 0' do + expect(described_class.process_start_time).to eq(0) + end + end end describe '.cpu_time' do -- cgit v1.2.1 From 4fae62b9efa985e4cf6a09cb6bd4b9cf665e6b32 Mon Sep 17 00:00:00 2001 From: Ryan Cobb Date: Mon, 20 May 2019 14:11:46 -0600 Subject: Fix typo in system.rb --- lib/gitlab/metrics/system.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/metrics/system.rb b/lib/gitlab/metrics/system.rb index d3cbd200584..81bd799bd1e 100644 --- a/lib/gitlab/metrics/system.rb +++ b/lib/gitlab/metrics/system.rb @@ -83,7 +83,7 @@ module Gitlab end def self.clk_tck - @clk_tck ||= `genconf CLK_TCK`.to_i + @clk_tck ||= `getconf CLK_TCK`.to_i end end end -- cgit v1.2.1 From d1a7368e8e232fe996e86938548cf720eee3e138 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Tue, 21 May 2019 02:27:24 +0000 Subject: Bumps Kubernetes version to 1.11.10 --- changelogs/unreleased/auto-devops-kubernestes-bump1-11-10.yml | 5 +++++ lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/auto-devops-kubernestes-bump1-11-10.yml diff --git a/changelogs/unreleased/auto-devops-kubernestes-bump1-11-10.yml b/changelogs/unreleased/auto-devops-kubernestes-bump1-11-10.yml new file mode 100644 index 00000000000..9ba55719bdf --- /dev/null +++ b/changelogs/unreleased/auto-devops-kubernestes-bump1-11-10.yml @@ -0,0 +1,5 @@ +--- +title: Bumps Kubernetes in Auto DevOps to 1.11.10 +merge_request: 28525 +author: +type: other diff --git a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml index 939112e6e41..87cd62ef2d4 100644 --- a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml @@ -50,7 +50,7 @@ variables: POSTGRES_DB: $CI_ENVIRONMENT_SLUG POSTGRES_VERSION: 9.6.2 - KUBERNETES_VERSION: 1.11.9 + KUBERNETES_VERSION: 1.11.10 HELM_VERSION: 2.13.1 DOCKER_DRIVER: overlay2 -- cgit v1.2.1 From fab6a50f17d15d21a157d4d561f41527fa943f27 Mon Sep 17 00:00:00 2001 From: James Edwards-Jones Date: Wed, 12 Dec 2018 14:45:55 +0000 Subject: Prevent password sign in restriction bypass --- app/controllers/sessions_controller.rb | 9 ++++++ .../security-jej-prevent-web-sign-in-bypass.yml | 5 ++++ spec/controllers/sessions_controller_spec.rb | 34 +++++++++++++++++++++- 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/security-jej-prevent-web-sign-in-bypass.yml diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 6fea61cf45d..a841859621e 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -18,6 +18,7 @@ class SessionsController < Devise::SessionsController prepend_before_action :store_redirect_uri, only: [:new] prepend_before_action :ldap_servers, only: [:new, :create] prepend_before_action :require_no_authentication_without_flash, only: [:new, :create] + prepend_before_action :ensure_password_authentication_enabled!, if: :password_based_login?, only: [:create] before_action :auto_sign_in_with_provider, only: [:new] before_action :load_recaptcha @@ -138,6 +139,14 @@ class SessionsController < Devise::SessionsController end # rubocop: enable CodeReuse/ActiveRecord + def ensure_password_authentication_enabled! + render_403 unless Gitlab::CurrentSettings.password_authentication_enabled_for_web? + end + + def password_based_login? + user_params[:login].present? || user_params[:password].present? + end + def user_params params.require(:user).permit(:login, :password, :remember_me, :otp_attempt, :device_response) end diff --git a/changelogs/unreleased/security-jej-prevent-web-sign-in-bypass.yml b/changelogs/unreleased/security-jej-prevent-web-sign-in-bypass.yml new file mode 100644 index 00000000000..02773fa1d7c --- /dev/null +++ b/changelogs/unreleased/security-jej-prevent-web-sign-in-bypass.yml @@ -0,0 +1,5 @@ +--- +title: Prevent bypass of restriction disabling web password sign in +merge_request: +author: +type: security diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 6bcff7f975c..9c4ddce5409 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -58,7 +58,26 @@ describe SessionsController do it 'authenticates user correctly' do post(:create, params: { user: user_params }) - expect(subject.current_user). to eq user + expect(subject.current_user).to eq user + end + + context 'with password authentication disabled' do + before do + stub_application_setting(password_authentication_enabled_for_web: false) + end + + it 'does not sign in the user' do + post(:create, params: { user: user_params }) + + expect(@request.env['warden']).not_to be_authenticated + expect(subject.current_user).to be_nil + end + + it 'returns status 403' do + post(:create, params: { user: user_params }) + + expect(response.status).to eq 403 + end end it 'creates an audit log record' do @@ -153,6 +172,19 @@ describe SessionsController do end end + context 'with password authentication disabled' do + before do + stub_application_setting(password_authentication_enabled_for_web: false) + end + + it 'allows 2FA stage of non-password login' do + authenticate_2fa(otp_attempt: user.current_otp) + + expect(@request.env['warden']).to be_authenticated + expect(subject.current_user).to eq user + end + end + ## # See #14900 issue # -- cgit v1.2.1 From a3f88356b533b9991fbf360ded47155050fa49e2 Mon Sep 17 00:00:00 2001 From: Mark Lapierre Date: Wed, 22 May 2019 15:04:04 +1000 Subject: Reload page to check if branch is deleted When the "Delete merged branches" button is used the UI doesn't update automatically. So when the page is refreshed it's possible that the branch will still be present. This checks for the branch to be gone and reloads if it is not, repeating until the default timeout (60s). --- qa/qa/page/project/branches/show.rb | 8 +++++--- .../3_create/repository/add_list_delete_branches_spec.rb | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/qa/qa/page/project/branches/show.rb b/qa/qa/page/project/branches/show.rb index 762eacdab15..480fc7d78cb 100644 --- a/qa/qa/page/project/branches/show.rb +++ b/qa/qa/page/project/branches/show.rb @@ -28,9 +28,11 @@ module QA finished_loading? end - def has_no_branch?(branch_name) - within_element(:all_branches) do - has_no_element?(:branch_name, text: branch_name, wait: Support::Waiter::DEFAULT_MAX_WAIT_TIME) + def has_no_branch?(branch_name, reload: false) + wait(reload: reload) do + within_element(:all_branches) do + has_no_element?(:branch_name, text: branch_name) + end end end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb index cf6c24fa873..37a784248d4 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/add_list_delete_branches_spec.rb @@ -84,7 +84,7 @@ module QA page.refresh Page::Project::Branches::Show.perform do |branches_view| - expect(branches_view).to have_no_branch(second_branch) + expect(branches_view).to have_no_branch(second_branch, reload: true) end end end -- cgit v1.2.1 From a76fdcb7a30c6244ffb11a2e672e16d1e5b413b2 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Mon, 20 May 2019 13:24:22 -0700 Subject: Reject slug+uri concat if slug is deemed unsafe First reported: https://gitlab.com/gitlab-org/gitlab-ce/issues/60143 When the page slug is "javascript:" and we attempt to link to a relative path (using `.` or `..`) the code will concatenate the slug and the uri. This MR adds a guard to that concat step that will return `nil` if the incoming slug matches against any of the "unsafe" slug regexes; currently this is only for the slug "javascript:" but can be extended if needed. Manually tested against a non-exhaustive list from OWASP of common javascript XSS exploits that have to to with mangling the "javascript:" method, and all are caught by this change or by existing code that ingests the user-specified slug. --- ...urity-60143-address-xss-issue-in-wiki-links.yml | 5 +++ lib/banzai/filter/wiki_link_filter/rewriter.rb | 8 +++++ spec/lib/banzai/filter/wiki_link_filter_spec.rb | 42 ++++++++++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 changelogs/unreleased/security-60143-address-xss-issue-in-wiki-links.yml diff --git a/changelogs/unreleased/security-60143-address-xss-issue-in-wiki-links.yml b/changelogs/unreleased/security-60143-address-xss-issue-in-wiki-links.yml new file mode 100644 index 00000000000..5b79258af54 --- /dev/null +++ b/changelogs/unreleased/security-60143-address-xss-issue-in-wiki-links.yml @@ -0,0 +1,5 @@ +--- +title: Filter relative links in wiki for XSS +merge_request: +author: +type: security diff --git a/lib/banzai/filter/wiki_link_filter/rewriter.rb b/lib/banzai/filter/wiki_link_filter/rewriter.rb index f4cc8beeb52..77b5053f38c 100644 --- a/lib/banzai/filter/wiki_link_filter/rewriter.rb +++ b/lib/banzai/filter/wiki_link_filter/rewriter.rb @@ -4,6 +4,8 @@ module Banzai module Filter class WikiLinkFilter < HTML::Pipeline::Filter class Rewriter + UNSAFE_SLUG_REGEXES = [/\Ajavascript:/i].freeze + def initialize(link_string, wiki:, slug:) @uri = Addressable::URI.parse(link_string) @wiki_base_path = wiki && wiki.wiki_base_path @@ -35,6 +37,8 @@ module Banzai # Of the form `./link`, `../link`, or similar def apply_hierarchical_link_rules! + return if slug_considered_unsafe? + @uri = Addressable::URI.join(@slug, @uri) if @uri.to_s[0] == '.' end @@ -54,6 +58,10 @@ module Banzai def repository_upload? @uri.relative? && @uri.path.starts_with?(Wikis::CreateAttachmentService::ATTACHMENT_PATH) end + + def slug_considered_unsafe? + UNSAFE_SLUG_REGEXES.any? { |r| r.match?(@slug) } + end end end end diff --git a/spec/lib/banzai/filter/wiki_link_filter_spec.rb b/spec/lib/banzai/filter/wiki_link_filter_spec.rb index b9059b85fdc..cce1cd0b284 100644 --- a/spec/lib/banzai/filter/wiki_link_filter_spec.rb +++ b/spec/lib/banzai/filter/wiki_link_filter_spec.rb @@ -70,5 +70,47 @@ describe Banzai::Filter::WikiLinkFilter do expect(filtered_link.attribute('href').value).to eq(invalid_link) end end + + context "when the slug is deemed unsafe or invalid" do + let(:link) { "alert(1);" } + + invalid_slugs = [ + "javascript:", + "JaVaScRiPt:", + "\u0001java\u0003script:", + "javascript :", + "javascript: ", + "javascript : ", + ":javascript:", + "javascript:", + "javascript:", + "javascript:", + "javascript:", + "java\0script:", + "  javascript:" + ] + + invalid_slugs.each do |slug| + context "with the slug #{slug}" do + it "doesn't rewrite a (.) relative link" do + filtered_link = filter( + "Link", + project_wiki: wiki, + page_slug: slug).children[0] + + expect(filtered_link.attribute('href').value).not_to include(slug) + end + + it "doesn't rewrite a (..) relative link" do + filtered_link = filter( + "Link", + project_wiki: wiki, + page_slug: slug).children[0] + + expect(filtered_link.attribute('href').value).not_to include(slug) + end + end + end + end end end -- cgit v1.2.1 From c8a1395f9e9ec3731255e32e5abcdd12f780f030 Mon Sep 17 00:00:00 2001 From: antonyliu Date: Fri, 17 May 2019 11:39:30 +0800 Subject: Externalize strings of password page in user profile --- app/controllers/profiles/passwords_controller.rb | 10 +++--- app/views/profiles/passwords/edit.html.haml | 22 ++++++------- app/views/profiles/passwords/new.html.haml | 6 ++-- locale/gitlab.pot | 42 ++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 19 deletions(-) diff --git a/app/controllers/profiles/passwords_controller.rb b/app/controllers/profiles/passwords_controller.rb index 7038447581c..d2787c2e450 100644 --- a/app/controllers/profiles/passwords_controller.rb +++ b/app/controllers/profiles/passwords_controller.rb @@ -14,7 +14,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController def create unless @user.password_automatically_set || @user.valid_password?(user_params[:current_password]) - redirect_to new_profile_password_path, alert: 'You must provide a valid current password' + redirect_to new_profile_password_path, alert: _('You must provide a valid current password') return end @@ -29,7 +29,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController if result[:status] == :success Users::UpdateService.new(current_user, user: @user, password_expires_at: nil).execute - redirect_to root_path, notice: 'Password successfully changed' + redirect_to root_path, notice: _('Password successfully changed') else render :new end @@ -45,14 +45,14 @@ class Profiles::PasswordsController < Profiles::ApplicationController password_attributes[:password_automatically_set] = false unless @user.password_automatically_set || @user.valid_password?(user_params[:current_password]) - redirect_to edit_profile_password_path, alert: 'You must provide a valid current password' + redirect_to edit_profile_password_path, alert: _('You must provide a valid current password') return end result = Users::UpdateService.new(current_user, password_attributes.merge(user: @user)).execute if result[:status] == :success - flash[:notice] = "Password was successfully updated. Please login with it" + flash[:notice] = _('Password was successfully updated. Please login with it') redirect_to new_user_session_path else @user.reset @@ -62,7 +62,7 @@ class Profiles::PasswordsController < Profiles::ApplicationController def reset current_user.send_reset_password_instructions - redirect_to edit_profile_password_path, notice: 'We sent you an email with reset password instructions' + redirect_to edit_profile_password_path, notice: _('We sent you an email with reset password instructions') end private diff --git a/app/views/profiles/passwords/edit.html.haml b/app/views/profiles/passwords/edit.html.haml index 0b4b9841ea1..9f4f0ac2019 100644 --- a/app/views/profiles/passwords/edit.html.haml +++ b/app/views/profiles/passwords/edit.html.haml @@ -1,5 +1,5 @@ -- breadcrumb_title "Edit Password" -- page_title "Password" +- breadcrumb_title _('Edit Password') +- page_title _('Password') - @content_class = "limit-container-width" unless fluid_layout .row.prepend-top-default @@ -7,28 +7,28 @@ %h4.prepend-top-0 = page_title %p - After a successful password update, you will be redirected to the login page where you can log in with your new password. + = _('After a successful password update, you will be redirected to the login page where you can log in with your new password.') .col-lg-8 %h5.prepend-top-0 - Change your password + = _('Change your password') - unless @user.password_automatically_set? - or recover your current one + = _('or recover your current one') = form_for @user, url: profile_password_path, method: :put, html: {class: "update-password"} do |f| = form_errors(@user) - unless @user.password_automatically_set? .form-group - = f.label :current_password, class: 'label-bold' + = f.label :current_password, _('Current password'), class: 'label-bold' = f.password_field :current_password, required: true, class: 'form-control' %p.form-text.text-muted - You must provide your current password in order to change it. + = _('You must provide your current password in order to change it.') .form-group - = f.label :password, 'New password', class: 'label-bold' + = f.label :password, _('New password'), class: 'label-bold' = f.password_field :password, required: true, class: 'form-control' .form-group - = f.label :password_confirmation, class: 'label-bold' + = f.label :password_confirmation, _('Password confirmation'), class: 'label-bold' = f.password_field :password_confirmation, required: true, class: 'form-control' .prepend-top-default.append-bottom-default - = f.submit 'Save password', class: "btn btn-success append-right-10" + = f.submit _('Save password'), class: "btn btn-success append-right-10" - unless @user.password_automatically_set? - = link_to "I forgot my password", reset_profile_password_path, method: :put, class: "account-btn-link" + = link_to _('I forgot my password'), reset_profile_password_path, method: :put, class: "account-btn-link" diff --git a/app/views/profiles/passwords/new.html.haml b/app/views/profiles/passwords/new.html.haml index 4b84835429c..6c0f456b1ad 100644 --- a/app/views/profiles/passwords/new.html.haml +++ b/app/views/profiles/passwords/new.html.haml @@ -13,13 +13,13 @@ - unless @user.password_automatically_set? .form-group.row - = f.label :current_password, class: 'col-form-label col-sm-2' + = f.label :current_password, _('Current password'), class: 'col-form-label col-sm-2' .col-sm-10= f.password_field :current_password, required: true, class: 'form-control' .form-group.row - = f.label :password, class: 'col-form-label col-sm-2' + = f.label :password, _('New Password'), class: 'col-form-label col-sm-2' .col-sm-10= f.password_field :password, required: true, class: 'form-control' .form-group.row - = f.label :password_confirmation, class: 'col-form-label col-sm-2' + = f.label :password_confirmation, _('Password confirmation'), class: 'col-form-label col-sm-2' .col-sm-10 = f.password_field :password_confirmation, required: true, class: 'form-control' .form-actions diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 700266bc026..33188eaf11d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -751,6 +751,9 @@ msgstr "" msgid "After a successful password update you will be redirected to login screen." msgstr "" +msgid "After a successful password update, you will be redirected to the login page where you can log in with your new password." +msgstr "" + msgid "All" msgstr "" @@ -1764,6 +1767,9 @@ msgstr "" msgid "Change title" msgstr "" +msgid "Change your password" +msgstr "" + msgid "ChangeTypeActionLabel|Pick into branch" msgstr "" @@ -3080,6 +3086,9 @@ msgstr "" msgid "Current Branch" msgstr "" +msgid "Current password" +msgstr "" + msgid "CurrentUser|Profile" msgstr "" @@ -3580,6 +3589,9 @@ msgstr "" msgid "Edit Milestone" msgstr "" +msgid "Edit Password" +msgstr "" + msgid "Edit Pipeline Schedule %{id}" msgstr "" @@ -4946,6 +4958,9 @@ msgstr "" msgid "I accept the|Terms of Service and Privacy Policy" msgstr "" +msgid "I forgot my password" +msgstr "" + msgid "I have read and agree to the Let's Encrypt %{link_start}Terms of Service%{link_end}" msgstr "" @@ -6305,6 +6320,9 @@ msgstr "" msgid "New milestone" msgstr "" +msgid "New password" +msgstr "" + msgid "New pipelines will cancel older, pending pipelines on the same branch" msgstr "" @@ -6748,6 +6766,15 @@ msgstr "" msgid "Password authentication is unavailable." msgstr "" +msgid "Password confirmation" +msgstr "" + +msgid "Password successfully changed" +msgstr "" + +msgid "Password was successfully updated. Please login with it" +msgstr "" + msgid "Past due" msgstr "" @@ -8399,6 +8426,9 @@ msgstr "" msgid "Save comment" msgstr "" +msgid "Save password" +msgstr "" + msgid "Save pipeline schedule" msgstr "" @@ -10918,6 +10948,9 @@ msgstr "" msgid "We heard back from your U2F device. You have been authenticated." msgstr "" +msgid "We sent you an email with reset password instructions" +msgstr "" + msgid "We want to be sure it is you, please confirm you are not a robot." msgstr "" @@ -11313,6 +11346,12 @@ msgstr "" msgid "You must have permission to create a project in a namespace before forking." msgstr "" +msgid "You must provide a valid current password" +msgstr "" + +msgid "You must provide your current password in order to change it." +msgstr "" + msgid "You need permission." msgstr "" @@ -11940,6 +11979,9 @@ msgstr "" msgid "or" msgstr "" +msgid "or recover your current one" +msgstr "" + msgid "out of %d total test" msgid_plural "out of %d total tests" msgstr[0] "" -- cgit v1.2.1 From 9342b22df895f71c29ddc338d4611dd9c93e5aef Mon Sep 17 00:00:00 2001 From: antonyliu Date: Fri, 17 May 2019 13:59:31 +0800 Subject: Externalize strings of Two-Factor Authentication page in user profile --- app/controllers/profiles/accounts_controller.rb | 2 +- .../profiles/notifications_controller.rb | 4 +- .../profiles/two_factor_auths_controller.rb | 6 +- .../profiles/two_factor_auths/_codes.html.haml | 9 +-- .../profiles/two_factor_auths/codes.html.haml | 5 +- .../profiles/two_factor_auths/create.html.haml | 4 +- app/views/profiles/two_factor_auths/show.html.haml | 61 +++++++------- locale/gitlab.pot | 93 ++++++++++++++++++++++ 8 files changed, 136 insertions(+), 48 deletions(-) diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb index 0d2a6145d0e..b03f4b7435f 100644 --- a/app/controllers/profiles/accounts_controller.rb +++ b/app/controllers/profiles/accounts_controller.rb @@ -17,7 +17,7 @@ class Profiles::AccountsController < Profiles::ApplicationController if unlink_provider_allowed?(provider) identity.destroy else - flash[:alert] = "You are not allowed to unlink your primary login account" + flash[:alert] = _("You are not allowed to unlink your primary login account") end redirect_to profile_account_path diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb index b719b70c56e..617e5bb7cb3 100644 --- a/app/controllers/profiles/notifications_controller.rb +++ b/app/controllers/profiles/notifications_controller.rb @@ -14,9 +14,9 @@ class Profiles::NotificationsController < Profiles::ApplicationController result = Users::UpdateService.new(current_user, user_params.merge(user: current_user)).execute if result[:status] == :success - flash[:notice] = "Notification settings saved" + flash[:notice] = _("Notification settings saved") else - flash[:alert] = "Failed to save new settings" + flash[:alert] = _("Failed to save new settings") end redirect_back_or_default(default: profile_notifications_path) diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb index 83e14275a8b..95b9344c551 100644 --- a/app/controllers/profiles/two_factor_auths_controller.rb +++ b/app/controllers/profiles/two_factor_auths_controller.rb @@ -18,7 +18,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController two_factor_authentication_reason( global: lambda do flash.now[:alert] = - s_('The global settings require you to enable Two-Factor Authentication for your account.') + _('The global settings require you to enable Two-Factor Authentication for your account.') end, group: lambda do |groups| flash.now[:alert] = groups_notification(groups) @@ -27,7 +27,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController unless two_factor_grace_period_expired? grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours - flash.now[:alert] = flash.now[:alert] + s_(" You need to do this before %{grace_period_deadline}.") % { grace_period_deadline: l(grace_period_deadline) } + flash.now[:alert] = flash.now[:alert] + _(" You need to do this before %{grace_period_deadline}.") % { grace_period_deadline: l(grace_period_deadline) } end end @@ -44,7 +44,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController render 'create' else - @error = s_('Invalid pin code') + @error = _('Invalid pin code') @qr_code = build_qr_code setup_u2f_registration render 'show' diff --git a/app/views/profiles/two_factor_auths/_codes.html.haml b/app/views/profiles/two_factor_auths/_codes.html.haml index 759d39cf5f5..c2427cc2395 100644 --- a/app/views/profiles/two_factor_auths/_codes.html.haml +++ b/app/views/profiles/two_factor_auths/_codes.html.haml @@ -1,8 +1,5 @@ %p.slead - Should you ever lose your phone or access to your one time password secret, each of these recovery codes can be used one - time each to regain access to your account. Please save them in a safe place, or you - %b will - lose access to your account. + = _('Should you ever lose your phone or access to your one time password secret, each of these recovery codes can be used one time each to regain access to your account. Please save them in a safe place, or you %{b_start}will%{b_end} lose access to your account.'.html_safe) % { b_start:''.html_safe, b_end:''.html_safe } .codes.card %ul @@ -11,5 +8,5 @@ %span.monospace= code .d-flex - = link_to 'Proceed', profile_account_path, class: 'btn btn-success append-right-10' - = link_to 'Download codes', "data:text/plain;charset=utf-8,#{CGI.escape(@codes.join("\n"))}", download: "gitlab-recovery-codes.txt", class: 'btn btn-default' + = link_to _('Proceed'), profile_account_path, class: 'btn btn-success append-right-10' + = link_to _('Download codes'), "data:text/plain;charset=utf-8,#{CGI.escape(@codes.join("\n"))}", download: "gitlab-recovery-codes.txt", class: 'btn btn-default' diff --git a/app/views/profiles/two_factor_auths/codes.html.haml b/app/views/profiles/two_factor_auths/codes.html.haml index addf356697a..53907ebffab 100644 --- a/app/views/profiles/two_factor_auths/codes.html.haml +++ b/app/views/profiles/two_factor_auths/codes.html.haml @@ -1,5 +1,6 @@ -- page_title 'Recovery Codes', 'Two-factor Authentication' +- page_title _('Recovery Codes'), _('Two-factor Authentication') -%h3.page-title Two-factor Authentication Recovery codes +%h3.page-title + = _('Two-factor Authentication Recovery codes') %hr = render 'codes' diff --git a/app/views/profiles/two_factor_auths/create.html.haml b/app/views/profiles/two_factor_auths/create.html.haml index e330aadac13..973eb8136c4 100644 --- a/app/views/profiles/two_factor_auths/create.html.haml +++ b/app/views/profiles/two_factor_auths/create.html.haml @@ -1,6 +1,6 @@ -- page_title 'Two-factor Authentication', 'Account' +- page_title _('Two-factor Authentication'), _('Account') .alert.alert-success - Congratulations! You have enabled Two-factor Authentication! + = _('Congratulations! You have enabled Two-factor Authentication!') = render 'codes' diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml index d986c566928..0d7dabddb3a 100644 --- a/app/views/profiles/two_factor_auths/show.html.haml +++ b/app/views/profiles/two_factor_auths/show.html.haml @@ -1,72 +1,68 @@ -- page_title 'Two-Factor Authentication', 'Account' -- add_to_breadcrumbs("Two-Factor Authentication", profile_account_path) +- page_title _('Two-Factor Authentication'), _('Account') +- add_to_breadcrumbs(_('Two-Factor Authentication'), profile_account_path) - @content_class = "limit-container-width" unless fluid_layout .js-two-factor-auth{ 'data-two-factor-skippable' => "#{two_factor_skippable?}", 'data-two_factor_skip_url' => skip_profile_two_factor_auth_path } .row.prepend-top-default .col-lg-4 %h4.prepend-top-0 - Register Two-Factor Authenticator + = _('Register Two-Factor Authenticator') %p - Use an one time password authenticator on your mobile device or computer to enable two-factor authentication (2FA). + = _('Use an one time password authenticator on your mobile device or computer to enable two-factor authentication (2FA).') .col-lg-8 - if current_user.two_factor_otp_enabled? %p - You've already enabled two-factor authentication using one time password authenticators. In order to register a different device, you must first disable two-factor authentication. + = _("You've already enabled two-factor authentication using one time password authenticators. In order to register a different device, you must first disable two-factor authentication.") %p - If you lose your recovery codes you can generate new ones, invalidating all previous codes. + = _('If you lose your recovery codes you can generate new ones, invalidating all previous codes.') %div - = link_to 'Disable two-factor authentication', profile_two_factor_auth_path, + = link_to _('Disable two-factor authentication'), profile_two_factor_auth_path, method: :delete, - data: { confirm: "Are you sure? This will invalidate your registered applications and U2F devices." }, + data: { confirm: _('Are you sure? This will invalidate your registered applications and U2F devices.') }, class: 'btn btn-danger append-right-10' = form_tag codes_profile_two_factor_auth_path, {style: 'display: inline-block', method: :post} do |f| - = submit_tag 'Regenerate recovery codes', class: 'btn' + = submit_tag _('Regenerate recovery codes'), class: 'btn' - else %p - Install a soft token authenticator like FreeOTP - or Google Authenticator from your application repository and scan this QR code. - More information is available in the #{link_to('documentation', help_page_path('user/profile/account/two_factor_authentication'))}. + - document_url = help_page_path('user/profile/account/two_factor_authentication') + - help_link_start = ''.html_safe % { url: document_url } + = _('Install a soft token authenticator like %{free_otp_link} or Google Authenticator from your application repository and scan this QR code. More information is available in the %{help_link_start}documentation%{help_link_end}.'.html_safe) % { free_otp_link:'FreeOTP'.html_safe, help_link_start:help_link_start, help_link_end:''.html_safe } .row.append-bottom-10 .col-md-4 = raw @qr_code .col-md-8 .account-well %p.prepend-top-0.append-bottom-0 - Can't scan the code? + = _("Can't scan the code?") %p.prepend-top-0.append-bottom-0 - To add the entry manually, provide the following details to the application on your phone. + = _('To add the entry manually, provide the following details to the application on your phone.') %p.prepend-top-0.append-bottom-0 - Account: - = @account_string + = _('Account: %{account}') % { account: @account_string } %p.prepend-top-0.append-bottom-0 - Key: - = current_user.otp_secret.scan(/.{4}/).join(' ') + = _('Key: %{key}') %{ key: current_user.otp_secret.scan(/.{4}/).join(' ') } %p.two-factor-new-manual-content - Time based: Yes + = _('Time based: Yes') = form_tag profile_two_factor_auth_path, method: :post do |f| - if @error .alert.alert-danger = @error .form-group - = label_tag :pin_code, nil, class: "label-bold" + = label_tag :pin_code, _('Pin code'), class: "label-bold" = text_field_tag :pin_code, nil, class: "form-control", required: true .prepend-top-default - = submit_tag 'Register with two-factor app', class: 'btn btn-success' + = submit_tag _('Register with two-factor app'), class: 'btn btn-success' %hr .row.prepend-top-default .col-lg-4 %h4.prepend-top-0 - Register Universal Two-Factor (U2F) Device + = _('Register Universal Two-Factor (U2F) Device') %p - Use a hardware device to add the second factor of authentication. + = _('Use a hardware device to add the second factor of authentication.') %p - As U2F devices are only supported by a few browsers, we require that you set up a - two-factor authentication app before a U2F device. That way you'll always be able to - log in - even when you're using an unsupported browser. + = _("As U2F devices are only supported by a few browsers, we require that you set up a two-factor authentication app before a U2F device. That way you'll always be able to log in - even when you're using an unsupported browser.") .col-lg-8 - if @u2f_registration.errors.present? = form_errors(@u2f_registration) @@ -74,7 +70,8 @@ %hr - %h5 U2F Devices (#{@u2f_registrations.length}) + %h5 + = _('U2F Devices (%{length})') % { length: @u2f_registrations.length } - if @u2f_registrations.present? .table-responsive @@ -85,16 +82,16 @@ %col{ width: "20%" } %thead %tr - %th Name - %th Registered On + %th= _('Name') + %th= _('Registered On') %th %tbody - @u2f_registrations.each do |registration| %tr - %td= registration.name.presence || "" + %td= registration.name.presence || _("") %td= registration.created_at.to_date.to_s(:medium) - %td= link_to "Delete", profile_u2f_registration_path(registration), method: :delete, class: "btn btn-danger float-right", data: { confirm: "Are you sure you want to delete this device? This action cannot be undone." } + %td= link_to _('Delete'), profile_u2f_registration_path(registration), method: :delete, class: "btn btn-danger float-right", data: { confirm: _('Are you sure you want to delete this device? This action cannot be undone.') } - else .settings-message.text-center - You don't have any U2F devices registered yet. + = _("You don't have any U2F devices registered yet.") diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 700266bc026..8759036152c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -385,6 +385,9 @@ msgstr "" msgid "\"johnsmith@example.com\": \"johnsmith@example.com\" will add \"By johnsmith@example.com\" to all issues and comments originally created by johnsmith@example.com. By default, the email address or username is masked to ensure the user's privacy. Use this option if you want to show the full email address." msgstr "" +msgid "" +msgstr "" + msgid "%{changedFilesLength} unstaged and %{stagedFilesLength} staged changes" msgstr "" @@ -499,6 +502,9 @@ msgstr "" msgid "Account and limit" msgstr "" +msgid "Account: %{account}" +msgstr "" + msgid "Active" msgstr "" @@ -1096,6 +1102,9 @@ msgstr "" msgid "Are you sure you want to cancel editing this comment?" msgstr "" +msgid "Are you sure you want to delete this device? This action cannot be undone." +msgstr "" + msgid "Are you sure you want to delete this list?" msgstr "" @@ -1138,9 +1147,15 @@ msgstr "" msgid "Are you sure?" msgstr "" +msgid "Are you sure? This will invalidate your registered applications and U2F devices." +msgstr "" + msgid "Artifacts" msgstr "" +msgid "As U2F devices are only supported by a few browsers, we require that you set up a two-factor authentication app before a U2F device. That way you'll always be able to log in - even when you're using an unsupported browser." +msgstr "" + msgid "AsanaService|%{user} pushed to branch %{branch} of %{project_name} ( %{commit_url} ):" msgstr "" @@ -1713,6 +1728,9 @@ msgstr "" msgid "Can't find variable: ZiteReader" msgstr "" +msgid "Can't scan the code?" +msgstr "" + msgid "Cancel" msgstr "" @@ -2762,6 +2780,9 @@ msgstr "" msgid "Confirmation required" msgstr "" +msgid "Congratulations! You have enabled Two-factor Authentication!" +msgstr "" + msgid "Connect" msgstr "" @@ -3463,6 +3484,9 @@ msgstr "" msgid "Disable shared Runners" msgstr "" +msgid "Disable two-factor authentication" +msgstr "" + msgid "Disabled" msgstr "" @@ -3541,6 +3565,9 @@ msgstr "" msgid "Download asset" msgstr "" +msgid "Download codes" +msgstr "" + msgid "Download export" msgstr "" @@ -5018,6 +5045,9 @@ msgstr "" msgid "If this was a mistake you can leave the %{source_type}." msgstr "" +msgid "If you lose your recovery codes you can generate new ones, invalidating all previous codes." +msgstr "" + msgid "If your HTTP repository is not publicly accessible, add authentication information to the URL: https://username:password@gitlab.company.com/group/project.git." msgstr "" @@ -5477,6 +5507,9 @@ msgstr "" msgid "Key (PEM)" msgstr "" +msgid "Key: %{key}" +msgstr "" + msgid "Kubernetes" msgstr "" @@ -6515,6 +6548,9 @@ msgstr "" msgid "Notification setting - %{notification_title}" msgstr "" +msgid "Notification settings saved" +msgstr "" + msgid "NotificationEvent|Close issue" msgstr "" @@ -6802,6 +6838,9 @@ msgstr "" msgid "Pick a name" msgstr "" +msgid "Pin code" +msgstr "" + msgid "Pipeline" msgstr "" @@ -7168,6 +7207,9 @@ msgstr "" msgid "Private projects can be created in your personal namespace with:" msgstr "" +msgid "Proceed" +msgstr "" + msgid "Profile" msgstr "" @@ -7957,6 +7999,9 @@ msgstr "" msgid "Recent searches" msgstr "" +msgid "Recovery Codes" +msgstr "" + msgid "Reference:" msgstr "" @@ -7968,21 +8013,36 @@ msgstr[1] "" msgid "Regenerate key" msgstr "" +msgid "Regenerate recovery codes" +msgstr "" + msgid "Regex pattern" msgstr "" msgid "Register / Sign In" msgstr "" +msgid "Register Two-Factor Authenticator" +msgstr "" + msgid "Register U2F device" msgstr "" +msgid "Register Universal Two-Factor (U2F) Device" +msgstr "" + msgid "Register and see your runners for this group." msgstr "" msgid "Register and see your runners for this project." msgstr "" +msgid "Register with two-factor app" +msgstr "" + +msgid "Registered On" +msgstr "" + msgid "Registry" msgstr "" @@ -10020,6 +10080,9 @@ msgstr "" msgid "Thursday" msgstr "" +msgid "Time based: Yes" +msgstr "" + msgid "Time before an issue gets scheduled" msgstr "" @@ -10219,6 +10282,9 @@ msgstr "" msgid "To add an SSH key you need to %{generate_link_start}generate one%{link_end} or use an %{existing_link_start}existing key%{link_end}." msgstr "" +msgid "To add the entry manually, provide the following details to the application on your phone." +msgstr "" + msgid "To define internal users, first enable new users set to external" msgstr "" @@ -10429,6 +10495,15 @@ msgstr "" msgid "Twitter" msgstr "" +msgid "Two-Factor Authentication" +msgstr "" + +msgid "Two-factor Authentication" +msgstr "" + +msgid "Two-factor Authentication Recovery codes" +msgstr "" + msgid "Two-factor Authentication has been disabled for this user" msgstr "" @@ -10438,6 +10513,9 @@ msgstr "" msgid "Type" msgstr "" +msgid "U2F Devices (%{length})" +msgstr "" + msgid "U2F only works with HTTPS-enabled websites. Contact your administrator for more details." msgstr "" @@ -10618,6 +10696,12 @@ msgstr "" msgid "Use %{native_redirect_uri} for local tests" msgstr "" +msgid "Use a hardware device to add the second factor of authentication." +msgstr "" + +msgid "Use an one time password authenticator on your mobile device or computer to enable two-factor authentication (2FA)." +msgstr "" + msgid "Use group milestones to manage issues from multiple projects in the same milestone." msgstr "" @@ -11169,6 +11253,9 @@ msgstr "" msgid "You are going to transfer %{project_full_name} to another owner. Are you ABSOLUTELY sure?" msgstr "" +msgid "You are not allowed to unlink your primary login account" +msgstr "" + msgid "You are now impersonating %{username}" msgstr "" @@ -11268,6 +11355,9 @@ msgstr "" msgid "You do not have permission to leave this %{namespaceType}." msgstr "" +msgid "You don't have any U2F devices registered yet." +msgstr "" + msgid "You don't have any applications" msgstr "" @@ -11388,6 +11478,9 @@ msgstr "" msgid "You're receiving this email because of your account on %{host}. %{manage_notifications_link} · %{help_link}" msgstr "" +msgid "You've already enabled two-factor authentication using one time password authenticators. In order to register a different device, you must first disable two-factor authentication." +msgstr "" + msgid "YouTube" msgstr "" -- cgit v1.2.1 From 30f2f757674d70bfcd094667b4bdf24b936c080a Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Mon, 27 May 2019 00:30:10 +0000 Subject: Un-quarantine Auto DevOps QA test --- .../7_configure/auto_devops/create_project_with_auto_devops_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb index 0971e551db1..a69899dd3e7 100644 --- a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb +++ b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb @@ -9,8 +9,7 @@ module QA Page::Main::Login.perform(&:sign_in_using_credentials) end - # Transient failure issue: https://gitlab.com/gitlab-org/quality/nightly/issues/68 - describe 'Auto DevOps support', :orchestrated, :kubernetes, :quarantine do + describe 'Auto DevOps support', :orchestrated, :kubernetes do context 'when rbac is enabled' do before(:all) do @cluster = Service::KubernetesCluster.new.create! -- cgit v1.2.1 From f31efe747376ae1b4505dd6a7aca6c71db82714f Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Mon, 27 May 2019 14:02:20 +1200 Subject: Move public class methods out of private private modifier by itself does nothing for class methods (need `private_class_method` for that). As these are documented to be public methods in README, move them to the top. --- qa/qa/resource/base.rb | 64 +++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/qa/qa/resource/base.rb b/qa/qa/resource/base.rb index 523d92c7ef3..283fc6cdbcb 100644 --- a/qa/qa/resource/base.rb +++ b/qa/qa/resource/base.rb @@ -15,6 +15,38 @@ module QA def_delegators :evaluator, :attribute + def self.fabricate!(*args, &prepare_block) + fabricate_via_api!(*args, &prepare_block) + rescue NotImplementedError + fabricate_via_browser_ui!(*args, &prepare_block) + end + + def self.fabricate_via_browser_ui!(*args, &prepare_block) + options = args.extract_options! + resource = options.fetch(:resource) { new } + parents = options.fetch(:parents) { [] } + + do_fabricate!(resource: resource, prepare_block: prepare_block, parents: parents) do + log_fabrication(:browser_ui, resource, parents, args) { resource.fabricate!(*args) } + + current_url + end + end + + def self.fabricate_via_api!(*args, &prepare_block) + options = args.extract_options! + resource = options.fetch(:resource) { new } + parents = options.fetch(:parents) { [] } + + raise NotImplementedError unless resource.api_support? + + resource.eager_load_api_client! + + do_fabricate!(resource: resource, prepare_block: prepare_block, parents: parents) do + log_fabrication(:api, resource, parents, args) { resource.fabricate_via_api! } + end + end + def fabricate!(*_args) raise NotImplementedError end @@ -55,38 +87,6 @@ module QA QA::Runtime::Logger.info "<#{self.class}> Attribute #{name.inspect} has both API response `#{api_value}` and a block. API response will be picked. Block will be ignored." end - def self.fabricate!(*args, &prepare_block) - fabricate_via_api!(*args, &prepare_block) - rescue NotImplementedError - fabricate_via_browser_ui!(*args, &prepare_block) - end - - def self.fabricate_via_browser_ui!(*args, &prepare_block) - options = args.extract_options! - resource = options.fetch(:resource) { new } - parents = options.fetch(:parents) { [] } - - do_fabricate!(resource: resource, prepare_block: prepare_block, parents: parents) do - log_fabrication(:browser_ui, resource, parents, args) { resource.fabricate!(*args) } - - current_url - end - end - - def self.fabricate_via_api!(*args, &prepare_block) - options = args.extract_options! - resource = options.fetch(:resource) { new } - parents = options.fetch(:parents) { [] } - - raise NotImplementedError unless resource.api_support? - - resource.eager_load_api_client! - - do_fabricate!(resource: resource, prepare_block: prepare_block, parents: parents) do - log_fabrication(:api, resource, parents, args) { resource.fabricate_via_api! } - end - end - def self.do_fabricate!(resource:, prepare_block:, parents: []) prepare_block.call(resource) if prepare_block -- cgit v1.2.1 From d7cbeda57ef5e7b7fce50049c3429e5d46c12043 Mon Sep 17 00:00:00 2001 From: Mark Lapierre Date: Mon, 27 May 2019 13:52:36 +1000 Subject: Fabricate a file via the API --- qa/qa/resource/file.rb | 31 ++++++++++++++++++++-- .../user_views_commit_diff_patch_spec.rb | 26 +++++++++--------- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/qa/qa/resource/file.rb b/qa/qa/resource/file.rb index 57e82ac19ad..ca74654bf90 100644 --- a/qa/qa/resource/file.rb +++ b/qa/qa/resource/file.rb @@ -3,9 +3,12 @@ module QA module Resource class File < Base - attr_accessor :name, + attr_accessor :author_email, + :author_name, + :branch, :content, - :commit_message + :commit_message, + :name attribute :project do Project.fabricate! do |resource| @@ -31,6 +34,30 @@ module QA page.commit_changes end end + + def resource_web_url(resource) + super + rescue ResourceURLMissingError + # this particular resource does not expose a web_url property + end + + def api_get_path + "/projects/#{CGI.escape(project.path_with_namespace)}/repository/files/#{CGI.escape(@name)}" + end + + def api_post_path + api_get_path + end + + def api_post_body + { + branch: @branch || "master", + author_email: @author_email || Runtime::User.default_email, + author_name: @author_name || Runtime::User.username, + content: content, + commit_message: commit_message + } + end end end end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/user_views_commit_diff_patch_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/user_views_commit_diff_patch_spec.rb index b7400cdca97..680c5e21fa4 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/user_views_commit_diff_patch_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/user_views_commit_diff_patch_spec.rb @@ -3,13 +3,16 @@ module QA context 'Create' do # failure reported: https://gitlab.com/gitlab-org/quality/nightly/issues/42 - # also failing in staging until the fix is picked into the next release: - # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24533 describe 'Commit data', :quarantine do before(:context) do Runtime::Browser.visit(:gitlab, Page::Main::Login) Page::Main::Login.perform(&:sign_in_using_credentials) + # Get the user's details to confirm they're included in the email patch + @user = Resource::User.fabricate_via_api! do |user| + user.username = Runtime::User.username + end + project_push = Resource::Repository::ProjectPush.fabricate! do |push| push.file_name = 'README.md' push.file_content = '# This is a test project' @@ -21,12 +24,13 @@ module QA # add second file to repo to enable diff from initial commit @commit_message = 'Add second file' - Page::Project::Show.perform(&:create_new_file!) - Page::File::Form.perform do |f| - f.add_name('second') - f.add_content('second file content') - f.add_commit_message(@commit_message) - f.commit_changes + Resource::File.fabricate_via_api! do |file| + file.project = @project + file.name = 'second' + file.content = 'second file content' + file.commit_message = @commit_message + file.author_name = @user.name + file.author_email = @user.public_email end end @@ -42,15 +46,11 @@ module QA end it 'user views raw email patch' do - user = Resource::User.fabricate_via_api! do |user| - user.username = Runtime::User.username - end - view_commit Page::Project::Commit::Show.perform(&:select_email_patches) - expect(page).to have_content("From: #{user.name} <#{user.public_email}>") + expect(page).to have_content("From: #{@user.name} <#{@user.public_email}>") expect(page).to have_content('Subject: [PATCH] Add second file') expect(page).to have_content('diff --git a/second b/second') end -- cgit v1.2.1 From 3141b800b74576d5b5dfc84a96353fea47ae974b Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Tue, 21 May 2019 10:50:59 +0800 Subject: Fix dropdown position when loading remote data Also fixes flaky spec --- app/assets/javascripts/gl_dropdown.js | 4 ++++ app/assets/stylesheets/framework/dropdowns.scss | 4 ++-- ...reates-project-label-spec-features-boards-sidebar_spec-rb-350.yml | 5 +++++ spec/features/boards/sidebar_spec.rb | 4 ++++ 4 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/61639-flaky-spec-issue-boards-labels-creates-project-label-spec-features-boards-sidebar_spec-rb-350.yml diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 1c6b18c0e03..b9fe75c3664 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -335,6 +335,10 @@ GitLabDropdown = (function() { _this.fullData = data; _this.parseData(_this.fullData); _this.focusTextInput(); + + // Update dropdown position since remote data may have changed dropdown size + _this.dropdown.find('.dropdown-menu-toggle').dropdown('update'); + if ( _this.options.filterable && _this.filter && diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 8fb4027bf97..cd951f67293 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -570,10 +570,10 @@ } .dropdown-menu-close { - right: 5px; + top: $gl-padding-4; + right: $gl-padding-8; width: 20px; height: 20px; - top: -1px; } .dropdown-menu-close-icon { diff --git a/changelogs/unreleased/61639-flaky-spec-issue-boards-labels-creates-project-label-spec-features-boards-sidebar_spec-rb-350.yml b/changelogs/unreleased/61639-flaky-spec-issue-boards-labels-creates-project-label-spec-features-boards-sidebar_spec-rb-350.yml new file mode 100644 index 00000000000..9b4f13353f5 --- /dev/null +++ b/changelogs/unreleased/61639-flaky-spec-issue-boards-labels-creates-project-label-spec-features-boards-sidebar_spec-rb-350.yml @@ -0,0 +1,5 @@ +--- +title: Fix dropdown position when loading remote data +merge_request: 28526 +author: +type: fixed diff --git a/spec/features/boards/sidebar_spec.rb b/spec/features/boards/sidebar_spec.rb index 87c0dc40e5c..b1798c11361 100644 --- a/spec/features/boards/sidebar_spec.rb +++ b/spec/features/boards/sidebar_spec.rb @@ -352,6 +352,8 @@ describe 'Issue Boards', :js do page.within('.labels') do click_link 'Edit' + wait_for_requests + click_link 'Create project label' fill_in 'new_label_name', with: 'test label' first('.suggest-colors-dropdown a').click @@ -368,6 +370,8 @@ describe 'Issue Boards', :js do page.within('.labels') do click_link 'Edit' + wait_for_requests + click_link 'Create project label' fill_in 'new_label_name', with: 'test label' first('.suggest-colors-dropdown a').click -- cgit v1.2.1 From 9fc4ff52a5e5ef2d09583ea0fe26594080fc7739 Mon Sep 17 00:00:00 2001 From: Mark Lapierre Date: Mon, 27 May 2019 16:18:03 +1000 Subject: Add file via browser when testing the UI Now that it's possible to add a file via the API we need to make sure the UI test uses the browser UI fabrication, not the API. --- .../3_create/repository/create_edit_delete_file_via_web_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/create_edit_delete_file_via_web_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/create_edit_delete_file_via_web_spec.rb index 46346d1b984..d345fbfe995 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/create_edit_delete_file_via_web_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/create_edit_delete_file_via_web_spec.rb @@ -12,7 +12,7 @@ module QA file_content = 'QA Test - File content' commit_message_for_create = 'QA Test - Create new file' - Resource::File.fabricate! do |file| + Resource::File.fabricate_via_browser_ui! do |file| file.name = file_name file.content = file_content file.commit_message = commit_message_for_create -- cgit v1.2.1 From 0d62d9bcab0a20df7fca087d0904f90344e37227 Mon Sep 17 00:00:00 2001 From: Tim Rizzi Date: Mon, 27 May 2019 18:26:03 +0000 Subject: Update maven.gitlab-ci.yml for GitLab Package --- lib/gitlab/ci/templates/Maven.gitlab-ci.yml | 68 ++++------------------------- 1 file changed, 9 insertions(+), 59 deletions(-) diff --git a/lib/gitlab/ci/templates/Maven.gitlab-ci.yml b/lib/gitlab/ci/templates/Maven.gitlab-ci.yml index c9838c7a7ff..f0432eefce1 100644 --- a/lib/gitlab/ci/templates/Maven.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Maven.gitlab-ci.yml @@ -1,18 +1,11 @@ ---- +# This file is a template, and might need editing before it works on your project. # Build JAVA applications using Apache Maven (http://maven.apache.org) # For docker image tags see https://hub.docker.com/_/maven/ -# # For general lifecycle information see https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html -# -# This template will build and test your projects as well as create the documentation. -# +# This template will build and test your projects # * Caches downloaded dependencies and plugins between invocation. # * Verify but don't deploy merge requests. # * Deploy built artifacts from master branch only. -# * Shows how to use multiple jobs in test stage for verifying functionality -# with multiple JDKs. -# * Uses site:stage to collect the documentation for multi-module projects. -# * Publishes the documentation for `master` branch. variables: # This will suppress any download for dependencies and plugins or upload messages which would clutter the console log. @@ -29,72 +22,29 @@ cache: paths: - .m2/repository -# This will only validate and compile stuff and run e.g. maven-enforcer-plugin. -# Because some enforcer rules might check dependency convergence and class duplications -# we use `test-compile` here instead of `validate`, so the correct classpath is picked up. -.validate: &validate - stage: build - script: - - 'mvn $MAVEN_CLI_OPTS test-compile' - # For merge requests do not `deploy` but only run `verify`. # See https://maven.apache.org/guides/introduction/introduction-to-the-lifecycle.html .verify: &verify stage: test script: - - 'mvn $MAVEN_CLI_OPTS verify site site:stage' + - 'mvn $MAVEN_CLI_OPTS verify' except: - master -# Validate merge requests using JDK7 -validate:jdk7: - <<: *validate - image: maven:3.3.9-jdk-7 - -# Validate merge requests using JDK8 -validate:jdk8: - <<: *validate - image: maven:3.3.9-jdk-8 - -# Verify merge requests using JDK7 -verify:jdk7: - <<: *verify - image: maven:3.3.9-jdk-7 - # Verify merge requests using JDK8 verify:jdk8: <<: *verify image: maven:3.3.9-jdk-8 # For `master` branch run `mvn deploy` automatically. -# Here you need to decide whether you want to use JDK7 or 8. -# To get this working you need to define a volume while configuring your gitlab-ci-multi-runner. -# Mount your `settings.xml` as `/root/.m2/settings.xml` which holds your secrets. -# See https://maven.apache.org/settings.html deploy:jdk8: - # Use stage test here, so the pages job may later pickup the created site. - stage: test - script: - - 'mvn $MAVEN_CLI_OPTS deploy site site:stage' - only: - - master - # Archive up the built documentation site. - artifacts: - paths: - - target/staging - image: maven:3.3.9-jdk-8 - -pages: - image: busybox:latest stage: deploy script: - # Because Maven appends the artifactId automatically to the staging path if you did define a parent pom, - # you might need to use `mv target/staging/YOUR_ARTIFACT_ID public` instead. - - mv target/staging public - dependencies: - - deploy:jdk8 - artifacts: - paths: - - public + - if [ ! -f ci_settings.xml ]; + then echo CI settings missing, please see https://gitlab.com/help/user/project/packages/maven_repository.md#creating-maven-packages-with-gitlab-cicd for instructions.; + fi + - 'cp ci_settings.xml /root/.m2/settings.xml' + - 'mvn $MAVEN_CLI_OPTS deploy' only: - master + image: maven:3.3.9-jdk-8 -- cgit v1.2.1 From 347d91f86b73158d56a244fb2a865379456bd8ed Mon Sep 17 00:00:00 2001 From: Marcia Ramos Date: Mon, 27 May 2019 20:53:21 +0100 Subject: Add doc type For ssot epic --- doc/ci/enable_or_disable_ci.md | 4 ++++ doc/ci/git_submodules.md | 4 ++++ doc/ci/junit_test_reports.md | 4 ++++ doc/ci/metrics_reports.md | 4 ++++ doc/ci/multi_project_pipelines.md | 4 ++++ doc/ci/pipelines.md | 4 ++++ 6 files changed, 24 insertions(+) diff --git a/doc/ci/enable_or_disable_ci.md b/doc/ci/enable_or_disable_ci.md index 7aa7de97c43..9934d543991 100644 --- a/doc/ci/enable_or_disable_ci.md +++ b/doc/ci/enable_or_disable_ci.md @@ -1,3 +1,7 @@ +--- +type: reference +--- + # How to enable or disable GitLab CI/CD To effectively use GitLab CI/CD, you need a valid [`.gitlab-ci.yml`](yaml/README.md) diff --git a/doc/ci/git_submodules.md b/doc/ci/git_submodules.md index 37078230b34..551044dd76f 100644 --- a/doc/ci/git_submodules.md +++ b/doc/ci/git_submodules.md @@ -1,3 +1,7 @@ +--- +type: reference +--- + # Using Git submodules with GitLab CI > **Notes:** diff --git a/doc/ci/junit_test_reports.md b/doc/ci/junit_test_reports.md index 799217c9a08..fa78f53f563 100644 --- a/doc/ci/junit_test_reports.md +++ b/doc/ci/junit_test_reports.md @@ -1,3 +1,7 @@ +--- +type: reference +--- + # JUnit test reports > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/45318) in GitLab 11.2. diff --git a/doc/ci/metrics_reports.md b/doc/ci/metrics_reports.md index 83a7094faaa..b7824402d45 100644 --- a/doc/ci/metrics_reports.md +++ b/doc/ci/metrics_reports.md @@ -1,3 +1,7 @@ +--- +type: reference +--- + # Metrics Reports **[PREMIUM]** > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/9788) in [GitLab Premium](https://about.gitlab.com/pricing) 11.10. diff --git a/doc/ci/multi_project_pipelines.md b/doc/ci/multi_project_pipelines.md index 556059c01b6..17d5599e40a 100644 --- a/doc/ci/multi_project_pipelines.md +++ b/doc/ci/multi_project_pipelines.md @@ -1,3 +1,7 @@ +--- +type: reference +--- + # Multi-project pipelines **[PREMIUM]** > [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/issues/2121) in diff --git a/doc/ci/pipelines.md b/doc/ci/pipelines.md index 4dbe1a85588..8b0634ed268 100644 --- a/doc/ci/pipelines.md +++ b/doc/ci/pipelines.md @@ -1,3 +1,7 @@ +--- +type: reference +--- + # Creating and using CI/CD pipelines > Introduced in GitLab 8.8. -- cgit v1.2.1 From c718d915aaaefb70e9487a6e92f14f5fa51bc18e Mon Sep 17 00:00:00 2001 From: antony liu Date: Tue, 28 May 2019 07:55:20 +0000 Subject: Apply suggestion to app/views/profiles/two_factor_auths/show.html.haml --- app/views/profiles/two_factor_auths/show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/profiles/two_factor_auths/show.html.haml b/app/views/profiles/two_factor_auths/show.html.haml index 0d7dabddb3a..7a998bc3a1a 100644 --- a/app/views/profiles/two_factor_auths/show.html.haml +++ b/app/views/profiles/two_factor_auths/show.html.haml @@ -83,7 +83,7 @@ %thead %tr %th= _('Name') - %th= _('Registered On') + %th= s_('2FADevice|Registered On') %th %tbody - @u2f_registrations.each do |registration| -- cgit v1.2.1 From 478c2ea06022678190f5d304afb3bc911a467fb4 Mon Sep 17 00:00:00 2001 From: antonyliu Date: Tue, 28 May 2019 17:45:34 +0800 Subject: Regenerate locale/gitlab.pot --- locale/gitlab.pot | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8759036152c..196c62294ca 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -343,6 +343,9 @@ msgstr "" msgid "2FA enabled" msgstr "" +msgid "2FADevice|Registered On" +msgstr "" + msgid "3 days" msgstr "" @@ -8040,9 +8043,6 @@ msgstr "" msgid "Register with two-factor app" msgstr "" -msgid "Registered On" -msgstr "" - msgid "Registry" msgstr "" -- cgit v1.2.1 From 8f53e9ccc8771f1c8542a0b013a6e99ad6158ecd Mon Sep 17 00:00:00 2001 From: Peter Marko Date: Sun, 12 May 2019 11:19:02 +0200 Subject: Add notify_only_default_branch option to PipelinesEmailService --- .../project_services/pipelines_email_service.rb | 18 ++++- .../pipelines-email-default-branch-filter.yml | 5 ++ doc/api/services.md | 1 + lib/api/helpers/services_helpers.rb | 6 ++ .../pipelines_email_service_spec.rb | 76 +++++++++++++++++----- 5 files changed, 86 insertions(+), 20 deletions(-) create mode 100644 changelogs/unreleased/pipelines-email-default-branch-filter.yml diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb index 7ba69370f14..ae5d5038099 100644 --- a/app/models/project_services/pipelines_email_service.rb +++ b/app/models/project_services/pipelines_email_service.rb @@ -2,11 +2,11 @@ class PipelinesEmailService < Service prop_accessor :recipients - boolean_accessor :notify_only_broken_pipelines + boolean_accessor :notify_only_broken_pipelines, :notify_only_default_branch validates :recipients, presence: true, if: :valid_recipients? def initialize_properties - self.properties ||= { notify_only_broken_pipelines: true } + self.properties ||= { notify_only_broken_pipelines: true, notify_only_default_branch: false } end def title @@ -54,7 +54,9 @@ class PipelinesEmailService < Service placeholder: _('Emails separated by comma'), required: true }, { type: 'checkbox', - name: 'notify_only_broken_pipelines' } + name: 'notify_only_broken_pipelines' }, + { type: 'checkbox', + name: 'notify_only_default_branch' } ] end @@ -67,6 +69,16 @@ class PipelinesEmailService < Service end def should_pipeline_be_notified?(data) + notify_for_pipeline_branch?(data) && notify_for_pipeline?(data) + end + + def notify_for_pipeline_branch?(data) + return true unless notify_only_default_branch? + + data[:object_attributes][:ref] == data[:project][:default_branch] + end + + def notify_for_pipeline?(data) case data[:object_attributes][:status] when 'success' !notify_only_broken_pipelines? diff --git a/changelogs/unreleased/pipelines-email-default-branch-filter.yml b/changelogs/unreleased/pipelines-email-default-branch-filter.yml new file mode 100644 index 00000000000..4c2a54af0bf --- /dev/null +++ b/changelogs/unreleased/pipelines-email-default-branch-filter.yml @@ -0,0 +1,5 @@ +--- +title: Add notify_only_default_branch option to PipelinesEmailService +merge_request: 28271 +author: Peter Marko +type: added diff --git a/doc/api/services.md b/doc/api/services.md index 742abccb69e..01df2a50198 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -754,6 +754,7 @@ Parameters: | `recipients` | string | yes | Comma-separated list of recipient email addresses | | `add_pusher` | boolean | no | Add pusher to recipients list | | `notify_only_broken_pipelines` | boolean | no | Notify only broken pipelines | +| `notify_only_default_branch` | boolean | no | Send notifications only for the default branch ([introduced in GitLab 12.0](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/28271)) | ### Delete Pipeline-Emails service diff --git a/lib/api/helpers/services_helpers.rb b/lib/api/helpers/services_helpers.rb index 953be7f3798..44c577204b8 100644 --- a/lib/api/helpers/services_helpers.rb +++ b/lib/api/helpers/services_helpers.rb @@ -563,6 +563,12 @@ module API name: :notify_only_broken_pipelines, type: Boolean, desc: 'Notify only broken pipelines' + }, + { + required: false, + name: :notify_only_default_branch, + type: Boolean, + desc: 'Send notifications only for the default branch' } ], 'pivotaltracker' => [ diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/project_services/pipelines_email_service_spec.rb index ca17e7453b8..b85565e0c25 100644 --- a/spec/models/project_services/pipelines_email_service_spec.rb +++ b/spec/models/project_services/pipelines_email_service_spec.rb @@ -4,7 +4,11 @@ require 'spec_helper' describe PipelinesEmailService, :mailer do let(:pipeline) do - create(:ci_pipeline, project: project, sha: project.commit('master').sha) + create(:ci_pipeline, :failed, + project: project, + sha: project.commit('master').sha, + ref: project.default_branch + ) end let(:project) { create(:project, :repository) } @@ -84,12 +88,7 @@ describe PipelinesEmailService, :mailer do subject.test(data) end - context 'when pipeline is failed' do - before do - data[:object_attributes][:status] = 'failed' - pipeline.update(status: 'failed') - end - + context 'when pipeline is failed and on default branch' do it_behaves_like 'sending email' end @@ -101,6 +100,25 @@ describe PipelinesEmailService, :mailer do it_behaves_like 'sending email' end + + context 'when pipeline is failed and on a non-default branch' do + before do + data[:object_attributes][:ref] = 'not-the-default-branch' + pipeline.update(ref: 'not-the-default-branch') + end + + context 'with notify_only_default branch on' do + before do + subject.notify_only_default_branch = true + end + + it_behaves_like 'sending email' + end + + context 'with notify_only_default_branch off' do + it_behaves_like 'sending email' + end + end end describe '#execute' do @@ -110,11 +128,6 @@ describe PipelinesEmailService, :mailer do context 'with recipients' do context 'with failed pipeline' do - before do - data[:object_attributes][:status] = 'failed' - pipeline.update(status: 'failed') - end - it_behaves_like 'sending email' end @@ -133,11 +146,6 @@ describe PipelinesEmailService, :mailer do end context 'with failed pipeline' do - before do - data[:object_attributes][:status] = 'failed' - pipeline.update(status: 'failed') - end - it_behaves_like 'sending email' end @@ -150,6 +158,40 @@ describe PipelinesEmailService, :mailer do it_behaves_like 'not sending email' end end + + context 'with notify_only_default_branch off' do + context 'with default branch' do + it_behaves_like 'sending email' + end + + context 'with non default branch' do + before do + data[:object_attributes][:ref] = 'not-the-default-branch' + pipeline.update(ref: 'not-the-default-branch') + end + + it_behaves_like 'sending email' + end + end + + context 'with notify_only_default_branch on' do + before do + subject.notify_only_default_branch = true + end + + context 'with default branch' do + it_behaves_like 'sending email' + end + + context 'with non default branch' do + before do + data[:object_attributes][:ref] = 'not-the-default-branch' + pipeline.update(ref: 'not-the-default-branch') + end + + it_behaves_like 'not sending email' + end + end end context 'with empty recipients list' do -- cgit v1.2.1 From aaa0fb726cc0bde21759b3aa7ebb8c50d62b49d6 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 28 May 2019 14:43:33 +0100 Subject: Fixes repository Vue router hiding elements on root URL --- .../javascripts/repository/components/table/index.vue | 2 +- app/assets/javascripts/repository/index.js | 13 +++++++++++++ app/assets/javascripts/repository/router.js | 9 +-------- app/views/projects/_files.html.haml | 2 +- app/views/projects/_home_panel.html.haml | 2 +- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/repository/components/table/index.vue b/app/assets/javascripts/repository/components/table/index.vue index 2b0a4644bf6..f4df98ac2ff 100644 --- a/app/assets/javascripts/repository/components/table/index.vue +++ b/app/assets/javascripts/repository/components/table/index.vue @@ -50,7 +50,7 @@ export default { ); }, showParentRow() { - return !this.isLoadingFiles && this.path !== ''; + return !this.isLoadingFiles && ['', '/'].indexOf(this.path) === -1; }, }, watch: { diff --git a/app/assets/javascripts/repository/index.js b/app/assets/javascripts/repository/index.js index a5c125c2ff7..f992d4b6d54 100644 --- a/app/assets/javascripts/repository/index.js +++ b/app/assets/javascripts/repository/index.js @@ -17,6 +17,19 @@ export default function setupVueRepositoryList() { }); router.afterEach(({ params: { pathMatch } }) => setTitle(pathMatch, ref, fullName)); + router.afterEach(to => { + const isRoot = to.params.pathMatch === undefined || to.params.pathMatch === '/'; + + if (!isRoot) { + document + .querySelectorAll('.js-keep-hidden-on-navigation') + .forEach(elem => elem.classList.add('hidden')); + } + + document + .querySelectorAll('.js-hide-on-navigation') + .forEach(elem => elem.classList.toggle('hidden', !isRoot)); + }); return new Vue({ el, diff --git a/app/assets/javascripts/repository/router.js b/app/assets/javascripts/repository/router.js index f7132b99d9e..9322c81ab97 100644 --- a/app/assets/javascripts/repository/router.js +++ b/app/assets/javascripts/repository/router.js @@ -16,15 +16,8 @@ export default function createRouter(base, baseRef) { name: 'treePath', component: TreePage, props: route => ({ - path: route.params.pathMatch.replace(/^\//, ''), + path: route.params.pathMatch && route.params.pathMatch.replace(/^\//, ''), }), - beforeEnter(to, from, next) { - document - .querySelectorAll('.js-hide-on-navigation') - .forEach(el => el.classList.add('hidden')); - - next(); - }, }, { path: '/', diff --git a/app/views/projects/_files.html.haml b/app/views/projects/_files.html.haml index bb46b440c18..7f50a7e4294 100644 --- a/app/views/projects/_files.html.haml +++ b/app/views/projects/_files.html.haml @@ -14,7 +14,7 @@ = render 'shared/commit_well', commit: commit, ref: ref, project: project - if is_project_overview - .project-buttons.append-bottom-default{ class: ("js-hide-on-navigation" if vue_file_list) } + .project-buttons.append-bottom-default{ class: ("js-keep-hidden-on-navigation" if vue_file_list) } = render 'stat_anchor_list', anchors: @project.statistics_buttons(show_auto_devops_callout: show_auto_devops_callout) - if vue_file_list diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index a97322dace4..6de8848d3a1 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -1,7 +1,7 @@ - empty_repo = @project.empty_repo? - show_auto_devops_callout = show_auto_devops_callout?(@project) - max_project_topic_length = 15 -.project-home-panel{ class: [("empty-project" if empty_repo), ("js-hide-on-navigation" if Feature.enabled?(:vue_file_list, @project))] } +.project-home-panel{ class: [("empty-project" if empty_repo), ("js-keep-hidden-on-navigation" if Feature.enabled?(:vue_file_list, @project))] } .row.append-bottom-8 .home-panel-title-row.col-md-12.col-lg-6.d-flex .avatar-container.rect-avatar.s64.home-panel-avatar.append-right-default.float-none -- cgit v1.2.1 From 10eb09b94eb41681556165bc8fdd86574aa0e705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Tue, 28 May 2019 09:06:45 +0000 Subject: doc: personal_access_tokens: add missing link to !5951 --- doc/user/profile/personal_access_tokens.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/user/profile/personal_access_tokens.md b/doc/user/profile/personal_access_tokens.md index 4085f3b678c..5166524d13b 100644 --- a/doc/user/profile/personal_access_tokens.md +++ b/doc/user/profile/personal_access_tokens.md @@ -49,6 +49,7 @@ the following table. [2fa]: ../account/two_factor_authentication.md [api]: ../../api/README.md [ce-3749]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3749 +[ce-5951]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5951 [ce-14838]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14838 [container registry]: ../project/container_registry.md [users]: ../../api/users.md -- cgit v1.2.1 From 93a5071a3bde5db57f8f763a1c958a00fa6b4bf8 Mon Sep 17 00:00:00 2001 From: Tiger Date: Tue, 28 May 2019 09:44:45 -0500 Subject: Remove unused fixture lines --- spec/lib/gitlab/import_export/project.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 43afc067e08..fb7bddb386c 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -184,8 +184,6 @@ "note": "Est reprehenderit quas aut aspernatur autem recusandae voluptatem.", "noteable_type": "Issue", "author_id": 25, - "note_html": "

dodgy html

", - "cached_markdown_version": 121212, "created_at": "2016-06-14T15:02:47.795Z", "updated_at": "2016-06-14T15:02:47.795Z", "project_id": 5, -- cgit v1.2.1 From 51183ad3bdad507325c14c916d70fe3ab9857bfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 1 May 2019 17:31:04 -0400 Subject: Add all reports scope to Ci::JobArtifact --- app/models/ci/job_artifact.rb | 4 ++++ spec/models/ci/job_artifact_spec.rb | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 3beb76ffc2b..16fa77e0048 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -66,6 +66,10 @@ module Ci where(file_type: types) end + scope :with_all_reports, -> do + where(file_type: self.file_types.values.drop(3)) + end + scope :test_reports, -> do with_file_types(TEST_REPORT_FILE_TYPES) end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 5964f66c398..d3809f0a460 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -23,6 +23,21 @@ describe Ci::JobArtifact do it_behaves_like 'having unique enum values' + describe '.with_all_reports' do + let!(:artifact) { create(:ci_job_artifact, :archive) } + + subject { described_class.with_all_reports } + + it { is_expected.to be_empty } + + context 'when there are reports' do + let!(:metrics_report) { create(:ci_job_artifact, :junit) } + let!(:codequality_report) { create(:ci_job_artifact, :codequality) } + + it { is_expected.to eq([metrics_report, codequality_report]) } + end + end + describe '.test_reports' do subject { described_class.test_reports } -- cgit v1.2.1 From 31bd09a3924d1b76cac38996267ffe6ec08cad46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 1 May 2019 18:29:15 -0400 Subject: Expose report download path in build details Exposes report download paths in build details. --- app/serializers/build_details_entity.rb | 4 ++++ app/serializers/job_artifact_entity.rb | 15 ++++++++++++++ spec/serializers/build_details_entity_spec.rb | 8 ++++++++ spec/serializers/job_artifact_entity_spec.rb | 28 +++++++++++++++++++++++++++ 4 files changed, 55 insertions(+) create mode 100644 app/serializers/job_artifact_entity.rb create mode 100644 spec/serializers/job_artifact_entity_spec.rb diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 6928968edc0..ee5adc69660 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -42,6 +42,10 @@ class BuildDetailsEntity < JobEntity end end + expose :reports, if: -> (*) { can?(current_user, :read_build, build) }, using: JobArtifactEntity do |build| + build.job_artifacts.merge(Ci::JobArtifact.with_all_reports) + end + expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity expose :erase_path, if: -> (*) { build.erasable? && can?(current_user, :erase_build, build) } do |build| erase_project_job_path(project, build) diff --git a/app/serializers/job_artifact_entity.rb b/app/serializers/job_artifact_entity.rb new file mode 100644 index 00000000000..76ded287c75 --- /dev/null +++ b/app/serializers/job_artifact_entity.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class JobArtifactEntity < Grape::Entity + include RequestAwareEntity + + expose :file_type + expose :file_format + expose :size + + expose :download_path do |artifact| + download_project_job_artifacts_path(job.project, job, file_type: artifact.file_format) + end + + alias_method :job, :object +end diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb index 9c2e5c79a9d..0b8ed873bde 100644 --- a/spec/serializers/build_details_entity_spec.rb +++ b/spec/serializers/build_details_entity_spec.rb @@ -146,5 +146,13 @@ describe BuildDetailsEntity do end end end + + context 'when the build has reports' do + let!(:report) { create(:ci_job_artifact, :codequality, job: build) } + + it 'exposes the report artifacts' do + expect(subject[:reports]).not_to be_empty + end + end end end diff --git a/spec/serializers/job_artifact_entity_spec.rb b/spec/serializers/job_artifact_entity_spec.rb new file mode 100644 index 00000000000..4188be56473 --- /dev/null +++ b/spec/serializers/job_artifact_entity_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe JobArtifactEntity do + let(:report) { create(:ci_job_artifact, :codequality) } + let(:entity) { described_class.new(report, request: double) } + + describe '#as_json' do + subject { entity.as_json } + + it 'exposes file_type' do + expect(subject[:file_type]).to eq(report.file_type) + end + + it 'exposes file_format' do + expect(subject[:file_format]).to eq(report.file_format) + end + + it 'exposes size' do + expect(subject[:size]).to eq(report.size) + end + + it 'exposes download path' do + expect(subject[:download_path]).to include("jobs/#{report.job.id}/artifacts/download") + end + end +end -- cgit v1.2.1 From 9e6f37744a7f30a181879a1e2799e129c5e795c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 1 May 2019 19:16:26 -0400 Subject: Move JobArtifactEntity to JobArtifactReportEntity --- app/serializers/build_details_entity.rb | 2 +- app/serializers/job_artifact_entity.rb | 15 ------------ app/serializers/job_artifact_report_entity.rb | 15 ++++++++++++ spec/serializers/job_artifact_entity_spec.rb | 28 ---------------------- .../serializers/job_artifact_report_entity_spec.rb | 28 ++++++++++++++++++++++ 5 files changed, 44 insertions(+), 44 deletions(-) delete mode 100644 app/serializers/job_artifact_entity.rb create mode 100644 app/serializers/job_artifact_report_entity.rb delete mode 100644 spec/serializers/job_artifact_entity_spec.rb create mode 100644 spec/serializers/job_artifact_report_entity_spec.rb diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index ee5adc69660..601b2c47678 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -42,7 +42,7 @@ class BuildDetailsEntity < JobEntity end end - expose :reports, if: -> (*) { can?(current_user, :read_build, build) }, using: JobArtifactEntity do |build| + expose :reports, if: -> (*) { can?(current_user, :read_build, build) }, using: JobArtifactReportEntity do |build| build.job_artifacts.merge(Ci::JobArtifact.with_all_reports) end diff --git a/app/serializers/job_artifact_entity.rb b/app/serializers/job_artifact_entity.rb deleted file mode 100644 index 76ded287c75..00000000000 --- a/app/serializers/job_artifact_entity.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -class JobArtifactEntity < Grape::Entity - include RequestAwareEntity - - expose :file_type - expose :file_format - expose :size - - expose :download_path do |artifact| - download_project_job_artifacts_path(job.project, job, file_type: artifact.file_format) - end - - alias_method :job, :object -end diff --git a/app/serializers/job_artifact_report_entity.rb b/app/serializers/job_artifact_report_entity.rb new file mode 100644 index 00000000000..857cbfe8ee8 --- /dev/null +++ b/app/serializers/job_artifact_report_entity.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class JobArtifactReportEntity < Grape::Entity + include RequestAwareEntity + + expose :file_type + expose :file_format + expose :size + + expose :download_path do |artifact| + download_project_job_artifacts_path(job.project, job, file_type: artifact.file_format) + end + + alias_method :job, :object +end diff --git a/spec/serializers/job_artifact_entity_spec.rb b/spec/serializers/job_artifact_entity_spec.rb deleted file mode 100644 index 4188be56473..00000000000 --- a/spec/serializers/job_artifact_entity_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe JobArtifactEntity do - let(:report) { create(:ci_job_artifact, :codequality) } - let(:entity) { described_class.new(report, request: double) } - - describe '#as_json' do - subject { entity.as_json } - - it 'exposes file_type' do - expect(subject[:file_type]).to eq(report.file_type) - end - - it 'exposes file_format' do - expect(subject[:file_format]).to eq(report.file_format) - end - - it 'exposes size' do - expect(subject[:size]).to eq(report.size) - end - - it 'exposes download path' do - expect(subject[:download_path]).to include("jobs/#{report.job.id}/artifacts/download") - end - end -end diff --git a/spec/serializers/job_artifact_report_entity_spec.rb b/spec/serializers/job_artifact_report_entity_spec.rb new file mode 100644 index 00000000000..eef5c16d0fb --- /dev/null +++ b/spec/serializers/job_artifact_report_entity_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe JobArtifactReportEntity do + let(:report) { create(:ci_job_artifact, :codequality) } + let(:entity) { described_class.new(report, request: double) } + + describe '#as_json' do + subject { entity.as_json } + + it 'exposes file_type' do + expect(subject[:file_type]).to eq(report.file_type) + end + + it 'exposes file_format' do + expect(subject[:file_format]).to eq(report.file_format) + end + + it 'exposes size' do + expect(subject[:size]).to eq(report.size) + end + + it 'exposes download path' do + expect(subject[:download_path]).to include("jobs/#{report.job.id}/artifacts/download") + end + end +end -- cgit v1.2.1 From c86fa0e45e4a053090b5c17b9678120e39f696a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 21 May 2019 00:56:19 +0200 Subject: Rename with_all_reports to with_reports --- app/models/ci/job_artifact.rb | 13 +++++++++---- app/serializers/build_details_entity.rb | 2 +- app/serializers/job_artifact_report_entity.rb | 4 +--- spec/models/ci/job_artifact_spec.rb | 4 ++-- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 16fa77e0048..0dbeab30498 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -26,10 +26,13 @@ module Ci metrics: 'metrics.txt' }.freeze - TYPE_AND_FORMAT_PAIRS = { + INTERNAL_TYPES = { archive: :zip, metadata: :gzip, - trace: :raw, + trace: :raw + }.freeze + + REPORT_TYPES = { junit: :gzip, metrics: :gzip, @@ -45,6 +48,8 @@ module Ci performance: :raw }.freeze + TYPE_AND_FORMAT_PAIRS = INTERNAL_TYPES.merge(REPORT_TYPES).freeze + belongs_to :project belongs_to :job, class_name: "Ci::Build", foreign_key: :job_id @@ -66,8 +71,8 @@ module Ci where(file_type: types) end - scope :with_all_reports, -> do - where(file_type: self.file_types.values.drop(3)) + scope :with_reports, -> do + with_file_types(REPORT_TYPES.keys.map(&:to_s)) end scope :test_reports, -> do diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 601b2c47678..6be0a8d1499 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -43,7 +43,7 @@ class BuildDetailsEntity < JobEntity end expose :reports, if: -> (*) { can?(current_user, :read_build, build) }, using: JobArtifactReportEntity do |build| - build.job_artifacts.merge(Ci::JobArtifact.with_all_reports) + build.job_artifacts.with_reports end expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity diff --git a/app/serializers/job_artifact_report_entity.rb b/app/serializers/job_artifact_report_entity.rb index 857cbfe8ee8..4280351a6b0 100644 --- a/app/serializers/job_artifact_report_entity.rb +++ b/app/serializers/job_artifact_report_entity.rb @@ -8,8 +8,6 @@ class JobArtifactReportEntity < Grape::Entity expose :size expose :download_path do |artifact| - download_project_job_artifacts_path(job.project, job, file_type: artifact.file_format) + download_project_job_artifacts_path(artifact.job.project, artifact.job, file_type: artifact.file_format) end - - alias_method :job, :object end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index d3809f0a460..e6d682c24d9 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -23,10 +23,10 @@ describe Ci::JobArtifact do it_behaves_like 'having unique enum values' - describe '.with_all_reports' do + describe '.with_reports' do let!(:artifact) { create(:ci_job_artifact, :archive) } - subject { described_class.with_all_reports } + subject { described_class.with_reports } it { is_expected.to be_empty } -- cgit v1.2.1 From 1542b160f18b30a8ec28fbdc9c52694b5dfd6f62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 25 May 2019 02:08:53 +0200 Subject: Extract Ci::Build#report_artifacts into method Extracts combining the job_artifacts relation with the with_reports scope for getting report artifacts into a method. --- app/models/ci/build.rb | 4 ++++ app/serializers/build_details_entity.rb | 7 ++++--- spec/models/ci/build_spec.rb | 12 ++++++++++++ spec/serializers/build_details_entity_spec.rb | 3 ++- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 1b67a7272bc..56786fae6ea 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -765,6 +765,10 @@ module Ci end end + def report_artifacts + job_artifacts.with_reports + end + # Virtual deployment status depending on the environment status. def deployment_status return unless starts_environment? diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb index 6be0a8d1499..67e44ee9d10 100644 --- a/app/serializers/build_details_entity.rb +++ b/app/serializers/build_details_entity.rb @@ -42,9 +42,10 @@ class BuildDetailsEntity < JobEntity end end - expose :reports, if: -> (*) { can?(current_user, :read_build, build) }, using: JobArtifactReportEntity do |build| - build.job_artifacts.with_reports - end + expose :report_artifacts, + as: :reports, + using: JobArtifactReportEntity, + if: -> (*) { can?(current_user, :read_build, build) } expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity expose :erase_path, if: -> (*) { build.erasable? && can?(current_user, :erase_build, build) } do |build| diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index bc81c34f7ab..32eef9e0e01 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3490,6 +3490,18 @@ describe Ci::Build do end end + describe '#report_artifacts' do + subject { build.report_artifacts } + + context 'when the build has reports' do + let!(:report) { create(:ci_job_artifact, :codequality, job: build) } + + it 'returns the artifacts with reports' do + expect(subject).to contain_exactly(report) + end + end + end + describe '#artifacts_metadata_entry' do set(:build) { create(:ci_build, project: project) } let(:path) { 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' } diff --git a/spec/serializers/build_details_entity_spec.rb b/spec/serializers/build_details_entity_spec.rb index 0b8ed873bde..d922e8246c7 100644 --- a/spec/serializers/build_details_entity_spec.rb +++ b/spec/serializers/build_details_entity_spec.rb @@ -151,7 +151,8 @@ describe BuildDetailsEntity do let!(:report) { create(:ci_job_artifact, :codequality, job: build) } it 'exposes the report artifacts' do - expect(subject[:reports]).not_to be_empty + expect(subject[:reports].count).to eq(1) + expect(subject[:reports].first[:file_type]).to eq('codequality') end end end -- cgit v1.2.1 From 40b67a4f6a91e718721857f4d34cc600d1f90b9b Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Tue, 21 May 2019 17:46:12 -0500 Subject: Allow issues to be sorted by relative_position - adding a "Manual" option to the dropdown - show 100 issues list when manually sorting --- app/controllers/concerns/issuable_collections.rb | 6 +++ app/helpers/sorting_helper.rb | 55 +++++++++++++--------- app/models/issue.rb | 8 ++-- app/views/shared/issuable/_sort_dropdown.html.haml | 15 +++--- .../unreleased/9121-sort-relative-position.yml | 5 ++ locale/gitlab.pot | 3 ++ .../controllers/projects/issues_controller_spec.rb | 31 ++++++++++++ spec/models/issue_spec.rb | 15 ++++++ 8 files changed, 105 insertions(+), 33 deletions(-) create mode 100644 changelogs/unreleased/9121-sort-relative-position.yml diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 91e875dca54..9cf25915e92 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -41,6 +41,7 @@ module IssuableCollections return if pagination_disabled? @issuables = @issuables.page(params[:page]) + @issuables = per_page_for_relative_position if params[:sort] == 'relative_position' @issuable_meta_data = issuable_meta_data(@issuables, collection_type) @total_pages = issuable_page_count end @@ -80,6 +81,11 @@ module IssuableCollections (row_count.to_f / limit).ceil end + # manual / relative_position sorting allows for 100 items on the page + def per_page_for_relative_position + @issuables.per(100) # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + def issuable_finder_for(finder_class) finder_class.new(current_user, finder_options) end diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb index f2d814e6930..26692934456 100644 --- a/app/helpers/sorting_helper.rb +++ b/app/helpers/sorting_helper.rb @@ -3,29 +3,30 @@ module SortingHelper def sort_options_hash { - sort_value_created_date => sort_title_created_date, - sort_value_downvotes => sort_title_downvotes, - sort_value_due_date => sort_title_due_date, - sort_value_due_date_later => sort_title_due_date_later, - sort_value_due_date_soon => sort_title_due_date_soon, - sort_value_label_priority => sort_title_label_priority, - sort_value_largest_group => sort_title_largest_group, - sort_value_largest_repo => sort_title_largest_repo, - sort_value_milestone => sort_title_milestone, - sort_value_milestone_later => sort_title_milestone_later, - sort_value_milestone_soon => sort_title_milestone_soon, - sort_value_name => sort_title_name, - sort_value_name_desc => sort_title_name_desc, - sort_value_oldest_created => sort_title_oldest_created, - sort_value_oldest_signin => sort_title_oldest_signin, - sort_value_oldest_updated => sort_title_oldest_updated, - sort_value_recently_created => sort_title_recently_created, - sort_value_recently_signin => sort_title_recently_signin, - sort_value_recently_updated => sort_title_recently_updated, - sort_value_popularity => sort_title_popularity, - sort_value_priority => sort_title_priority, - sort_value_upvotes => sort_title_upvotes, - sort_value_contacted_date => sort_title_contacted_date + sort_value_created_date => sort_title_created_date, + sort_value_downvotes => sort_title_downvotes, + sort_value_due_date => sort_title_due_date, + sort_value_due_date_later => sort_title_due_date_later, + sort_value_due_date_soon => sort_title_due_date_soon, + sort_value_label_priority => sort_title_label_priority, + sort_value_largest_group => sort_title_largest_group, + sort_value_largest_repo => sort_title_largest_repo, + sort_value_milestone => sort_title_milestone, + sort_value_milestone_later => sort_title_milestone_later, + sort_value_milestone_soon => sort_title_milestone_soon, + sort_value_name => sort_title_name, + sort_value_name_desc => sort_title_name_desc, + sort_value_oldest_created => sort_title_oldest_created, + sort_value_oldest_signin => sort_title_oldest_signin, + sort_value_oldest_updated => sort_title_oldest_updated, + sort_value_recently_created => sort_title_recently_created, + sort_value_recently_signin => sort_title_recently_signin, + sort_value_recently_updated => sort_title_recently_updated, + sort_value_popularity => sort_title_popularity, + sort_value_priority => sort_title_priority, + sort_value_upvotes => sort_title_upvotes, + sort_value_contacted_date => sort_title_contacted_date, + sort_value_relative_position => sort_title_relative_position } end @@ -397,6 +398,10 @@ module SortingHelper s_('SortOptions|Recent last activity') end + def sort_title_relative_position + s_('SortOptions|Manual') + end + # Values. def sort_value_access_level_asc 'access_level_asc' @@ -545,4 +550,8 @@ module SortingHelper def sort_value_recently_last_activity 'last_activity_on_desc' end + + def sort_value_relative_position + 'relative_position' + end end diff --git a/app/models/issue.rb b/app/models/issue.rb index eb5544f2a12..6da6fbe55cb 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -58,6 +58,7 @@ class Issue < ApplicationRecord scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') } scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') } scope :order_closest_future_date, -> { reorder('CASE WHEN issues.due_date >= CURRENT_DATE THEN 0 ELSE 1 END ASC, ABS(CURRENT_DATE - issues.due_date) ASC') } + scope :order_relative_position_asc, -> { reorder(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')) } scope :preload_associations, -> { preload(:labels, project: :namespace) } scope :with_api_entity_associations, -> { preload(:timelogs, :assignees, :author, :notes, :labels, project: [:route, { namespace: :route }] ) } @@ -130,9 +131,10 @@ class Issue < ApplicationRecord def self.sort_by_attribute(method, excluded_labels: []) case method.to_s when 'closest_future_date' then order_closest_future_date - when 'due_date' then order_due_date_asc - when 'due_date_asc' then order_due_date_asc - when 'due_date_desc' then order_due_date_desc + when 'due_date' then order_due_date_asc + when 'due_date_asc' then order_due_date_asc + when 'due_date_desc' then order_due_date_desc + when 'relative_position' then order_relative_position_asc else super end diff --git a/app/views/shared/issuable/_sort_dropdown.html.haml b/app/views/shared/issuable/_sort_dropdown.html.haml index 967f31c8325..9c151dc96f3 100644 --- a/app/views/shared/issuable/_sort_dropdown.html.haml +++ b/app/views/shared/issuable/_sort_dropdown.html.haml @@ -10,12 +10,13 @@ = icon('chevron-down') %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable.dropdown-menu-sort %li - = sortable_item(sort_title_priority, page_filter_path(sort: sort_value_priority), sort_title) - = sortable_item(sort_title_created_date, page_filter_path(sort: sort_value_created_date), sort_title) - = sortable_item(sort_title_recently_updated, page_filter_path(sort: sort_value_recently_updated), sort_title) - = sortable_item(sort_title_milestone, page_filter_path(sort: sort_value_milestone), sort_title) - = sortable_item(sort_title_due_date, page_filter_path(sort: sort_value_due_date), sort_title) if viewing_issues - = sortable_item(sort_title_popularity, page_filter_path(sort: sort_value_popularity), sort_title) - = sortable_item(sort_title_label_priority, page_filter_path(sort: sort_value_label_priority), sort_title) + = sortable_item(sort_title_priority, page_filter_path(sort: sort_value_priority), sort_title) + = sortable_item(sort_title_created_date, page_filter_path(sort: sort_value_created_date), sort_title) + = sortable_item(sort_title_recently_updated, page_filter_path(sort: sort_value_recently_updated), sort_title) + = sortable_item(sort_title_milestone, page_filter_path(sort: sort_value_milestone), sort_title) + = sortable_item(sort_title_due_date, page_filter_path(sort: sort_value_due_date), sort_title) if viewing_issues + = sortable_item(sort_title_popularity, page_filter_path(sort: sort_value_popularity), sort_title) + = sortable_item(sort_title_label_priority, page_filter_path(sort: sort_value_label_priority), sort_title) + = sortable_item(sort_title_relative_position, page_filter_path(sort: sort_value_relative_position), sort_title) if viewing_issues = render_if_exists('shared/ee/issuable/sort_dropdown', viewing_issues: viewing_issues, sort_title: sort_title) = issuable_sort_direction_button(sort_value) diff --git a/changelogs/unreleased/9121-sort-relative-position.yml b/changelogs/unreleased/9121-sort-relative-position.yml new file mode 100644 index 00000000000..adc9e87e5bb --- /dev/null +++ b/changelogs/unreleased/9121-sort-relative-position.yml @@ -0,0 +1,5 @@ +--- +title: Allow issue list to be sorted by relative order +merge_request: 28566 +author: +type: added diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 72bff621059..c0b3eaf1e8b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9061,6 +9061,9 @@ msgstr "" msgid "SortOptions|Least popular" msgstr "" +msgid "SortOptions|Manual" +msgstr "" + msgid "SortOptions|Milestone due date" msgstr "" diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 0c46b43f080..32607fc5f56 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -130,6 +130,37 @@ describe Projects::IssuesController do end end + context 'with relative_position sorting' do + let!(:issue_list) { create_list(:issue, 2, project: project) } + + before do + sign_in(user) + project.add_developer(user) + allow(Kaminari.config).to receive(:default_per_page).and_return(1) + end + + it 'overrides the number allowed on the page' do + get :index, + params: { + namespace_id: project.namespace.to_param, + project_id: project, + sort: 'relative_position' + } + + expect(assigns(:issues).count).to eq 2 + end + + it 'allows the default number on the page' do + get :index, + params: { + namespace_id: project.namespace.to_param, + project_id: project + } + + expect(assigns(:issues).count).to eq 1 + end + end + context 'external authorization' do before do sign_in user diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index cc777cbf749..a5c7e9db2a1 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -93,6 +93,21 @@ describe Issue do end end + describe '#sort' do + let(:project) { create(:project) } + + context "by relative_position" do + let!(:issue) { create(:issue, project: project) } + let!(:issue2) { create(:issue, project: project, relative_position: 2) } + let!(:issue3) { create(:issue, project: project, relative_position: 1) } + + it "sorts asc with nulls at the end" do + issues = project.issues.sort_by_attribute('relative_position') + expect(issues).to eq([issue3, issue2, issue]) + end + end + end + describe '#card_attributes' do it 'includes the author name' do allow(subject).to receive(:author).and_return(double(name: 'Robert')) -- cgit v1.2.1 From 703dd456e38d2cd357c3e7ec15e3b088f8ea4235 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Tue, 28 May 2019 11:35:45 -0500 Subject: Check for manual_sorting feature flag --- app/views/shared/issuable/_sort_dropdown.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/issuable/_sort_dropdown.html.haml b/app/views/shared/issuable/_sort_dropdown.html.haml index 9c151dc96f3..1dd97bc4ed1 100644 --- a/app/views/shared/issuable/_sort_dropdown.html.haml +++ b/app/views/shared/issuable/_sort_dropdown.html.haml @@ -17,6 +17,6 @@ = sortable_item(sort_title_due_date, page_filter_path(sort: sort_value_due_date), sort_title) if viewing_issues = sortable_item(sort_title_popularity, page_filter_path(sort: sort_value_popularity), sort_title) = sortable_item(sort_title_label_priority, page_filter_path(sort: sort_value_label_priority), sort_title) - = sortable_item(sort_title_relative_position, page_filter_path(sort: sort_value_relative_position), sort_title) if viewing_issues + = sortable_item(sort_title_relative_position, page_filter_path(sort: sort_value_relative_position), sort_title) if viewing_issues && Feature.enabled?(:manual_sorting) = render_if_exists('shared/ee/issuable/sort_dropdown', viewing_issues: viewing_issues, sort_title: sort_title) = issuable_sort_direction_button(sort_value) -- cgit v1.2.1 From 0e4d47e790d72be9445643798ef56239b0dd4303 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 28 May 2019 10:26:53 -0700 Subject: Add a column header to admin/jobs page This column was blank before. It contain one or two pieces of information: * Job duration * Job completion timestamp For brevity, use `Timing` as the header. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/62347 --- app/views/projects/jobs/_table.html.haml | 2 +- changelogs/unreleased/sh-add-header-to-jobs-admin-page.yml | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/sh-add-header-to-jobs-admin-page.yml diff --git a/app/views/projects/jobs/_table.html.haml b/app/views/projects/jobs/_table.html.haml index d124d3ebfc1..b08223546f7 100644 --- a/app/views/projects/jobs/_table.html.haml +++ b/app/views/projects/jobs/_table.html.haml @@ -16,7 +16,7 @@ %th Runner %th Stage %th Name - %th + %th Timing %th Coverage %th diff --git a/changelogs/unreleased/sh-add-header-to-jobs-admin-page.yml b/changelogs/unreleased/sh-add-header-to-jobs-admin-page.yml new file mode 100644 index 00000000000..b089e6e4f37 --- /dev/null +++ b/changelogs/unreleased/sh-add-header-to-jobs-admin-page.yml @@ -0,0 +1,5 @@ +--- +title: Add a column header to admin/jobs page +merge_request: 28837 +author: +type: other -- cgit v1.2.1 From 4f14ec1d63706994963c55edea1637d9155e2cd1 Mon Sep 17 00:00:00 2001 From: Thiago Presa Date: Mon, 27 May 2019 12:31:46 -0300 Subject: Show if a user is using a license seat --- app/views/admin/users/show.html.haml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index c4178296e67..dcd6f7c8078 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -124,6 +124,8 @@ %strong = Gitlab::Access.human_access_with_none(@user.highest_role) + = render_if_exists 'admin/users/using_license_seat', user: @user + - if @user.ldap_user? %li %span.light LDAP uid: -- cgit v1.2.1 From ec1a4032d3cc64293d65485b8ffe84462c1959a9 Mon Sep 17 00:00:00 2001 From: Raphael Das Gupta Date: Tue, 28 May 2019 21:53:49 +0000 Subject: Improve documentation of RegExp in CI/CD job ref patterns as requested by https://gitlab.com/gitlab-org/gitlab-ce/issues/30477#note_29060568 in #30477 --- doc/ci/yaml/README.md | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 8667eacd3d5..18c85618b1b 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -386,17 +386,12 @@ job: - branches@gitlab-org/gitlab-ce except: - master@gitlab-org/gitlab-ce - - release/.*@gitlab-org/gitlab-ce + - /^release/.*$/@gitlab-org/gitlab-ce ``` The above example will run `job` for all branches on `gitlab-org/gitlab-ce`, except `master` and those with names prefixed with `release/`. -NOTE: **Note:** -Because `@` is used to denote the beginning of a ref's repository path, -matching a ref name containing the `@` character in a regular expression -requires the use of the hex character code match `\x40`. - If a job does not have an `only` rule, `only: ['branches', 'tags']` is set by default. If it doesn't have an `except` rule, it is empty. @@ -415,6 +410,28 @@ job: only: ['branches', 'tags'] ``` +#### Regular expressions + +Because `@` is used to denote the beginning of a ref's repository path, +matching a ref name containing the `@` character in a regular expression +requires the use of the hex character code match `\x40`. + +Only the tag or branch name can be matched by a regular expression. +The repository path, if given, is always matched literally. + +If a regular expression shall be used to match the tag or branch name, +the entire ref name part of the pattern has to be a regular expression, +and must be surrounded by `/`. +(With regular expression flags appended after the closing `/`.) +So `issue-/.*/` won't work to match all tag names or branch names +that begin with `issue-`. + +TIP: **Tip** +Use anchors `^` and `$` to avoid the regular expression +matching only a substring of the tag name or branch name. +For example, `/^issue-.*$/` is equivalent to `/^issue-/`, +while just `/issue/` would also match a branch called `severe-issues`. + ### Supported `only`/`except` regexp syntax CAUTION: **Warning:** -- cgit v1.2.1 From d03538d0ce474511cef521c37f47f6f69db1d321 Mon Sep 17 00:00:00 2001 From: Phy Date: Tue, 28 May 2019 23:36:50 +0000 Subject: Correct CI_API_V4_URL CI variable name --- doc/ci/variables/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 67e1d316f02..d33275cae4f 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -612,8 +612,8 @@ $'\''git'\'' "checkout" "-f" "-q" "dd648b2e48ce6518303b0bb580b2ee32fadaf045" Running on runner-8a2f473d-project-1796893-concurrent-0 via runner-8a2f473d-machine-1480971377-317a7d0f-digital-ocean-4gb... ++ export CI=true ++ CI=true -++ export CI_API_V4_API_URL=https://example.com:3000/api/v4 -++ CI_API_V4_API_URL=https://example.com:3000/api/v4 +++ export CI_API_V4_URL=https://example.com:3000/api/v4 +++ CI_API_V4_URL=https://example.com:3000/api/v4 ++ export CI_DEBUG_TRACE=false ++ CI_DEBUG_TRACE=false ++ export CI_COMMIT_SHA=dd648b2e48ce6518303b0bb580b2ee32fadaf045 @@ -652,8 +652,8 @@ Running on runner-8a2f473d-project-1796893-concurrent-0 via runner-8a2f473d-mach ++ GITLAB_CI=true ++ export CI=true ++ CI=true -++ export CI_API_V4_API_URL=https://example.com:3000/api/v4 -++ CI_API_V4_API_URL=https://example.com:3000/api/v4 +++ export CI_API_V4_URL=https://example.com:3000/api/v4 +++ CI_API_V4_URL=https://example.com:3000/api/v4 ++ export GITLAB_CI=true ++ GITLAB_CI=true ++ export CI_JOB_ID=7046507 -- cgit v1.2.1 From b41b9e6005df7d32a586bf92d858291480beb83a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Lafoucri=C3=A8re?= Date: Wed, 29 May 2019 01:11:45 +0000 Subject: Disable proxy in container scanning template closes https://gitlab.com/gitlab-org/gitlab-ee/issues/11105 --- changelogs/unreleased/11105-fix-cs-with-proxy.yml | 5 +++++ lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml | 3 +++ 2 files changed, 8 insertions(+) create mode 100644 changelogs/unreleased/11105-fix-cs-with-proxy.yml diff --git a/changelogs/unreleased/11105-fix-cs-with-proxy.yml b/changelogs/unreleased/11105-fix-cs-with-proxy.yml new file mode 100644 index 00000000000..ee32427d20e --- /dev/null +++ b/changelogs/unreleased/11105-fix-cs-with-proxy.yml @@ -0,0 +1,5 @@ +--- +title: Fix proxy support in Container Scanning +merge_request: 27246 +author: +type: fixed diff --git a/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml index 324e39c7747..5372ec6cceb 100644 --- a/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml @@ -23,6 +23,9 @@ container_scanning: DOCKER_HOST: tcp://${DOCKER_SERVICE}:2375/ # https://hub.docker.com/r/arminc/clair-local-scan/tags CLAIR_LOCAL_SCAN_VERSION: v2.0.8_fe9b059d930314b54c78f75afe265955faf4fdc1 + ## Disable the proxy for clair-local-scan, otherwise Container Scanning will + ## fail when a proxy is used. + NO_PROXY: ${DOCKER_SERVICE},localhost allow_failure: true services: - docker:stable-dind -- cgit v1.2.1 From dea58cb8651d510fd4a75b2ac084552fa3d81946 Mon Sep 17 00:00:00 2001 From: antonyliu Date: Wed, 29 May 2019 12:36:33 +0800 Subject: Apply suggestion to app/views/profiles/passwords/edit.html.haml Apply @reprazent suggestion. See gitlab-org/gitlab-ee!13339 --- app/views/profiles/passwords/edit.html.haml | 7 ++++--- locale/gitlab.pot | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/views/profiles/passwords/edit.html.haml b/app/views/profiles/passwords/edit.html.haml index 9f4f0ac2019..031f0524094 100644 --- a/app/views/profiles/passwords/edit.html.haml +++ b/app/views/profiles/passwords/edit.html.haml @@ -10,9 +10,10 @@ = _('After a successful password update, you will be redirected to the login page where you can log in with your new password.') .col-lg-8 %h5.prepend-top-0 - = _('Change your password') - - unless @user.password_automatically_set? - = _('or recover your current one') + - if @user.password_automatically set + = _('Change your password') + - else + = _('Change your password or recover your current one') = form_for @user, url: profile_password_path, method: :put, html: {class: "update-password"} do |f| = form_errors(@user) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 75bf093ee88..11f19027464 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1776,6 +1776,9 @@ msgstr "" msgid "Change your password" msgstr "" +msgid "Change your password or recover your current one" +msgstr "" + msgid "ChangeTypeActionLabel|Pick into branch" msgstr "" @@ -12102,9 +12105,6 @@ msgstr "" msgid "or" msgstr "" -msgid "or recover your current one" -msgstr "" - msgid "out of %d total test" msgid_plural "out of %d total tests" msgstr[0] "" -- cgit v1.2.1 From ede97703ea306b0bcb2672703e635692da70b194 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Wed, 29 May 2019 11:38:00 +0800 Subject: Increase global karma timeout Revert useless timeout added in karma spec --- spec/javascripts/notes/components/note_app_spec.js | 4 ++-- spec/javascripts/test_bundle.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/javascripts/notes/components/note_app_spec.js b/spec/javascripts/notes/components/note_app_spec.js index f04af04f852..ef876dc2941 100644 --- a/spec/javascripts/notes/components/note_app_spec.js +++ b/spec/javascripts/notes/components/note_app_spec.js @@ -195,7 +195,7 @@ describe('note_app', () => { setTimeout(() => { done(); }); - }, 2000); + }); }); describe('discussion note', () => { @@ -230,7 +230,7 @@ describe('note_app', () => { setTimeout(() => { done(); }); - }, 2000); + }); }); }); diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index 87ef0885d8c..8c80a425581 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -111,7 +111,7 @@ let longRunningTestTimeoutHandle; beforeEach(done => { longRunningTestTimeoutHandle = setTimeout(() => { done.fail('Test is running too long!'); - }, 2000); + }, 4000); done(); }); -- cgit v1.2.1 From c3980a2222143a35890b1ac5dd837450ac53c946 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Fri, 24 May 2019 13:35:46 +0300 Subject: Backport of https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/13025 --- doc/api/geo_nodes.md | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/doc/api/geo_nodes.md b/doc/api/geo_nodes.md index a1cb524499f..ea31abdd87e 100644 --- a/doc/api/geo_nodes.md +++ b/doc/api/geo_nodes.md @@ -192,12 +192,10 @@ Example response: "job_artifacts_failed_count": nil, "job_artifacts_synced_missing_on_primary_count": 0, "job_artifacts_synced_in_percentage": "0.00%", - "repositories_count": 41, "projects_count": 41, "repositories_failed_count": nil, "repositories_synced_count": nil, "repositories_synced_in_percentage": "0.00%", - "wikis_count": 41, "wikis_failed_count": nil, "wikis_synced_count": nil, "wikis_synced_in_percentage": "0.00%", @@ -257,12 +255,10 @@ Example response: "job_artifacts_failed_count": 1, "job_artifacts_synced_missing_on_primary_count": 0, "job_artifacts_synced_in_percentage": "50.00%", - "repositories_count": 41, "projects_count": 41, "repositories_failed_count": 1, "repositories_synced_count": 40, "repositories_synced_in_percentage": "97.56%", - "wikis_count": 41, "wikis_failed_count": 0, "wikis_synced_count": 41, "wikis_synced_in_percentage": "100.00%", @@ -300,7 +296,8 @@ Example response: ] ``` -Note: fields `wikis_count` and `repositories_count` are deprecated and will be deleted soon. Please use `projects_count` instead. +NOTE: **Note:** +In GitLab 12.0, deprecated fields `wikis_count` and `repositories_count` were removed. Use `projects_count` instead. ## Retrieve status about a specific Geo node @@ -337,12 +334,10 @@ Example response: "job_artifacts_failed_count": 1, "job_artifacts_synced_missing_on_primary_count": 0, "job_artifacts_synced_in_percentage": "50.00%", - "repositories_count": 41, "projects_count": 41, "repositories_failed_count": 1, "repositories_synced_count": 40, "repositories_synced_in_percentage": "97.56%", - "wikis_count": 41, "wikis_failed_count": 0, "wikis_synced_count": 41, "wikis_synced_in_percentage": "100.00%", @@ -362,7 +357,8 @@ Example response: Note: The `health_status` parameter can only be in an "Healthy" or "Unhealthy" state, while the `health` parameter can be empty, "Healthy", or contain the actual error message. -Note: Fields `wikis_count` and `repositories_count` are deprecated and will be deleted soon. Please use `projects_count` instead. +NOTE: **Note:** +In GitLab 12.0, deprecated fields `wikis_count` and `repositories_count` were removed. Use `projects_count` instead. ## Retrieve project sync or verification failures that occurred on the current node -- cgit v1.2.1 From 2d19bf65512d9dcd43965b87eabec7f9c5a3e7e0 Mon Sep 17 00:00:00 2001 From: Rajat Jain Date: Wed, 29 May 2019 14:13:57 +0530 Subject: Add referenced-commands in no overflow list --- app/assets/stylesheets/pages/labels.scss | 3 ++- changelogs/unreleased/referenced-labels.yml | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/referenced-labels.yml diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index 13288d8bad1..11e8a32389f 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/app/assets/stylesheets/pages/labels.scss @@ -456,8 +456,9 @@ // Don't hide the overflow in system messages .system-note-message, -.issuable-detail, +.issuable-details, .md-preview-holder, +.referenced-commands, .note-body { .scoped-label-wrapper { .badge { diff --git a/changelogs/unreleased/referenced-labels.yml b/changelogs/unreleased/referenced-labels.yml new file mode 100644 index 00000000000..c39ef4c2478 --- /dev/null +++ b/changelogs/unreleased/referenced-labels.yml @@ -0,0 +1,5 @@ +--- +title: Add referenced-commands in no overflow list +merge_request: 28858 +author: +type: fixed -- cgit v1.2.1 From 1224aa516eefe7be1c8ca306625d5d0b6d524d76 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 29 May 2019 10:24:35 +0100 Subject: Created repository list breadcrumbs Vue app --- .../repository/components/breadcrumbs.vue | 61 +++++++++++ app/assets/javascripts/repository/index.js | 25 ++++- .../repository/queries/getProjectShortPath.graphql | 3 + app/assets/javascripts/repository/utils/title.js | 2 + app/helpers/projects_helper.rb | 4 + app/views/projects/_files.html.haml | 7 +- app/views/projects/_home_panel.html.haml | 2 +- app/views/projects/tree/_readme.html.haml | 2 +- app/views/projects/tree/_tree_header.html.haml | 121 +++++++++++---------- locale/gitlab.pot | 3 + .../project_owner_creates_license_file_spec.rb | 1 + .../projects/files/user_creates_files_spec.rb | 1 + .../show/user_sees_collaboration_links_spec.rb | 1 + .../repository/components/breadcrumbs_spec.js | 44 ++++++++ 14 files changed, 208 insertions(+), 69 deletions(-) create mode 100644 app/assets/javascripts/repository/components/breadcrumbs.vue create mode 100644 app/assets/javascripts/repository/queries/getProjectShortPath.graphql create mode 100644 spec/frontend/repository/components/breadcrumbs_spec.js diff --git a/app/assets/javascripts/repository/components/breadcrumbs.vue b/app/assets/javascripts/repository/components/breadcrumbs.vue new file mode 100644 index 00000000000..6eca015036f --- /dev/null +++ b/app/assets/javascripts/repository/components/breadcrumbs.vue @@ -0,0 +1,61 @@ + + + diff --git a/app/assets/javascripts/repository/index.js b/app/assets/javascripts/repository/index.js index f992d4b6d54..52f53be045b 100644 --- a/app/assets/javascripts/repository/index.js +++ b/app/assets/javascripts/repository/index.js @@ -1,24 +1,27 @@ import Vue from 'vue'; import createRouter from './router'; import App from './components/app.vue'; +import Breadcrumbs from './components/breadcrumbs.vue'; import apolloProvider from './graphql'; import { setTitle } from './utils/title'; export default function setupVueRepositoryList() { const el = document.getElementById('js-tree-list'); - const { projectPath, ref, fullName } = el.dataset; + const { projectPath, projectShortPath, ref, fullName } = el.dataset; const router = createRouter(projectPath, ref); apolloProvider.clients.defaultClient.cache.writeData({ data: { projectPath, + projectShortPath, ref, }, }); - router.afterEach(({ params: { pathMatch } }) => setTitle(pathMatch, ref, fullName)); - router.afterEach(to => { - const isRoot = to.params.pathMatch === undefined || to.params.pathMatch === '/'; + router.afterEach(({ params: { pathMatch } }) => { + const isRoot = pathMatch === undefined || pathMatch === '/'; + + setTitle(pathMatch, ref, fullName); if (!isRoot) { document @@ -31,6 +34,20 @@ export default function setupVueRepositoryList() { .forEach(elem => elem.classList.toggle('hidden', !isRoot)); }); + // eslint-disable-next-line no-new + new Vue({ + el: document.getElementById('js-repo-breadcrumb'), + router, + apolloProvider, + render(h) { + return h(Breadcrumbs, { + props: { + currentPath: this.$route.params.pathMatch, + }, + }); + }, + }); + return new Vue({ el, router, diff --git a/app/assets/javascripts/repository/queries/getProjectShortPath.graphql b/app/assets/javascripts/repository/queries/getProjectShortPath.graphql new file mode 100644 index 00000000000..34eb26598c2 --- /dev/null +++ b/app/assets/javascripts/repository/queries/getProjectShortPath.graphql @@ -0,0 +1,3 @@ +query getProjectShortPath { + projectShortPath @client +} diff --git a/app/assets/javascripts/repository/utils/title.js b/app/assets/javascripts/repository/utils/title.js index 3c3e918c0a8..4e194640e92 100644 --- a/app/assets/javascripts/repository/utils/title.js +++ b/app/assets/javascripts/repository/utils/title.js @@ -1,5 +1,7 @@ // eslint-disable-next-line import/prefer-default-export export const setTitle = (pathMatch, ref, project) => { + if (!pathMatch) return; + const path = pathMatch.replace(/^\//, ''); const isEmpty = path === ''; diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index f798bfbf703..4cf1d6e93a5 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -655,4 +655,8 @@ module ProjectsHelper project.builds_enabled? && !project.repository.gitlab_ci_yml end + + def vue_file_list_enabled? + Gitlab::Graphql.enabled? && Feature.enabled?(:vue_file_list, @project) + end end diff --git a/app/views/projects/_files.html.haml b/app/views/projects/_files.html.haml index 7f50a7e4294..2b0c3985755 100644 --- a/app/views/projects/_files.html.haml +++ b/app/views/projects/_files.html.haml @@ -4,7 +4,6 @@ - project = local_assigns.fetch(:project) { @project } - content_url = local_assigns.fetch(:content_url) { @tree.readme ? project_blob_path(@project, tree_join(@ref, @tree.readme.path)) : project_tree_path(@project, @ref) } - show_auto_devops_callout = show_auto_devops_callout?(@project) -- vue_file_list = Feature.enabled?(:vue_file_list, @project) #tree-holder.tree-holder.clearfix .nav-block @@ -14,11 +13,11 @@ = render 'shared/commit_well', commit: commit, ref: ref, project: project - if is_project_overview - .project-buttons.append-bottom-default{ class: ("js-keep-hidden-on-navigation" if vue_file_list) } + .project-buttons.append-bottom-default{ class: ("js-keep-hidden-on-navigation" if vue_file_list_enabled?) } = render 'stat_anchor_list', anchors: @project.statistics_buttons(show_auto_devops_callout: show_auto_devops_callout) - - if vue_file_list - #js-tree-list{ data: { project_path: @project.full_path, full_name: @project.name_with_namespace, ref: ref } } + - if vue_file_list_enabled? + #js-tree-list{ data: { project_path: @project.full_path, project_short_path: @project.path, ref: ref, full_name: @project.name_with_namespace } } - if @tree.readme = render "projects/tree/readme", readme: @tree.readme - else diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index 6de8848d3a1..9f5241344a7 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -1,7 +1,7 @@ - empty_repo = @project.empty_repo? - show_auto_devops_callout = show_auto_devops_callout?(@project) - max_project_topic_length = 15 -.project-home-panel{ class: [("empty-project" if empty_repo), ("js-keep-hidden-on-navigation" if Feature.enabled?(:vue_file_list, @project))] } +.project-home-panel{ class: [("empty-project" if empty_repo), ("js-keep-hidden-on-navigation" if vue_file_list_enabled?)] } .row.append-bottom-8 .home-panel-title-row.col-md-12.col-lg-6.d-flex .avatar-container.rect-avatar.s64.home-panel-avatar.append-right-default.float-none diff --git a/app/views/projects/tree/_readme.html.haml b/app/views/projects/tree/_readme.html.haml index e935af23659..4f6c7e1f9a6 100644 --- a/app/views/projects/tree/_readme.html.haml +++ b/app/views/projects/tree/_readme.html.haml @@ -1,5 +1,5 @@ - if readme.rich_viewer - %article.file-holder.readme-holder{ id: 'readme', class: [("limited-width-container" unless fluid_layout), ("js-hide-on-navigation" if Feature.enabled?(:vue_file_list, @project))] } + %article.file-holder.readme-holder{ id: 'readme', class: [("limited-width-container" unless fluid_layout), ("js-hide-on-navigation" if vue_file_list_enabled?)] } .js-file-title.file-title = blob_icon readme.mode, readme.name = link_to project_blob_path(@project, tree_join(@ref, readme.path)) do diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml index ec8e5234bd4..ea6349f2f57 100644 --- a/app/views/projects/tree/_tree_header.html.haml +++ b/app/views/projects/tree/_tree_header.html.haml @@ -6,71 +6,74 @@ = render 'shared/ref_switcher', destination: 'tree', path: @path, show_create: true - if on_top_of_branch? - - addtotree_toggle_attributes = { href: '#', 'data-toggle': 'dropdown', 'data-target': '.add-to-tree-dropdown', 'data-boundary': 'window' } + - addtotree_toggle_attributes = { 'data-toggle': 'dropdown', 'data-target': '.add-to-tree-dropdown', 'data-boundary': 'window' } - else - addtotree_toggle_attributes = { title: _("You can only add files when you are on a branch"), data: { container: 'body' }, class: 'disabled has-tooltip' } - %ul.breadcrumb.repo-breadcrumb - %li.breadcrumb-item - = link_to project_tree_path(@project, @ref) do - = @project.path - - path_breadcrumbs do |title, path| + - if vue_file_list_enabled? + #js-repo-breadcrumb + - else + %ul.breadcrumb.repo-breadcrumb %li.breadcrumb-item - = link_to truncate(title, length: 40), project_tree_path(@project, tree_join(@ref, path)) + = link_to project_tree_path(@project, @ref) do + = @project.path + - path_breadcrumbs do |title, path| + %li.breadcrumb-item + = link_to truncate(title, length: 40), project_tree_path(@project, tree_join(@ref, path)) - - if can_collaborate || can_create_mr_from_fork - %li.breadcrumb-item - %a.btn.add-to-tree.qa-add-to-tree{ addtotree_toggle_attributes } - = sprite_icon('plus', size: 16, css_class: 'float-left') - = sprite_icon('arrow-down', size: 16, css_class: 'float-left') - - if on_top_of_branch? - .add-to-tree-dropdown - %ul.dropdown-menu - - if can_edit_tree? - %li.dropdown-header - #{ _('This directory') } - %li - = link_to project_new_blob_path(@project, @id), class: 'qa-new-file-option' do - #{ _('New file') } - %li - = link_to '#modal-upload-blob', { 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal' } do - #{ _('Upload file') } - %li - = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal' } do - #{ _('New directory') } - - elsif can?(current_user, :fork_project, @project) && can?(current_user, :create_merge_request_in, @project) - %li - - continue_params = { to: project_new_blob_path(@project, @id), - notice: edit_in_new_fork_notice, - notice_now: edit_in_new_fork_notice_now } - - fork_path = project_forks_path(@project, namespace_key: current_user.namespace.id, continue: continue_params) - = link_to fork_path, method: :post do - #{ _('New file') } - %li - - continue_params = { to: request.fullpath, - notice: edit_in_new_fork_notice + " Try to upload a file again.", - notice_now: edit_in_new_fork_notice_now } - - fork_path = project_forks_path(@project, namespace_key: current_user.namespace.id, continue: continue_params) - = link_to fork_path, method: :post do - #{ _('Upload file') } - %li - - continue_params = { to: request.fullpath, - notice: edit_in_new_fork_notice + " Try to create a new directory again.", - notice_now: edit_in_new_fork_notice_now } - - fork_path = project_forks_path(@project, namespace_key: current_user.namespace.id, continue: continue_params) - = link_to fork_path, method: :post do - #{ _('New directory') } + - if can_collaborate || can_create_mr_from_fork + %li.breadcrumb-item + %button.btn.add-to-tree.qa-add-to-tree{ addtotree_toggle_attributes, type: 'button' } + = sprite_icon('plus', size: 16, css_class: 'float-left') + = sprite_icon('arrow-down', size: 16, css_class: 'float-left') + - if on_top_of_branch? + .add-to-tree-dropdown + %ul.dropdown-menu + - if can_edit_tree? + %li.dropdown-header + #{ _('This directory') } + %li + = link_to project_new_blob_path(@project, @id), class: 'qa-new-file-option' do + #{ _('New file') } + %li + = link_to '#modal-upload-blob', { 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal' } do + #{ _('Upload file') } + %li + = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal' } do + #{ _('New directory') } + - elsif can?(current_user, :fork_project, @project) && can?(current_user, :create_merge_request_in, @project) + %li + - continue_params = { to: project_new_blob_path(@project, @id), + notice: edit_in_new_fork_notice, + notice_now: edit_in_new_fork_notice_now } + - fork_path = project_forks_path(@project, namespace_key: current_user.namespace.id, continue: continue_params) + = link_to fork_path, method: :post do + #{ _('New file') } + %li + - continue_params = { to: request.fullpath, + notice: edit_in_new_fork_notice + " Try to upload a file again.", + notice_now: edit_in_new_fork_notice_now } + - fork_path = project_forks_path(@project, namespace_key: current_user.namespace.id, continue: continue_params) + = link_to fork_path, method: :post do + #{ _('Upload file') } + %li + - continue_params = { to: request.fullpath, + notice: edit_in_new_fork_notice + " Try to create a new directory again.", + notice_now: edit_in_new_fork_notice_now } + - fork_path = project_forks_path(@project, namespace_key: current_user.namespace.id, continue: continue_params) + = link_to fork_path, method: :post do + #{ _('New directory') } - - if can?(current_user, :push_code, @project) - %li.divider - %li.dropdown-header - #{ _('This repository') } - %li - = link_to new_project_branch_path(@project) do - #{ _('New branch') } - %li - = link_to new_project_tag_path(@project) do - #{ _('New tag') } + - if can?(current_user, :push_code, @project) + %li.divider + %li.dropdown-header + #{ _('This repository') } + %li + = link_to new_project_branch_path(@project) do + #{ _('New branch') } + %li + = link_to new_project_tag_path(@project) do + #{ _('New tag') } .tree-controls = link_to s_('Commits|History'), project_commits_path(@project, @id), class: 'btn' diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9e7ff8d1847..44dda25c38c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4389,6 +4389,9 @@ msgstr "" msgid "Files" msgstr "" +msgid "Files breadcrumb" +msgstr "" + msgid "Files, directories, and submodules in the path %{path} for commit reference %{ref}" msgstr "" diff --git a/spec/features/projects/files/project_owner_creates_license_file_spec.rb b/spec/features/projects/files/project_owner_creates_license_file_spec.rb index 6762460971f..44715261b8b 100644 --- a/spec/features/projects/files/project_owner_creates_license_file_spec.rb +++ b/spec/features/projects/files/project_owner_creates_license_file_spec.rb @@ -5,6 +5,7 @@ describe 'Projects > Files > Project owner creates a license file', :js do let(:project_maintainer) { project.owner } before do + stub_feature_flags(vue_file_list: false) project.repository.delete_file(project_maintainer, 'LICENSE', message: 'Remove LICENSE', branch_name: 'master') sign_in(project_maintainer) diff --git a/spec/features/projects/files/user_creates_files_spec.rb b/spec/features/projects/files/user_creates_files_spec.rb index dd2964c2186..69f8bd4d319 100644 --- a/spec/features/projects/files/user_creates_files_spec.rb +++ b/spec/features/projects/files/user_creates_files_spec.rb @@ -12,6 +12,7 @@ describe 'Projects > Files > User creates files' do let(:user) { create(:user) } before do + stub_feature_flags(vue_file_list: false) stub_feature_flags(web_ide_default: false) project.add_maintainer(user) diff --git a/spec/features/projects/show/user_sees_collaboration_links_spec.rb b/spec/features/projects/show/user_sees_collaboration_links_spec.rb index 24777788248..46586b891e7 100644 --- a/spec/features/projects/show/user_sees_collaboration_links_spec.rb +++ b/spec/features/projects/show/user_sees_collaboration_links_spec.rb @@ -5,6 +5,7 @@ describe 'Projects > Show > Collaboration links' do let(:user) { create(:user) } before do + stub_feature_flags(vue_file_list: false) project.add_developer(user) sign_in(user) end diff --git a/spec/frontend/repository/components/breadcrumbs_spec.js b/spec/frontend/repository/components/breadcrumbs_spec.js new file mode 100644 index 00000000000..068fa317a87 --- /dev/null +++ b/spec/frontend/repository/components/breadcrumbs_spec.js @@ -0,0 +1,44 @@ +import { shallowMount, RouterLinkStub } from '@vue/test-utils'; +import Breadcrumbs from '~/repository/components/breadcrumbs.vue'; + +let vm; + +function factory(currentPath) { + vm = shallowMount(Breadcrumbs, { + propsData: { + currentPath, + }, + stubs: { + RouterLink: RouterLinkStub, + }, + }); +} + +describe('Repository breadcrumbs component', () => { + afterEach(() => { + vm.destroy(); + }); + + it.each` + path | linkCount + ${'/'} | ${1} + ${'app'} | ${2} + ${'app/assets'} | ${3} + ${'app/assets/javascripts'} | ${4} + `('renders $linkCount links for path $path', ({ path, linkCount }) => { + factory(path); + + expect(vm.findAll(RouterLinkStub).length).toEqual(linkCount); + }); + + it('renders last link as active', () => { + factory('app/assets'); + + expect( + vm + .findAll(RouterLinkStub) + .at(2) + .attributes('aria-current'), + ).toEqual('page'); + }); +}); -- cgit v1.2.1 From 301a7d32b40128d388aa42b487de367c1cdbc1cd Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 28 May 2019 14:41:57 +0100 Subject: Enable GraphQL batch requests --- app/assets/javascripts/lib/graphql.js | 20 ++++++++++++++------ app/controllers/graphql_controller.rb | 3 ++- package.json | 2 ++ yarn.lock | 20 +++++++++++++++++++- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/lib/graphql.js b/app/assets/javascripts/lib/graphql.js index 47e91dedd5a..5857f9e22ae 100644 --- a/app/assets/javascripts/lib/graphql.js +++ b/app/assets/javascripts/lib/graphql.js @@ -1,6 +1,8 @@ import { ApolloClient } from 'apollo-client'; import { InMemoryCache } from 'apollo-cache-inmemory'; import { createUploadLink } from 'apollo-upload-client'; +import { ApolloLink } from 'apollo-link'; +import { BatchHttpLink } from 'apollo-link-batch-http'; import csrf from '~/lib/utils/csrf'; export default (resolvers = {}, config = {}) => { @@ -11,13 +13,19 @@ export default (resolvers = {}, config = {}) => { uri = `${config.baseUrl}${uri}`.replace(/\/{3,}/g, '/'); } + const httpOptions = { + uri, + headers: { + [csrf.headerKey]: csrf.token, + }, + }; + return new ApolloClient({ - link: createUploadLink({ - uri, - headers: { - [csrf.headerKey]: csrf.token, - }, - }), + link: ApolloLink.split( + operation => operation.getContext().hasUpload, + createUploadLink(httpOptions), + new BatchHttpLink(httpOptions), + ), cache: new InMemoryCache(config.cacheConfig), resolvers, }); diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index e8f38899647..1ce0afac83b 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -53,7 +53,8 @@ class GraphqlController < ApplicationController { query: single_query_info[:query], variables: build_variables(single_query_info[:variables]), - operation_name: single_query_info[:operationName] + operation_name: single_query_info[:operationName], + context: context } end end diff --git a/package.json b/package.json index a4ec2dce0df..f2f784ef4d7 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,8 @@ "@gitlab/ui": "^3.10.0", "apollo-cache-inmemory": "^1.5.1", "apollo-client": "^2.5.1", + "apollo-link": "^1.2.11", + "apollo-link-batch-http": "^1.2.11", "apollo-upload-client": "^10.0.0", "at.js": "^1.5.4", "autosize": "^4.0.0", diff --git a/yarn.lock b/yarn.lock index 7c119e2c9dd..70dc7b8d15f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1398,6 +1398,24 @@ apollo-client@^2.5.1: tslib "^1.9.3" zen-observable "^0.8.0" +apollo-link-batch-http@^1.2.11: + version "1.2.11" + resolved "https://registry.yarnpkg.com/apollo-link-batch-http/-/apollo-link-batch-http-1.2.11.tgz#ae42dbcc02820658e1e267d05bf2aae7ac208088" + integrity sha512-f+KEdbP51I3AeEaBDW2lKS3eaPK/1IXaTM9F2moj02s1hgC/TzeUORRuUeOExW8ggXveW1Jzp6aYMJ2SQkZJyA== + dependencies: + apollo-link "^1.2.11" + apollo-link-batch "^1.1.12" + apollo-link-http-common "^0.2.13" + tslib "^1.9.3" + +apollo-link-batch@^1.1.12: + version "1.1.12" + resolved "https://registry.yarnpkg.com/apollo-link-batch/-/apollo-link-batch-1.1.12.tgz#64eb231082f182b0395ef7ab903600627f6c7fe8" + integrity sha512-6NqLiB9tEGxRiyhtnX/7CPHkmFG0IXfEP7pC5kirhjV+4KxqBaWvJnJGKpGp7Owgdph7KJlV+9+niOKEkcwreg== + dependencies: + apollo-link "^1.2.11" + tslib "^1.9.3" + apollo-link-dedup@^1.0.0: version "1.0.10" resolved "https://registry.yarnpkg.com/apollo-link-dedup/-/apollo-link-dedup-1.0.10.tgz#7b94589fe7f969777efd18a129043c78430800ae" @@ -1405,7 +1423,7 @@ apollo-link-dedup@^1.0.0: dependencies: apollo-link "^1.2.3" -apollo-link-http-common@^0.2.8: +apollo-link-http-common@^0.2.13, apollo-link-http-common@^0.2.8: version "0.2.13" resolved "https://registry.yarnpkg.com/apollo-link-http-common/-/apollo-link-http-common-0.2.13.tgz#c688f6baaffdc7b269b2db7ae89dae7c58b5b350" integrity sha512-Uyg1ECQpTTA691Fwx5e6Rc/6CPSu4TB4pQRTGIpwZ4l5JDOQ+812Wvi/e3IInmzOZpwx5YrrOfXrtN8BrsDXoA== -- cgit v1.2.1 From aa520060ecacd11578e4dad5e9d04a477f61d689 Mon Sep 17 00:00:00 2001 From: Lukas 'Eipi' Eipert Date: Wed, 29 May 2019 09:59:53 +0000 Subject: Update dependency @gitlab/ui to ^3.10.1 --- package.json | 2 +- yarn.lock | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index a4ec2dce0df..a8a7ee7ceb9 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "@babel/preset-env": "^7.4.4", "@gitlab/csslab": "^1.9.0", "@gitlab/svgs": "^1.63.0", - "@gitlab/ui": "^3.10.0", + "@gitlab/ui": "^3.10.1", "apollo-cache-inmemory": "^1.5.1", "apollo-client": "^2.5.1", "apollo-upload-client": "^10.0.0", diff --git a/yarn.lock b/yarn.lock index 7c119e2c9dd..45729eda890 100644 --- a/yarn.lock +++ b/yarn.lock @@ -698,10 +698,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.63.0.tgz#9dd544026d203e4ce6efed72b05db68f710c4d49" integrity sha512-YztrReFTg31B7v5wtUC5j15KHNcMebtW+kACytEU42XomMaIwk4USIbygqWlq0VRHA2VHJrHApfJHIjxiCCQcA== -"@gitlab/ui@^3.10.0": - version "3.10.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-3.10.0.tgz#0f16772b7fe8052dabd37aba2ae436255b9e0f0a" - integrity sha512-po6fh2T8esa2Nach73AYLdoTg8N0YrRa5GkJk5GoxVrHYoAUD8T1Rn3pXXXKSsQdQcYjIZJ6uvY8sL+qg+Yjww== +"@gitlab/ui@^3.10.1": + version "3.10.1" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-3.10.1.tgz#473e1383b05847d7226257228a8fd8d2905a9415" + integrity sha512-LReB+EIBvZlfeEX2s/azuz6NvegrwwzficvBxK5yPCztjDeegcEsTDlt0F32mX6DcvNFacuuAyWN8QtgpaWkhA== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.2.1" @@ -712,7 +712,7 @@ js-beautify "^1.8.8" lodash "^4.17.11" url-search-params-polyfill "^5.0.0" - vue "^2.5.21" + vue "^2.6.10" vue-loader "^15.4.2" "@gitlab/vue-toasted@^1.2.1": @@ -11353,7 +11353,7 @@ vue-virtual-scroll-list@^1.3.1: resolved "https://registry.yarnpkg.com/vue-virtual-scroll-list/-/vue-virtual-scroll-list-1.3.1.tgz#efcb83d3a3dcc69cd886fa4de1130a65493e8f76" integrity sha512-PMTxiK9/P1LtgoWWw4n1QnmDDkYqIdWWCNdt1L4JD9g6rwDgnsGsSV10bAnd5n7DQLHGWHjRex+zAbjXWT8t0g== -vue@^2.5.21, vue@^2.6.10: +vue@^2.6.10: version "2.6.10" resolved "https://registry.yarnpkg.com/vue/-/vue-2.6.10.tgz#a72b1a42a4d82a721ea438d1b6bf55e66195c637" integrity sha512-ImThpeNU9HbdZL3utgMCq0oiMzAkt1mcgy3/E6zWC/G6AaQoeuFdsl9nDhTDU3X1R6FK7nsIUuRACVcjI+A2GQ== -- cgit v1.2.1 From a2aa160cea36fd8969e38eafb352154ee7d8f6f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cunha?= Date: Tue, 9 Apr 2019 20:45:58 +0100 Subject: Adapt functions to work for external Knative Remove Kn services cache from Clusters::Application::Knative Knative function can exist even if user did not installed Knative via GitLab managed apps. -> Move responsibility of finding services into the Cluster -> Responsability is inside Clusters::Cluster::KnativeServiceFinder -> Projects::Serverless::FunctionsFinder now calls depends solely on a cluster to find the Kn services. -> Detect Knative by resource presence instead of service presence -> Mock knative_installed response temporarily for frontend to develop Display loader while `installed === 'checking'` Added frontend work to determine if Knative is installed Memoize with_reactive_cache(*args, &block) to avoid race conditions When calling with_reactive_cache more than once, it's possible that the second call will already have the value populated. Therefore, in cases where we need the sequential calls to have consistent results, we'd fall under a race condition. Check knative installation via Knative resource presence Only load pods if Knative is discovered Always return a response in FunctionsController#index - Always indicate if Knative is installed, not installed or checking - Always indicate the partial response for functions. Final response is guaranteed when knative_installed is either true | false. Adds specs for Clusters::Cluster#knative_services_finder Fix method name when calling on specs Add an explicit check for functions Added an explicit check to see if there are any functions available Fix Serverless feature spec - we don't find knative installation via database anymore, rather via Knative resource Display error message for request timeouts Display an error message if the request times out Adds feature specs for when functions exist Remove a test purposed hardcoded flag Add ability to partially load functions Added the ability to partially load functions on the frontend Add frontend unit tests Added tests for the new frontend additions Generate new translations Generated new frontend translations Address review comments Cleaned up the frontend unit test. Added computed prop for `isInstalled`. Move string to constant Simplify nil to array conversion Put knative_installed states in a frozen hash for better read Pluralize list of Knative states Quey services and pods filtering name This way we don't need to filter the namespace in memory. Also, the data we get from the network is much smaller. Simplify cache_key and fix bug - Simplifies the cache_key by removing namespace duplicate - Fixes a bug with reactive_cache memoization --- .../serverless/components/functions.vue | 36 ++++--- app/assets/javascripts/serverless/constants.js | 4 + .../javascripts/serverless/serverless_bundle.js | 3 +- app/assets/javascripts/serverless/store/actions.js | 29 ++++-- .../javascripts/serverless/store/mutation_types.js | 2 + .../javascripts/serverless/store/mutations.js | 15 ++- app/assets/javascripts/serverless/store/state.js | 1 + .../projects/serverless/functions_controller.rb | 10 +- .../clusters/cluster/knative_services_finder.rb | 114 +++++++++++++++++++++ .../projects/serverless/functions_finder.rb | 39 ++++--- app/models/clusters/applications/knative.rb | 48 --------- app/models/clusters/cluster.rb | 4 + ...rverless-with-existing-knative-installation.yml | 5 + locale/gitlab.pot | 3 + .../serverless/functions_controller_spec.rb | 92 ++++++++++++++--- .../features/projects/serverless/functions_spec.rb | 59 ++++++++--- .../cluster/knative_services_finder_spec.rb | 105 +++++++++++++++++++ .../projects/serverless/functions_finder_spec.rb | 68 +++++++----- .../serverless/components/environment_row_spec.js | 8 +- .../serverless/components/functions_spec.js | 27 ++++- spec/frontend/serverless/mock_data.js | 110 ++++++++++---------- spec/frontend/serverless/store/getters_spec.js | 2 +- spec/frontend/serverless/store/mutations_spec.js | 4 +- spec/models/clusters/applications/knative_spec.rb | 76 -------------- spec/models/clusters/cluster_spec.rb | 5 + spec/support/helpers/kubernetes_helpers.rb | 46 +++++++-- 26 files changed, 617 insertions(+), 298 deletions(-) create mode 100644 app/finders/clusters/cluster/knative_services_finder.rb create mode 100644 changelogs/unreleased/58941-use-gitlab-serverless-with-existing-knative-installation.yml create mode 100644 spec/finders/clusters/cluster/knative_services_finder_spec.rb diff --git a/app/assets/javascripts/serverless/components/functions.vue b/app/assets/javascripts/serverless/components/functions.vue index f9b4e789563..94341050b86 100644 --- a/app/assets/javascripts/serverless/components/functions.vue +++ b/app/assets/javascripts/serverless/components/functions.vue @@ -4,6 +4,7 @@ import { GlLoadingIcon } from '@gitlab/ui'; import FunctionRow from './function_row.vue'; import EnvironmentRow from './environment_row.vue'; import EmptyState from './empty_state.vue'; +import { CHECKING_INSTALLED } from '../constants'; export default { components: { @@ -13,10 +14,6 @@ export default { GlLoadingIcon, }, props: { - installed: { - type: Boolean, - required: true, - }, clustersPath: { type: String, required: true, @@ -31,8 +28,15 @@ export default { }, }, computed: { - ...mapState(['isLoading', 'hasFunctionData']), + ...mapState(['installed', 'isLoading', 'hasFunctionData']), ...mapGetters(['getFunctions']), + + checkingInstalled() { + return this.installed === CHECKING_INSTALLED; + }, + isInstalled() { + return this.installed === true; + }, }, created() { this.fetchFunctions({ @@ -47,15 +51,16 @@ export default {