Commit d5bbb49f authored by Andy Soiron's avatar Andy Soiron

Remove jira_connect_asymmetric_jwt feature flag

The feature flag was added to prevent the following scenario:

1. Atlassian loads the old version of app_descriptor_controller.rb from production.
2. A user installs the app and the installed request hits canary.
3. The JWT header does not contain a kid and the JWT verification fails.

Changelog: changed
parent 914861cd
......@@ -32,7 +32,7 @@ class JiraConnect::AppDescriptorController < JiraConnect::ApplicationController
apiVersion: 1,
apiMigrations: {
'context-qsh': true,
'signed-install': signed_install_active?,
'signed-install': true,
gdpr: true
}
}
......
......@@ -74,8 +74,4 @@ class JiraConnect::ApplicationController < ApplicationController
params[:jwt] || request.headers['Authorization']&.split(' ', 2)&.last
end
end
def signed_install_active?
Feature.enabled?(:jira_connect_asymmetric_jwt, default_enabled: :yaml)
end
end
......@@ -4,14 +4,9 @@ class JiraConnect::EventsController < JiraConnect::ApplicationController
# See https://developer.atlassian.com/cloud/jira/software/app-descriptor/#lifecycle
skip_before_action :verify_atlassian_jwt!
before_action :verify_asymmetric_atlassian_jwt!, if: :signed_install_active?
before_action :verify_atlassian_jwt!, only: :uninstalled, unless: :signed_install_active?
before_action :verify_qsh_claim!, only: :uninstalled, unless: :signed_install_active?
before_action :verify_asymmetric_atlassian_jwt!
def installed
return head :ok if !signed_install_active? && atlassian_jwt_valid?
return head :ok if current_jira_installation
installation = JiraConnectInstallation.new(event_params)
......
---
name: jira_connect_asymmetric_jwt
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71080
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/342808
milestone: '14.4'
type: development
group: group::integrations
default_enabled: true
......@@ -90,17 +90,5 @@ RSpec.describe JiraConnect::AppDescriptorController do
)
)
end
context 'when jira_connect_asymmetric_jwt is disabled' do
before do
stub_feature_flags(jira_connect_asymmetric_jwt: false)
end
specify do
get :show
expect(json_response).to include('apiMigrations' => include('signed-install' => false))
end
end
end
end
......@@ -77,18 +77,6 @@ RSpec.describe JiraConnect::EventsController do
expect(installation.base_url).to eq('https://test.atlassian.net')
end
context 'when jira_connect_asymmetric_jwt is disabled' do
before do
stub_feature_flags(jira_connect_asymmetric_jwt: false)
end
it 'saves the jira installation data without JWT validation' do
expect(Atlassian::JiraConnect::AsymmetricJwt).not_to receive(:new)
expect { subject }.to change { JiraConnectInstallation.count }.by(1)
end
end
context 'when it is a version update and shared_secret is not sent' do
let(:params) do
{
......@@ -110,22 +98,6 @@ RSpec.describe JiraConnect::EventsController do
expect { subject }.not_to change { JiraConnectInstallation.count }
expect(response).to have_gitlab_http_status(:ok)
end
context 'when jira_connect_asymmetric_jwt is disabled' do
before do
stub_feature_flags(jira_connect_asymmetric_jwt: false)
end
it 'decodes the JWT token in authorization header and returns 200 without creating a new installation' do
request.headers["Authorization"] = "Bearer #{Atlassian::Jwt.encode({ iss: client_key }, shared_secret)}"
expect(Atlassian::JiraConnect::AsymmetricJwt).not_to receive(:new)
expect { subject }.not_to change { JiraConnectInstallation.count }
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
end
......@@ -153,23 +125,6 @@ RSpec.describe JiraConnect::EventsController do
it 'does not delete the installation' do
expect { post_uninstalled }.not_to change { JiraConnectInstallation.count }
end
context 'when jira_connect_asymmetric_jwt is disabled' do
before do
stub_feature_flags(jira_connect_asymmetric_jwt: false)
request.headers['Authorization'] = 'JWT invalid token'
end
it 'returns 403' do
post_uninstalled
expect(response).to have_gitlab_http_status(:forbidden)
end
it 'does not delete the installation' do
expect { post_uninstalled }.not_to change { JiraConnectInstallation.count }
end
end
end
context 'when JWT is valid' do
......@@ -197,36 +152,6 @@ RSpec.describe JiraConnect::EventsController do
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
context 'when jira_connect_asymmetric_jwt is disabled' do
before do
stub_feature_flags(jira_connect_asymmetric_jwt: false)
request.headers['Authorization'] = "JWT #{Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret)}"
end
let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/events/uninstalled', 'POST', 'https://gitlab.test') }
it 'calls the DestroyService and returns ok in case of success' do
expect_next_instance_of(JiraConnectInstallations::DestroyService, installation, jira_base_path, jira_event_path) do |destroy_service|
expect(destroy_service).to receive(:execute).and_return(true)
end
post_uninstalled
expect(response).to have_gitlab_http_status(:ok)
end
it 'calls the DestroyService and returns unprocessable_entity in case of failure' do
expect_next_instance_of(JiraConnectInstallations::DestroyService, installation, jira_base_path, jira_event_path) do |destroy_service|
expect(destroy_service).to receive(:execute).and_return(false)
end
post_uninstalled
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
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