Commit 6b88a315 authored by Alex Kalderimis's avatar Alex Kalderimis

Only create jira connect NS subscriptions for admins

This prevents non-admins from creating Jira Connect namespace
subscriptions.

This additional check is controlled by a new ops feature flag
(`jira_connect_require_site_admins`) so that it can be disabled
for self-hosted installations if required.

Changelog: security
parent 575f3788
...@@ -38,12 +38,30 @@ class JiraConnect::ApplicationController < ApplicationController ...@@ -38,12 +38,30 @@ class JiraConnect::ApplicationController < ApplicationController
end end
def installation_from_jwt def installation_from_jwt
return unless auth_token
strong_memoize(:installation_from_jwt) do strong_memoize(:installation_from_jwt) do
next unless claims['iss']
JiraConnectInstallation.find_by_client_key(claims['iss'])
end
end
def claims
strong_memoize(:claims) do
next {} unless auth_token
# Decode without verification to get `client_key` in `iss` # Decode without verification to get `client_key` in `iss`
payload, _ = Atlassian::Jwt.decode(auth_token, nil, false) payload, _ = Atlassian::Jwt.decode(auth_token, nil, false)
JiraConnectInstallation.find_by_client_key(payload['iss']) payload
end
end
def jira_user
strong_memoize(:jira_user) do
next unless installation_from_jwt
next unless claims['sub']
# This only works for Jira Cloud installations.
installation_from_jwt.client.user_info(claims['sub'])
end end
end end
......
...@@ -44,7 +44,9 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController ...@@ -44,7 +44,9 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController
def destroy def destroy
subscription = current_jira_installation.subscriptions.find(params[:id]) subscription = current_jira_installation.subscriptions.find(params[:id])
if subscription.destroy if !jira_user&.site_admin?
render json: { error: 'forbidden' }, status: :forbidden
elsif subscription.destroy
render json: { success: true } render json: { success: true }
else else
render json: { error: subscription.errors.full_messages.join(', ') }, status: :unprocessable_entity render json: { error: subscription.errors.full_messages.join(', ') }, status: :unprocessable_entity
...@@ -54,7 +56,7 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController ...@@ -54,7 +56,7 @@ class JiraConnect::SubscriptionsController < JiraConnect::ApplicationController
private private
def create_service def create_service
JiraConnectSubscriptions::CreateService.new(current_jira_installation, current_user, namespace_path: params['namespace_path']) JiraConnectSubscriptions::CreateService.new(current_jira_installation, current_user, namespace_path: params['namespace_path'], jira_user: jira_user)
end end
def allow_rendering_in_iframe def allow_rendering_in_iframe
......
...@@ -20,4 +20,8 @@ class JiraConnectInstallation < ApplicationRecord ...@@ -20,4 +20,8 @@ class JiraConnectInstallation < ApplicationRecord
id: JiraConnectSubscription.for_project(project) id: JiraConnectSubscription.for_project(project)
}) })
} }
def client
Atlassian::JiraConnect::Client.new(base_url, shared_secret)
end
end end
...@@ -5,8 +5,11 @@ module JiraConnectSubscriptions ...@@ -5,8 +5,11 @@ module JiraConnectSubscriptions
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
MERGE_REQUEST_SYNC_BATCH_SIZE = 20 MERGE_REQUEST_SYNC_BATCH_SIZE = 20
MERGE_REQUEST_SYNC_BATCH_DELAY = 1.minute.freeze MERGE_REQUEST_SYNC_BATCH_DELAY = 1.minute.freeze
NOT_SITE_ADMIN = 'The Jira user is not a site administrator.'
def execute def execute
return error(NOT_SITE_ADMIN, 403) unless can_administer_jira?
unless namespace && can?(current_user, :create_jira_connect_subscription, namespace) unless namespace && can?(current_user, :create_jira_connect_subscription, namespace)
return error('Invalid namespace. Please make sure you have sufficient permissions', 401) return error('Invalid namespace. Please make sure you have sufficient permissions', 401)
end end
...@@ -16,6 +19,10 @@ module JiraConnectSubscriptions ...@@ -16,6 +19,10 @@ module JiraConnectSubscriptions
private private
def can_administer_jira?
@params[:jira_user]&.site_admin?
end
def create_subscription def create_subscription
subscription = JiraConnectSubscription.new(installation: jira_connect_installation, namespace: namespace) subscription = JiraConnectSubscription.new(installation: jira_connect_installation, namespace: namespace)
......
...@@ -6,6 +6,10 @@ info: To determine the technical writer assigned to the Stage/Group associated w ...@@ -6,6 +6,10 @@ info: To determine the technical writer assigned to the Stage/Group associated w
# GitLab.com for Jira Cloud app **(FREE)** # GitLab.com for Jira Cloud app **(FREE)**
NOTE:
Only Jira users with administrator level access are able to install or configure
the GitLab app for Jira Cloud.
## GitLab.com for Jira Cloud app **(FREE SAAS)** ## GitLab.com for Jira Cloud app **(FREE SAAS)**
You can integrate GitLab.com and Jira Cloud using the You can integrate GitLab.com and Jira Cloud using the
...@@ -39,7 +43,8 @@ For a walkthrough of the integration with GitLab.com for Jira Cloud app, watch ...@@ -39,7 +43,8 @@ For a walkthrough of the integration with GitLab.com for Jira Cloud app, watch
![Sign in to GitLab.com in GitLab.com for Jira Cloud app](img/jira_dev_panel_setup_com_3_v13_9.png) ![Sign in to GitLab.com in GitLab.com for Jira Cloud app](img/jira_dev_panel_setup_com_3_v13_9.png)
1. Select **Add namespace** to open the list of available namespaces. 1. Select **Add namespace** to open the list of available namespaces.
1. Identify the namespace you want to link, and select **Link**. 1. Identify the namespace you want to link, and select **Link**. Only Jira site
administrators are permitted to add or remove namespaces for an installation.
![Link namespace in GitLab.com for Jira Cloud app](img/jira_dev_panel_setup_com_4_v13_9.png) ![Link namespace in GitLab.com for Jira Cloud app](img/jira_dev_panel_setup_com_4_v13_9.png)
......
...@@ -30,8 +30,21 @@ module Atlassian ...@@ -30,8 +30,21 @@ module Atlassian
responses.compact responses.compact
end end
def user_info(account_id)
r = get('/rest/api/3/user', { accountId: account_id, expand: 'groups' })
JiraUser.new(r.parsed_response) if r.code == 200
end
private private
def get(path, query_params)
uri = URI.join(@base_uri, path)
uri.query = URI.encode_www_form(query_params)
self.class.get(uri, headers: headers(uri, 'GET'))
end
def store_ff_info(project:, feature_flags:, **opts) def store_ff_info(project:, feature_flags:, **opts)
items = feature_flags.map { |flag| ::Atlassian::JiraConnect::Serializers::FeatureFlagEntity.represent(flag, opts) } items = feature_flags.map { |flag| ::Atlassian::JiraConnect::Serializers::FeatureFlagEntity.represent(flag, opts) }
items.reject! { |item| item.issue_keys.empty? } items.reject! { |item| item.issue_keys.empty? }
...@@ -99,10 +112,11 @@ module Atlassian ...@@ -99,10 +112,11 @@ module Atlassian
self.class.post(uri, headers: headers(uri), body: metadata.merge(payload).to_json) self.class.post(uri, headers: headers(uri), body: metadata.merge(payload).to_json)
end end
def headers(uri) def headers(uri, http_method = 'POST')
{ {
'Authorization' => "JWT #{jwt_token('POST', uri)}", 'Authorization' => "JWT #{jwt_token(http_method, uri)}",
'Content-Type' => 'application/json' 'Content-Type' => 'application/json',
'Accept' => 'application/json'
} }
end end
......
# frozen_string_literal: true
module Atlassian
module JiraConnect
class JiraUser
def initialize(data)
@data = data
end
def site_admin?
groups = @data.dig('groups', 'items')
return false unless groups
groups.any? { |g| g['name'] == 'site-admins' }
end
end
end
end
...@@ -102,11 +102,17 @@ RSpec.describe JiraConnect::SubscriptionsController do ...@@ -102,11 +102,17 @@ RSpec.describe JiraConnect::SubscriptionsController do
end end
context 'with valid JWT' do 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 context 'signed in to GitLab' do
before do before do
sign_in(user) 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 end
context 'dev panel integration is available' do context 'dev panel integration is available' do
...@@ -120,6 +126,16 @@ RSpec.describe JiraConnect::SubscriptionsController do ...@@ -120,6 +126,16 @@ RSpec.describe JiraConnect::SubscriptionsController do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
end end
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 end
context 'not signed in to GitLab' do context 'not signed in to GitLab' do
...@@ -134,8 +150,14 @@ RSpec.describe JiraConnect::SubscriptionsController do ...@@ -134,8 +150,14 @@ RSpec.describe JiraConnect::SubscriptionsController do
describe '#destroy' do describe '#destroy' do
let(:subscription) { create(:jira_connect_subscription, installation: installation) } 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 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 } delete :destroy, params: { jwt: jwt, id: subscription.id }
end end
...@@ -148,12 +170,23 @@ RSpec.describe JiraConnect::SubscriptionsController do ...@@ -148,12 +170,23 @@ RSpec.describe JiraConnect::SubscriptionsController do
end end
context 'with valid JWT' do 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 it 'deletes the subscription' do
expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound expect { subscription.reload }.to raise_error ActiveRecord::RecordNotFound
expect(response).to have_gitlab_http_status(:ok) 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 '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 end
end end
...@@ -7,8 +7,10 @@ RSpec.describe JiraConnectSubscriptions::CreateService do ...@@ -7,8 +7,10 @@ RSpec.describe JiraConnectSubscriptions::CreateService do
let(:current_user) { create(:user) } let(:current_user) { create(:user) }
let(:group) { create(:group) } let(:group) { create(:group) }
let(:path) { group.full_path } 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 before do
group.add_maintainer(current_user) group.add_maintainer(current_user)
...@@ -24,6 +26,30 @@ RSpec.describe JiraConnectSubscriptions::CreateService do ...@@ -24,6 +26,30 @@ RSpec.describe JiraConnectSubscriptions::CreateService do
end end
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 context 'when user does have access' do
it 'creates a subscription' do it 'creates a subscription' do
expect { subject }.to change { installation.subscriptions.count }.from(0).to(1) expect { subject }.to change { installation.subscriptions.count }.from(0).to(1)
......
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