Commit 5ad818a1 authored by Robert Hunt's avatar Robert Hunt Committed by Markus Koller

The SSH Key page should mention that expired keys only cause warnings and will continue working

parent 6798107e
......@@ -37,4 +37,18 @@ module ProfilesHelper
def user_status_set_to_busy?(status)
status&.availability == availability_values[:busy]
end
# Overridden in EE::ProfilesHelper#ssh_key_expiration_tooltip
def ssh_key_expiration_tooltip(key)
return key.errors.full_messages.join(', ') if key.errors.full_messages.any?
s_('Profiles|Key usable beyond expiration date.') if key.expired?
end
# Overridden in EE::ProfilesHelper#ssh_key_expires_field_description
def ssh_key_expires_field_description
s_('Profiles|Key can still be used after expiration.')
end
end
ProfilesHelper.prepend_ee_mod
......@@ -15,11 +15,12 @@
.col.form-group
= f.label :expires_at, s_('Profiles|Expires at'), class: 'label-bold'
= f.date_field :expires_at, class: "form-control input-lg", min: Date.tomorrow, data: { qa_selector: 'key_expiry_date_field' }
%p.form-text.text-muted{ data: { qa_selector: 'key_expiry_date_field_description' } }= ssh_key_expires_field_description
.js-add-ssh-key-validation-warning.hide
.bs-callout.bs-callout-warning{ role: 'alert', aria_live: 'assertive' }
%strong= _('Oops, are you sure?')
%p= s_("Profiles|This doesn't look like a public SSH key, are you sure you want to add it? It will be publicly visible.")
%p= s_("Profiles|Publicly visible private SSH keys can compromise your system.")
%button.btn.gl-button.btn-confirm.js-add-ssh-key-validation-confirm-submit= _("Yes, add it")
......
- icon_classes = 'settings-list-icon gl-display-none gl-sm-display-block'
%li.key-list-item
.gl-display-flex.gl-align-items-flex-start
.key-list-item-info.gl-w-full.float-none
......@@ -5,15 +7,11 @@
= key.title
.gl-display-flex.gl-align-items-center.gl-mt-2
- if key.valid?
- if key.expired?
%span.gl-display-inline-block.has-tooltip{ title: s_('Profiles|Your key has expired') }
= sprite_icon('warning-solid', css_class: 'settings-list-icon gl-display-none gl-sm-display-block')
- else
= sprite_icon('key', css_class: 'settings-list-icon gl-display-none gl-sm-display-block')
- if key.valid? && !key.expired?
= sprite_icon('key', css_class: icon_classes)
- else
%span.gl-display-inline-block.has-tooltip{ title: key.errors.full_messages.join(', ') }
= sprite_icon('warning-solid', css_class: 'settings-list-icon gl-display-none gl-sm-display-block')
%span.gl-display-inline-block.has-tooltip{ title: ssh_key_expiration_tooltip(key) }
= sprite_icon('warning-solid', css_class: icon_classes)
%span.gl-text-truncate.gl-sm-ml-3
= key.fingerprint
......
# frozen_string_literal: true
module EE
module ProfilesHelper
extend ::Gitlab::Utils::Override
override :ssh_key_expiration_tooltip
def ssh_key_expiration_tooltip(key)
return super unless ::Key.expiration_enforced? && key.expired?
# The key returns a validation error for an invalid keys which is displayed in git related operations.
# However, on the UI we don't want to show this message,
# so we strip it out and check for any other errors
return s_('Profiles|Invalid key.') unless key.errors.full_messages.reject do |m|
m.eql?('Key has expired and the instance administrator has enforced expiration')
end.empty?
s_('Profiles|Expired key is not valid.')
end
override :ssh_key_expires_field_description
def ssh_key_expires_field_description
return super unless ::Key.expiration_enforced?
s_('Profiles|Key will be deleted on this date.')
end
end
end
---
title: SSH key page text now reflects the expiration enforcement setting being used on the instance
merge_request: 57494
author:
type: changed
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ProfilesHelper do
before do
allow(Key).to receive(:enforce_ssh_key_expiration_feature_available?).and_return(true)
end
describe "#ssh_key_expiration_tooltip" do
using RSpec::Parameterized::TableSyntax
error_message = 'Key type is forbidden. Must be DSA, ECDSA, or ED25519'
where(:error, :expired, :enforced, :result) do
false | false | false | nil
true | false | false | error_message
true | false | true | error_message
true | true | false | error_message
true | true | true | 'Invalid key.'
false | true | true | 'Expired key is not valid.'
false | true | false | 'Key usable beyond expiration date.'
end
with_them do
let_it_be(:key) { build(:personal_key) }
it do
allow(Key).to receive(:expiration_enforced?).and_return(enforced)
key.expires_at = expired ? 2.days.ago : 2.days.from_now
key.errors.add(:base, error_message) if error
expect(helper.ssh_key_expiration_tooltip(key)).to eq(result)
end
end
context 'when enforced and expired' do
let_it_be(:key) { build(:personal_key) }
it 'does not return the expiration validation error message', :aggregate_failures do
allow(Key).to receive(:expiration_enforced?).and_return(true)
key.expires_at = 2.days.ago
expect(key.invalid?).to eq(true)
expect(helper.ssh_key_expiration_tooltip(key)).to eq('Expired key is not valid.')
end
end
end
describe "#ssh_key_expires_field_description" do
using RSpec::Parameterized::TableSyntax
where(:expiration_enforced, :result) do
true | "Key will be deleted on this date."
false | "Key can still be used after expiration."
end
with_them do
it do
allow(Key).to receive(:expiration_enforced?).and_return(expiration_enforced)
expect(helper.ssh_key_expires_field_description).to eq(result)
end
end
end
end
......@@ -23877,6 +23877,9 @@ msgstr ""
msgid "Profiles|Enter your name, so people you know can recognize you"
msgstr ""
msgid "Profiles|Expired key is not valid."
msgstr ""
msgid "Profiles|Expires at"
msgstr ""
......@@ -23907,6 +23910,9 @@ msgstr ""
msgid "Profiles|Increase your account's security by enabling Two-Factor Authentication (2FA)"
msgstr ""
msgid "Profiles|Invalid key."
msgstr ""
msgid "Profiles|Invalid password"
msgstr ""
......@@ -23916,6 +23922,15 @@ msgstr ""
msgid "Profiles|Key"
msgstr ""
msgid "Profiles|Key can still be used after expiration."
msgstr ""
msgid "Profiles|Key usable beyond expiration date."
msgstr ""
msgid "Profiles|Key will be deleted on this date."
msgstr ""
msgid "Profiles|Last used:"
msgstr ""
......@@ -23961,6 +23976,9 @@ msgstr ""
msgid "Profiles|Public email"
msgstr ""
msgid "Profiles|Publicly visible private SSH keys can compromise your system."
msgstr ""
msgid "Profiles|Remove avatar"
msgstr ""
......@@ -23985,9 +24003,6 @@ msgstr ""
msgid "Profiles|The maximum file size allowed is 200KB."
msgstr ""
msgid "Profiles|This doesn't look like a public SSH key, are you sure you want to add it? It will be publicly visible."
msgstr ""
msgid "Profiles|This email will be displayed on your public profile"
msgstr ""
......@@ -24075,9 +24090,6 @@ msgstr ""
msgid "Profiles|Your email address was automatically set based on your %{provider_label} account"
msgstr ""
msgid "Profiles|Your key has expired"
msgstr ""
msgid "Profiles|Your location was automatically set based on your %{provider_label} account"
msgstr ""
......
......@@ -112,6 +112,46 @@ RSpec.describe ProfilesHelper do
end
end
describe "#ssh_key_expiration_tooltip" do
using RSpec::Parameterized::TableSyntax
before do
allow(Key).to receive(:enforce_ssh_key_expiration_feature_available?).and_return(false)
end
error_message = 'Key type is forbidden. Must be DSA, ECDSA, or ED25519'
where(:error, :expired, :result) do
false | false | nil
true | false | error_message
false | true | 'Key usable beyond expiration date.'
true | true | error_message
end
with_them do
let_it_be(:key) do
build(:personal_key)
end
it do
key.expires_at = expired ? 2.days.ago : 2.days.from_now
key.errors.add(:base, error_message) if error
expect(helper.ssh_key_expiration_tooltip(key)).to eq(result)
end
end
end
describe "#ssh_key_expires_field_description" do
before do
allow(Key).to receive(:enforce_ssh_key_expiration_feature_available?).and_return(false)
end
it 'returns the description' do
expect(helper.ssh_key_expires_field_description).to eq('Key can still be used after expiration.')
end
end
def stub_cas_omniauth_provider
provider = OpenStruct.new(
'name' => 'cas3',
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'profiles/keys/_form.html.haml' do
let_it_be(:key) { Key.new }
let(:page) { Capybara::Node::Simple.new(rendered) }
before do
assign(:key, key)
end
context 'when the form partial is used' do
before do
allow(view).to receive(:ssh_key_expires_field_description).and_return('Key can still be used after expiration.')
render
end
it 'renders the form with the correct action' do
expect(page.find('form')['action']).to eq('/-/profile/keys')
end
it 'has the key field', :aggregate_failures do
expect(rendered).to have_field('Key', type: 'textarea', placeholder: 'Typically starts with "ssh-ed25519 …" or "ssh-rsa …"')
expect(rendered).to have_text("Paste your public SSH key, which is usually contained in the file '~/.ssh/id_ed25519.pub' or '~/.ssh/id_rsa.pub' and begins with 'ssh-ed25519' or 'ssh-rsa'. Do not paste your private SSH key, as that can compromise your identity.")
end
it 'has the title field', :aggregate_failures do
expect(rendered).to have_field('Title', type: 'text', placeholder: 'e.g. My MacBook key')
expect(rendered).to have_text('Give your individual key a title.')
end
it 'has the expires at field', :aggregate_failures do
expect(rendered).to have_field('Expires at', type: 'date')
expect(page.find_field('Expires at')['min']).to eq(l(1.day.from_now, format: "%Y-%m-%d"))
expect(rendered).to have_text('Key can still be used after expiration.')
end
it 'has the validation warning', :aggregate_failures do
expect(rendered).to have_text("Oops, are you sure? Publicly visible private SSH keys can compromise your system.")
expect(rendered).to have_button('Yes, add it')
end
it 'has the submit button' do
expect(rendered).to have_button('Add key')
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'profiles/keys/_key.html.haml' do
let_it_be(:user) { create(:user) }
before do
allow(view).to receive(:key).and_return(key)
allow(view).to receive(:is_admin).and_return(false)
end
context 'when the key partial is used' do
let_it_be(:key) do
create(:personal_key,
user: user,
last_used_at: 7.days.ago,
expires_at: 2.days.from_now)
end
it 'displays the correct values', :aggregate_failures do
render
expect(rendered).to have_text(key.title)
expect(rendered).to have_css('[data-testid="key-icon"]')
expect(rendered).to have_text(key.fingerprint)
expect(rendered).to have_text(l(key.last_used_at, format: "%b %d, %Y"))
expect(rendered).to have_text(l(key.created_at, format: "%b %d, %Y"))
expect(rendered).to have_text(key.expires_at.to_date)
expect(response).to render_template(partial: 'shared/ssh_keys/_key_delete')
end
context 'when the key has not been used' do
let_it_be(:key) do
create(:personal_key,
user: user,
last_used_at: nil)
end
it 'renders "Never" for last used' do
render
expect(rendered).to have_text('Last used: Never')
end
end
context 'when the key does not have an expiration date' do
let_it_be(:key) do
create(:personal_key,
user: user,
expires_at: nil)
end
it 'renders "Never" for expires' do
render
expect(rendered).to have_text('Expires: Never')
end
end
context 'when the key is not deletable' do
# Turns out key.can_delete? is only false for LDAP keys
# but LDAP keys don't exist outside EE
before do
allow(key).to receive(:can_delete?).and_return(false)
end
it 'does not render the partial' do
render
expect(response).not_to render_template(partial: 'shared/ssh_keys/_key_delete')
end
end
context 'icon tooltip' do
using RSpec::Parameterized::TableSyntax
where(:valid, :expiry, :result) do
false | 2.days.from_now | 'Key type is forbidden. Must be DSA, ECDSA, or ED25519'
false | 2.days.ago | 'Key type is forbidden. Must be DSA, ECDSA, or ED25519'
true | 2.days.ago | 'Key usable beyond expiration date.'
true | 2.days.from_now | ''
end
with_them do
let_it_be(:key) do
create(:personal_key, user: user)
end
it 'renders the correct icon', :aggregate_failures do
unless valid
stub_application_setting(rsa_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE)
end
key.expires_at = expiry
render
if result.empty?
expect(rendered).to have_css('[data-testid="key-icon"]')
else
expect(rendered).to have_css('[data-testid="warning-solid-icon"]')
expect(rendered).to have_selector("span.has-tooltip[title='#{result}']")
end
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