diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index fe4fbf36e59d490f26c6b5178d98a5d2d3d35e36..4fd8cb0882a7343249136cff2693cd34ab6891d6 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -999,6 +999,8 @@ - 2 - - security_scans_purge_by_job_id - 1 +- - security_secret_detection_partner_token_verification + - 1 - - security_secret_detection_update_token_status - 1 - - security_sync_linked_pipeline_execution_policy_configs diff --git a/db/docs/p_ci_pipelines_config.yml b/db/docs/p_ci_pipelines_config.yml new file mode 100644 index 0000000000000000000000000000000000000000..2afd7d5ba4f796e77b86ea935dbeaa41710dfcc0 --- /dev/null +++ b/db/docs/p_ci_pipelines_config.yml @@ -0,0 +1,15 @@ +--- +table_name: p_ci_pipelines_config +classes: +- Ci::PipelineConfig +feature_categories: +- continuous_integration +description: Routing table for ci_pipelines_config +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/164455 +milestone: '12.7' +gitlab_schema: gitlab_ci +sharding_key: + project_id: projects +table_size: small +removed_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/194730 +removed_in_milestone: '18.2' diff --git a/db/migrate/20250719093354_add_sdrs_jwt_public_key.rb b/db/migrate/20250719093354_add_sdrs_jwt_public_key.rb new file mode 100644 index 0000000000000000000000000000000000000000..f061806042521940262a6e430c3c16c6b3272811 --- /dev/null +++ b/db/migrate/20250719093354_add_sdrs_jwt_public_key.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddSdrsJwtPublicKey < Gitlab::Database::Migration[2.3] + milestone '18.3' + disable_ddl_transaction! + + def up + with_lock_retries do + add_column :application_settings, :sdrs_jwt_public_key, :jsonb, null: true, if_not_exists: true + end + end + + def down + with_lock_retries do + remove_column :application_settings, :sdrs_jwt_public_key + end + end +end diff --git a/db/schema_migrations/20250719093354 b/db/schema_migrations/20250719093354 new file mode 100644 index 0000000000000000000000000000000000000000..e25283f9c2c8ca91ffe6b12fd87eaaa30f8f3492 --- /dev/null +++ b/db/schema_migrations/20250719093354 @@ -0,0 +1 @@ +bf2386326498c20aa99030d97525510931519533ba657200357280cdbf012256 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index f185f7888c54b876b73b05e2649cde0daba60334..d2608c4c6dba1a8a198c7d8b7176853a35181c61 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9577,6 +9577,7 @@ CREATE TABLE application_settings ( default_profile_preferences jsonb DEFAULT '{}'::jsonb NOT NULL, sdrs_enabled boolean DEFAULT false NOT NULL, sdrs_jwt_signing_key jsonb, + sdrs_jwt_public_key jsonb, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), diff --git a/ee/app/helpers/ee/application_settings_helper.rb b/ee/app/helpers/ee/application_settings_helper.rb index 45491f41042ba65f88bedf62db208ac68713a791..20def1e7dd3964153c5dcb5bd730831205d1c1c9 100644 --- a/ee/app/helpers/ee/application_settings_helper.rb +++ b/ee/app/helpers/ee/application_settings_helper.rb @@ -163,6 +163,7 @@ def self.possible_licensed_attributes secret_detection_service_auth_token secret_detection_service_url virtual_registries_endpoints_api_limit + sdrs_callback_per_jti_api disable_invite_members ] end diff --git a/ee/app/models/ee/application_setting.rb b/ee/app/models/ee/application_setting.rb index a952932bfb7e10d28740935605ae62296f766928..4eba8f2f0550b378bc594e3bd80dbb863d834f66 100644 --- a/ee/app/models/ee/application_setting.rb +++ b/ee/app/models/ee/application_setting.rb @@ -138,10 +138,15 @@ module ApplicationSetting encrypts :sdrs_jwt_signing_key - validates :sdrs_jwt_signing_key, json_schema: { filename: 'application_setting_sdrs_jwt_signing_key' }, + validates :sdrs_jwt_signing_key, json_schema: { filename: 'application_setting_sdrs_jwt_key_configuration' }, allow_nil: true validates :sdrs_jwt_signing_key, length: { maximum: 10_000 } + encrypts :sdrs_jwt_public_key + validates :sdrs_jwt_public_key, json_schema: { filename: 'application_setting_sdrs_jwt_key_configuration' }, + allow_nil: true + validates :sdrs_jwt_public_key, length: { maximum: 10_000 } + validates :mirror_capacity_threshold, :mirror_max_capacity, :elasticsearch_indexed_file_size_limit_kb, diff --git a/ee/app/models/vulnerabilities/finding.rb b/ee/app/models/vulnerabilities/finding.rb index bbe7651e42ef287a0b5d3874ccc568476ee186e2..1a0ff7f9abf9ae41092e5cbded9105280be36aa2 100644 --- a/ee/app/models/vulnerabilities/finding.rb +++ b/ee/app/models/vulnerabilities/finding.rb @@ -168,6 +168,7 @@ class Finding < ::SecApplicationRecord scope :eager_load_comparison_entities, -> { includes(:scanner, :primary_identifier) } scope :by_primary_identifiers, ->(identifier_ids) { where(primary_identifier: identifier_ids) } scope :by_latest_pipeline, ->(pipeline_id) { where(latest_pipeline_id: pipeline_id) } + scope :with_project, -> { includes(:project) } scope :all_preloaded, -> do preload(:scanner, :identifiers, :feedbacks, project: [:namespace, :project_feature]) diff --git a/ee/app/models/vulnerabilities/finding_token_status.rb b/ee/app/models/vulnerabilities/finding_token_status.rb index 9f957d93249f8e85851aebdbab41fcb414ab9f80..a0d2b57bdfb6db294864da6f11a23155ce2cbb80 100644 --- a/ee/app/models/vulnerabilities/finding_token_status.rb +++ b/ee/app/models/vulnerabilities/finding_token_status.rb @@ -8,7 +8,10 @@ class FindingTokenStatus < ::SecApplicationRecord enum :status, { unknown: 0, active: 1, - inactive: 2 + inactive: 2, + pending: 3, + error: 4, + revoked: 5 }, prefix: true belongs_to :finding, diff --git a/ee/app/services/authz/sdrs_authentication_service.rb b/ee/app/services/authz/sdrs_authentication_service.rb index 563648717f0055f521d1a2fa7f512cce23561862..08da26b8dabc0e0d50f965693348652054f136b1 100644 --- a/ee/app/services/authz/sdrs_authentication_service.rb +++ b/ee/app/services/authz/sdrs_authentication_service.rb @@ -2,42 +2,136 @@ module Authz class SdrsAuthenticationService < BaseService + # When GitLab sends JWT to SDRS for verification requests AUDIENCE = 'sdrs' ISSUER = 'gitlab-secret-detection' + + # When SDRS sends JWT to GitLab for callbacks + AUDIENCE_CALLBACK = 'sdrs-callback' + ISSUER_CALLBACK = 'gitlab-secret-detection-callback' + ALGORITHM = 'RS256' VALIDITY_TIME = 1.hour + JTI_EXPIRY_TIME = 2.hours + LEEWAY = 30.seconds + SigningKeyNotConfigured = Class.new(StandardError) + PublicKeyNotConfigured = Class.new(StandardError) - def self.generate_token(user:, project:, finding_id:) - claims = { - iss: ISSUER, - sub: "user:#{user.id}", - aud: AUDIENCE, - exp: VALIDITY_TIME.from_now.to_i, - iat: Time.current.to_i, - jti: SecureRandom.uuid, - gitlab: { - user_id: user.id, - project_id: project.id, - finding_id: finding_id, - service: 'token-verification', - scopes: ['token:verify'] + class << self + def generate_token(user:, project:, finding_id:) + claims = { + iss: ISSUER, + sub: "user:#{user.id}", + aud: AUDIENCE, + exp: VALIDITY_TIME.from_now.to_i, + iat: Time.current.to_i, + nbf: now, + jti: SecureRandom.uuid, + gitlab: { + user_id: user.id, + project_id: project.id, + finding_id: finding_id, + service: 'token-verification', + scopes: ['token:verify'] + } } - } - begin - JWT.encode(claims, signing_key, ALGORITHM) - rescue OpenSSL::PKey::RSAError => e - Gitlab::ErrorTracking.log_and_raise_exception(e) + begin + JWT.encode(claims, signing_key, ALGORITHM) + rescue OpenSSL::PKey::RSAError => e + Gitlab::ErrorTracking.log_and_raise_exception(e) + end end - end - def self.signing_key - key_pem = ::Gitlab::CurrentSettings.sdrs_jwt_signing_key - raise SigningKeyNotConfigured, 'SDRS JWT signing key not configured' if key_pem.blank? + def verify_callback_token(token) + return unless token.present? + + payload = JWT.decode( + token, + sdrs_public_key, + true, + { + algorithm: ALGORITHM, + verify_iss: true, + iss: ISSUER_CALLBACK, + verify_aud: true, + aud: AUDIENCE_CALLBACK, + verify_exp: true, + verify_iat: true, + verify_jti: true, + + # Allow small clock skew + exp_leeway: LEEWAY, + iat_leeway: LEEWAY, + nbf_leeway: LEEWAY + } + ).first + + # Validate required GitLab claims + return unless valid_gitlab_claims?(payload) + + # Prevent replay attacks using JTI + return unless register_jti(payload['jti']) + + payload + rescue JWT::DecodeError, JWT::ExpiredSignature, JWT::InvalidIssuerError, + JWT::InvalidAudError, JWT::InvalidIatError, JWT::InvalidJtiError => e + log_jwt_error(e, token) + nil + rescue StandardError => e + Gitlab::ErrorTracking.track_exception(e, jwt_context: { service: self.class.name }) + nil + end + + private + + def signing_key + key_pem = ::Gitlab::CurrentSettings.sdrs_jwt_signing_key + raise SigningKeyNotConfigured, 'SDRS JWT signing key not configured' if key_pem.blank? + + OpenSSL::PKey::RSA.new(key_pem) + end + + def sdrs_public_key + key_pem = ::Gitlab::CurrentSettings.sdrs_jwt_public_key + raise SigningKeyNotConfigured, 'SDRS public key not configured' unless key_pem.present? + + OpenSSL::PKey::RSA.new(key_pem) + end - OpenSSL::PKey::RSA.new(key_pem) + def valid_gitlab_claims?(payload) + gitlab_claims = payload['gitlab'] + + return false unless gitlab_claims.is_a?(Hash) + return false unless gitlab_claims['finding_id'].is_a?(Integer) + return false if gitlab_claims['finding_id'] <= 0 + + true + end + + def register_jti(jti) + return false if jti.blank? + + # Use Redis to track JTIs for replay prevention + redis_key = "sdrs:callback:jti:#{jti}" + + Gitlab::Redis::SharedState.with do |redis| + # SET NX returns true only if key doesn't exist + # EX sets expiration + redis.set(redis_key, Time.current.to_i, nx: true, ex: JTI_EXPIRY_TIME.to_i) + end + end + + def log_jwt_error(error, token) + Gitlab::AppLogger.warn( + message: 'SDRS JWT verification failed', + error_class: error.class.name, + error_message: error.message, + token_length: token&.length + ) + end end end end diff --git a/ee/app/services/security/secret_detection/partner_token_verification_service.rb b/ee/app/services/security/secret_detection/partner_token_verification_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..df292b1a114fc632879823ca6fc0b24f72b581b8 --- /dev/null +++ b/ee/app/services/security/secret_detection/partner_token_verification_service.rb @@ -0,0 +1,210 @@ +# frozen_string_literal: true + +module Security + module SecretDetection + class PartnerTokenVerificationService < ::BaseService + include Gitlab::Utils::StrongMemoize + + SDRS_REQUEST_TIMEOUT = 30.seconds + SDRS_ENDPOINT = '/api/v1/token/verify' + + attr_reader :finding, :current_user, :project + + def initialize(current_user, finding) + @finding = finding + @current_user = current_user + @project = finding&.project + end + + def execute + return error('Feature is disabled') unless feature_enabled? + return error('SDRS not configured') unless sdrs_configured? + return error('Invalid finding') unless valid_finding? + return error('Unauthorized') unless can_verify_token? + + update_token_status('pending') + + jwt_token = generate_jwt_token + return error('Failed to generate JWT') unless jwt_token + + response = send_verification_request(jwt_token) + return response if response.error? + + handle_sdrs_response(response.payload[:http_response]) + rescue StandardError => e + handle_unexpected_error(e) + end + + private + + def can_verify_token? + can?(current_user, :read_vulnerability, project) + end + + def feature_enabled? + project.present? && project.licensed_feature_available?(:secret_detection) && + Feature.enabled?(:token_verification_flow, project, type: :development) + end + + def sdrs_configured? + ::Gitlab::CurrentSettings.sdrs_enabled && sdrs_url.present? + end + + def valid_finding? + finding.report_type.to_s == 'secret_detection' && token_value.present? + end + + def generate_jwt_token + ::Authz::SdrsAuthenticationService.generate_token( + user: current_user, + project: project, + finding_id: finding.id + ) + rescue ::Authz::SdrsAuthenticationService::SigningKeyNotConfigured => e + log_error("JWT signing key not configured", e) + update_token_status('error', error: 'Configuration error') + nil + rescue StandardError => e + log_error("Failed to generate JWT token", e) + update_token_status('error', error: 'Token generation failed') + nil + end + + def send_verification_request(jwt_token) + http_response = Gitlab::HTTP.post( + request_url, + headers: request_headers(jwt_token), + body: request_body.to_json, + timeout: SDRS_REQUEST_TIMEOUT, + open_timeout: 5.seconds, + read_timeout: SDRS_REQUEST_TIMEOUT + ) + success(http_response: http_response) + rescue StandardError => e + log_error("Unexpected error during SDRS request", e) + update_token_status('error', error: 'Unexpected error during request') + error("Unexpected error during SDRS request: #{e.message}", e.class) + end + + def handle_sdrs_response(response) + return error('No response received') unless response + + if response.code == 202 + log_info("Token verification request accepted by SDRS") + success(finding_id: finding.id, request_id: request_id) + else + log_error("Unexpected SDRS response", nil, response_code: response.code) + update_token_status('error', error: 'Unexpected response', code: response.code) + error('Unexpected SDRS response', :unexpected_sdrs_response) + end + end + + def handle_unexpected_error(exception) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception( + exception, + finding_id: finding.id, + user_id: current_user.id, + project_id: project.id, + token_type: token_type + ) + update_token_status('error', error: exception.message) + error(exception.message, :internal_error) + end + + def update_token_status(status, metadata = {}) + log_info("Token status metadata", metadata) if metadata.present? + + ::Vulnerabilities::FindingTokenStatuses::CreateOrUpdateService + .new(finding: finding, current_user: current_user) + .execute(status: status) + rescue StandardError => e + log_error("Failed to update token status", e, status: status) + end + + def request_url + "#{sdrs_url}#{SDRS_ENDPOINT}" + end + + def request_headers(jwt_token) + { + 'Authorization' => "Bearer #{jwt_token}", + 'Content-Type' => 'application/json', + 'Accept' => 'application/json', + 'X-Request-ID' => request_id + } + end + + def request_body + { + token_type: token_type, + token_value: token_value, + finding_id: finding.id, + callback_url: callback_url + } + end + + def token_type + finding.metadata['token_type'] || finding.primary_identifier&.external_id + end + strong_memoize_attr :token_type + + def token_value + finding.metadata['raw_source_code_extract'] || + finding.metadata['token_value'] + end + strong_memoize_attr :token_value + + def callback_url + # TODO: Change this to Gitlab::Routing.url_helpers.api_v4_internal_token_status_callback_url + # once https://gitlab.com/gitlab-org/gitlab/-/issues/551363 is done + "#{Gitlab.config.gitlab.url}/api/v4/internal/token_status/callback" + end + strong_memoize_attr :callback_url + + def sdrs_url + ::Gitlab::CurrentSettings.sdrs_url + end + strong_memoize_attr :sdrs_url + + def request_id + @request_id ||= SecureRandom.uuid + end + + def log_info(message, context = {}) + Gitlab::AppLogger.info(structured_payload(message, context)) + end + + def log_error(message, exception = nil, context = {}) + payload = structured_payload(message, context) + payload[:exception] = exception.class.name if exception + payload[:exception_message] = exception.message if exception + payload[:exception_backtrace] = Gitlab::BacktraceCleaner.clean_backtrace(exception.backtrace) if exception + + Gitlab::AppLogger.error(payload) + end + + def structured_payload(message, context = {}) + { + message: message, + service_class: self.class.name, + finding_id: finding.id, + project_id: project.id, + project_path: project.full_path, + user_id: current_user.id, + username: current_user.username, + token_type: token_type, + request_id: request_id, + feature_flag_enabled: feature_enabled? + }.merge(context) + end + + def error(message, type = nil) + ServiceResponse.error(message: message, payload: { error_type: type }) + end + + def success(payload) + ServiceResponse.success(payload: payload) + end + end + end +end diff --git a/ee/app/services/security/secret_detection/token_verification_callback_service.rb b/ee/app/services/security/secret_detection/token_verification_callback_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..47de5ceaac333a198328ddc9756b40611a55d15f --- /dev/null +++ b/ee/app/services/security/secret_detection/token_verification_callback_service.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true + +module Security + module SecretDetection + class TokenVerificationCallbackService < ::BaseService + RateLimitExceededError = Class.new(StandardError) + FindingMismatchError = Class.new(StandardError) + InvalidJWTError = Class.new(StandardError) + + attr_reader :jwt_payload, :params, :request_headers + + def initialize(jwt_payload:, params:, request_headers: {}) + @jwt_payload = jwt_payload + @params = params + @request_headers = request_headers + end + + def execute + validate_request! + check_rate_limit! + + finding = find_and_validate_finding + return error('Finding not found', :not_found) unless finding + + update_token_status(finding) + rescue RateLimitExceededError => e + error(e.message, :rate_limited) + rescue FindingMismatchError => e + error(e.message, :forbidden) + rescue InvalidJWTError => e + error(e.message, :unauthorized) + rescue ActiveRecord::RecordInvalid => e + log_callback_attempt(status: 'error', finding_id: params[:finding_id], error: e.message) + error(e.record.errors.full_messages.to_sentence, :bad_request) + rescue StandardError => e + handle_unexpected_error(e) + end + + private + + def validate_request! + raise InvalidJWTError, 'Invalid JWT' unless jwt_payload.present? + + jwt_finding_id = jwt_payload.dig('gitlab', 'finding_id') + return unless params[:finding_id] != jwt_finding_id + + raise FindingMismatchError, 'Finding ID mismatch' + end + + def check_rate_limit! + jti = jwt_payload['jti'] + + if Gitlab::ApplicationRateLimiter.throttled?( + :sdrs_callback_per_jti_api, + scope: [jti], + threshold: 10, + interval: 1.minute + ) + raise RateLimitExceededError, 'Rate limit exceeded' + end + end + + def find_and_validate_finding + finding_id = jwt_payload.dig('gitlab', 'finding_id') + finding = ::Vulnerabilities::Finding.find_by_id(finding_id) + + return unless finding && finding.finding_token_status + + finding + end + + def update_token_status(finding) + token_status = finding.finding_token_status + + token_status.update!( + status: params[:status] + ) + + log_callback_attempt( + status: 'success', + finding_id: finding.id + ) + + success(finding: finding) + end + + def handle_unexpected_error(exception) + log_callback_attempt( + status: 'error', + finding_id: params[:finding_id], + error: exception.message + ) + + Gitlab::ErrorTracking.track_exception( + exception, + finding_id: params[:finding_id], + jwt_payload: jwt_payload + ) + + error('Internal error processing callback', :internal_server_error) + end + + def log_callback_attempt(status:, finding_id:, error: nil) + Gitlab::AppLogger.info( + message: 'SDRS callback received', + status: status, + finding_id: finding_id, + jti: jwt_payload&.dig('jti'), + request_id: request_headers['X-Request-ID'], + error: error + ) + end + + def error(message, reason) + ServiceResponse.error(message: message, reason: reason) + end + + def success(payload = {}) + ServiceResponse.success(payload: payload) + end + end + end +end diff --git a/ee/app/services/vulnerabilities/finding_token_statuses/create_or_update_service.rb b/ee/app/services/vulnerabilities/finding_token_statuses/create_or_update_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..db99808cc061985185199872df66b201ec5f0bc3 --- /dev/null +++ b/ee/app/services/vulnerabilities/finding_token_statuses/create_or_update_service.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Vulnerabilities + module FindingTokenStatuses + class CreateOrUpdateService < ::BaseService + attr_reader :finding, :current_user, :project + + def initialize(finding:, current_user:) + @finding = finding + @current_user = current_user + @project = finding.project + end + + def execute(status:) + return ServiceResponse.error(message: 'Invalid parameters') unless valid_status?(status) + + token_status = finding.finding_token_status || + finding.build_finding_token_status + + token_status.assign_attributes( + status: status, + project_id: project.id + ) + + if token_status.save + ServiceResponse.success(payload: { token_status: token_status }) + else + ServiceResponse.error(message: token_status.errors.full_messages.to_sentence) + end + end + + private + + def valid_status?(status) + status.present? && status.in?(%w[pending active revoked error unknown inactive]) + end + end + end +end diff --git a/ee/app/validators/json_schemas/application_setting_sdrs_jwt_key_configuration.json b/ee/app/validators/json_schemas/application_setting_sdrs_jwt_key_configuration.json new file mode 100644 index 0000000000000000000000000000000000000000..66f8bace2dccbd2327105d7e2825b193cb3f9aa6 --- /dev/null +++ b/ee/app/validators/json_schemas/application_setting_sdrs_jwt_key_configuration.json @@ -0,0 +1,6 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "SDRS JWT Key Configuration", + "type": "string", + "description": "PEM-encoded RSA key (private or public) for JWT operations" +} diff --git a/ee/app/validators/json_schemas/application_setting_sdrs_jwt_signing_key.json b/ee/app/validators/json_schemas/application_setting_sdrs_jwt_signing_key.json deleted file mode 100644 index 7ade3054926f6fe2ba8fc4ad6527dfaf5ae3e187..0000000000000000000000000000000000000000 --- a/ee/app/validators/json_schemas/application_setting_sdrs_jwt_signing_key.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-07/schema#", - "title": "SDRS JWT Signing Key Configuration", - "type": "string", - "description": "PEM-encoded RSA private key for JWT signing" -} diff --git a/ee/app/workers/all_queues.yml b/ee/app/workers/all_queues.yml index 7f3373da70e2e94936f81914b569a4c930003b03..3ed5ec68597dd17863bba81dea9b4317d306481d 100644 --- a/ee/app/workers/all_queues.yml +++ b/ee/app/workers/all_queues.yml @@ -3784,6 +3784,16 @@ :idempotent: true :tags: [] :queue_namespace: +- :name: security_secret_detection_partner_token_verification + :worker_name: Security::SecretDetection::PartnerTokenVerificationWorker + :feature_category: :secret_detection + :has_external_dependencies: false + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] + :queue_namespace: - :name: security_secret_detection_update_token_status :worker_name: Security::SecretDetection::UpdateTokenStatusWorker :feature_category: :secret_detection diff --git a/ee/app/workers/security/secret_detection/partner_token_verification_worker.rb b/ee/app/workers/security/secret_detection/partner_token_verification_worker.rb new file mode 100644 index 0000000000000000000000000000000000000000..f0908c15e261675088a3ae73e71bab20f3268d35 --- /dev/null +++ b/ee/app/workers/security/secret_detection/partner_token_verification_worker.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +module Security + module SecretDetection + class PartnerTokenVerificationWorker + include ApplicationWorker + include Gitlab::Utils::StrongMemoize + + # Retryable exceptions - network and temporary errors that make sense to retry + RETRYABLE_EXCEPTIONS = [Gitlab::HTTP::HTTP_TIMEOUT_ERRORS, EOFError, SocketError, OpenSSL::SSL::SSLError, + OpenSSL::OpenSSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Errno::ENETUNREACH, + Errno::ETIMEDOUT, Net::ReadTimeout, Net::OpenTimeout, Timeout::Error].flatten.freeze + + data_consistency :sticky + feature_category :secret_detection + urgency :low + defer_on_database_health_signal :gitlab_main + idempotent! + + # Control retry behavior through sidekiq_retry_in + sidekiq_retry_in do |count, exception| + # Only retry up to 3 times for retryable exceptions + if count < 3 && RETRYABLE_EXCEPTIONS.include?(exception.class) + # Exponential backoff: 4, 16, 64 seconds + 4**count + else + # For non-retryable exceptions or after 3 retries, don't retry + false + end + end + + def perform(finding_id, user_id) + @finding_id = finding_id + @user_id = user_id + + return log_and_return('Finding not found') unless finding + return log_and_return('User not found') unless user + + result = ::Security::SecretDetection::PartnerTokenVerificationService + .new(user, finding) + .execute + + log_result(result) + + # Re-raise retryable exceptions for Sidekiq to handle + handle_service_errors(result) unless result.success? + end + + private + + attr_reader :finding_id, :user_id + + def finding + Vulnerabilities::Finding.with_project.find_by_id(finding_id) + end + strong_memoize_attr :finding + + def user + User.find_by_id(user_id) + end + strong_memoize_attr :user + + def log_result(result) + if result.success? + log_extra_metadata_on_done(:status, 'success') + log_extra_metadata_on_done(:finding_id, result.payload[:finding_id]) + log_extra_metadata_on_done(:request_id, result.payload[:request_id]) if result.payload[:request_id] + else + log_extra_metadata_on_done(:status, 'error') + log_extra_metadata_on_done(:error_message, result.message) + log_extra_metadata_on_done(:error_type, result.payload[:error_type]) if result.payload[:error_type] + end + end + + def handle_service_errors(result) + error_type = result.payload[:error_type] + + # Only retry when send_verification_request fails with retryable network exceptions + raise StandardError, result.message if retryable_exception_class?(error_type) + + # For all other error types (configuration, authorization, validation, etc.) + # don't retry as they won't be resolved by retrying + log_extra_metadata_on_done(:retry_decision, 'not_retryable') + end + + def retryable_exception_class?(exception_class) + return false unless exception_class + + RETRYABLE_EXCEPTIONS.include?(exception_class) + end + + def log_and_return(message) + log_extra_metadata_on_done(:status, 'skipped') + log_extra_metadata_on_done(:reason, message) + + Gitlab::AppLogger.info( + message: message, + worker_class: self.class.name, + finding_id: finding_id, + user_id: user_id + ) + end + end + end +end diff --git a/ee/config/feature_flags/development/token_verification_flow.yml b/ee/config/feature_flags/development/token_verification_flow.yml new file mode 100644 index 0000000000000000000000000000000000000000..59546ecb35b0f8d87628b3858f7432e1d9662b35 --- /dev/null +++ b/ee/config/feature_flags/development/token_verification_flow.yml @@ -0,0 +1,8 @@ +--- +name: token_verification_flow +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/197819 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/551358 +milestone: '18.3' +type: development +group: group::secret detection +default_enabled: false \ No newline at end of file diff --git a/ee/lib/api/internal/app_sec/vulnerabilities/finding/token_status.rb b/ee/lib/api/internal/app_sec/vulnerabilities/finding/token_status.rb new file mode 100644 index 0000000000000000000000000000000000000000..cbf08398655a48fb4aa8f09eac655187675c7938 --- /dev/null +++ b/ee/lib/api/internal/app_sec/vulnerabilities/finding/token_status.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module API + module Internal + module AppSec + module Vulnerabilities + module Finding + class TokenStatus < ::API::Base + before { authenticate_sdrs_callback! } + + helpers do + def authenticate_sdrs_callback! + unauthorized!('Missing authorization header') unless jwt_token.present? + + @jwt_payload = ::Authz::SdrsAuthenticationService.verify_callback_token(jwt_token) + unauthorized!('Invalid JWT') unless @jwt_payload.present? + end + + def jwt_token + @jwt_token ||= request.headers['Authorization']&.split(' ')&.last + end + + def callback_service + ::Security::SecretDetection::TokenVerificationCallbackService.new( + jwt_payload: @jwt_payload, + params: declared_params, + request_headers: { + 'X-Request-Id' => request.headers['X-Request-Id'] + } + ) + end + end + + namespace 'internal/vulnerabilities/finding' do + desc 'Receive token status callback from SDRS' do + detail 'Updates the token verification status for a vulnerability finding' + tags %w[internal] + end + params do + requires :finding_id, type: Integer, desc: 'The vulnerability finding ID' + requires :status, type: String, + values: %w[active unknown inactive], + desc: 'Token verification status' + end + post 'token_status/callback', urgency: :low, feature_category: :secret_detection do + result = callback_service.execute + + if result.success? + status 200 + { success: true } + else + case result.reason + when :not_found + not_found!(result.message) + when :forbidden + forbidden!(result.message) + when :unauthorized + unauthorized!(result.message) + when :bad_request + bad_request!(result.message) + when :rate_limited + too_many_requests!(result.message) + else + internal_server_error!(result.message) + end + end + end + end + end + end + end + end + end +end diff --git a/ee/lib/ee/api/api.rb b/ee/lib/ee/api/api.rb index 3d9993000cf4ba4840813fdbb8408b3c9cecd95f..ad795c7082d0a14d415a06578729fbe0650240ee 100644 --- a/ee/lib/ee/api/api.rb +++ b/ee/lib/ee/api/api.rb @@ -89,6 +89,7 @@ module API mount ::API::VirtualRegistries::Packages::Maven::Endpoints mount ::API::Internal::AppSec::Dast::SiteValidations + mount ::API::Internal::AppSec::Vulnerabilities::Finding::TokenStatus mount ::API::Internal::Search::Zoekt mount ::API::Internal::SuggestedReviewers mount ::API::Internal::Ai::XRay::Scan diff --git a/ee/lib/ee/gitlab/application_rate_limiter.rb b/ee/lib/ee/gitlab/application_rate_limiter.rb index a6d17ae5d56aac82d9c28acf7db42b88e5fe8593..073650f4b6350e03ca16dea21e7ff7faba8331a7 100644 --- a/ee/lib/ee/gitlab/application_rate_limiter.rb +++ b/ee/lib/ee/gitlab/application_rate_limiter.rb @@ -35,6 +35,7 @@ def rate_limits interval: 1.day }, container_scanning_for_registry_scans: { threshold: 50, interval: 1.day }, + sdrs_callback_per_jti_api: { threshold: 5, interval: 1.minute }, virtual_registries_endpoints_api_limit: { threshold: -> { application_settings.virtual_registries_endpoints_api_limit }, interval: 15.seconds } diff --git a/ee/spec/models/vulnerabilities/finding_token_status_spec.rb b/ee/spec/models/vulnerabilities/finding_token_status_spec.rb index 7fd6aaf777a1d25fd65b5727d76284f39e1365ae..514f9f545147a29ec20eed7cffe94e3c70ae694b 100644 --- a/ee/spec/models/vulnerabilities/finding_token_status_spec.rb +++ b/ee/spec/models/vulnerabilities/finding_token_status_spec.rb @@ -69,7 +69,10 @@ expect(described_class.statuses).to eq({ 'unknown' => 0, 'active' => 1, - 'inactive' => 2 + 'inactive' => 2, + 'pending' => 3, + 'error' => 4, + 'revoked' => 5 }) end diff --git a/ee/spec/requests/api/internal/app_sec/vulnerabilities/finding/token_status_spec.rb b/ee/spec/requests/api/internal/app_sec/vulnerabilities/finding/token_status_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9776d8734a6a208d05c8832aac2311d4eb3a8041 --- /dev/null +++ b/ee/spec/requests/api/internal/app_sec/vulnerabilities/finding/token_status_spec.rb @@ -0,0 +1,137 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Internal::AppSec::Vulnerabilities::Finding::TokenStatus, feature_category: :secret_detection do + describe 'POST /internal/vulnerabilities/finding/token_status/callback' do + let(:finding) { create(:vulnerabilities_finding) } + let(:valid_params) { { finding_id: finding.id, status: 'active' } } + let(:jwt_payload) { { finding_id: finding.id, iss: 'sdrs' } } + let(:valid_jwt) { 'valid.jwt.token' } + let(:headers) { { 'Authorization' => "Bearer #{valid_jwt}" } } + + before do + allow(::Authz::SdrsAuthenticationService).to receive( + :verify_callback_token).with(valid_jwt).and_return(jwt_payload) + end + + context 'when authenticating' do + it 'returns 401 when authorization header is missing' do + post api( + '/internal/vulnerabilities/finding/token_status/callback'), params: valid_params + + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['message']).to eq('401 Unauthorized - Missing authorization header') + end + + it 'returns 401 when JWT is invalid' do + allow(::Authz::SdrsAuthenticationService).to receive( + :verify_callback_token).with('invalid.token').and_return(nil) + + post api('/internal/vulnerabilities/finding/token_status/callback'), + params: valid_params, + headers: { 'Authorization' => 'Bearer invalid.token' } + + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response['message']).to eq('401 Unauthorized - Invalid JWT') + end + end + + context 'when validating parameters' do + it 'returns 400 when finding_id is missing' do + post api('/internal/vulnerabilities/finding/token_status/callback'), + params: { status: 'active' }, + headers: headers + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 when status is missing' do + post api('/internal/vulnerabilities/finding/token_status/callback'), + params: { finding_id: finding.id }, + headers: headers + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 when status is invalid' do + post api('/internal/vulnerabilities/finding/token_status/callback'), + params: { finding_id: finding.id, status: 'invalid_status' }, + headers: headers + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when callback is successful' do + let(:service_result) { ServiceResponse.success } + + before do + allow_next_instance_of(::Security::SecretDetection::TokenVerificationCallbackService) do |service| + allow(service).to receive(:execute).and_return(service_result) + end + end + + it 'returns 200 with success response' do + post api('/internal/vulnerabilities/finding/token_status/callback'), + params: valid_params, + headers: headers + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq('success' => true) + end + + it 'passes request headers to service' do + expect(::Security::SecretDetection::TokenVerificationCallbackService).to receive(:new).with( + jwt_payload: jwt_payload, + params: valid_params, + request_headers: { 'X-Request-Id' => 'test-request-id' } + ).and_call_original + + post api('/internal/vulnerabilities/finding/token_status/callback'), + params: valid_params, + headers: headers.merge('X-Request-Id' => 'test-request-id') + end + + %w[active unknown inactive].each do |status_value| + it "accepts '#{status_value}' as valid status" do + post api('/internal/vulnerabilities/finding/token_status/callback'), + params: { finding_id: finding.id, status: status_value }, + headers: headers + + expect(response).to have_gitlab_http_status(:ok) + end + end + end + + context 'when service returns error' do + using RSpec::Parameterized::TableSyntax + + where(:reason, :http_status, :message) do + :not_found | 404 | 'Not Found' + :forbidden | 403 | 'Access denied' + :unauthorized | 401 | 'Unauthorized access' + :bad_request | 400 | 'Invalid request' + :rate_limited | 429 | 'Rate limit exceeded' + :unknown_error | 500 | 'Internal error' + end + + with_them do + it 'returns appropriate error response' do + service_result = ServiceResponse.error(message: message, reason: reason) + + allow_next_instance_of(::Security::SecretDetection::TokenVerificationCallbackService) do |service| + allow(service).to receive(:execute).and_return(service_result) + end + + post api('/internal/vulnerabilities/finding/token_status/callback'), + params: valid_params, + headers: headers + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to include(message) + end + end + end + end +end diff --git a/ee/spec/services/authz/sdrs_authentication_service_spec.rb b/ee/spec/services/authz/sdrs_authentication_service_spec.rb index d4d2b04364ccfb94c0019f27bee74962eec1cf81..2a8f71e62340e99956119fc8d7507b21c9e37fcb 100644 --- a/ee/spec/services/authz/sdrs_authentication_service_spec.rb +++ b/ee/spec/services/authz/sdrs_authentication_service_spec.rb @@ -7,6 +7,7 @@ let_it_be(:project) { create(:project) } let(:finding_id) { 123 } let(:rsa_key) { OpenSSL::PKey::RSA.generate(2048) } + let(:sdrs_rsa_key) { OpenSSL::PKey::RSA.generate(2048) } describe '.generate_token' do subject(:token) { described_class.generate_token(user: user, project: project, finding_id: finding_id) } @@ -18,6 +19,7 @@ it 'includes correct claims', :freeze_time do allow(SecureRandom).to receive(:uuid).and_return('test-uuid-12345') + decoded = JWT.decode(token, rsa_key.public_key, true, algorithm: 'RS256') claims = decoded.first @@ -27,6 +29,7 @@ 'sub' => "user:#{user.id}", 'exp' => 1.hour.from_now.to_i, 'iat' => Time.current.to_i, + 'nbf' => Time.current.to_i, 'jti' => 'test-uuid-12345', 'gitlab' => { 'user_id' => user.id, @@ -47,6 +50,13 @@ expect(jti1).not_to eq(jti2) end + + it 'uses RS256 algorithm' do + decoded = JWT.decode(token, rsa_key.public_key, true, algorithm: 'RS256') + header = decoded.last + + expect(header['alg']).to eq('RS256') + end end context 'when signing key is not configured' do @@ -67,9 +77,314 @@ stub_application_setting(sdrs_jwt_signing_key: 'invalid-key') end - it 'raises an OpenSSL error' do + it 'logs and raises an OpenSSL error' do + expect(Gitlab::ErrorTracking).to receive(:log_and_raise_exception) + .with(instance_of(OpenSSL::PKey::RSAError)) + expect { token }.to raise_error(OpenSSL::PKey::RSAError) end end end + + describe '.verify_callback_token' do + let(:jti) { 'unique-jti-12345' } + let(:valid_payload) do + { + iss: 'gitlab-secret-detection-callback', + aud: 'sdrs-callback', + sub: 'sdrs-service', + exp: 1.hour.from_now.to_i, + iat: Time.current.to_i, + nbf: Time.current.to_i, + jti: jti, + gitlab: { + finding_id: finding_id + } + } + end + + subject(:verify) { described_class.verify_callback_token(token) } + + before do + stub_application_setting(sdrs_jwt_public_key: sdrs_rsa_key.public_key.to_pem) + end + + context 'with valid token' do + let(:token) { JWT.encode(valid_payload, sdrs_rsa_key, 'RS256') } + + it 'returns the decoded payload' do + expect(verify).to include( + 'iss' => 'gitlab-secret-detection-callback', + 'aud' => 'sdrs-callback', + 'gitlab' => { 'finding_id' => finding_id } + ) + end + + it 'registers JTI to prevent replay attacks' do + expect(Gitlab::Redis::SharedState).to receive(:with).and_call_original + + verify + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.exists("sdrs:callback:jti:#{jti}")).to be true + expect(redis.ttl("sdrs:callback:jti:#{jti}")).to be > 0 + expect(redis.ttl("sdrs:callback:jti:#{jti}")).to be <= 2.hours.to_i + end + end + + it 'rejects token if JTI was already used' do + # First use succeeds + expect(verify).not_to be_nil + + # Second use with same JTI fails + expect(described_class.verify_callback_token(token)).to be_nil + end + + context 'with clock skew' do + let(:token) do + payload = valid_payload.merge( + iat: 25.seconds.ago.to_i, + nbf: 25.seconds.ago.to_i + ) + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it 'accepts token within leeway' do + expect(verify).not_to be_nil + end + end + end + + context 'with nil token' do + let(:token) { nil } + + it 'returns nil' do + expect(verify).to be_nil + end + end + + context 'with empty token' do + let(:token) { '' } + + it 'returns nil' do + expect(verify).to be_nil + end + end + + context 'when public key is not configured' do + before do + stub_application_setting(sdrs_jwt_public_key: nil) + end + + let(:token) { JWT.encode(valid_payload, sdrs_rsa_key, 'RS256') } + + it 'raises PublicKeyNotConfigured error' do + expect { verify }.to raise_error( + described_class::PublicKeyNotConfigured, + 'SDRS public key not configured' + ) + end + end + + context 'with invalid signature' do + let(:wrong_key) { OpenSSL::PKey::RSA.generate(2048) } + let(:token) { JWT.encode(valid_payload, wrong_key, 'RS256') } + + it 'returns nil and logs error' do + expect(Gitlab::AppLogger).to receive(:warn).with( + hash_including( + message: 'SDRS JWT verification failed', + error_class: 'JWT::VerificationError' + ) + ) + + expect(verify).to be_nil + end + end + + context 'with wrong algorithm' do + let(:token) { JWT.encode(valid_payload, 'secret', 'HS256') } + + it 'returns nil' do + expect(verify).to be_nil + end + end + + context 'with expired token' do + let(:token) do + payload = valid_payload.merge(exp: 1.hour.ago.to_i) + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it 'returns nil and logs error' do + expect(Gitlab::AppLogger).to receive(:warn).with( + hash_including( + message: 'SDRS JWT verification failed', + error_class: 'JWT::ExpiredSignature' + ) + ) + + expect(verify).to be_nil + end + end + + context 'with invalid issuer' do + let(:token) do + payload = valid_payload.merge(iss: 'wrong-issuer') + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it 'returns nil and logs error' do + expect(Gitlab::AppLogger).to receive(:warn).with( + hash_including( + message: 'SDRS JWT verification failed', + error_class: 'JWT::InvalidIssuerError' + ) + ) + + expect(verify).to be_nil + end + end + + context 'with invalid audience' do + let(:token) do + payload = valid_payload.merge(aud: 'wrong-audience') + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it 'returns nil and logs error' do + expect(Gitlab::AppLogger).to receive(:warn).with( + hash_including( + message: 'SDRS JWT verification failed', + error_class: 'JWT::InvalidAudError' + ) + ) + + expect(verify).to be_nil + end + end + + context 'with missing gitlab claims' do + let(:token) do + payload = valid_payload.except(:gitlab) + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it 'returns nil' do + expect(verify).to be_nil + end + end + + context 'with invalid gitlab claims' do + shared_examples 'invalid gitlab claims' do + it 'returns nil' do + expect(verify).to be_nil + end + end + + context 'when gitlab claims is not a hash' do + let(:token) do + payload = valid_payload.merge(gitlab: 'not-a-hash') + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it_behaves_like 'invalid gitlab claims' + end + + context 'when finding_id is missing' do + let(:token) do + payload = valid_payload + payload[:gitlab].delete(:finding_id) + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it_behaves_like 'invalid gitlab claims' + end + + context 'when finding_id is not an integer' do + let(:token) do + payload = valid_payload + payload[:gitlab][:finding_id] = 'not-an-integer' + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it_behaves_like 'invalid gitlab claims' + end + + context 'when finding_id is zero' do + let(:token) do + payload = valid_payload + payload[:gitlab][:finding_id] = 0 + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it_behaves_like 'invalid gitlab claims' + end + + context 'when finding_id is negative' do + let(:token) do + payload = valid_payload + payload[:gitlab][:finding_id] = -1 + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it_behaves_like 'invalid gitlab claims' + end + end + + context 'with missing JTI' do + let(:token) do + payload = valid_payload.except(:jti) + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it 'returns nil' do + expect(verify).to be_nil + end + end + + context 'with empty JTI' do + let(:token) do + payload = valid_payload.merge(jti: '') + JWT.encode(payload, sdrs_rsa_key, 'RS256') + end + + it 'returns nil' do + expect(verify).to be_nil + end + end + + context 'when Redis is unavailable' do + let(:token) { JWT.encode(valid_payload, sdrs_rsa_key, 'RS256') } + + before do + allow(Gitlab::Redis::SharedState).to receive(:with).and_raise(Redis::CannotConnectError) + end + + it 'tracks exception and returns nil' do + expect(Gitlab::ErrorTracking).to receive(:track_exception) + .with( + instance_of(Redis::CannotConnectError), + jwt_context: { service: 'Authz::SdrsAuthenticationService' } + ) + + expect(verify).to be_nil + end + end + + context 'with malformed token' do + let(:token) { 'not.a.jwt' } + + it 'returns nil and logs error' do + expect(Gitlab::AppLogger).to receive(:warn).with( + hash_including( + message: 'SDRS JWT verification failed', + error_class: 'JWT::DecodeError' + ) + ) + + expect(verify).to be_nil + end + end + end end diff --git a/ee/spec/services/security/secret_detection/partner_token_verification_service_spec.rb b/ee/spec/services/security/secret_detection/partner_token_verification_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..948ddd3cb85fa9e5e6d36b85e5242bca6bc08e02 --- /dev/null +++ b/ee/spec/services/security/secret_detection/partner_token_verification_service_spec.rb @@ -0,0 +1,276 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::SecretDetection::PartnerTokenVerificationService, feature_category: :secret_detection do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + + let(:finding) do + build_stubbed(:vulnerabilities_finding, + project: project, + report_type: 'secret_detection', + raw_metadata: Gitlab::Json.dump(finding_metadata)) + end + + let(:finding_metadata) do + { + 'token_type' => 'gitlab_pat', + 'raw_source_code_extract' => 'gtlb_test123abc' + } + end + + let(:service) { described_class.new(user, finding) } + let(:sdrs_url) { 'https://sdrs.example.com' } + let(:jwt_token) { 'valid.jwt.token' } + + before do + stub_licensed_features(secret_detection: true) + stub_application_setting( + sdrs_enabled: true, + sdrs_url: sdrs_url, + sdrs_jwt_signing_key: OpenSSL::PKey::RSA.generate(2048).to_pem + ) + + allow(Ability).to receive(:allowed?).with(user, :read_vulnerability, project).and_return(true) + allow(::Authz::SdrsAuthenticationService) + .to receive(:generate_token).and_return(jwt_token) + end + + describe '#execute' do + subject(:execute_service) { service.execute } + + context 'when feature is disabled' do + before do + stub_feature_flags(token_verification_flow: false) + end + + it 'returns error' do + result = execute_service + expect(result).to be_error + expect(result.message).to eq('Feature is disabled') + end + end + + context 'when SDRS is not configured' do + before do + stub_application_setting(sdrs_enabled: false) + end + + it 'returns error' do + result = execute_service + expect(result).to be_error + expect(result.message).to eq('SDRS not configured') + end + end + + context 'when finding is invalid' do + let(:finding_metadata) { { 'token_type' => 'gitlab_pat' } } + + it 'returns error' do + result = execute_service + expect(result).to be_error + expect(result.message).to eq('Invalid finding') + end + end + + context 'when user unauthorized' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_vulnerability, project).and_return(false) + end + + it 'returns error' do + result = execute_service + expect(result).to be_error + expect(result.message).to eq('Unauthorized') + end + end + + context 'when all validations pass' do + let(:status_service) { instance_double(Vulnerabilities::FindingTokenStatuses::CreateOrUpdateService) } + + before do + allow(Vulnerabilities::FindingTokenStatuses::CreateOrUpdateService) + .to receive(:new).with(finding: finding, current_user: user) + .and_return(status_service) + allow(status_service).to receive(:execute).and_return(ServiceResponse.success) + end + + context 'when JWT generation fails' do + before do + allow(::Authz::SdrsAuthenticationService) + .to receive(:generate_token) + .and_raise(::Authz::SdrsAuthenticationService::SigningKeyNotConfigured) + end + + it 'returns error' do + result = execute_service + expect(result).to be_error + expect(result.message).to eq('Failed to generate JWT') + end + + it 'updates token status to pending then error' do + expect(status_service).to receive(:execute).with(status: 'pending').ordered + expect(status_service).to receive(:execute).with(status: 'error').ordered + + execute_service + end + end + + context 'when sending request to SDRS' do + let(:request_stub) do + stub_request(:post, "#{sdrs_url}/api/v1/token/verify") + .with( + headers: { + 'Authorization' => "Bearer #{jwt_token}", + 'Content-Type' => 'application/json' + }, + body: hash_including( + 'token_type' => 'gitlab_pat', + 'token_value' => 'gtlb_test123abc', + 'finding_id' => finding.id + ) + ) + end + + context 'when request is accepted' do + before do + request_stub.to_return(status: 202, body: '{}') + end + + it 'returns success' do + result = execute_service + expect(result).to be_success + expect(result.payload).to include(finding_id: finding.id) + end + + it 'updates token status to pending' do + expect(status_service).to receive(:execute).with(status: 'pending') + execute_service + end + end + + context 'when request fails' do + before do + request_stub.to_return(status: 400, body: '{}') + end + + it 'returns error' do + result = execute_service + expect(result).to be_error + expect(result.message).to eq('Unexpected SDRS response') + end + + it 'updates token status to pending then error' do + expect(status_service).to receive(:execute).with(status: 'pending').ordered + expect(status_service).to receive(:execute).with(status: 'error').ordered + + execute_service + end + end + + context 'when network error occurs' do + before do + request_stub.to_timeout + end + + it 'handles network error gracefully' do + response = execute_service + + expect(response).to be_error + expect(response.message).to eq('Unexpected error during SDRS request: execution expired') + expect(response.payload[:error_type]).to eq(Net::OpenTimeout) + end + + it 'updates token status to pending then error' do + expect(status_service).to receive(:execute).with(status: 'pending').ordered + expect(status_service).to receive(:execute).with(status: 'error').ordered + + response = execute_service + expect(response.payload[:error_type]).to eq(Net::OpenTimeout) + end + end + + context 'when unexpected error occurs during response handling' do + before do + request_stub.to_return(status: 202, body: { status: 'pending' }.to_json) + + allow(service).to receive(:handle_sdrs_response).and_raise(StandardError.new('Unexpected error')) + end + + it 'tracks the error and returns error response' do + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) + .with( + instance_of(StandardError), + hash_including( + finding_id: finding.id, + user_id: user.id, + project_id: project.id, + token_type: 'gitlab_pat' + ) + ) + + response = execute_service + + expect(response).to be_error + expect(response.message).to eq('Unexpected error') + expect(response.payload[:error_type]).to eq(:internal_error) + end + + it 'updates token status to pending then error' do + expect(status_service).to receive(:execute).with(status: 'pending').ordered + expect(status_service).to receive(:execute).with(status: 'error').ordered + + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) + + execute_service + end + end + end + end + + describe 'token type detection' do + let(:status_service) { instance_double(Vulnerabilities::FindingTokenStatuses::CreateOrUpdateService) } + + before do + allow(Vulnerabilities::FindingTokenStatuses::CreateOrUpdateService) + .to receive(:new).and_return(status_service) + allow(status_service).to receive(:execute).and_return(ServiceResponse.success) + end + + context 'when token_type is in metadata' do + it 'uses token_type from metadata' do + stub_request(:post, "#{sdrs_url}/api/v1/token/verify") + .with(body: hash_including('token_type' => 'gitlab_pat')) + .to_return(status: 202) + + execute_service + + expect(WebMock).to have_requested(:post, "#{sdrs_url}/api/v1/token/verify") + .with(body: hash_including('token_type' => 'gitlab_pat')) + end + end + + context 'when token_type is not in metadata' do + let(:finding_metadata) { { 'raw_source_code_extract' => 'gtlb_test123abc' } } + + before do + allow(finding).to receive(:primary_identifier).and_return( + build_stubbed(:vulnerabilities_identifier, external_id: 'gitlab_pat') + ) + end + + it 'uses token_type from primary identifier' do + stub_request(:post, "#{sdrs_url}/api/v1/token/verify") + .with(body: hash_including('token_type' => 'gitlab_pat')) + .to_return(status: 202) + + execute_service + + expect(WebMock).to have_requested(:post, "#{sdrs_url}/api/v1/token/verify") + .with(body: hash_including('token_type' => 'gitlab_pat')) + end + end + end + end +end diff --git a/ee/spec/services/security/secret_detection/token_verification_callback_service_spec.rb b/ee/spec/services/security/secret_detection/token_verification_callback_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..088bdffa7e24ccfe296257817522314fe2a5eccd --- /dev/null +++ b/ee/spec/services/security/secret_detection/token_verification_callback_service_spec.rb @@ -0,0 +1,213 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::SecretDetection::TokenVerificationCallbackService, feature_category: :secret_detection do + let_it_be(:project) { create(:project) } + let_it_be(:finding) { create(:vulnerabilities_finding, project: project, report_type: 'secret_detection') } + let_it_be(:finding_token_status) { create(:finding_token_status, finding: finding, status: 'unknown') } + + let(:jti) { SecureRandom.uuid } + + let(:jwt_payload) do + { + 'jti' => jti, + 'iat' => Time.current.to_i, + 'exp' => 5.minutes.from_now.to_i, + 'gitlab' => { + 'finding_id' => finding.id + } + } + end + + let(:params) do + { + finding_id: finding.id, + status: 'active' + } + end + + let(:request_headers) { { 'X-Request-ID' => 'test-request-123' } } + + let(:service) do + described_class.new( + jwt_payload: jwt_payload, + params: params, + request_headers: request_headers + ) + end + + describe '#execute' do + subject(:execute_service) { service.execute } + + context 'when request is valid' do + it 'updates the token status' do + expect { execute_service } + .to change { finding_token_status.reload.status } + .from('unknown') + .to('active') + end + + it 'returns success' do + result = execute_service + + expect(result).to be_success + expect(result.payload[:finding]).to eq(finding) + end + + it 'logs successful callback' do + expect(Gitlab::AppLogger).to receive(:info).with( + hash_including( + message: 'SDRS callback received', + status: 'success', + finding_id: finding.id, + jti: jti, + request_id: 'test-request-123' + ) + ) + + execute_service + end + end + + context 'when JWT is invalid' do + let(:jwt_payload) { nil } + + it 'returns error with unauthorized reason' do + result = execute_service + + expect(result).to be_error + expect(result.message).to eq('Invalid JWT') + expect(result.reason).to eq(:unauthorized) + end + end + + context 'when finding_id does not match JWT' do + let(:params) { super().merge(finding_id: finding.id + 1) } + + it 'returns error with forbidden reason' do + result = execute_service + + expect(result).to be_error + expect(result.message).to eq('Finding ID mismatch') + expect(result.reason).to eq(:forbidden) + end + end + + context 'when rate limit is exceeded' do + before do + allow(Gitlab::ApplicationRateLimiter).to receive(:throttled?) + .with(:sdrs_callback_per_jti_api, scope: [jti], threshold: 10, interval: 1.minute) + .and_return(true) + end + + it 'returns error with rate_limited reason' do + result = execute_service + + expect(result).to be_error + expect(result.message).to eq('Rate limit exceeded') + expect(result.reason).to eq(:rate_limited) + end + end + + context 'when finding does not exist' do + let(:jwt_payload) do + super().tap { |p| p['gitlab']['finding_id'] = non_existing_record_id } + end + + let(:params) { super().merge(finding_id: non_existing_record_id) } + + it 'returns error with not_found reason' do + result = execute_service + + expect(result).to be_error + expect(result.message).to eq('Finding not found') + expect(result.reason).to eq(:not_found) + end + end + + context 'when finding has no token status' do + let_it_be(:finding_without_status) do + create(:vulnerabilities_finding, project: project, report_type: 'secret_detection') + end + + let(:jwt_payload) do + super().tap { |p| p['gitlab']['finding_id'] = finding_without_status.id } + end + + let(:params) { super().merge(finding_id: finding_without_status.id) } + + it 'returns error with not_found reason' do + result = execute_service + + expect(result).to be_error + expect(result.message).to eq('Finding not found') + expect(result.reason).to eq(:not_found) + end + end + + context 'when validation fails' do + before do + allow_next_instance_of(Vulnerabilities::FindingTokenStatus) do |instance| + allow(instance).to receive(:update!) + .and_raise(ActiveRecord::RecordInvalid.new(finding_token_status)) + end + + finding_token_status.errors.add(:status, 'is not included in the list') + end + + it 'returns error with bad_request reason' do + result = execute_service + + expect(result).to be_error + expect(result.message).to eq('Status is not included in the list') + expect(result.reason).to eq(:bad_request) + end + end + + context 'when unexpected error occurs' do + let(:error) { StandardError.new('Database error') } + + before do + # Force an error by making the status update fail + allow_next_instance_of(Vulnerabilities::FindingTokenStatus) do |instance| + allow(instance).to receive(:update!) + .and_raise(error) + end + end + + it 'returns error with internal_server_error reason' do + result = execute_service + + expect(result).to be_error + expect(result.message).to eq('Internal error processing callback') + expect(result.reason).to eq(:internal_server_error) + end + + it 'tracks the exception' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( + error, + hash_including( + finding_id: finding.id, + jwt_payload: jwt_payload + ) + ) + + execute_service + end + + it 'logs error' do + expect(Gitlab::AppLogger).to receive(:info).with( + hash_including( + message: 'SDRS callback received', + status: 'error', + finding_id: finding.id, + error: 'Database error' + ) + ) + + execute_service + end + end + end +end diff --git a/ee/spec/services/vulnerabilities/finding_token_statuses/create_or_update_service_spec.rb b/ee/spec/services/vulnerabilities/finding_token_statuses/create_or_update_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..563aff654f99172eb69afb50e4d33780ff945846 --- /dev/null +++ b/ee/spec/services/vulnerabilities/finding_token_statuses/create_or_update_service_spec.rb @@ -0,0 +1,157 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Vulnerabilities::FindingTokenStatuses::CreateOrUpdateService, feature_category: :secret_detection do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + + let(:finding) { create(:vulnerabilities_finding, project: project) } + let(:service) { described_class.new(finding: finding, current_user: user) } + let(:status) { 'active' } + + describe '#execute' do + subject(:execute_service) { service.execute(status: status) } + + shared_examples 'a successful response' do + it 'returns success' do + expect(execute_service).to be_success + end + + it 'returns the token status in payload' do + result = execute_service + + expect(result.payload[:token_status]).to be_a(Vulnerabilities::FindingTokenStatus) + expect(result.payload[:token_status]).to have_attributes( + status: status, + project_id: project.id + ) + end + end + + shared_examples 'an error response' do |error_message| + it 'returns error' do + expect(execute_service).to be_error + end + + it 'returns the expected error message' do + expect(execute_service.message).to eq(error_message) + end + + it 'does not create or update any records' do + expect { execute_service }.not_to change { Vulnerabilities::FindingTokenStatus.count } + end + end + + context 'when creating a new token status' do + context 'with valid parameters' do + %w[pending active revoked error unknown inactive].each do |status_value| + context "with status #{status_value}" do + let(:status) { status_value } + + it_behaves_like 'a successful response' + + it 'creates a new token status record' do + expect { execute_service }.to change { Vulnerabilities::FindingTokenStatus.count }.by(1) + end + + it 'associates the token status with the finding' do + execute_service + + expect(finding.reload.finding_token_status).to be_present + expect(finding.finding_token_status.status).to eq(status) + end + end + end + end + end + + context 'when updating an existing token status' do + let!(:existing_status) do + create(:finding_token_status, + finding: finding, + status: 'pending', + project: project) + end + + context 'with valid status transition' do + let(:status) { 'active' } + + it_behaves_like 'a successful response' + + it 'does not create a new record' do + expect { execute_service }.not_to change { Vulnerabilities::FindingTokenStatus.count } + end + + it 'updates the existing status' do + expect { execute_service }.to change { existing_status.reload.status } + .from('pending') + .to('active') + end + end + + context 'when transitioning through different states' do + [ + %w[pending active], + %w[pending revoked], + %w[pending error], + %w[active revoked], + %w[active inactive], + %w[error pending] + ].each do |from_status, to_status| + context "when #{from_status} to #{to_status}" do + let!(:existing_status) do + create(:finding_token_status, + finding: finding, + status: from_status, + project: project) + end + + let(:status) { to_status } + + it_behaves_like 'a successful response' + + it 'transitions the status correctly' do + expect { execute_service }.to change { existing_status.reload.status } + .from(from_status) + .to(to_status) + end + end + end + end + end + + context 'with invalid parameters' do + context 'when status is blank' do + let(:status) { '' } + + it_behaves_like 'an error response', 'Invalid parameters' + end + + context 'when status is nil' do + let(:status) { nil } + + it_behaves_like 'an error response', 'Invalid parameters' + end + + context 'when status is not in allowed values' do + let(:status) { 'invalid_status' } + + it_behaves_like 'an error response', 'Invalid parameters' + end + end + + context 'when save fails due to validation errors' do + before do + allow_next_instance_of(Vulnerabilities::FindingTokenStatus) do |instance| + allow(instance).to receive_messages( + save: false, + errors: instance_double(ActiveModel::Errors, full_messages: ['Custom validation error']) + ) + end + end + + it_behaves_like 'an error response', 'Custom validation error' + end + end +end diff --git a/ee/spec/workers/security/secret_detection/partner_token_verification_worker_spec.rb b/ee/spec/workers/security/secret_detection/partner_token_verification_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0756e33a8c32b7ee27a90c56be6e871fb7301fc4 --- /dev/null +++ b/ee/spec/workers/security/secret_detection/partner_token_verification_worker_spec.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::SecretDetection::PartnerTokenVerificationWorker, feature_category: :secret_detection do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:finding) { create(:vulnerabilities_finding, project: project) } + + subject(:worker) { described_class.new } + + describe '#perform' do + let(:service) { instance_double(Security::SecretDetection::PartnerTokenVerificationService) } + let(:service_response) { ServiceResponse.success(payload: { finding_id: finding.id, request_id: 'abc-123' }) } + + before do + allow(Security::SecretDetection::PartnerTokenVerificationService) + .to receive(:new).with(user, finding).and_return(service) + allow(service).to receive(:execute).and_return(service_response) + end + + context 'when finding and user exist' do + it 'calls the service with correct parameters' do + worker.perform(finding.id, user.id) + + expect(Security::SecretDetection::PartnerTokenVerificationService) + .to have_received(:new).with(user, finding) + expect(service).to have_received(:execute) + end + + context 'when service returns success' do + it 'logs success metadata' do + expect(worker).to receive(:log_extra_metadata_on_done).with(:status, 'success') + expect(worker).to receive(:log_extra_metadata_on_done).with(:finding_id, finding.id) + expect(worker).to receive(:log_extra_metadata_on_done).with(:request_id, 'abc-123') + + worker.perform(finding.id, user.id) + end + end + + context 'when service returns error' do + let(:service_response) do + ServiceResponse.error( + message: 'Configuration error', + payload: { error_type: :configuration_error } + ) + end + + it 'logs error metadata and does not retry' do + expect(worker).to receive(:log_extra_metadata_on_done).with(:status, 'error') + expect(worker).to receive(:log_extra_metadata_on_done).with(:error_message, 'Configuration error') + expect(worker).to receive(:log_extra_metadata_on_done).with(:error_type, :configuration_error) + expect(worker).to receive(:log_extra_metadata_on_done).with(:retry_decision, 'not_retryable') + + worker.perform(finding.id, user.id) + end + end + + context 'when service returns retryable network error' do + let(:service_response) do + ServiceResponse.error( + message: 'Unexpected error during SDRS request: Connection timeout', + payload: { error_type: Net::OpenTimeout } + ) + end + + it 'raises exception to trigger retry' do + expect { worker.perform(finding.id, user.id) } + .to raise_error(StandardError, 'Unexpected error during SDRS request: Connection timeout') + end + end + + context 'when service returns non-retryable error' do + let(:service_response) do + ServiceResponse.error( + message: 'Unauthorized', + payload: { error_type: :unauthorized } + ) + end + + it 'does not raise exception' do + expect { worker.perform(finding.id, user.id) }.not_to raise_error + end + end + end + + context 'when finding does not exist' do + it 'logs and returns early' do + expect(Gitlab::AppLogger).to receive(:info).with( + message: 'Finding not found', + worker_class: described_class.name, + finding_id: 999, + user_id: user.id + ) + expect(worker).to receive(:log_extra_metadata_on_done).with(:status, 'skipped') + expect(worker).to receive(:log_extra_metadata_on_done).with(:reason, 'Finding not found') + + worker.perform(999, user.id) + + expect(service).not_to have_received(:execute) + end + end + + context 'when user does not exist' do + it 'logs and returns early' do + expect(Gitlab::AppLogger).to receive(:info).with( + message: 'User not found', + worker_class: described_class.name, + finding_id: finding.id, + user_id: 999 + ) + expect(worker).to receive(:log_extra_metadata_on_done).with(:status, 'skipped') + expect(worker).to receive(:log_extra_metadata_on_done).with(:reason, 'User not found') + + worker.perform(finding.id, 999) + + expect(service).not_to have_received(:execute) + end + end + end + + describe '.sidekiq_retry_in' do + it 'has correct retry configuration' do + expect(described_class.sidekiq_options_hash['retry']).to be(true) + end + end + + describe 'worker configuration' do + it 'has correct feature category' do + expect(described_class.get_feature_category).to eq(:secret_detection) + end + + it 'has correct urgency' do + expect(described_class.get_urgency).to eq(:low) + end + end + + describe 'retryable exceptions' do + it 'includes expected network exceptions' do + expect(described_class::RETRYABLE_EXCEPTIONS).to include( + Net::OpenTimeout, + EOFError, + SocketError, + OpenSSL::SSL::SSLError, + Errno::ECONNRESET, + Errno::ECONNREFUSED + ) + end + end +end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 7bf064d48e2b2fa57e363f2923bea6b72ddc4026..7f7fee9db65ed873347f3c74e75f488251d68e43 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -575,6 +575,10 @@ def service_unavailable!(message = nil) render_api_error!(message || '503 Service Unavailable', 503) end + def internal_server_error!(message = nil) + render_api_error!(message || '500 Internal Server Error', 500) + end + def conflict!(message = nil) render_api_error!(message || '409 Conflict', 409) end