diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-03-01 18:34:52 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-03-01 18:35:10 +0000 |
commit | f5d158fe8b0ff20b6bdf64a22e4ee8751116d83a (patch) | |
tree | 473ac8dc2d09841eb54290a09cdeb4280cf075be | |
parent | d8de601662db1f93fb66b0447ef4de2ac6831ee1 (diff) | |
download | gitlab-ce-f5d158fe8b0ff20b6bdf64a22e4ee8751116d83a.tar.gz |
Add latest changes from gitlab-org/security/gitlab@15-7-stable-ee
-rw-r--r-- | app/models/integration.rb | 2 | ||||
-rw-r--r-- | app/services/resource_access_tokens/create_service.rb | 2 | ||||
-rw-r--r-- | doc/development/integrations/index.md | 9 | ||||
-rw-r--r-- | spec/models/integration_spec.rb | 9 | ||||
-rw-r--r-- | spec/services/resource_access_tokens/create_service_spec.rb | 51 |
5 files changed, 44 insertions, 29 deletions
diff --git a/app/models/integration.rb b/app/models/integration.rb index a630a6dee11..8fe04cadc7a 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -508,7 +508,7 @@ class Integration < ApplicationRecord end def api_field_names - fields.reject { _1[:type] == 'password' }.pluck(:name) + fields.reject { _1[:type] == 'password' || _1[:name] == 'webhook' }.pluck(:name) end def form_fields diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index c6948536053..f6fe23b4555 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -125,7 +125,7 @@ module ResourceAccessTokens def do_not_allow_owner_access_level_for_project_bot?(access_level) resource.is_a?(Project) && - access_level == Gitlab::Access::OWNER && + access_level.to_i == Gitlab::Access::OWNER && !current_user.can?(:manage_owners, resource) end end diff --git a/doc/development/integrations/index.md b/doc/development/integrations/index.md index 1c9144a1163..9b5be1eb99c 100644 --- a/doc/development/integrations/index.md +++ b/doc/development/integrations/index.md @@ -249,6 +249,15 @@ To expose the integration in the [REST API](../../api/integrations.md): You can also refer to our [REST API style guide](../api_styleguide.md). +Sensitive fields are not exposed over the API. Sensitive fields are those fields that contain any of the following in their name: + +- `key` +- `passphrase` +- `password` +- `secret` +- `token` +- `webhook` + #### GraphQL API Integrations use the `Types::Projects::ServiceType` type by default, diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 78b30221a24..9353f58df2f 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Integration do +RSpec.describe Integration, feature_category: :integrations do using RSpec::Parameterized::TableSyntax let_it_be(:group) { create(:group) } @@ -852,6 +852,7 @@ RSpec.describe Integration do { name: 'api_key', type: 'password' }, { name: 'password', type: 'password' }, { name: 'password_field', type: 'password' }, + { name: 'webhook' }, { name: 'some_safe_field' }, { name: 'safe_field' }, { name: 'url' }, @@ -879,6 +880,7 @@ RSpec.describe Integration do field :api_key, type: 'password' field :password, type: 'password' field :password_field, type: 'password' + field :webhook field :some_safe_field field :safe_field field :url @@ -1088,6 +1090,8 @@ RSpec.describe Integration do field :bar, type: 'password' field :password + field :webhook + field :with_help, help: -> { 'help' } field :select, type: 'select' field :boolean, type: 'checkbox' @@ -1138,7 +1142,7 @@ RSpec.describe Integration do it 'registers fields in the fields list' do expect(integration.fields.pluck(:name)).to match_array %w[ - foo foo_p foo_dt bar password with_help select boolean + foo foo_p foo_dt bar password with_help select boolean webhook ] expect(integration.api_field_names).to match_array %w[ @@ -1153,6 +1157,7 @@ RSpec.describe Integration do have_attributes(name: 'foo_dt', type: 'text'), have_attributes(name: 'bar', type: 'password'), have_attributes(name: 'password', type: 'password'), + have_attributes(name: 'webhook', type: 'text'), have_attributes(name: 'with_help', help: 'help'), have_attributes(name: 'select', type: 'select'), have_attributes(name: 'boolean', type: 'checkbox') diff --git a/spec/services/resource_access_tokens/create_service_spec.rb b/spec/services/resource_access_tokens/create_service_spec.rb index 442232920f9..a8c8d41ca09 100644 --- a/spec/services/resource_access_tokens/create_service_spec.rb +++ b/spec/services/resource_access_tokens/create_service_spec.rb @@ -27,6 +27,13 @@ RSpec.describe ResourceAccessTokens::CreateService do end end + shared_examples 'correct error message' do + it 'returns correct error message' do + expect(subject.error?).to be true + expect(subject.errors).to include(error_message) + end + end + shared_examples 'allows creation of bot with valid params' do it { expect { subject }.to change { User.count }.by(1) } @@ -200,16 +207,11 @@ RSpec.describe ResourceAccessTokens::CreateService do end context 'when invalid scope is passed' do + let(:error_message) { 'Scopes can only contain available scopes' } let_it_be(:params) { { scopes: [:invalid_scope] } } it_behaves_like 'token creation fails' - - it 'returns the scope error message' do - response = subject - - expect(response.error?).to be true - expect(response.errors).to include("Scopes can only contain available scopes") - end + it_behaves_like 'correct error message' end end @@ -217,6 +219,7 @@ RSpec.describe ResourceAccessTokens::CreateService do let_it_be(:bot_user) { create(:user, :project_bot) } let(:unpersisted_member) { build(:project_member, source: resource, user: bot_user) } + let(:error_message) { 'Could not provision maintainer access to project access token' } before do allow_next_instance_of(ResourceAccessTokens::CreateService) do |service| @@ -226,13 +229,7 @@ RSpec.describe ResourceAccessTokens::CreateService do end it_behaves_like 'token creation fails' - - it 'returns the provisioning error message' do - response = subject - - expect(response.error?).to be true - expect(response.errors).to include("Could not provision maintainer access to project access token") - end + it_behaves_like 'correct error message' end end @@ -246,14 +243,10 @@ RSpec.describe ResourceAccessTokens::CreateService do end shared_examples 'when user does not have permission to create a resource bot' do - it_behaves_like 'token creation fails' - - it 'returns the permission error message' do - response = subject + let(:error_message) { "User does not have permission to create #{resource_type} access token" } - expect(response.error?).to be true - expect(response.errors).to include("User does not have permission to create #{resource_type} access token") - end + it_behaves_like 'token creation fails' + it_behaves_like 'correct error message' end context 'when resource is a project' do @@ -273,11 +266,19 @@ RSpec.describe ResourceAccessTokens::CreateService do let_it_be(:params) { { access_level: Gitlab::Access::OWNER } } context 'when the executor is a MAINTAINER' do - it 'does not add the bot user with the specified access level in the resource' do - response = subject + let(:error_message) { 'Could not provision owner access to project access token' } - expect(response.error?).to be true - expect(response.errors).to include('Could not provision owner access to project access token') + context 'with OWNER access_level, in integer format' do + it_behaves_like 'token creation fails' + it_behaves_like 'correct error message' + end + + context 'with OWNER access_level, in string format' do + let(:error_message) { 'Could not provision owner access to project access token' } + let_it_be(:params) { { access_level: Gitlab::Access::OWNER.to_s } } + + it_behaves_like 'token creation fails' + it_behaves_like 'correct error message' end end |