Commit 93d074db authored by Reuben Pereira's avatar Reuben Pereira Committed by Rémy Coutable

Skip user auth check in alerts_controller#notify

Do not check if the user can read project in the notify action of the
alerts_controller because that API is hit by alertmanager, and there
will never be a user signed in. Project access will be checked in
the NotifyService which checks if the alertmanager token in the request
matches the token in the DB for the given project.
parent 80855ba9
...@@ -7,6 +7,10 @@ module Projects ...@@ -7,6 +7,10 @@ module Projects
protect_from_forgery except: [:notify] protect_from_forgery except: [:notify]
skip_before_action :project, only: [:notify]
prepend_before_action :repository, :project_without_auth, only: [:notify]
before_action :authorize_read_prometheus_alerts!, except: [:notify] before_action :authorize_read_prometheus_alerts!, except: [:notify]
before_action :authorize_admin_project!, except: [:notify] before_action :authorize_admin_project!, except: [:notify]
before_action :alert, only: [:update, :show, :destroy] before_action :alert, only: [:update, :show, :destroy]
...@@ -102,6 +106,15 @@ module Projects ...@@ -102,6 +106,15 @@ module Projects
def extract_alert_manager_token(request) def extract_alert_manager_token(request)
Doorkeeper::OAuth::Token.from_bearer_authorization(request) Doorkeeper::OAuth::Token.from_bearer_authorization(request)
end end
def project_without_auth
return @project if @project
namespace = params[:namespace_id]
id = params[:project_id]
@project = Project.find_by_full_path("#{namespace}/#{id}")
end
end end
end end
end end
...@@ -9,7 +9,7 @@ module Projects ...@@ -9,7 +9,7 @@ module Projects
return false unless valid_alert_manager_token?(token) return false unless valid_alert_manager_token?(token)
send_alert_email(project, firings) if firings.any? send_alert_email(project, firings) if firings.any?
persist_events(project, current_user, params) persist_events(project, params)
true true
end end
...@@ -88,8 +88,8 @@ module Projects ...@@ -88,8 +88,8 @@ module Projects
.prometheus_alerts_fired(project, firings) .prometheus_alerts_fired(project, firings)
end end
def persist_events(project, current_user, params) def persist_events(project, params)
CreateEventsService.new(project, current_user, params).execute CreateEventsService.new(project, nil, params).execute
end end
end end
end end
......
---
title: Fix alert notifications for non-public projects
merge_request: 9636
author:
type: fixed
...@@ -88,10 +88,12 @@ describe Projects::Prometheus::AlertsController do ...@@ -88,10 +88,12 @@ describe Projects::Prometheus::AlertsController do
let(:notify_service) { spy } let(:notify_service) { spy }
before do before do
sign_out(user)
expect(Projects::Prometheus::Alerts::NotifyService) expect(Projects::Prometheus::Alerts::NotifyService)
.to receive(:new) .to receive(:new)
.with(project, nil, duck_type(:permitted?))
.and_return(notify_service) .and_return(notify_service)
.with(project, user, duck_type(:permitted?))
end end
it 'renders ok if notification succeeds' do it 'renders ok if notification succeeds' do
......
...@@ -3,10 +3,9 @@ ...@@ -3,10 +3,9 @@
require 'spec_helper' require 'spec_helper'
describe Projects::Prometheus::Alerts::NotifyService do describe Projects::Prometheus::Alerts::NotifyService do
set(:user) { create(:user) }
set(:project) { create(:project) } set(:project) { create(:project) }
let(:service) { described_class.new(project, user, payload) } let(:service) { described_class.new(project, nil, payload) }
let(:token_input) { 'token' } let(:token_input) { 'token' }
let(:subject) { service.execute(token_input) } let(:subject) { service.execute(token_input) }
......
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