Skip to content
Snippets Groups Projects
Commit 6c2adbb0 authored by Stan Hu's avatar Stan Hu Committed by Ezekiel Kigbo
Browse files

Fix deploy keys breaking protected branch dropdown in FIPS mode

In FIPS mode, if a deploy key were present, clicking on the "Allowed
to push" dropdown would fail with a "Failed to load groups, users and
deploy keys" message. This occurred because the JavaScript attempted
to use the `fingerprint` attribute of the key.  However, on a FIPS
system, the MD5 fingerprint is not available, and the `null` value
breaks the dropdown.

To fix this, we use the `fingerprint_sha256` attribute instead. This
commit also puts the SHA256 fingerprint first in the HTML views to
avoid confusion.

Relates to #364562

Changelog: fixed
parent 59a796ba
No related branches found
No related tags found
1 merge request!91627Fix deploy keys breaking protected branch dropdown in FIPS mode
Showing with 119 additions and 32 deletions
......@@ -34,9 +34,13 @@ export default {
key: 'title',
label: __('Title'),
},
{
key: 'fingerprint_sha256',
label: __('Fingerprint (SHA256)'),
},
{
key: 'fingerprint',
label: __('Fingerprint'),
label: __('Fingerprint (MD5)'),
},
{
key: 'projects',
......@@ -130,10 +134,18 @@ export default {
}
this.items = items.map(
({ id, title, fingerprint, projects_with_write_access, created_at }) => ({
({
id,
title,
fingerprint,
fingerprint_sha256,
projects_with_write_access,
created_at,
}) => ({
id,
title,
fingerprint,
fingerprint_sha256,
projects: projects_with_write_access,
created: created_at,
}),
......@@ -196,8 +208,12 @@ export default {
>
</template>
<template #cell(fingerprint_sha256)="{ item: { fingerprint_sha256 } }">
<span v-if="fingerprint_sha256" class="monospace">{{ fingerprint_sha256 }}</span>
</template>
<template #cell(fingerprint)="{ item: { fingerprint } }">
<code>{{ fingerprint }}</code>
<span v-if="fingerprint" class="monospace">{{ fingerprint }}</span>
</template>
<template #cell(created)="{ item: { created } }">
......
......@@ -115,10 +115,20 @@ export default {
<div role="rowheader" class="table-mobile-header">{{ s__('DeployKeys|Deploy key') }}</div>
<div class="table-mobile-content" data-qa-selector="key_container">
<strong class="title" data-qa-selector="key_title_content"> {{ deployKey.title }} </strong>
<div class="fingerprint" data-qa-selector="key_md5_fingerprint_content">
{{ __('MD5') }}:{{ deployKey.fingerprint }}
</div>
<div class="fingerprint">{{ __('SHA256') }}:{{ deployKey.fingerprint_sha256 }}</div>
<dl>
<dt>{{ __('SHA256') }}</dt>
<dd class="fingerprint" data-qa-selector="key_sha256_fingerprint_content">
{{ deployKey.fingerprint_sha256 }}
</dd>
<template v-if="deployKey.fingerprint">
<dt>
{{ __('MD5') }}
</dt>
<dd class="fingerprint" data-qa-selector="key_md5_fingerprint_content">
{{ deployKey.fingerprint }}
</dd>
</template>
</dl>
</div>
</div>
<div class="table-section section-30 section-wrap">
......
......@@ -441,11 +441,13 @@ export default class AccessDropdown {
const {
id,
fingerprint,
fingerprint_sha256: fingerprintSha256,
title,
owner: { avatar_url, name, username },
} = response;
const shortFingerprint = `(${fingerprint.substring(0, 14)}...)`;
const availableFingerprint = fingerprintSha256 || fingerprint;
const shortFingerprint = `(${availableFingerprint.substring(0, 14)}...)`;
return {
id,
......
......@@ -203,11 +203,13 @@ export default {
const {
id,
fingerprint,
fingerprint_sha256: fingerprintSha256,
title,
owner: { avatar_url, name, username },
} = response;
const shortFingerprint = `(${fingerprint.substring(0, 14)}...)`;
const availableFingerprint = fingerprintSha256 || fingerprint;
const shortFingerprint = `(${availableFingerprint.substring(0, 14)}...)`;
return {
id,
......@@ -387,7 +389,7 @@ export default {
}}</gl-dropdown-section-header>
<gl-dropdown-item
v-for="key in deployKeys"
:key="`${key.id}${key.fingerprint}`"
:key="`${key.id}-{key.title}`"
data-testid="deploy_key-dropdown-item"
is-check-item
:is-checked="isSelected(key)"
......
......@@ -18,9 +18,14 @@
= _('Paste a public key here. %{link_start}How do I generate it?%{link_end}').html_safe % { link_start: link_start, link_end: link_end.html_safe }
= form.text_area :key, class: 'form-control gl-form-input thin_area', rows: 5, data: { qa_selector: 'deploy_key_field' }
- else
= form.label :fingerprint, class: 'col-form-label col-sm-2'
.col-sm-10
= form.text_field :fingerprint, class: 'form-control gl-form-input', readonly: 'readonly'
- if deploy_key.fingerprint_sha256.present?
= form.label :fingerprint, _('Fingerprint (SHA256)'), class: 'col-form-label col-sm-2'
.col-sm-10
= form.text_field :fingerprint_sha256, class: 'form-control gl-form-input', readonly: 'readonly'
- if deploy_key.fingerprint.present?
= form.label :fingerprint, _('Fingerprint (MD5)'), class: 'col-form-label col-sm-2'
.col-sm-10
= form.text_field :fingerprint, class: 'form-control gl-form-input', readonly: 'readonly'
- if deploy_keys_project.present?
= form.fields_for :deploy_keys_projects, deploy_keys_project do |deploy_keys_project_form|
......
......@@ -16371,7 +16371,10 @@ msgstr ""
msgid "Find file"
msgstr ""
 
msgid "Fingerprint"
msgid "Fingerprint (MD5)"
msgstr ""
msgid "Fingerprint (SHA256)"
msgstr ""
 
msgid "Fingerprints"
......@@ -20,6 +20,12 @@
expires_at { Date.today.beginning_of_day + 3.hours }
end
trait :without_md5_fingerprint do
after(:create) do |key|
key.update_column(:fingerprint, nil)
end
end
factory :key_without_comment do
key { SSHData::PrivateKey::RSA.generate(3072, unsafe_allow_small_key: true).public_key.openssh }
end
......
......@@ -3,22 +3,38 @@
require 'spec_helper'
RSpec.describe 'Project deploy keys', :js do
let(:user) { create(:user) }
let(:project) { create(:project_empty_repo) }
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project_empty_repo) }
let_it_be(:deploy_keys_project) { create(:deploy_keys_project, project: project) }
let_it_be(:deploy_key) { deploy_keys_project.deploy_key }
before do
project.add_maintainer(user)
sign_in(user)
end
context 'editing key' do
it 'shows fingerprints' do
visit edit_project_deploy_key_path(project, deploy_key)
expect(page).to have_content('Fingerprint (SHA256)')
expect(find('#deploy_key_fingerprint_sha256').value).to eq(deploy_key.fingerprint_sha256)
if Gitlab::FIPS.enabled?
expect(page).not_to have_content('Fingerprint (MD5)')
else
expect(page).to have_content('Fingerprint (MD5)')
expect(find('#deploy_key_fingerprint').value).to eq(deploy_key.fingerprint)
end
end
end
describe 'removing key' do
before do
create(:deploy_keys_project, project: project)
visit project_settings_repository_path(project)
end
it 'removes association between project and deploy key' do
visit project_settings_repository_path(project)
page.within(find('.rspec-deploy-keys-settings')) do
expect(page).to have_selector('.deploy-key', count: 1)
......
......@@ -27,6 +27,7 @@ describe('DeployKeysTable', () => {
const deployKey = responseBody[0];
const deployKey2 = responseBody[1];
const deployKeyWithoutMd5Fingerprint = responseBody[2];
const createComponent = (provide = {}) => {
wrapper = mountExtended(DeployKeysTable, {
......@@ -57,9 +58,10 @@ describe('DeployKeysTable', () => {
const timeAgoTooltip = findTimeAgoTooltip(expectedRowIndex);
expect(wrapper.findByText(expectedDeployKey.title).exists()).toBe(true);
expect(wrapper.findByText(expectedDeployKey.fingerprint, { selector: 'code' }).exists()).toBe(
true,
);
expect(
wrapper.findByText(expectedDeployKey.fingerprint_sha256, { selector: 'span' }).exists(),
).toBe(true);
expect(timeAgoTooltip.exists()).toBe(true);
expect(timeAgoTooltip.props('time')).toBe(expectedDeployKey.created_at);
expect(editButton.exists()).toBe(true);
......@@ -67,6 +69,13 @@ describe('DeployKeysTable', () => {
expect(findRemoveButton(expectedRowIndex).exists()).toBe(true);
};
const expectDeployKeyWithFingerprintIsRendered = (expectedDeployKey, expectedRowIndex) => {
expect(wrapper.findByText(expectedDeployKey.fingerprint, { selector: 'span' }).exists()).toBe(
true,
);
expectDeployKeyIsRendered(expectedDeployKey, expectedRowIndex);
};
const itRendersTheEmptyState = () => {
it('renders empty state', () => {
const emptyState = wrapper.findComponent(GlEmptyState);
......@@ -127,8 +136,12 @@ describe('DeployKeysTable', () => {
});
it('renders deploy keys in table', () => {
expectDeployKeyIsRendered(deployKey, 0);
expectDeployKeyIsRendered(deployKey2, 1);
expectDeployKeyWithFingerprintIsRendered(deployKey, 0);
expectDeployKeyWithFingerprintIsRendered(deployKey2, 1);
});
it('renders deploy keys that do not have an MD5 fingerprint', () => {
expectDeployKeyIsRendered(deployKeyWithoutMd5Fingerprint, 2);
});
describe('when delete button is clicked', () => {
......@@ -157,7 +170,7 @@ describe('DeployKeysTable', () => {
beforeEach(() => {
Api.deployKeys.mockResolvedValueOnce({
data: [deployKey],
headers: { 'x-total': '2' },
headers: { 'x-total': '3' },
});
createComponent();
......@@ -179,7 +192,7 @@ describe('DeployKeysTable', () => {
describe('when pagination is changed', () => {
it('calls API with `page` parameter', async () => {
const pagination = findPagination();
expectDeployKeyIsRendered(deployKey, 0);
expectDeployKeyWithFingerprintIsRendered(deployKey, 0);
Api.deployKeys.mockResolvedValue({
data: [deployKey2],
......@@ -199,7 +212,7 @@ describe('DeployKeysTable', () => {
page: 2,
public: true,
});
expectDeployKeyIsRendered(deployKey2, 0);
expectDeployKeyWithFingerprintIsRendered(deployKey2, 0);
});
});
});
......
......@@ -11,6 +11,7 @@
let_it_be(:project2) { create(:project) }
let_it_be(:deploy_key) { create(:deploy_key, public: true) }
let_it_be(:deploy_key2) { create(:deploy_key, public: true) }
let_it_be(:deploy_key_without_fingerprint) { create(:deploy_key, :without_md5_fingerprint, public: true) }
let_it_be(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) }
let_it_be(:deploy_keys_project2) { create(:deploy_keys_project, :write_access, project: project2, deploy_key: deploy_key) }
let_it_be(:deploy_keys_project3) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key2) }
......
......@@ -29,9 +29,20 @@ jest.mock('~/projects/settings/api/access_dropdown_api', () => ({
}),
getDeployKeys: jest.fn().mockResolvedValue({
data: [
{ id: 10, title: 'key10', fingerprint: 'abcdefghijklmnop', owner: { name: 'user1' } },
{ id: 11, title: 'key11', fingerprint: 'abcdefghijklmnop', owner: { name: 'user2' } },
{ id: 12, title: 'key12', fingerprint: 'abcdefghijklmnop', owner: { name: 'user3' } },
{
id: 10,
title: 'key10',
fingerprint: 'md5-abcdefghijklmnop',
fingerprint_sha256: 'sha256-abcdefghijklmnop',
owner: { name: 'user1' },
},
{
id: 11,
title: 'key11',
fingerprint_sha256: 'sha256-abcdefghijklmnop',
owner: { name: 'user2' },
},
{ id: 12, title: 'key12', fingerprint: 'md5-abcdefghijklmnop', owner: { name: 'user3' } },
],
}),
}));
......@@ -279,6 +290,7 @@ describe('Access Level Dropdown', () => {
{ id: 115, type: 'group', group_id: 5 },
{ id: 118, type: 'user', user_id: 8, name: 'user2' },
{ id: 121, type: 'deploy_key', deploy_key_id: 11 },
{ id: 122, type: 'deploy_key', deploy_key_id: 12 },
];
const findSelected = (type) =>
......@@ -309,8 +321,9 @@ describe('Access Level Dropdown', () => {
it('should set selected deploy keys as intersection between the server response and preselected mapping some keys', () => {
const selectedDeployKeys = findSelected(LEVEL_TYPES.DEPLOY_KEY);
expect(selectedDeployKeys).toHaveLength(1);
expect(selectedDeployKeys.at(0).text()).toContain('key11 (abcdefghijklmn...)');
expect(selectedDeployKeys).toHaveLength(2);
expect(selectedDeployKeys.at(0).text()).toContain('key11 (sha256-abcdefg...)');
expect(selectedDeployKeys.at(1).text()).toContain('key12 (md5-abcdefghij...)');
});
});
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment