Commit d109b8f5 authored by Jan Provaznik's avatar Jan Provaznik

Merge branch 'use-alert-service-token-in-notify-service' into 'master'

Add proper token check for Notify Service

Closes #14792

See merge request gitlab-org/gitlab!17759
parents 6edea27f 16991649
...@@ -5,14 +5,9 @@ module Projects ...@@ -5,14 +5,9 @@ module Projects
class NotifyService < BaseService class NotifyService < BaseService
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
# Prevents users to use WIP feature on private GitLab instances
# by enabling 'generic_alert_endpoint' feature manually.
# TODO: https://gitlab.com/gitlab-org/gitlab/issues/14792
DEV_TOKEN = :development_token
def execute(token) def execute(token)
return forbidden unless alerts_service_activated?
return unauthorized unless valid_token?(token) return unauthorized unless valid_token?(token)
return forbidden unless create_issue?
process_incident_issues process_incident_issues
...@@ -23,6 +18,8 @@ module Projects ...@@ -23,6 +18,8 @@ module Projects
private private
delegate :alerts_service, to: :project
def generic_alert_endpoint_enabled? def generic_alert_endpoint_enabled?
Feature.enabled?(:generic_alert_endpoint, project) Feature.enabled?(:generic_alert_endpoint, project)
end end
...@@ -31,8 +28,10 @@ module Projects ...@@ -31,8 +28,10 @@ module Projects
project.feature_available?(:incident_management) project.feature_available?(:incident_management)
end end
def create_issue? def alerts_service_activated?
incident_management_available? && generic_alert_endpoint_enabled? incident_management_available? &&
generic_alert_endpoint_enabled? &&
alerts_service.try(:active?)
end end
def process_incident_issues def process_incident_issues
...@@ -45,7 +44,7 @@ module Projects ...@@ -45,7 +44,7 @@ module Projects
end end
def valid_token?(token) def valid_token?(token)
token == DEV_TOKEN token == alerts_service.token
end end
def bad_request def bad_request
......
...@@ -6,4 +6,14 @@ FactoryBot.define do ...@@ -6,4 +6,14 @@ FactoryBot.define do
active true active true
type 'GitlabSlackApplicationService' type 'GitlabSlackApplicationService'
end end
factory :alerts_service do
project
type 'AlertsService'
active true
trait :inactive do
active false
end
end
end end
...@@ -36,7 +36,7 @@ describe Projects::Alerting::NotifyService do ...@@ -36,7 +36,7 @@ describe Projects::Alerting::NotifyService do
end end
describe '#execute' do describe '#execute' do
let(:token) { :development_token } let(:token) { 'invalid-token' }
let(:starts_at) { Time.now.change(usec: 0) } let(:starts_at) { Time.now.change(usec: 0) }
let(:service) { described_class.new(project, nil, payload) } let(:service) { described_class.new(project, nil, payload) }
let(:payload_raw) do let(:payload_raw) do
...@@ -59,26 +59,36 @@ describe Projects::Alerting::NotifyService do ...@@ -59,26 +59,36 @@ describe Projects::Alerting::NotifyService do
stub_feature_flags(generic_alert_endpoint: true) stub_feature_flags(generic_alert_endpoint: true)
end end
context 'with valid token' do context 'with activated Alerts Service' do
context 'with a valid payload' do let!(:alerts_service) { create(:alerts_service, project: project) }
it_behaves_like 'processes incident issues', 1
end context 'with valid token' do
let(:token) { alerts_service.token }
context 'with a valid payload' do
it_behaves_like 'processes incident issues', 1
end
context 'with an invalid payload' do
before do
allow(Gitlab::Alerting::NotificationPayloadParser)
.to receive(:call)
.and_raise(Gitlab::Alerting::NotificationPayloadParser::BadPayloadError)
end
context 'with an invalid payload' do it_behaves_like 'does not process incident issues', http_status: 400
before do
allow(Gitlab::Alerting::NotificationPayloadParser)
.to receive(:call)
.and_raise(Gitlab::Alerting::NotificationPayloadParser::BadPayloadError)
end end
end
it_behaves_like 'does not process incident issues', http_status: 400 context 'with invalid token' do
it_behaves_like 'does not process incident issues', http_status: 401
end end
end end
context 'with invalid token' do context 'with deactivated Alerts Service' do
let(:token) { 'invalid-token' } let!(:alerts_service) { create(:alerts_service, :inactive, project: project) }
it_behaves_like 'does not process incident issues', http_status: 401 it_behaves_like 'does not process incident issues', http_status: 403
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment