Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/controllers/concerns/analytics/cycle_analytics/value_stream_actions.rb7
-rw-r--r--app/helpers/users_helper.rb2
-rw-r--r--app/policies/project_policy.rb2
-rw-r--r--doc/ci/jobs/ci_job_token.md13
-rw-r--r--doc/update/index.md15
-rw-r--r--lib/api/entities/issue.rb9
-rw-r--r--lib/banzai/filter/references/reference_cache.rb31
-rw-r--r--spec/features/admin/users/user_spec.rb8
-rw-r--r--spec/lib/api/entities/issue_spec.rb47
-rw-r--r--spec/policies/project_policy_spec.rb46
-rw-r--r--spec/requests/api/npm_project_packages_spec.rb4
-rw-r--r--spec/requests/lfs_http_spec.rb6
12 files changed, 120 insertions, 70 deletions
diff --git a/app/controllers/concerns/analytics/cycle_analytics/value_stream_actions.rb b/app/controllers/concerns/analytics/cycle_analytics/value_stream_actions.rb
index f10b23d1664..cf0430307a3 100644
--- a/app/controllers/concerns/analytics/cycle_analytics/value_stream_actions.rb
+++ b/app/controllers/concerns/analytics/cycle_analytics/value_stream_actions.rb
@@ -7,6 +7,9 @@ module Analytics
included do
before_action :authorize
+ # Defining the before action here, because in the EE module we cannot define a before_action.
+ # Reason: this is a module which is being included into a controller. This module is extended in EE.
+ before_action :authorize_modification, only: %i[create destroy update] # rubocop:disable Rails/LexicallyScopedActionFilter
end
def index
@@ -25,6 +28,10 @@ module Analytics
def authorize
authorize_read_cycle_analytics!
end
+
+ def authorize_modification
+ # no-op, overridden in EE
+ end
end
end
end
diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb
index c8002c437a9..acc7d8a5a10 100644
--- a/app/helpers/users_helper.rb
+++ b/app/helpers/users_helper.rb
@@ -136,7 +136,7 @@ module UsersHelper
def confirm_user_data(user)
message = if user.unconfirmed_email.present?
- _('This user has an unconfirmed email address (%{email}). You may force a confirmation.') % { email: user.unconfirmed_email }
+ safe_format(_('This user has an unconfirmed email address (%{email}). You may force a confirmation.'), email: user.unconfirmed_email)
else
_('This user has an unconfirmed email address. You may force a confirmation.')
end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index c70dc288710..cdb7c3eca46 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -670,7 +670,7 @@ class ProjectPolicy < BasePolicy
enable :read_project_for_iids
end
- rule { ~public_project & ~internal_access & ~project_allowed_for_job_token }.prevent_all
+ rule { ~project_allowed_for_job_token }.prevent_all
rule { can?(:public_access) }.policy do
enable :read_package
diff --git a/doc/ci/jobs/ci_job_token.md b/doc/ci/jobs/ci_job_token.md
index a60aaa04ea1..fa1ba4be2ec 100644
--- a/doc/ci/jobs/ci_job_token.md
+++ b/doc/ci/jobs/ci_job_token.md
@@ -67,12 +67,10 @@ tries to steal tokens from other jobs.
You can control what projects a CI/CD job token can access to increase the
job token's security. A job token might give extra permissions that aren't necessary
-to access specific private resources. The job token scope only controls access
-to private projects. If an accessed project is public or internal, token scoping does
-not apply.
+to access specific resources.
If a job token is leaked, it could potentially be used to access private data
-to the job token's user. By limiting the job token access scope, private data cannot
+to the job token's user. By limiting the job token access scope, project data cannot
be accessed unless projects are explicitly authorized.
There is a proposal to add more strategic control of the access permissions,
@@ -92,8 +90,7 @@ their `CI_JOB_TOKEN`.
For example, project `A` can add project `B` to the allowlist. CI/CD jobs
in project `B` (the "allowed project") can now use their CI/CD job token to
-authenticate API calls to access project `A`. If project `A` is public or internal,
-the project can be accessed by project `B` without adding it to the allowlist.
+authenticate API calls to access project `A`.
By default, the allowlist of any project only includes itself.
@@ -168,9 +165,7 @@ limited only by the user's access permissions.
For example, when the setting is enabled, jobs in a pipeline in project `A` have
a `CI_JOB_TOKEN` scope limited to project `A`. If the job needs to use the token
-to make an API request to a private project `B`, then `B` must be added to the allowlist for `A`.
-If project `B` is public or internal, you do not need to add
-`B` to the allowlist to grant access.
+to make an API request to project `B`, then `B` must be added to the allowlist for `A`.
### Configure the job token scope
diff --git a/doc/update/index.md b/doc/update/index.md
index 0380f1a69ef..7bcbc32a0e9 100644
--- a/doc/update/index.md
+++ b/doc/update/index.md
@@ -270,6 +270,11 @@ NOTE:
Specific information that follow related to Ruby and Git versions do not apply to [Omnibus installations](https://docs.gitlab.com/omnibus/)
and [Helm Chart deployments](https://docs.gitlab.com/charts/). They come with appropriate Ruby and Git versions and are not using system binaries for Ruby and Git. There is no need to install Ruby or Git when utilizing these two approaches.
+### 16.1.1
+
+- Accessing a public or internal project with a [CI/CD job token](../ci/jobs/ci_job_token.md)
+ now needs explicit authorization in the target project's allowlist.
+
### 16.1.0
- A `MigrateHumanUserType` background migration will be finalized with
@@ -285,6 +290,11 @@ and [Helm Chart deployments](https://docs.gitlab.com/charts/). They come with ap
migration may take multiple days to complete on larger GitLab instances. Make sure the migration
has completed successfully before upgrading to 16.1.0.
+### 16.0.6
+
+- Accessing a public or internal project with a [CI/CD job token](../ci/jobs/ci_job_token.md)
+ now needs explicit authorization in the target project's allowlist.
+
### 16.0.0
- Sidekiq crashes if there are non-ASCII characters in the GitLab.rb file. You can fix this
@@ -295,6 +305,11 @@ and [Helm Chart deployments](https://docs.gitlab.com/charts/). They come with ap
- Docker 20.10.10 or later is required to run the GitLab Docker image. Older versions
[throw errors on startup](../install/docker.md#threaderror-cant-create-thread-operation-not-permitted).
+### 15.11.10
+
+- Accessing a public or internal project with a [CI/CD job token](../ci/jobs/ci_job_token.md)
+ now needs explicit authorization in the target project's allowlist.
+
### 15.11.1
- Many [project importers](../user/project/import/index.md) and [group importers](../user/group/import/index.md) now
diff --git a/lib/api/entities/issue.rb b/lib/api/entities/issue.rb
index 56e942a0383..79adad92d6f 100644
--- a/lib/api/entities/issue.rb
+++ b/lib/api/entities/issue.rb
@@ -55,7 +55,14 @@ module API
end
expose :moved_to_id
- expose :service_desk_reply_to
+ expose :service_desk_reply_to do |issue|
+ issue.present(
+ current_user: options[:current_user],
+ # We need to pass it explicitly to account for the case where `issue`
+ # is a `WorkItem` which doesn't have a presenter yet.
+ presenter_class: IssuePresenter
+ ).service_desk_reply_to
+ end
end
end
end
diff --git a/lib/banzai/filter/references/reference_cache.rb b/lib/banzai/filter/references/reference_cache.rb
index c8370d5f9c1..759c34ab7e6 100644
--- a/lib/banzai/filter/references/reference_cache.rb
+++ b/lib/banzai/filter/references/reference_cache.rb
@@ -29,15 +29,19 @@ module Banzai
@references_per_parent[parent_type] ||= begin
refs = Hash.new { |hash, key| hash[key] = Set.new }
- prepare_doc_for_scan.to_enum(:scan, regex).each do
- parent_path = if parent_type == :project
- full_project_path($~[:namespace], $~[:project])
- else
- full_group_path($~[:group])
- end
-
- ident = filter.identifier($~)
- refs[parent_path] << ident if ident
+ [filter.object_class.link_reference_pattern, filter.object_class.reference_pattern].each do |pattern|
+ next unless pattern
+
+ prepare_doc_for_scan.to_enum(:scan, pattern).each do
+ parent_path = if parent_type == :project
+ full_project_path($~[:namespace], $~[:project])
+ else
+ full_group_path($~[:group])
+ end
+
+ ident = filter.identifier($~)
+ refs[parent_path] << ident if ident
+ end
end
refs
@@ -171,15 +175,6 @@ module Banzai
delegate :project, :group, :parent, :parent_type, to: :filter
- def regex
- strong_memoize(:regex) do
- [
- filter.object_class.link_reference_pattern,
- filter.object_class.reference_pattern
- ].compact.reduce { |a, b| Regexp.union(a, b) }
- end
- end
-
def refs_cache
Gitlab::SafeRequestStore["banzai_#{parent_type}_refs".to_sym] ||= {}
end
diff --git a/spec/features/admin/users/user_spec.rb b/spec/features/admin/users/user_spec.rb
index 18d62cb585f..f8f1fdaabb4 100644
--- a/spec/features/admin/users/user_spec.rb
+++ b/spec/features/admin/users/user_spec.rb
@@ -504,7 +504,9 @@ RSpec.describe 'Admin::Users::User', feature_category: :user_management do
end
context 'when user has an unconfirmed email', :js do
- let(:unconfirmed_user) { create(:user, :unconfirmed) }
+ # Email address contains HTML to ensure email address is displayed in an HTML safe way.
+ let_it_be(:unconfirmed_email) { "#{generate(:email)}<h2>testing<img/src=http://localhost:8000/test.png>" }
+ let_it_be(:unconfirmed_user) { create(:user, :unconfirmed, unconfirmed_email: unconfirmed_email) }
where(:path_helper) do
[
@@ -524,7 +526,9 @@ RSpec.describe 'Admin::Users::User', feature_category: :user_management do
within_modal do
expect(page).to have_content("Confirm user #{unconfirmed_user.name}?")
- expect(page).to have_content('This user has an unconfirmed email address. You may force a confirmation.')
+ expect(page).to have_content(
+ "This user has an unconfirmed email address (#{unconfirmed_email}). You may force a confirmation."
+ )
click_button 'Confirm user'
end
diff --git a/spec/lib/api/entities/issue_spec.rb b/spec/lib/api/entities/issue_spec.rb
new file mode 100644
index 00000000000..15886542571
--- /dev/null
+++ b/spec/lib/api/entities/issue_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::API::Entities::Issue, feature_category: :team_planning do
+ let_it_be(:project) { create(:project) }
+ let(:issue) { build_stubbed(:issue, project: project) }
+ let(:current_user) { build_stubbed(:user) }
+ let(:options) { { current_user: current_user }.merge(option_addons) }
+ let(:option_addons) { {} }
+ let(:entity) { described_class.new(issue, options) }
+
+ subject(:json) { entity.as_json }
+
+ describe '#service_desk_reply_to', feature_category: :service_desk do
+ # Setting to true (default) doesn't play nice with stubs
+ let(:option_addons) { { include_subscribed: false } }
+ let(:issue) { build_stubbed(:issue, project: project, service_desk_reply_to: email) }
+ let(:email) { 'creator@example.com' }
+ let(:role) { :developer }
+
+ subject { json[:service_desk_reply_to] }
+
+ context 'as developer' do
+ before do
+ stub_member_access_level(issue.project, developer: current_user)
+ end
+
+ it { is_expected.to eq(email) }
+ end
+
+ context 'as guest' do
+ before do
+ stub_member_access_level(issue.project, guest: current_user)
+ end
+
+ it { is_expected.to eq('cr*****@e*****.c**') }
+ end
+
+ context 'without email' do
+ let(:email) { nil }
+
+ specify { expect(json).to have_key(:service_desk_reply_to) }
+ it { is_expected.to eq(nil) }
+ end
+ end
+end
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index ee8d811971a..200e2025517 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -2552,42 +2552,24 @@ RSpec.describe ProjectPolicy, feature_category: :system_access do
describe 'when user is authenticated via CI_JOB_TOKEN', :request_store do
using RSpec::Parameterized::TableSyntax
- where(:project_visibility, :user_role, :external_user, :scope_project_type, :token_scope_enabled, :result) do
- :private | :reporter | false | :same | true | true
- :private | :reporter | false | :same | false | true
- :private | :reporter | false | :different | true | false
- :private | :reporter | false | :different | false | true
- :private | :guest | false | :same | true | true
- :private | :guest | false | :same | false | true
- :private | :guest | false | :different | true | false
- :private | :guest | false | :different | false | true
-
- :internal | :reporter | false | :same | true | true
- :internal | :reporter | true | :same | true | true
- :internal | :reporter | false | :same | false | true
- :internal | :reporter | false | :different | true | true
- :internal | :reporter | true | :different | true | false
- :internal | :reporter | false | :different | false | true
- :internal | :guest | false | :same | true | true
- :internal | :guest | true | :same | true | true
- :internal | :guest | false | :same | false | true
- :internal | :guest | false | :different | true | true
- :internal | :guest | true | :different | true | false
- :internal | :guest | false | :different | false | true
-
- :public | :reporter | false | :same | true | true
- :public | :reporter | false | :same | false | true
- :public | :reporter | false | :different | true | true
- :public | :reporter | false | :different | false | true
- :public | :guest | false | :same | true | true
- :public | :guest | false | :same | false | true
- :public | :guest | false | :different | true | true
- :public | :guest | false | :different | false | true
+ where(:user_role, :external_user, :scope_project_type, :token_scope_enabled, :result) do
+ :reporter | false | :same | true | true
+ :reporter | true | :same | true | true
+ :reporter | false | :same | false | true
+ :reporter | false | :different | true | false
+ :reporter | true | :different | true | false
+ :reporter | false | :different | false | true
+ :guest | false | :same | true | true
+ :guest | true | :same | true | true
+ :guest | false | :same | false | true
+ :guest | false | :different | true | false
+ :guest | true | :different | true | false
+ :guest | false | :different | false | true
end
with_them do
let(:current_user) { public_send(user_role) }
- let(:project) { public_send("#{project_visibility}_project") }
+ let(:project) { public_project }
let(:job) { build_stubbed(:ci_build, project: scope_project, user: current_user) }
let(:scope_project) do
diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb
index 60d4bddc502..b18d99ca884 100644
--- a/spec/requests/api/npm_project_packages_spec.rb
+++ b/spec/requests/api/npm_project_packages_spec.rb
@@ -111,7 +111,7 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do
context 'with a job token for a different user' do
let_it_be(:other_user) { create(:user) }
- let_it_be_with_reload(:other_job) { create(:ci_build, :running, user: other_user) }
+ let_it_be_with_reload(:other_job) { create(:ci_build, :running, user: other_user, project: project) }
let(:headers) { build_token_auth_header(other_job.token) }
@@ -160,7 +160,7 @@ RSpec.describe API::NpmProjectPackages, feature_category: :package_registry do
context 'with a job token for a different user' do
let_it_be(:other_user) { create(:user) }
- let_it_be_with_reload(:other_job) { create(:ci_build, :running, user: other_user) }
+ let_it_be_with_reload(:other_job) { create(:ci_build, :running, user: other_user, project: project) }
let(:headers) { build_token_auth_header(other_job.token) }
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index b07296a0df2..81d6b5465e3 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -677,8 +677,7 @@ RSpec.describe 'Git LFS API and storage', feature_category: :source_code_managem
context 'tries to push to other project' do
let(:pipeline) { create(:ci_empty_pipeline, project: other_project) }
- # I'm not sure what this tests that is different from the previous test
- it_behaves_like 'LFS http 403 response'
+ it_behaves_like 'LFS http 404 response'
end
end
@@ -1198,8 +1197,7 @@ RSpec.describe 'Git LFS API and storage', feature_category: :source_code_managem
context 'tries to push to other project' do
let(:pipeline) { create(:ci_empty_pipeline, project: other_project) }
- # I'm not sure what this tests that is different from the previous test
- it_behaves_like 'LFS http 403 response'
+ it_behaves_like 'LFS http 404 response'
end
end