Merge branch 'security-358-operations-page-visible-to-reporters' into 'master'

[master] Operations settings page visible to reporter users

See merge request gitlab/gitlab-ee!724

5 files changed, 75 insertions, 69 deletions

diff --git a/ee/app/controllers/projects/settings/operations_controller.rb b/ee/app/controllers/projects/settings/operations_controller.rb
index 1935ac11372..ef1246f1645 100644
--- a/ee/app/controllers/projects/settings/operations_controller.rb
+++ b/ee/app/controllers/projects/settings/operations_controller.rb
@@ -4,8 +4,7 @@ module Projects
   module Settings
     class OperationsController < Projects::ApplicationController
       before_action :check_license
-      before_action :authorize_update_environment!, only: [:create, :update]
-      before_action :authorize_read_environment!, only: [:show]
+      before_action :authorize_update_environment!
 
       def show
         @tracing_settings ||= ProjectTracingSetting.for_project(@project)
diff --git a/ee/app/controllers/projects/tracings_controller.rb b/ee/app/controllers/projects/tracings_controller.rb
index f8ad88f712a..215da9f0a73 100644
--- a/ee/app/controllers/projects/tracings_controller.rb
+++ b/ee/app/controllers/projects/tracings_controller.rb
@@ -2,7 +2,7 @@
 
 class Projects::TracingsController < Projects::ApplicationController
   before_action :check_license
-  before_action :authorize_read_environment!, only: [:show]
+  before_action :authorize_update_environment!
 
   def show
   end
diff --git a/ee/changelogs/unreleased/security-358-operations-page-visible-to-reporters.yml b/ee/changelogs/unreleased/security-358-operations-page-visible-to-reporters.yml
new file mode 100644
index 00000000000..dc0a8a7f2e5
--- /dev/null
+++ b/ee/changelogs/unreleased/security-358-operations-page-visible-to-reporters.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent reporter roles from viewing the Jaeger tracing settings page
+merge_request:
+author:
+type: security
diff --git a/ee/spec/controllers/projects/settings/operations_controller_spec.rb b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
index c899b5ce5e4..a27e56d0c98 100644
--- a/ee/spec/controllers/projects/settings/operations_controller_spec.rb
+++ b/ee/spec/controllers/projects/settings/operations_controller_spec.rb
@@ -10,21 +10,27 @@ describe Projects::Settings::OperationsController do
   end
 
   describe 'GET show' do
-    shared_examples 'user without access to project' do |project_visibility|
+    shared_examples 'user without read access' do |project_visibility|
       let(:project) { create(:project, project_visibility) }
 
-      it 'returns 404' do
-        get :show, namespace_id: project.namespace, project_id: project
+      %w[guest reporter developer].each do |role|
+        before do
+          project.public_send("add_#{role}", user)
+        end
 
-        expect(response).to have_gitlab_http_status(:not_found)
+        it 'returns 404' do
+          get :show, namespace_id: project.namespace, project_id: project
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
       end
     end
 
-    shared_examples 'user with access to project' do |project_visibility|
+    shared_examples 'user with read access' do |project_visibility|
       let(:project) { create(:project, project_visibility) }
 
       before do
-        project.add_reporter(user)
+        project.add_maintainer(user)
       end
 
       it 'renders ok' do
@@ -50,16 +56,16 @@ describe Projects::Settings::OperationsController do
         stub_licensed_features(tracing: true)
       end
 
-      context 'when logged in with correct permission' do
-        it_behaves_like 'user with access to project', :public
-        it_behaves_like 'user with access to project', :private
-        it_behaves_like 'user with access to project', :internal
+      context 'with maintainer role' do
+        it_behaves_like 'user with read access', :public
+        it_behaves_like 'user with read access', :private
+        it_behaves_like 'user with read access', :internal
       end
 
-      context 'when logged in without correct permission' do
-        it_behaves_like 'user without access to project', :public
-        it_behaves_like 'user without access to project', :private
-        it_behaves_like 'user without access to project', :internal
+      context 'without maintainer role' do
+        it_behaves_like 'user without read access', :public
+        it_behaves_like 'user without read access', :private
+        it_behaves_like 'user without read access', :internal
       end
 
       context 'when user not logged in' do
@@ -67,7 +73,7 @@ describe Projects::Settings::OperationsController do
           sign_out(user)
         end
 
-        it_behaves_like 'user without access to project', :public
+        it_behaves_like 'user without read access', :public
 
         it_behaves_like 'user needs to login', :private
         it_behaves_like 'user needs to login', :internal
@@ -79,9 +85,9 @@ describe Projects::Settings::OperationsController do
         stub_licensed_features(tracing: false)
       end
 
-      it_behaves_like 'user without access to project', :public
-      it_behaves_like 'user without access to project', :private
-      it_behaves_like 'user without access to project', :internal
+      it_behaves_like 'user without read access', :public
+      it_behaves_like 'user without read access', :private
+      it_behaves_like 'user without read access', :internal
     end
   end
 
