Add latest changes from gitlab-org/security/gitlab@14-2-stable-ee

3 files changed, 66 insertions, 4 deletions

diff --git a/spec/controllers/jira_connect/app_descriptor_controller_spec.rb b/spec/controllers/jira_connect/app_descriptor_controller_spec.rb
index 98f4db13a1d..25c11d92b4e 100644
--- a/spec/controllers/jira_connect/app_descriptor_controller_spec.rb
+++ b/spec/controllers/jira_connect/app_descriptor_controller_spec.rb
@@ -54,7 +54,10 @@ RSpec.describe JiraConnect::AppDescriptorController do
         postInstallPage: {
           key: 'gitlab-configuration',
           name: { value: 'GitLab Configuration' },
-          url: '/subscriptions'
+          url: '/subscriptions',
+          conditions: contain_exactly(
+            a_hash_including(condition: 'user_is_admin', invert: false)
+          )
         },
         jiraDevelopmentTool: {
           actions: {
diff --git a/spec/controllers/jira_connect/subscriptions_controller_spec.rb b/spec/controllers/jira_connect/subscriptions_controller_spec.rb
index e32915d55a1..f548c1f399d 100644
--- a/spec/controllers/jira_connect/subscriptions_controller_spec.rb
+++ b/spec/controllers/jira_connect/subscriptions_controller_spec.rb
@@ -102,11 +102,17 @@ RSpec.describe JiraConnect::SubscriptionsController do
     end
 
     context 'with valid JWT' do
-      let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) }
+      let(:claims) { { iss: installation.client_key, sub: 1234 } }
+      let(:jwt) { Atlassian::Jwt.encode(claims, installation.shared_secret) }
+      let(:jira_user) { { 'groups' => { 'items' => [{ 'name' => jira_group_name }] } } }
+      let(:jira_group_name) { 'site-admins' }
 
       context 'signed in to GitLab' do
         before do
           sign_in(user)
+          WebMock
+            .stub_request(:get, "#{installation.base_url}/rest/api/3/user?accountId=1234&expand=groups")
+            .to_return(body: jira_user.to_json, status: 200, headers: { 'Content-Type' => 'application/json' })
         end
 
         context 'dev panel integration is available' do
@@ -120,6 +126,16 @@ RSpec.describe JiraConnect::SubscriptionsController do
             expect(response).to have_gitlab_http_status(:ok)
           end
         end
+
+        context 'when the Jira user is not a site-admin' do
+          let(:jira_group_name) { 'some-other-group' }
+
+          it 'returns forbidden' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
+        end
       end
 
       context 'not signed in to GitLab' do
@@ -134,8 +150,14 @@ RSpec.describe JiraConnect::SubscriptionsController do
 
   describe '#destroy' do
     let(:subscription) { create(:jira_connect_subscription, installation: installation) }
+    let(:jira_user) { { 'groups' => { 'items' => [{ 'name' => jira_group_name }] } } }
+    let(:jira_group_name) { 'site-admins' }
 
     before do
+      WebMock
+        .stub_request(:get, "#{installation.base_url}/rest/api/3/user?accountId=1234&expand=groups")
+        .to_return(body: jira_user.to_json, status: 200, headers: { 'Content-Type' => 'application/json' })
+
       delete :destroy, params: { jwt: jwt, id: subscription.id }
     end
 
@@ -148,12 +170,23 @@ RSpec.describe JiraConnect::SubscriptionsController do
     end
 
     context 'with valid JWT' do
-      let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key }, installation.shared_secret) }
+      let(:claims) { { iss: installation.client_key, sub: 1234 } }
+      let(:jwt) { Atlassian::Jwt.encode(claims, installation.shared_secret) }
 
       it 'deletes the subscription' do
         expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound
         expect(response).to have_gitlab_http_status(:ok)
       end
+
+      context 'when the Jira user is not a site admin' do
+        let(:jira_group_name) { 'some-other-group' }
+
+        it 'does not delete the subscription' do
+          expect(response).to have_gitlab_http_status(:forbidden)
+
+          expect { subscription.reload }.not_to raise_error
+        end
+      end
     end
   end
 end
diff --git a/spec/services/jira_connect_subscriptions/create_service_spec.rb b/spec/services/jira_connect_subscriptions/create_service_spec.rb
index 5f467a07a78..cde4753cde7 100644
--- a/spec/services/jira_connect_subscriptions/create_service_spec.rb
+++ b/spec/services/jira_connect_subscriptions/create_service_spec.rb
@@ -7,8 +7,10 @@ RSpec.describe JiraConnectSubscriptions::CreateService do
   let(:current_user) { create(:user) }
   let(:group) { create(:group) }
   let(:path) { group.full_path }
+  let(:params) { { namespace_path: path, jira_user: jira_user } }
+  let(:jira_user) { double(:JiraUser, site_admin?: true) }
 
-  subject { described_class.new(installation, current_user, namespace_path: path).execute }
+  subject { described_class.new(installation, current_user, params).execute }
 
   before do
     group.add_maintainer(current_user)
@@ -24,6 +26,30 @@ RSpec.describe JiraConnectSubscriptions::CreateService do
     end
   end
 
+  context 'remote user does not have access' do
+    let(:jira_user) { double(site_admin?: false) }
+
+    it 'does not create a subscription' do
+      expect { subject }.not_to change { installation.subscriptions.count }
+    end
+
+    it 'returns error' do
+      expect(subject[:status]).to eq(:error)
+    end
+  end
+
+  context 'remote user cannot be retrieved' do
+    let(:jira_user) { nil }
+
+    it 'does not create a subscription' do
+      expect { subject }.not_to change { installation.subscriptions.count }
+    end
+
+    it 'returns error' do
+      expect(subject[:status]).to eq(:error)
+    end
+  end
+
   context 'when user does have access' do
     it 'creates a subscription' do
       expect { subject }.to change { installation.subscriptions.count }.from(0).to(1)