Commit 19722d41 authored by Andy Soiron's avatar Andy Soiron Committed by Markus Koller

Handle Jira Connect installation updates

The previous logic ignored installed hooks when the installation already
exists. This was because we assumed that the hook would only indicate
an App version update, but don't contain any data we need to process.

It turns out installed hooks for existing installation are aiming to
update those installations with a new auth token or base_url

This broke some active installations when auth tokens were rotated.

Changelog: fixed
parent b2d6e6d6
......@@ -7,11 +7,13 @@ class JiraConnect::EventsController < JiraConnect::ApplicationController
before_action :verify_asymmetric_atlassian_jwt!
def installed
unless Feature.enabled?(:jira_connect_installation_update, default_enabled: :yaml)
return head :ok if current_jira_installation
end
installation = JiraConnectInstallation.new(event_params)
success = current_jira_installation ? update_installation : create_installation
if installation.save
if success
head :ok
else
head :unprocessable_entity
......@@ -28,8 +30,24 @@ class JiraConnect::EventsController < JiraConnect::ApplicationController
private
def event_params
params.permit(:clientKey, :sharedSecret, :baseUrl).transform_keys(&:underscore)
def create_installation
JiraConnectInstallation.new(create_params).save
end
def update_installation
current_jira_installation.update(update_params)
end
def create_params
transformed_params.permit(:client_key, :shared_secret, :base_url)
end
def update_params
transformed_params.permit(:shared_secret, :base_url)
end
def transformed_params
@transformed_params ||= params.transform_keys(&:underscore)
end
def verify_asymmetric_atlassian_jwt!
......@@ -43,7 +61,7 @@ class JiraConnect::EventsController < JiraConnect::ApplicationController
def jwt_verification_claims
{
aud: jira_connect_base_url(protocol: 'https'),
iss: event_params[:client_key],
iss: transformed_params[:client_key],
qsh: Atlassian::Jwt.create_query_string_hash(request.url, request.method, jira_connect_base_url)
}
end
......
---
name: jira_connect_installation_update
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83038
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/356083
milestone: '14.9'
type: development
group: group::integrations
default_enabled: false
......@@ -43,14 +43,15 @@ RSpec.describe JiraConnect::EventsController do
end
describe '#installed' do
let(:client_key) { '1234' }
let(:shared_secret) { 'secret' }
let_it_be(:client_key) { '1234' }
let_it_be(:shared_secret) { 'secret' }
let_it_be(:base_url) { 'https://test.atlassian.net' }
let(:params) do
{
clientKey: client_key,
sharedSecret: shared_secret,
baseUrl: 'https://test.atlassian.net'
baseUrl: base_url
}
end
......@@ -77,11 +78,11 @@ RSpec.describe JiraConnect::EventsController do
expect(installation.base_url).to eq('https://test.atlassian.net')
end
context 'when it is a version update and shared_secret is not sent' do
context 'when the shared_secret param is missing' do
let(:params) do
{
clientKey: client_key,
baseUrl: 'https://test.atlassian.net'
baseUrl: base_url
}
end
......@@ -90,14 +91,49 @@ RSpec.describe JiraConnect::EventsController do
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
context 'and an installation exists' do
let!(:installation) { create(:jira_connect_installation, client_key: client_key, shared_secret: shared_secret) }
context 'when an installation already exists' do
let_it_be(:installation) { create(:jira_connect_installation, base_url: base_url, client_key: client_key, shared_secret: shared_secret) }
it 'validates the JWT token in authorization header and returns 200 without creating a new installation' do
it 'validates the JWT token in authorization header and returns 200 without creating a new installation', :aggregate_failures do
expect { subject }.not_to change { JiraConnectInstallation.count }
expect(response).to have_gitlab_http_status(:ok)
end
context 'when parameters include a new shared secret and base_url' do
let(:shared_secret) { 'new_secret' }
let(:base_url) { 'https://new_test.atlassian.net' }
it 'updates the installation', :aggregate_failures do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(installation.reload).to have_attributes(
shared_secret: shared_secret,
base_url: base_url
)
end
context 'when the `jira_connect_installation_update` feature flag is disabled' do
before do
stub_feature_flags(jira_connect_installation_update: false)
end
it 'does not update the installation', :aggregate_failures do
expect { subject }.not_to change { installation.reload.attributes }
expect(response).to have_gitlab_http_status(:ok)
end
end
end
context 'when the new base_url is invalid' do
let(:base_url) { 'invalid' }
it 'renders 422', :aggregate_failures do
expect { subject }.not_to change { installation.reload.base_url }
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
end
end
......
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