summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2019-10-02 09:05:53 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2019-10-02 09:05:53 +0000
commit95793b2325f6a9add2395d89447bd0e64b870cd1 (patch)
treeb16717b39c378af5cf9356dbe0eea1cad2cfc67c
parent404bb44ef7dfc2b0d4da6b946b8b96007aca4b56 (diff)
downloadgitlab-ce-95793b2325f6a9add2395d89447bd0e64b870cd1.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--.gitlab/CODEOWNERS2
-rw-r--r--CHANGELOG-EE.md7
-rw-r--r--CHANGELOG.md7
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--GITLAB_WORKHORSE_VERSION2
-rw-r--r--app/models/ci/build_trace_section.rb3
-rw-r--r--app/models/project.rb7
-rw-r--r--app/models/project_feature.rb3
-rw-r--r--changelogs/unreleased/ak-upgrade-workhorse.yml5
-rw-r--r--changelogs/unreleased/gitaly-version-v1.66.0.yml5
-rw-r--r--changelogs/unreleased/ignore-id-column-ci_build_trace_sections.yml5
-rw-r--r--changelogs/unreleased/security-29491.yml5
-rw-r--r--doc/development/chatops_on_gitlabcom.md2
-rw-r--r--spec/frontend/helpers/dom_shims/README.md12
-rw-r--r--spec/frontend/helpers/dom_shims/get_client_rects.js50
-rw-r--r--spec/frontend/helpers/dom_shims/get_client_rects_spec.js71
-rw-r--r--spec/frontend/helpers/dom_shims/index.js1
-rw-r--r--spec/frontend/test_setup.js2
-rw-r--r--spec/models/project_feature_spec.rb24
-rw-r--r--spec/models/project_spec.rb30
-rw-r--r--spec/support/helpers/project_helpers.rb25
-rw-r--r--spec/support/shared_contexts/policies/project_policy_table_shared_context.rb118
22 files changed, 382 insertions, 6 deletions
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 <https://ops.gitlab.net/users/sign_in> using the same username as for GitLab.com.
+1. Log into <https://ops.gitlab.net/users/sign_in> **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 <username> 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