From 95793b2325f6a9add2395d89447bd0e64b870cd1 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 2 Oct 2019 09:05:53 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .gitlab/CODEOWNERS | 2 +- CHANGELOG-EE.md | 7 ++ CHANGELOG.md | 7 ++ GITALY_SERVER_VERSION | 2 +- GITLAB_WORKHORSE_VERSION | 2 +- app/models/ci/build_trace_section.rb | 3 + app/models/project.rb | 7 +- app/models/project_feature.rb | 3 +- changelogs/unreleased/ak-upgrade-workhorse.yml | 5 + changelogs/unreleased/gitaly-version-v1.66.0.yml | 5 + .../ignore-id-column-ci_build_trace_sections.yml | 5 + changelogs/unreleased/security-29491.yml | 5 + doc/development/chatops_on_gitlabcom.md | 2 +- spec/frontend/helpers/dom_shims/README.md | 12 +++ .../frontend/helpers/dom_shims/get_client_rects.js | 50 +++++++++ .../helpers/dom_shims/get_client_rects_spec.js | 71 +++++++++++++ spec/frontend/helpers/dom_shims/index.js | 1 + spec/frontend/test_setup.js | 2 + spec/models/project_feature_spec.rb | 24 +++++ spec/models/project_spec.rb | 30 ++++++ spec/support/helpers/project_helpers.rb | 25 +++++ .../project_policy_table_shared_context.rb | 118 +++++++++++++++++++++ 22 files changed, 382 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/ak-upgrade-workhorse.yml create mode 100644 changelogs/unreleased/gitaly-version-v1.66.0.yml create mode 100644 changelogs/unreleased/ignore-id-column-ci_build_trace_sections.yml create mode 100644 changelogs/unreleased/security-29491.yml create mode 100644 spec/frontend/helpers/dom_shims/README.md create mode 100644 spec/frontend/helpers/dom_shims/get_client_rects.js create mode 100644 spec/frontend/helpers/dom_shims/get_client_rects_spec.js create mode 100644 spec/frontend/helpers/dom_shims/index.js create mode 100644 spec/support/helpers/project_helpers.rb create mode 100644 spec/support/shared_contexts/policies/project_policy_table_shared_context.rb diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 7ed75125391..527454dd3f9 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -23,6 +23,6 @@ lib/gitlab/github_import/ @gitlab-org/maintainers/database /lib/gitlab/auth/ldap/ @dblessing @mkozono /lib/gitlab/ci/templates/ @nolith @zj /lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml @DylanGriffith @mayra-cabrera @tkuah -/lib/gitlab/ci/templates/Security/ @plafoucriere @gonzoyumo @twoodham +/lib/gitlab/ci/templates/Security/ @plafoucriere @gonzoyumo @twoodham @sethgitlab /ee/app/models/project_alias.rb @patrickbajao /ee/lib/api/project_aliases.rb @patrickbajao diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md index 5bd8f7166bb..fd7b523af97 100644 --- a/CHANGELOG-EE.md +++ b/CHANGELOG-EE.md @@ -195,6 +195,13 @@ Please view this file on the master branch, on stable branches it's out of date. - Fixes style-lint errors and warnings for EE builds.scss file. +## 12.2.7 + +### Security (1 change) + +- Restrict access for security reports in MR widget. + + ## 12.2.6 ### Security (3 changes) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2ffec09cb2..6eab7f944e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -299,6 +299,13 @@ entry. - Updates tooltip of 'detached' label/state. +## 12.2.7 + +### Security (1 change) + +- Fix private feature Elasticsearch leak. + + ## 12.2.6 ### Security (11 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 09f245728f6..b6148bc0a75 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.65.1 +1.66.0 diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index db3905bf80f..e51b3430127 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.11.0 +8.12.0 diff --git a/app/models/ci/build_trace_section.rb b/app/models/ci/build_trace_section.rb index 8be42eb48d6..7fe6b753da1 100644 --- a/app/models/ci/build_trace_section.rb +++ b/app/models/ci/build_trace_section.rb @@ -4,6 +4,9 @@ module Ci class BuildTraceSection < ApplicationRecord extend Gitlab::Ci::Model + # Only remove > 2019-11-22 and > 12.5 + self.ignored_columns += %i[id] + belongs_to :build, class_name: 'Ci::Build' belongs_to :project belongs_to :section_name, class_name: 'Ci::BuildTraceSectionName' diff --git a/app/models/project.rb b/app/models/project.rb index 318d1473a70..512734e9b3f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -482,7 +482,7 @@ class Project < ApplicationRecord # the feature is either public, enabled, or internal with permission for the user. # Note: this scope doesn't enforce that the user has access to the projects, it just checks # that the user has access to the feature. It's important to use this scope with others - # that checks project authorizations first. + # that checks project authorizations first (e.g. `filter_by_feature_visibility`). # # This method uses an optimised version of `with_feature_access_level` for # logged in users to more efficiently get private projects with the given @@ -510,6 +510,11 @@ class Project < ApplicationRecord end end + # This scope returns projects where user has access to both the project and the feature. + def self.filter_by_feature_visibility(feature, user) + with_feature_available_for_user(feature, user).public_or_visible_to_user(user) + end + scope :active, -> { joins(:issues, :notes, :merge_requests).order('issues.created_at, notes.created_at, merge_requests.created_at DESC') } scope :abandoned, -> { where('projects.last_activity_at < ?', 6.months.ago) } diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 13b20b1fead..2013f620b5b 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -62,7 +62,8 @@ class ProjectFeature < ApplicationRecord private def ensure_feature!(feature) - feature = feature.model_name.plural.to_sym if feature.respond_to?(:model_name) + feature = feature.model_name.plural if feature.respond_to?(:model_name) + feature = feature.to_sym raise ArgumentError, "invalid project feature: #{feature}" unless FEATURES.include?(feature) feature diff --git a/changelogs/unreleased/ak-upgrade-workhorse.yml b/changelogs/unreleased/ak-upgrade-workhorse.yml new file mode 100644 index 00000000000..f92e8f876a6 --- /dev/null +++ b/changelogs/unreleased/ak-upgrade-workhorse.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade gitlab-workhorse to 8.12.0 +merge_request: 17892 +author: +type: changed diff --git a/changelogs/unreleased/gitaly-version-v1.66.0.yml b/changelogs/unreleased/gitaly-version-v1.66.0.yml new file mode 100644 index 00000000000..41a0e1ab87a --- /dev/null +++ b/changelogs/unreleased/gitaly-version-v1.66.0.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade to Gitaly v1.66.0 +merge_request: 17900 +author: +type: changed diff --git a/changelogs/unreleased/ignore-id-column-ci_build_trace_sections.yml b/changelogs/unreleased/ignore-id-column-ci_build_trace_sections.yml new file mode 100644 index 00000000000..4208273be19 --- /dev/null +++ b/changelogs/unreleased/ignore-id-column-ci_build_trace_sections.yml @@ -0,0 +1,5 @@ +--- +title: Ignore id column of ci_build_trace_sections table +merge_request: 17805 +author: +type: change diff --git a/changelogs/unreleased/security-29491.yml b/changelogs/unreleased/security-29491.yml new file mode 100644 index 00000000000..ec4ada47c62 --- /dev/null +++ b/changelogs/unreleased/security-29491.yml @@ -0,0 +1,5 @@ +--- +title: Fix private feature Elasticsearch leak +merge_request: +author: +type: security diff --git a/doc/development/chatops_on_gitlabcom.md b/doc/development/chatops_on_gitlabcom.md index 09d6638924a..a1c07ee2a1e 100644 --- a/doc/development/chatops_on_gitlabcom.md +++ b/doc/development/chatops_on_gitlabcom.md @@ -13,7 +13,7 @@ tasks such as: To request access to Chatops on GitLab.com: -1. Log into using the same username as for GitLab.com. +1. Log into **using the same username** as for GitLab.com (you may have to rename it). 1. Ask [anyone in the `chatops` project](https://gitlab.com/gitlab-com/chatops/-/project_members) to add you by running `/chatops run member add gitlab-com/chatops --ops`. ## See also diff --git a/spec/frontend/helpers/dom_shims/README.md b/spec/frontend/helpers/dom_shims/README.md new file mode 100644 index 00000000000..1105e4b0c4c --- /dev/null +++ b/spec/frontend/helpers/dom_shims/README.md @@ -0,0 +1,12 @@ +## Jest DOM shims + +This is where we shim parts of JSDom. It is imported in our root `test_setup.js`. + +### Why do we need this? + +Since JSDom mocks a real DOM environment (which is a good thing), it +unfortunately does not support some jQuery matchers. + +### References + +- https://gitlab.com/gitlab-org/gitlab/merge_requests/17906#note_224448120 diff --git a/spec/frontend/helpers/dom_shims/get_client_rects.js b/spec/frontend/helpers/dom_shims/get_client_rects.js new file mode 100644 index 00000000000..d740c1bf154 --- /dev/null +++ b/spec/frontend/helpers/dom_shims/get_client_rects.js @@ -0,0 +1,50 @@ +function hasHiddenStyle(node) { + if (!node.style) { + return false; + } else if (node.style.display === 'none' || node.style.visibility === 'hidden') { + return true; + } + + return false; +} + +function createDefaultClientRect() { + return { + bottom: 0, + height: 0, + left: 0, + right: 0, + top: 0, + width: 0, + x: 0, + y: 0, + }; +} + +/** + * This is needed to get the `toBeVisible` matcher to work in `jsdom` + * + * Reference: + * - https://github.com/jsdom/jsdom/issues/1322 + * - https://github.com/unindented/custom-jquery-matchers/blob/v2.1.0/packages/custom-jquery-matchers/src/matchers.js#L157 + */ +window.Element.prototype.getClientRects = function getClientRects() { + let node = this; + + while (node) { + if (node === document) { + break; + } + + if (hasHiddenStyle(node)) { + return []; + } + node = node.parentNode; + } + + if (!node) { + return []; + } + + return [createDefaultClientRect()]; +}; diff --git a/spec/frontend/helpers/dom_shims/get_client_rects_spec.js b/spec/frontend/helpers/dom_shims/get_client_rects_spec.js new file mode 100644 index 00000000000..e7b8f1e235b --- /dev/null +++ b/spec/frontend/helpers/dom_shims/get_client_rects_spec.js @@ -0,0 +1,71 @@ +const createTestElement = () => { + const element = document.createElement('div'); + + element.textContent = 'Hello World!'; + + return element; +}; + +describe('DOM patch for getClientRects', () => { + let origHtml; + let el; + + beforeEach(() => { + origHtml = document.body.innerHTML; + el = createTestElement(); + }); + + afterEach(() => { + document.body.innerHTML = origHtml; + }); + + describe('toBeVisible matcher', () => { + describe('when not attached to document', () => { + it('does not match', () => { + expect(el).not.toBeVisible(); + }); + }); + + describe('when attached to document', () => { + beforeEach(() => { + document.body.appendChild(el); + }); + + it('matches', () => { + expect(el).toBeVisible(); + }); + }); + + describe('with parent and attached to document', () => { + let parentEl; + + beforeEach(() => { + parentEl = createTestElement(); + parentEl.appendChild(el); + document.body.appendChild(parentEl); + }); + + it('matches', () => { + expect(el).toBeVisible(); + }); + + describe.each` + style + ${{ display: 'none' }} + ${{ visibility: 'hidden' }} + `('with style $style', ({ style }) => { + it('does not match when applied to element', () => { + Object.assign(el.style, style); + + expect(el).not.toBeVisible(); + }); + + it('does not match when applied to parent', () => { + Object.assign(parentEl.style, style); + + expect(el).not.toBeVisible(); + }); + }); + }); + }); +}); diff --git a/spec/frontend/helpers/dom_shims/index.js b/spec/frontend/helpers/dom_shims/index.js new file mode 100644 index 00000000000..40256398e6d --- /dev/null +++ b/spec/frontend/helpers/dom_shims/index.js @@ -0,0 +1 @@ +import './get_client_rects'; diff --git a/spec/frontend/test_setup.js b/spec/frontend/test_setup.js index 6e1f1038dcd..0b9cfa44409 100644 --- a/spec/frontend/test_setup.js +++ b/spec/frontend/test_setup.js @@ -8,6 +8,8 @@ import { getJSONFixture, loadHTMLFixture, setHTMLFixture } from './helpers/fixtu import { setupManualMocks } from './mocks/mocks_helper'; import customMatchers from './matchers'; +import './helpers/dom_shims'; + // Expose jQuery so specs using jQuery plugins can be imported nicely. // Here is an issue to explore better alternatives: // https://gitlab.com/gitlab-org/gitlab/issues/12448 diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index e6505bb4a51..5b448c343a0 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -228,4 +228,28 @@ describe ProjectFeature do end end end + + describe '.required_minimum_access_level' do + it 'handles reporter level' do + expect(described_class.required_minimum_access_level(:merge_requests)).to eq(Gitlab::Access::REPORTER) + end + + it 'handles guest level' do + expect(described_class.required_minimum_access_level(:issues)).to eq(Gitlab::Access::GUEST) + end + + it 'accepts ActiveModel' do + expect(described_class.required_minimum_access_level(MergeRequest)).to eq(Gitlab::Access::REPORTER) + end + + it 'accepts string' do + expect(described_class.required_minimum_access_level('merge_requests')).to eq(Gitlab::Access::REPORTER) + end + + it 'raises error if feature is invalid' do + expect do + described_class.required_minimum_access_level(:foos) + end.to raise_error + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index daccd143b6d..45812a83c68 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3469,6 +3469,36 @@ describe Project do end end + describe '.filter_by_feature_visibility' do + include_context 'ProjectPolicyTable context' + include ProjectHelpers + using RSpec::Parameterized::TableSyntax + + set(:group) { create(:group) } + let!(:project) { create(:project, project_level, namespace: group ) } + let(:user) { create_user_from_membership(project, membership) } + + context 'reporter level access' do + let(:feature) { MergeRequest } + + where(:project_level, :feature_access_level, :membership, :expected_count) do + permission_table_for_reporter_feature_access + end + + with_them do + it "respects visibility" do + update_feature_access_level(project, feature_access_level) + + expected_objects = expected_count == 1 ? [project] : [] + + expect( + described_class.filter_by_feature_visibility(feature, user) + ).to eq(expected_objects) + end + end + end + end + describe '#pages_available?' do let(:project) { create(:project, group: group) } diff --git a/spec/support/helpers/project_helpers.rb b/spec/support/helpers/project_helpers.rb new file mode 100644 index 00000000000..61056b47aed --- /dev/null +++ b/spec/support/helpers/project_helpers.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module ProjectHelpers + # @params target [Project] membership target + # @params membership [Symbol] accepts the membership levels :guest, :reporter... + # and phony levels :non_member and :anonymous + def create_user_from_membership(target, membership) + case membership + when :anonymous + nil + when :non_member + create(:user, name: membership) + else + create(:user, name: membership).tap { |u| target.add_user(u, membership) } + end + end + + def update_feature_access_level(project, access_level) + project.update!( + repository_access_level: access_level, + merge_requests_access_level: access_level, + builds_access_level: access_level + ) + end +end diff --git a/spec/support/shared_contexts/policies/project_policy_table_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_table_shared_context.rb new file mode 100644 index 00000000000..e666b346b8b --- /dev/null +++ b/spec/support/shared_contexts/policies/project_policy_table_shared_context.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +RSpec.shared_context 'ProjectPolicyTable context' do + using RSpec::Parameterized::TableSyntax + + # rubocop:disable Metrics/AbcSize + def permission_table_for_reporter_feature_access + :public | :enabled | :reporter | 1 + :public | :enabled | :guest | 1 + :public | :enabled | :non_member | 1 + :public | :enabled | :anonymous | 1 + + :public | :private | :reporter | 1 + :public | :private | :guest | 0 + :public | :private | :non_member | 0 + :public | :private | :anonymous | 0 + + :public | :disabled | :reporter | 0 + :public | :disabled | :guest | 0 + :public | :disabled | :non_member | 0 + :public | :disabled | :anonymous | 0 + + :internal | :enabled | :reporter | 1 + :internal | :enabled | :guest | 1 + :internal | :enabled | :non_member | 1 + :internal | :enabled | :anonymous | 0 + + :internal | :private | :reporter | 1 + :internal | :private | :guest | 0 + :internal | :private | :non_member | 0 + :internal | :private | :anonymous | 0 + + :internal | :disabled | :reporter | 0 + :internal | :disabled | :guest | 0 + :internal | :disabled | :non_member | 0 + :internal | :disabled | :anonymous | 0 + + :private | :enabled | :reporter | 1 + :private | :enabled | :guest | 1 + :private | :enabled | :non_member | 0 + :private | :enabled | :anonymous | 0 + + :private | :private | :reporter | 1 + :private | :private | :guest | 0 + :private | :private | :non_member | 0 + :private | :private | :anonymous | 0 + + :private | :disabled | :reporter | 0 + :private | :disabled | :guest | 0 + :private | :disabled | :non_member | 0 + :private | :disabled | :anonymous | 0 + end + + def permission_table_for_guest_feature_access + :public | :enabled | :reporter | 1 + :public | :enabled | :guest | 1 + :public | :enabled | :non_member | 1 + :public | :enabled | :anonymous | 1 + + :public | :private | :reporter | 1 + :public | :private | :guest | 1 + :public | :private | :non_member | 0 + :public | :private | :anonymous | 0 + + :public | :disabled | :reporter | 0 + :public | :disabled | :guest | 0 + :public | :disabled | :non_member | 0 + :public | :disabled | :anonymous | 0 + + :internal | :enabled | :reporter | 1 + :internal | :enabled | :guest | 1 + :internal | :enabled | :non_member | 1 + :internal | :enabled | :anonymous | 0 + + :internal | :private | :reporter | 1 + :internal | :private | :guest | 1 + :internal | :private | :non_member | 0 + :internal | :private | :anonymous | 0 + + :internal | :disabled | :reporter | 0 + :internal | :disabled | :guest | 0 + :internal | :disabled | :non_member | 0 + :internal | :disabled | :anonymous | 0 + + :private | :enabled | :reporter | 1 + :private | :enabled | :guest | 1 + :private | :enabled | :non_member | 0 + :private | :enabled | :anonymous | 0 + + :private | :private | :reporter | 1 + :private | :private | :guest | 1 + :private | :private | :non_member | 0 + :private | :private | :anonymous | 0 + + :private | :disabled | :reporter | 0 + :private | :disabled | :guest | 0 + :private | :disabled | :non_member | 0 + :private | :disabled | :anonymous | 0 + end + + def permission_table_for_project_access + :public | :reporter | 1 + :public | :guest | 1 + :public | :non_member | 1 + :public | :anonymous | 1 + + :internal | :reporter | 1 + :internal | :guest | 1 + :internal | :non_member | 1 + :internal | :anonymous | 0 + + :private | :reporter | 1 + :private | :guest | 1 + :private | :non_member | 0 + :private | :anonymous | 0 + end + # rubocop:enable Metrics/AbcSize +end -- cgit v1.2.1