Add latest changes from gitlab-org/security/gitlab@15-9-stable-ee

6 files changed, 116 insertions, 9 deletions

diff --git a/app/models/integrations/prometheus.rb b/app/models/integrations/prometheus.rb
index 142f466018b..2f0995e9ab0 100644
--- a/app/models/integrations/prometheus.rb
+++ b/app/models/integrations/prometheus.rb
@@ -3,6 +3,7 @@
 module Integrations
   class Prometheus < BaseMonitoring
     include PrometheusAdapter
+    include Gitlab::Utils::StrongMemoize
 
     field :manual_configuration,
       type: 'checkbox',
@@ -81,7 +82,7 @@ module Integrations
         allow_local_requests: allow_local_api_url?
       )
 
-      if behind_iap?
+      if behind_iap? && iap_client
         # Adds the Authorization header
         options[:headers] = iap_client.apply({})
       end
@@ -106,6 +107,22 @@ module Integrations
       should_return_client?
     end
 
+    alias_method :google_iap_service_account_json_raw, :google_iap_service_account_json
+    private :google_iap_service_account_json_raw
+
+    MASKED_VALUE = '*' * 8
+
+    def google_iap_service_account_json
+      json = google_iap_service_account_json_raw
+      return json unless json.present?
+
+      Gitlab::Json.parse(json)
+        .then { |hash| hash.transform_values { MASKED_VALUE } }
+        .then { |hash| Gitlab::Json.generate(hash) }
+    rescue Gitlab::Json.parser_error
+      json
+    end
+
     private
 
     delegate :allow_local_requests_from_web_hooks_and_services?, to: :current_settings, private: true
@@ -155,17 +172,21 @@ module Integrations
     end
 
     def clean_google_iap_service_account
-      return unless google_iap_service_account_json
+      json = google_iap_service_account_json_raw
+      return unless json.present?
 
-      google_iap_service_account_json
-        .then { |json| Gitlab::Json.parse(json) }
-        .except('token_credential_uri')
+      Gitlab::Json.parse(json).except('token_credential_uri')
+    rescue Gitlab::Json.parser_error
+      {}
     end
 
     def iap_client
       @iap_client ||= Google::Auth::Credentials
         .new(clean_google_iap_service_account, target_audience: google_iap_audience_client_id)
         .client
+    rescue StandardError
+      nil
     end
+    strong_memoize_attr :iap_client
   end
 end
diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb
index 0a9705181ba..7e9fd9dad54 100644
--- a/app/services/groups/transfer_service.rb
+++ b/app/services/groups/transfer_service.rb
@@ -51,6 +51,7 @@ module Groups
       publish_event(old_root_ancestor_id)
     end
 
+    # Overridden in EE
     def ensure_allowed_transfer
       raise_transfer_error(:group_is_already_root) if group_is_already_root?
       raise_transfer_error(:same_parent_as_current) if same_parent?
@@ -208,6 +209,7 @@ module Groups
       raise TransferError, localized_error_messages[message]
     end
 
