Commit 4e1f2a3c authored by Alessio Caiazza's avatar Alessio Caiazza

Merge branch '217014-grafanametricembedservice-undefined-method-id-for-nil-nilclass' into 'master'

Allow anonymous users to view embedded Grafana metrics in public project

Closes #217014

See merge request gitlab-org/gitlab!37844
parents 8b762734 6ee44ea0
...@@ -33,7 +33,7 @@ module Metrics ...@@ -33,7 +33,7 @@ module Metrics
def from_cache(project_id, user_id, grafana_url) def from_cache(project_id, user_id, grafana_url)
project = Project.find(project_id) project = Project.find(project_id)
user = User.find(user_id) user = User.find(user_id) if user_id.present?
new(project, user, grafana_url: grafana_url) new(project, user, grafana_url: grafana_url)
end end
...@@ -56,7 +56,7 @@ module Metrics ...@@ -56,7 +56,7 @@ module Metrics
end end
def cache_key(*args) def cache_key(*args)
[project.id, current_user.id, grafana_url] [project.id, current_user&.id, grafana_url]
end end
# Required for ReactiveCaching; Usage overridden by # Required for ReactiveCaching; Usage overridden by
......
---
title: Allow anonymous users to view embedded Grafana metrics in public project
merge_request: 37844
author:
type: fixed
...@@ -7,7 +7,7 @@ RSpec.describe Metrics::Dashboard::GrafanaMetricEmbedService do ...@@ -7,7 +7,7 @@ RSpec.describe Metrics::Dashboard::GrafanaMetricEmbedService do
include ReactiveCachingHelpers include ReactiveCachingHelpers
include GrafanaApiHelpers include GrafanaApiHelpers
let_it_be(:project) { build(:project) } let_it_be(:project) { create(:project) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:grafana_integration) { create(:grafana_integration, project: project) } let_it_be(:grafana_integration) { create(:grafana_integration, project: project) }
...@@ -15,7 +15,7 @@ RSpec.describe Metrics::Dashboard::GrafanaMetricEmbedService do ...@@ -15,7 +15,7 @@ RSpec.describe Metrics::Dashboard::GrafanaMetricEmbedService do
valid_grafana_dashboard_link(grafana_integration.grafana_url) valid_grafana_dashboard_link(grafana_integration.grafana_url)
end end
before do before_all do
project.add_maintainer(user) project.add_maintainer(user)
end end
...@@ -58,6 +58,31 @@ RSpec.describe Metrics::Dashboard::GrafanaMetricEmbedService do ...@@ -58,6 +58,31 @@ RSpec.describe Metrics::Dashboard::GrafanaMetricEmbedService do
expect(subject.current_user).to eq(user) expect(subject.current_user).to eq(user)
expect(subject.params[:grafana_url]).to eq(grafana_url) expect(subject.params[:grafana_url]).to eq(grafana_url)
end end
context 'with unknown users' do
let(:params) { [project.id, current_user_id, grafana_url] }
context 'when anonymous' do
where(:current_user_id) do
[nil, '']
end
with_them do
it 'sets current_user as nil' do
expect(subject.current_user).to be_nil
end
end
end
context 'when invalid' do
let(:current_user_id) { non_existing_record_id }
it 'raise record not found error' do
expect { subject }
.to raise_error(ActiveRecord::RecordNotFound, /Couldn't find User/)
end
end
end
end end
describe '#get_dashboard', :use_clean_rails_memory_store_caching do describe '#get_dashboard', :use_clean_rails_memory_store_caching do
...@@ -145,8 +170,18 @@ RSpec.describe Metrics::Dashboard::GrafanaMetricEmbedService do ...@@ -145,8 +170,18 @@ RSpec.describe Metrics::Dashboard::GrafanaMetricEmbedService do
stub_datasource_request(grafana_integration.grafana_url) stub_datasource_request(grafana_integration.grafana_url)
end end
context 'when project is private and user is member' do
it_behaves_like 'valid embedded dashboard service response' it_behaves_like 'valid embedded dashboard service response'
end end
context 'when project is public and user is anonymous' do
let(:project) { create(:project, :public) }
let(:user) { nil }
let(:grafana_integration) { create(:grafana_integration, project: project) }
it_behaves_like 'valid embedded dashboard service response'
end
end
end end
context 'with caching', :use_clean_rails_memory_store_caching do context 'with caching', :use_clean_rails_memory_store_caching do
......
...@@ -78,6 +78,12 @@ RSpec.shared_examples 'raises error for users with insufficient permissions' do ...@@ -78,6 +78,12 @@ RSpec.shared_examples 'raises error for users with insufficient permissions' do
it_behaves_like 'misconfigured dashboard service response', :unauthorized it_behaves_like 'misconfigured dashboard service response', :unauthorized
end end
context 'when the user is anonymous' do
let(:user) { nil }
it_behaves_like 'misconfigured dashboard service response', :unauthorized
end
end end
RSpec.shared_examples 'valid dashboard cloning process' do |dashboard_template, sequence| RSpec.shared_examples 'valid dashboard cloning process' do |dashboard_template, sequence|
......
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