@@ -99,10 +105,16 @@ describe Projects::Settings::OperationsController do
     shared_examples 'user without write access' do |project_visibility|
       let(:project) { create(:project, project_visibility) }
 
-      it 'does not update tracing external_url' do
-        update_project(project, external_url: 'https://gitlab.com')
+      %w[guest reporter developer].each do |role|
+        before do
+          project.public_send("add_#{role}", user)
+        end
+
+        it 'does not update tracing external_url' do
+          update_project(project, external_url: 'https://gitlab.com')
 
-        expect(project.tracing_setting).to be_nil
+          expect(project.tracing_setting).to be_nil
+        end
       end
     end
 
@@ -125,13 +137,13 @@ describe Projects::Settings::OperationsController do
         end
       end
 
-      context 'with authorized user' do
+      context 'with maintainer role' do
         it_behaves_like 'user with write access', :public, 'https://gitlab.com', 'https://gitlab.com'
         it_behaves_like 'user with write access', :private, 'https://gitlab.com', 'https://gitlab.com'
         it_behaves_like 'user with write access', :internal, 'https://gitlab.com', 'https://gitlab.com'
       end
 
-      context 'with unauthorized user' do
+      context 'with non maintainer roles' do
         it_behaves_like 'user without write access', :public
         it_behaves_like 'user without write access', :private
         it_behaves_like 'user without write access', :internal
diff --git a/ee/spec/controllers/projects/tracings_controller_spec.rb b/ee/spec/controllers/projects/tracings_controller_spec.rb
index 085ae59b82b..eace5752a54 100644
--- a/ee/spec/controllers/projects/tracings_controller_spec.rb
+++ b/ee/spec/controllers/projects/tracings_controller_spec.rb
@@ -6,36 +6,27 @@ describe Projects::TracingsController do
   set(:user) { create(:user) }
 
   describe 'GET show' do
-    describe 'with valid license' do
+    shared_examples 'user with read access' do |visibility_level|
+      let(:project) { create(:project, visibility_level) }
+
       before do
-        stub_licensed_features(tracing: true)
+        project.add_maintainer(user)
       end
 
-      shared_examples 'authorized user' do |visibility_level|
-        let(:project) { create(:project, visibility_level) }
-
-        before do
-          project.add_reporter(user)
-          sign_in(user)
-        end
-
-        it 'renders OK' do
-          get :show, namespace_id: project.namespace, project_id: project
+      it 'renders OK' do
+        get :show, namespace_id: project.namespace, project_id: project
 
-          expect(response).to have_gitlab_http_status(200)
-          expect(response).to render_template(:show)
-        end
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to render_template(:show)
       end
+    end
 
-      it_behaves_like 'authorized user', :public
-      it_behaves_like 'authorized user', :internal
-      it_behaves_like 'authorized user', :private
-
-      shared_examples 'unauthorized user' do |visibility_level|
-        let(:project) { create(:project, visibility_level) }
+    shared_examples 'user without read access' do |visibility_level|
+      let(:project) { create(:project, visibility_level) }
 
+      %w[guest reporter developer].each do |role|
         before do
-          sign_in(user)
+          project.public_send("add_#{role}", user)
         end
 
         it 'returns 404' do
@@ -44,37 +35,36 @@ describe Projects::TracingsController do
           expect(response).to have_gitlab_http_status(:not_found)
         end
       end
-
-      it_behaves_like 'unauthorized user', :public
-      it_behaves_like 'unauthorized user', :internal
-      it_behaves_like 'unauthorized user', :private
     end
 
-    context 'with invalid license' do
+    describe 'with valid license' do
       before do
-        stub_licensed_features(tracing: false)
+        stub_licensed_features(tracing: true)
         sign_in(user)
       end
 
-      shared_examples 'invalid license' do |visibility_level|
-        let(:project) { create(:project, visibility_level) }
-
-        before do
-          stub_licensed_features(tracing: false)
-          project.add_reporter(user)
-          sign_in(user)
-        end
+      context 'with maintainer role' do
+        it_behaves_like 'user with read access', :public
+        it_behaves_like 'user with read access', :internal
+        it_behaves_like 'user with read access', :private
+      end
 
-        it 'returns 404' do
-          get :show, namespace_id: project.namespace, project_id: project
+      context 'without maintainer role' do
+        it_behaves_like 'user without read access', :public
+        it_behaves_like 'user without read access', :internal
+        it_behaves_like 'user without read access', :private
+      end
+    end
 
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
+    context 'with invalid license' do
+      before do
+        stub_licensed_features(tracing: false)
+        sign_in(user)
       end
 
-      it_behaves_like 'invalid license', :public
-      it_behaves_like 'invalid license', :internal
-      it_behaves_like 'invalid license', :private
+      it_behaves_like 'user without read access', :public
+      it_behaves_like 'user without read access', :internal
+      it_behaves_like 'user without read access', :private
     end
   end
 end