+    # Overridden in EE
     def localized_error_messages
       {
         database_not_supported: s_('TransferGroup|Database is not supported.'),
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index ad2bbf90917..fa99a1a2bc8 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -86,11 +86,15 @@ module API
       get ':id/repository/commits', urgency: :low do
         not_found! 'Repository' unless user_project.repository_exists?
 
+        page = params[:page] > 0 ? params[:page] : 1
+        per_page = params[:per_page] > 0 ? params[:per_page] : Kaminari.config.default_per_page
+        limit = [per_page, Kaminari.config.max_per_page].min
+        offset = (page - 1) * limit
+
         path = params[:path]
         before = params[:until]
         after = params[:since]
         ref = params[:ref_name].presence || user_project.default_branch unless params[:all]
-        offset = (params[:page] - 1) * params[:per_page]
         all = params[:all]
         with_stats = params[:with_stats]
         first_parent = params[:first_parent]
@@ -98,7 +102,7 @@ module API
 
         commits = user_project.repository.commits(ref,
                                                   path: path,
-                                                  limit: params[:per_page],
+                                                  limit: limit,
                                                   offset: offset,
                                                   before: before,
                                                   after: after,
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index a811c9b16bb..88a17b7d697 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -45053,6 +45053,9 @@ msgstr ""
 msgid "TransferGroup|Group is already associated to the parent group."
 msgstr ""
 
+msgid "TransferGroup|SAML Provider or SCIM Token is configured for this group."
+msgstr ""
+
 msgid "TransferGroup|The parent group already has a subgroup or a project with the same path."
 msgstr ""
 
diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb
index 3c3850854b3..aa248abd3bb 100644
--- a/spec/models/integrations/prometheus_spec.rb
+++ b/spec/models/integrations/prometheus_spec.rb
@@ -239,6 +239,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching,
 
     context 'behind IAP' do
       let(:manual_configuration) { true }
+      let(:google_iap_service_account_json) { Gitlab::Json.generate(google_iap_service_account) }
 
       let(:google_iap_service_account) do
         {
@@ -259,7 +260,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching,
       end
 
       def stub_iap_request
-        integration.google_iap_service_account_json = Gitlab::Json.generate(google_iap_service_account)
+        integration.google_iap_service_account_json = google_iap_service_account_json
         integration.google_iap_audience_client_id = 'IAP_CLIENT_ID.apps.googleusercontent.com'
 
         stub_request(:post, 'https://oauth2.googleapis.com/token')
@@ -278,6 +279,17 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching,
         expect(integration.prometheus_client.send(:options)[:headers]).to eq(authorization: "Bearer FOO")
       end
 
+      context 'with invalid IAP JSON' do
+        let(:google_iap_service_account_json) { 'invalid json' }
+
+        it 'does not include authorization header' do
+          stub_iap_request
+
+          expect(integration.prometheus_client).not_to be_nil
+          expect(integration.prometheus_client.send(:options)).not_to have_key(:headers)
+        end
+      end
+
       context 'when passed with token_credential_uri', issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/284819' do
         let(:malicious_host) { 'http://example.com' }
 
@@ -477,4 +489,45 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching,
       end
     end
   end
+
+  describe '#google_iap_service_account_json' do
+    subject(:iap_details) { integration.google_iap_service_account_json }
+
+    before do
+      integration.google_iap_service_account_json = value
+    end
+
+    context 'with valid JSON' do
+      let(:masked_value) { described_class::MASKED_VALUE }
+      let(:json) { Gitlab::Json.parse(iap_details) }
+
+      let(:value) do
+        Gitlab::Json.generate({
+          type: 'service_account',
+          private_key: 'SECRET',
+          foo: 'secret',
+          nested: {
+            key: 'value'
+          }
+        })
+      end
+
+      it 'masks all JSON values', issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/384580' do
+        expect(json).to eq(
+          'type' => masked_value,
+          'private_key' => masked_value,
+          'foo' => masked_value,
+          'nested' => masked_value
+        )
+      end
+    end
+
+    context 'with invalid JSON' do
+      where(:value) { [nil, '', ' ', 'invalid json'] }
+
+      with_them do
+        it { is_expected.to eq(value) }
+      end
+    end
+  end
 end
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 3932abd20cc..bcc27a80cf8 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -249,6 +249,18 @@ RSpec.describe API::Commits, feature_category: :source_code_management do
               end
             end
 
+            context 'when per_page is over 100' do
+              let(:per_page) { 101 }
+
+              it 'returns 100 commits (maximum)' do
+                expect(Gitlab::Git::Commit).to receive(:where).with(
+                  hash_including(ref: ref_name, limit: 100, offset: 0)
+                )
+
+                request
+              end
+            end
+
             context 'when pagination params are invalid' do
               let_it_be(:project) { create(:project, :repository) }
 
@@ -279,7 +291,7 @@ RSpec.describe API::Commits, feature_category: :source_code_management do
 
                 where(:page, :per_page, :error_message, :status) do
                   0   | nil  | nil                               | :success
-                  -10 | nil  | nil                               | :internal_server_error
+                  -10 | nil  | nil                               | :success
                   'a' | nil | 'page is invalid'                  | :bad_request
                   nil | 0   | 'per_page has a value not allowed' | :bad_request
                   nil | -1  | nil                                | :success
@@ -297,6 +309,18 @@ RSpec.describe API::Commits, feature_category: :source_code_management do
                     end
                   end
                 end
+
+                context 'when per_page is below 0' do
+                  let(:per_page) { -100 }
+
+                  it 'returns 20 commits (default)' do
+                    expect(Gitlab::Git::Commit).to receive(:where).with(
+                      hash_including(ref: ref_name, limit: 20, offset: 0)
+                    )
+
+                    request
+                  end
+                end
               end
             end
           end