diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 5ff8c4f5d2ad2fab0f675c329d99cc618ac3e74c..5db08bf2dc57979f9a3785791285b2724e9d45b7 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.26.0 +1.27.0 diff --git a/app/assets/javascripts/emoji/index.js b/app/assets/javascripts/emoji/index.js index bb5085a1911849941eef5e921ae13208f1a90bba..b9b3b34452473794e03552140dddb1df686e5580 100644 --- a/app/assets/javascripts/emoji/index.js +++ b/app/assets/javascripts/emoji/index.js @@ -33,10 +33,7 @@ export function initEmojiMap() { } axiosInstance - .get( - `${gon.asset_host || ''}${gon.relative_url_root || - ''}/-/emojis/${EMOJI_VERSION}/emojis.json`, - ) + .get(`${gon.relative_url_root || ''}/-/emojis/${EMOJI_VERSION}/emojis.json`) .then(({ data }) => { emojiMap = data; validEmojiNames = [...Object.keys(emojiMap), ...Object.keys(emojiAliases)]; diff --git a/app/assets/javascripts/pipelines/pipeline_details_mediator.js b/app/assets/javascripts/pipelines/pipeline_details_mediator.js index bd1e1895660db3fb4d47b7f227707f7bf396ba80..d67d88c4dba6201988c7a8b476546026e04095f0 100644 --- a/app/assets/javascripts/pipelines/pipeline_details_mediator.js +++ b/app/assets/javascripts/pipelines/pipeline_details_mediator.js @@ -19,6 +19,7 @@ export default class pipelinesMediator { this.poll = new Poll({ resource: this.service, method: 'getPipeline', + data: this.store.state.expandedPipelines ? this.getExpandedParameters() : undefined, successCallback: this.successCallback.bind(this), errorCallback: this.errorCallback.bind(this), }); @@ -56,6 +57,19 @@ export default class pipelinesMediator { .getPipeline() .then(response => this.successCallback(response)) .catch(() => this.errorCallback()) - .finally(() => this.poll.restart()); + .finally(() => + this.poll.restart( + this.store.state.expandedPipelines ? this.getExpandedParameters() : undefined, + ), + ); + } + + /** + * Backend expects paramets in the following format: `expanded[]=id&expanded[]=id` + */ + getExpandedParameters() { + return { + expanded: this.store.state.expandedPipelines, + }; } } diff --git a/app/assets/javascripts/pipelines/services/pipeline_service.js b/app/assets/javascripts/pipelines/services/pipeline_service.js index a53a9cc8365a9d6fe6259d5cd85e0653f73b2cb5..e44eb9cdfd11201a8b99b6fdf412c67488af2c9d 100644 --- a/app/assets/javascripts/pipelines/services/pipeline_service.js +++ b/app/assets/javascripts/pipelines/services/pipeline_service.js @@ -5,8 +5,8 @@ export default class PipelineService { this.pipeline = endpoint; } - getPipeline() { - return axios.get(this.pipeline); + getPipeline(params) { + return axios.get(this.pipeline, { params }); } // eslint-disable-next-line class-methods-use-this diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index e74e5f008d7f5c941374397bf964c3ca920dfe43..db49d3bed9c10ef5c241daeedfa171ca0c77328a 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -26,7 +26,7 @@ class GroupPolicy < BasePolicy condition(:can_change_parent_share_with_group_lock) { can?(:change_share_with_group_lock, @subject.parent) } condition(:has_projects) do - GroupProjectsFinder.new(group: @subject, current_user: @user, options: { include_subgroups: true }).execute.any? + GroupProjectsFinder.new(group: @subject, current_user: @user, options: { include_subgroups: true, only_owned: true }).execute.any? end condition(:has_clusters, scope: :subject) { clusterable_has_clusters? } @@ -55,6 +55,7 @@ class GroupPolicy < BasePolicy rule { has_projects }.policy do enable :read_list enable :read_label + enable :read_group end rule { has_access }.enable :read_namespace diff --git a/changelogs/unreleased/modify_group_policy.yml b/changelogs/unreleased/modify_group_policy.yml new file mode 100644 index 0000000000000000000000000000000000000000..cd9fc340faa4e864c1b10beffdd0f3c9eef9759f --- /dev/null +++ b/changelogs/unreleased/modify_group_policy.yml @@ -0,0 +1,5 @@ +--- +title: Allow project members to see private group if the project is in the group namespace +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/sh-fix-blank-codeowners-ce.yml b/changelogs/unreleased/sh-fix-blank-codeowners-ce.yml new file mode 100644 index 0000000000000000000000000000000000000000..05ea5869eb1dd4cc6136e0d4c080f54bccd467d1 --- /dev/null +++ b/changelogs/unreleased/sh-fix-blank-codeowners-ce.yml @@ -0,0 +1,5 @@ +--- +title: Fix 500 error caused by CODEOWNERS with no matches +merge_request: 26072 +author: +type: fixed diff --git a/changelogs/unreleased/sh-revert-rack-request-health-checks.yml b/changelogs/unreleased/sh-revert-rack-request-health-checks.yml new file mode 100644 index 0000000000000000000000000000000000000000..5dd5e5b731c95d96532baf2f54c0da1514b3a90c --- /dev/null +++ b/changelogs/unreleased/sh-revert-rack-request-health-checks.yml @@ -0,0 +1,5 @@ +--- +title: Fix health checks not working behind load balancers +merge_request: 26055 +author: +type: fixed diff --git a/changelogs/unreleased/sh-rugged-commit-tree-entry.yml b/changelogs/unreleased/sh-rugged-commit-tree-entry.yml new file mode 100644 index 0000000000000000000000000000000000000000..bcefa2c7112bac2e3faac4e7be57588620c59b32 --- /dev/null +++ b/changelogs/unreleased/sh-rugged-commit-tree-entry.yml @@ -0,0 +1,5 @@ +--- +title: Bring back Rugged implementation of commit_tree_entry +merge_request: 25896 +author: +type: other diff --git a/config/application.rb b/config/application.rb index 1c11e34728107a61ae3269e70396cfd17d07655d..6bdf61edfb171c439e4822f161b9a891a5aa1157 100644 --- a/config/application.rb +++ b/config/application.rb @@ -94,6 +94,7 @@ module Gitlab # - Webhook URLs (:hook) # - Sentry DSN (:sentry_dsn) # - File content from Web Editor (:content) + # - Jira shared secret (:sharedSecret) # # NOTE: It is **IMPORTANT** to also update gitlab-workhorse's filter when adding parameters here to not # introduce another security vulnerability: https://gitlab.com/gitlab-org/gitlab-workhorse/issues/182 @@ -108,6 +109,7 @@ module Gitlab trace variables content + sharedSecret ) # Enable escaping HTML in JSON. diff --git a/doc/user/group/index.md b/doc/user/group/index.md index c1f50bcc593bdc2f1356a6d9d8a7a265513f16ec..1fe8017adbc690cfe931ea8b4d12bf4b4fb6bfce 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -168,20 +168,21 @@ Alternatively, you can [lock the sharing with group feature](#share-with-group-l In GitLab Enterprise Edition it is possible to manage GitLab group memberships using LDAP groups. See [the GitLab Enterprise Edition documentation](../../integration/ldap.md) for more information. -## Transfer groups to another group +## Transferring groups -From 10.5 there are two different ways to transfer a group: +From GitLab 10.5, groups can be transferred in the following ways: -- Either by transferring a group into another group (making it a subgroup of that group). -- Or by converting a subgroup into a root group (a group with no parent). +- Top-level groups can be transferred to a group, converting them into subgroups. +- Subgroups can be transferred to a new parent group. +- Subgroups can be transferred out from a parent group, converting them into top-level groups. -Please make sure to understand that: +When transferring groups, note: -- Changing a group's parent can have unintended side effects. See [Redirects when changing repository paths](https://docs.gitlab.com/ce/user/project/index.html#redirects-when-changing-repository-paths) -- You can only transfer the group to a group you manage. +- Changing a group's parent can have unintended side effects. See [Redirects when changing repository paths](../project/index.md#redirects-when-changing-repository-paths). +- You can only transfer groups to groups you manage. - You will need to update your local repositories to point to the new location. -- If the parent group's visibility is lower than the group current visibility, visibility levels for subgroups and projects will be changed to match the new parent group's visibility. -- Only explicit group membership is transferred, not the inherited membership. If this would leave the group without an owner, the transferring user is added as owner instead. +- If the parent group's visibility is lower than the group's current visibility, visibility levels for subgroups and projects will be changed to match the new parent group's visibility. +- Only explicit group membership is transferred, not inherited membership. If the group's owners have only inherited membership, this would leave the group without an owner. In this case, the user transferring the group becomes the group's owner. ## Group settings diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 491e4b471963179b964d8fad886db14c3d7b10b6..e5bbd500e9833a6c2489ed6cdfeb68260b6b3119 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -314,11 +314,16 @@ module Gitlab def tree_entry(path) return unless path.present? + commit_tree_entry(path) + end + + def commit_tree_entry(path) # We're only interested in metadata, so limit actual data to 1 byte # since Gitaly doesn't support "send no data" option. entry = @repository.gitaly_commit_client.tree_entry(id, path, 1) return unless entry + # To be compatible with the rugged format entry = entry.to_h entry.delete(:data) entry[:name] = File.basename(path) diff --git a/lib/gitlab/git/rugged_impl/commit.rb b/lib/gitlab/git/rugged_impl/commit.rb index 251802878c303f3cefa9b5e3fccc6d94eeac32c5..f6777dfa0c3de1f146473474c1634dff0c9153a4 100644 --- a/lib/gitlab/git/rugged_impl/commit.rb +++ b/lib/gitlab/git/rugged_impl/commit.rb @@ -43,6 +43,30 @@ module Gitlab end end + override :commit_tree_entry + def commit_tree_entry(path) + if Feature.enabled?(:rugged_commit_tree_entry) + rugged_tree_entry(path) + else + super + end + end + + # Is this the same as Blob.find_entry_by_path ? + def rugged_tree_entry(path) + rugged_commit.tree.path(path) + rescue Rugged::TreeError + nil + end + + def rugged_commit + @rugged_commit ||= if raw_commit.is_a?(Rugged::Commit) + raw_commit + else + @repository.rev_parse_target(id) + end + end + def init_from_rugged(commit) author = commit.author committer = commit.committer diff --git a/lib/gitlab/git/rugged_impl/repository.rb b/lib/gitlab/git/rugged_impl/repository.rb index fe0120b119936b2dfb342545b1ba7419fea62284..c0a91f59ab9a6bd197c8eef83aeb75407845c528 100644 --- a/lib/gitlab/git/rugged_impl/repository.rb +++ b/lib/gitlab/git/rugged_impl/repository.rb @@ -12,7 +12,7 @@ module Gitlab module Repository extend ::Gitlab::Utils::Override - FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor).freeze + FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor rugged_commit_tree_entry).freeze def alternate_object_directories relative_object_directories.map { |d| File.join(path, d) } diff --git a/lib/gitlab/middleware/basic_health_check.rb b/lib/gitlab/middleware/basic_health_check.rb index acf8c301b8fe3bbd524c495645f034bc6162b233..84e49805428be2bb9d1cba56381b8d58132c9dcb 100644 --- a/lib/gitlab/middleware/basic_health_check.rb +++ b/lib/gitlab/middleware/basic_health_check.rb @@ -24,7 +24,13 @@ module Gitlab def call(env) return @app.call(env) unless env['PATH_INFO'] == HEALTH_PATH - request = ActionDispatch::Request.new(env) + # We should be using ActionDispatch::Request instead of + # Rack::Request to be consistent with Rails, but due to a Rails + # bug described in + # https://gitlab.com/gitlab-org/gitlab-ce/issues/58573#note_149799010 + # hosts behind a load balancer will only see 127.0.0.1 for the + # load balancer's IP. + request = Rack::Request.new(env) return OK_RESPONSE if client_ip_whitelisted?(request) diff --git a/lib/gitlab/request_context.rb b/lib/gitlab/request_context.rb index d9811e036d3833741a5813765c9df690db52fef1..f6d289476c5c711362e1bfd8adc2b0218e628b32 100644 --- a/lib/gitlab/request_context.rb +++ b/lib/gitlab/request_context.rb @@ -13,7 +13,13 @@ module Gitlab end def call(env) - req = ActionDispatch::Request.new(env) + # We should be using ActionDispatch::Request instead of + # Rack::Request to be consistent with Rails, but due to a Rails + # bug described in + # https://gitlab.com/gitlab-org/gitlab-ce/issues/58573#note_149799010 + # hosts behind a load balancer will only see 127.0.0.1 for the + # load balancer's IP. + req = Rack::Request.new(env) Gitlab::SafeRequestStore[:client_ip] = req.ip diff --git a/lib/gitlab/user_extractor.rb b/lib/gitlab/user_extractor.rb index 874599688bbc0330ba64027c5e043bd6dbe629c3..b41d085ee7704b1cbb60774544c4364d8f01983d 100644 --- a/lib/gitlab/user_extractor.rb +++ b/lib/gitlab/user_extractor.rb @@ -16,6 +16,7 @@ module Gitlab def users return User.none unless @text.present? + return User.none if references.empty? @users ||= User.from_union(union_relations) end diff --git a/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb index 4070a2252603974c03fa58950244089ae86a284f..d8609aa037a68058749a6fc3049b92d3b5b385dd 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb @@ -19,7 +19,7 @@ module QA page.add_member(user.username) end - expect(page).to have_content("#{user.name} @#{user.username} Given access") + expect(page).to have_content(/#{user.name} (. )?@#{user.username} Given access/) end end end diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index 8a44ce528492861d7c01ba6ab02b3cda5abf9066..ee5d27355f131231d638dcd0cbd85852a22afb91 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -82,6 +82,12 @@ FactoryBot.define do end end + trait :with_job do + after(:build) do |pipeline, evaluator| + pipeline.builds << build(:ci_build, pipeline: pipeline, project: pipeline.project) + end + end + trait :auto_devops_source do config_source { Ci::Pipeline.config_sources[:auto_devops_source] } end diff --git a/spec/features/security/group/private_access_spec.rb b/spec/features/security/group/private_access_spec.rb index 3238e07fe157833489f6020aa52168baef0770c2..de38a2c020472667692d7108eeb79f0df627a9b1 100644 --- a/spec/features/security/group/private_access_spec.rb +++ b/spec/features/security/group/private_access_spec.rb @@ -27,7 +27,7 @@ describe 'Private Group access' do it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) } - it { is_expected.to be_denied_for(project_guest) } + it { is_expected.to be_allowed_for(project_guest) } it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -42,7 +42,7 @@ describe 'Private Group access' do it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) } - it { is_expected.to be_denied_for(project_guest) } + it { is_expected.to be_allowed_for(project_guest) } it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -58,7 +58,7 @@ describe 'Private Group access' do it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) } - it { is_expected.to be_denied_for(project_guest) } + it { is_expected.to be_allowed_for(project_guest) } it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -73,7 +73,7 @@ describe 'Private Group access' do it { is_expected.to be_allowed_for(:developer).of(group) } it { is_expected.to be_allowed_for(:reporter).of(group) } it { is_expected.to be_allowed_for(:guest).of(group) } - it { is_expected.to be_denied_for(project_guest) } + it { is_expected.to be_allowed_for(project_guest) } it { is_expected.to be_denied_for(:user) } it { is_expected.to be_denied_for(:external) } it { is_expected.to be_denied_for(:visitor) } @@ -96,6 +96,7 @@ describe 'Private Group access' do describe 'GET /groups/:path for shared projects' do let(:project) { create(:project, :public) } + before do Projects::GroupLinks::CreateService.new( project, diff --git a/spec/lib/gitlab/middleware/basic_health_check_spec.rb b/spec/lib/gitlab/middleware/basic_health_check_spec.rb index 187d903a5e192414b5aa31e757d036c70640403a..86bdc479b66154de25c1c1493957f425fc9031a9 100644 --- a/spec/lib/gitlab/middleware/basic_health_check_spec.rb +++ b/spec/lib/gitlab/middleware/basic_health_check_spec.rb @@ -28,6 +28,35 @@ describe Gitlab::Middleware::BasicHealthCheck do end end + context 'with X-Forwarded-For headers' do + let(:load_balancer_ip) { '1.2.3.4' } + + before do + env['HTTP_X_FORWARDED_FOR'] = "#{load_balancer_ip}, 127.0.0.1" + env['REMOTE_ADDR'] = '127.0.0.1' + env['PATH_INFO'] = described_class::HEALTH_PATH + end + + it 'returns 200 response when endpoint is allowed' do + allow(Settings.monitoring).to receive(:ip_whitelist).and_return([load_balancer_ip]) + expect(app).not_to receive(:call) + + response = middleware.call(env) + + expect(response[0]).to eq(200) + expect(response[1]).to eq({ 'Content-Type' => 'text/plain' }) + expect(response[2]).to eq(['GitLab OK']) + end + + it 'returns 404 when whitelist is not configured' do + allow(Settings.monitoring).to receive(:ip_whitelist).and_return([]) + + response = middleware.call(env) + + expect(response[0]).to eq(404) + end + end + context 'whitelisted IP' do before do env['REMOTE_ADDR'] = '127.0.0.1' diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb index fd443cc1f71daf97b232052cc3264000ca2cb08e..3ed57c2c9166f559f73529bfbafce55ef0062dd4 100644 --- a/spec/lib/gitlab/request_context_spec.rb +++ b/spec/lib/gitlab/request_context_spec.rb @@ -6,6 +6,31 @@ describe Gitlab::RequestContext do let(:app) { -> (env) {} } let(:env) { Hash.new } + context 'with X-Forwarded-For headers', :request_store do + let(:load_balancer_ip) { '1.2.3.4' } + let(:headers) do + { + 'HTTP_X_FORWARDED_FOR' => "#{load_balancer_ip}, 127.0.0.1", + 'REMOTE_ADDR' => '127.0.0.1' + } + end + + let(:env) { Rack::MockRequest.env_for("/").merge(headers) } + + it 'returns the load balancer IP' do + client_ip = nil + + endpoint = proc do + client_ip = Gitlab::SafeRequestStore[:client_ip] + [200, {}, ["Hello"]] + end + + Rails.application.middleware.build(endpoint).call(env) + + expect(client_ip).to eq(load_balancer_ip) + end + end + context 'when RequestStore::Middleware is used' do around do |example| RequestStore::Middleware.new(-> (env) { example.run }).call({}) @@ -15,7 +40,7 @@ describe Gitlab::RequestContext do let(:ip) { '192.168.1.11' } before do - allow_any_instance_of(ActionDispatch::Request).to receive(:ip).and_return(ip) + allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip) described_class.new(app).call(env) end diff --git a/spec/lib/gitlab/user_extractor_spec.rb b/spec/lib/gitlab/user_extractor_spec.rb index fcc05ab3a0c8c6fc6bfd59e537a5f282b1011a56..6e2bb81fbda2a5426df7152bf4a16362e1fc7d69 100644 --- a/spec/lib/gitlab/user_extractor_spec.rb +++ b/spec/lib/gitlab/user_extractor_spec.rb @@ -48,6 +48,14 @@ describe Gitlab::UserExtractor do it 'includes all mentioned usernames' do expect(extractor.matches[:usernames]).to contain_exactly('user-1', 'user-2', 'user-4') end + + context 'input has no matching e-mail or usernames' do + it 'returns an empty list of users' do + extractor = described_class.new('My test') + + expect(extractor.users).to be_empty + end + end end describe '#references' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index baad83521857c72b2574326645472c8af3fcdb0e..9d4e18534ae92ea9f1f7a560625b5b43d7bb6361 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -542,7 +542,7 @@ eos end end - describe '#uri_type' do + shared_examples '#uri_type' do it 'returns the URI type at the given path' do expect(commit.uri_type('files/html')).to be(:tree) expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) @@ -561,6 +561,20 @@ eos end end + describe '#uri_type with Gitaly enabled' do + it_behaves_like "#uri_type" + end + + describe '#uri_type with Rugged enabled', :enable_rugged do + it 'calls out to the Rugged implementation' do + allow_any_instance_of(Rugged::Tree).to receive(:path).with('files/html').and_call_original + + commit.uri_type('files/html') + end + + it_behaves_like '#uri_type' + end + describe '.from_hash' do let(:new_commit) { described_class.from_hash(commit.to_hash, project) }