Commit 86d76a25 authored by huzaifaiftikhar1's avatar huzaifaiftikhar1 Committed by Huzaifa Iftikhar

Update the logic to decide if a key is valid and update the docs

Any key that has a validity period i.e. the difference of its
expires_at and created_at greater than the max SSH key lifetime
set by the instance administrator is invalid.
parent 02fef58e
...@@ -61,6 +61,11 @@ module ProfilesHelper ...@@ -61,6 +61,11 @@ module ProfilesHelper
def ssh_key_expires_field_description def ssh_key_expires_field_description
s_('Profiles|Key can still be used after expiration.') s_('Profiles|Key can still be used after expiration.')
end end
# Overridden in EE::ProfilesHelper#ssh_key_expiration_policy_enabled?
def ssh_key_expiration_policy_enabled?
false
end
end end
ProfilesHelper.prepend_mod ProfilesHelper.prepend_mod
...@@ -194,7 +194,7 @@ To set a limit on how long these sessions are valid: ...@@ -194,7 +194,7 @@ To set a limit on how long these sessions are valid:
## Limit the lifetime of SSH keys **(ULTIMATE SELF)** ## Limit the lifetime of SSH keys **(ULTIMATE SELF)**
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/1007) in GitLab 14.6. [with a flag](../../../administration/feature_flags.md) named `ff_limit_ssh_key_lifetime`. Disabled by default. > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/1007) in GitLab 14.6 [with a flag](../../../administration/feature_flags.md) named `ff_limit_ssh_key_lifetime`. Disabled by default.
FLAG: FLAG:
On self-managed GitLab, by default this feature is not available. To make it available, On self-managed GitLab, by default this feature is not available. To make it available,
......
...@@ -16,9 +16,9 @@ module EE ...@@ -16,9 +16,9 @@ module EE
return super unless ::Key.expiration_enforced? return super unless ::Key.expiration_enforced?
if ssh_key_expiration_policy_enabled? if ssh_key_expiration_policy_enabled?
s_('Profiles|Key will be deleted on this date. Maximum lifetime for SSH keys is %{max_ssh_key_lifetime} days') % { max_ssh_key_lifetime: ::Gitlab::CurrentSettings.max_ssh_key_lifetime } s_('Profiles|Key becomes invalid on this date. Maximum lifetime for SSH keys is %{max_ssh_key_lifetime} days') % { max_ssh_key_lifetime: ::Gitlab::CurrentSettings.max_ssh_key_lifetime }
else else
s_('Profiles|Key will be deleted on this date.') s_('Profiles|Key becomes invalid on this date.')
end end
end end
...@@ -30,6 +30,7 @@ module EE ...@@ -30,6 +30,7 @@ module EE
::Gitlab::CurrentSettings.max_ssh_key_lifetime_from_now ::Gitlab::CurrentSettings.max_ssh_key_lifetime_from_now
end end
override :ssh_key_expiration_policy_enabled?
def ssh_key_expiration_policy_enabled? def ssh_key_expiration_policy_enabled?
::Gitlab::CurrentSettings.max_ssh_key_lifetime && ssh_key_expiration_policy_licensed? && ::Feature.enabled?(:ff_limit_ssh_key_lifetime) ::Gitlab::CurrentSettings.max_ssh_key_lifetime && ssh_key_expiration_policy_licensed? && ::Feature.enabled?(:ff_limit_ssh_key_lifetime)
end end
......
...@@ -33,9 +33,11 @@ module EE ...@@ -33,9 +33,11 @@ module EE
end end
def expires_at_before_max_expiry_date def expires_at_before_max_expiry_date
return errors.add(:key, message: 'does not have an expiry date but maximum allowable lifetime for SSH keys is enforced by the instance administrator') if expires_at.blank? return errors.add(:key, message: 'has no expiry date but an expiry date is required for SSH keys on this instance. Contact the instance administrator.') if expires_at.blank?
errors.add(:key, message: 'has greater than the maximum allowable lifetime for SSH keys enforced by the instance administrator') if expires_at > ssh_key_max_expiry_date # when the key is not yet persisted the `created_at` field is nil
key_creation_date = created_at.presence || Time.current
errors.add(:key, message: 'has an invalid expiry date. Set a shorter lifetime for the key or contact the instance administrator.') if expires_at > key_creation_date + ::Gitlab::CurrentSettings.max_ssh_key_lifetime.days
end end
end end
......
...@@ -2,6 +2,6 @@ ...@@ -2,6 +2,6 @@
- form = local_assigns.fetch(:form) - form = local_assigns.fetch(:form)
.form-group .form-group
= form.label :max_ssh_key_lifetime, _('Maximum allowable lifetime for SSH keys (days)'), class: 'label-light' = form.label :max_ssh_key_lifetime, _('Maximum allowed lifetime for SSH keys (in days)'), class: 'label-light'
= form.number_field :max_ssh_key_lifetime, class: 'form-control gl-form-input input-xs' = form.number_field :max_ssh_key_lifetime, class: 'form-control gl-form-input input-xs'
%span.form-text.text-muted#max_ssh_key_lifetime= _('When enabled, existing SSH keys may be revoked. Leave blank for no limit.') %span.form-text.text-muted#max_ssh_key_lifetime= _('When enabled, SSH keys with no expiry date or an invalid expiration date are no longer accepted. Leave blank for no limit.')
...@@ -51,7 +51,7 @@ RSpec.describe ProfilesHelper do ...@@ -51,7 +51,7 @@ RSpec.describe ProfilesHelper do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:expiration_enforced, :result) do where(:expiration_enforced, :result) do
true | "Key will be deleted on this date." true | "Key becomes invalid on this date."
false | "Key can still be used after expiration." false | "Key can still be used after expiration."
end end
......
...@@ -26,22 +26,46 @@ RSpec.describe Key do ...@@ -26,22 +26,46 @@ RSpec.describe Key do
describe '#expires_at_before_max_expiry_date' do describe '#expires_at_before_max_expiry_date' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:key, :max_ssh_key_lifetime, :valid) do context 'for a range of key expiry combinations' do
build(:personal_key, expires_at: 20.days.from_now) | 10 | false where(:key, :max_ssh_key_lifetime, :valid) do
build(:personal_key, expires_at: 7.days.from_now) | 10 | true build(:personal_key, created_at: Time.current, expires_at: nil) | nil | true
build(:personal_key, expires_at: 20.days.from_now) | nil | true build(:personal_key, created_at: Time.current, expires_at: 20.days.from_now) | nil | true
build(:personal_key, expires_at: nil) | 20 | false build(:personal_key, created_at: 1.day.ago, expires_at: 20.days.from_now) | 10 | false
build(:personal_key, expires_at: nil) | nil | true build(:personal_key, created_at: 6.days.ago, expires_at: 3.days.from_now) | 10 | true
build(:personal_key, created_at: 10.days.ago, expires_at: 7.days.from_now) | 10 | false
build(:personal_key, created_at: Time.current, expires_at: nil) | 20 | false
build(:personal_key, expires_at: nil) | 30 | false
end
with_them do
before do
stub_licensed_features(ssh_key_expiration_policy: true)
stub_application_setting(max_ssh_key_lifetime: max_ssh_key_lifetime)
end
it 'checks if ssh key expiration is valid' do
expect(key.valid?).to eq(valid)
end
end
end end
with_them do context 'when keys and max expiry are set' do
before do where(:key, :max_ssh_key_lifetime, :valid) do
stub_licensed_features(ssh_key_expiration_policy: true) build(:personal_key, created_at: Time.current, expires_at: 20.days.from_now) | 10 | false
stub_application_setting(max_ssh_key_lifetime: max_ssh_key_lifetime) build(:personal_key, created_at: Time.current, expires_at: 7.days.from_now) | 10 | true
end end
it 'checks if ssh key expiration is valid' do with_them do
expect(key.valid?).to eq(valid) before do
stub_licensed_features(ssh_key_expiration_policy: true)
stub_application_setting(max_ssh_key_lifetime: max_ssh_key_lifetime)
end
it 'checks validity properly in the future too' do
# Travel to the day before the key is set to 'expire'.
# max_ssh_key_lifetime should still be enforced correctly.
travel_to(key.expires_at - 1) do
expect(key.valid?).to eq(valid)
end
end
end end
end end
end end
......
...@@ -21493,10 +21493,10 @@ msgstr "" ...@@ -21493,10 +21493,10 @@ msgstr ""
msgid "Maximum Users" msgid "Maximum Users"
msgstr "" msgstr ""
msgid "Maximum allowable lifetime for SSH keys (days)" msgid "Maximum allowable lifetime for personal access token (days)"
msgstr "" msgstr ""
msgid "Maximum allowable lifetime for personal access token (days)" msgid "Maximum allowed lifetime for SSH keys (in days)"
msgstr "" msgstr ""
msgid "Maximum artifacts size" msgid "Maximum artifacts size"
...@@ -26756,16 +26756,16 @@ msgstr "" ...@@ -26756,16 +26756,16 @@ msgstr ""
msgid "Profiles|Key" msgid "Profiles|Key"
msgstr "" msgstr ""
msgid "Profiles|Key can still be used after expiration." msgid "Profiles|Key becomes invalid on this date."
msgstr "" msgstr ""
msgid "Profiles|Key usable beyond expiration date." msgid "Profiles|Key becomes invalid on this date. Maximum lifetime for SSH keys is %{max_ssh_key_lifetime} days"
msgstr "" msgstr ""
msgid "Profiles|Key will be deleted on this date." msgid "Profiles|Key can still be used after expiration."
msgstr "" msgstr ""
msgid "Profiles|Key will be deleted on this date. Maximum lifetime for SSH keys is %{max_ssh_key_lifetime} days" msgid "Profiles|Key usable beyond expiration date."
msgstr "" msgstr ""
msgid "Profiles|Last used:" msgid "Profiles|Last used:"
...@@ -39227,7 +39227,7 @@ msgstr "" ...@@ -39227,7 +39227,7 @@ msgstr ""
msgid "When a runner is locked, it cannot be assigned to other projects" msgid "When a runner is locked, it cannot be assigned to other projects"
msgstr "" msgstr ""
msgid "When enabled, existing SSH keys may be revoked. Leave blank for no limit." msgid "When enabled, SSH keys with no expiry date or an invalid expiration date are no longer accepted. Leave blank for no limit."
msgstr "" msgstr ""
msgid "When enabled, existing personal access tokens may be revoked. Leave blank for no limit." msgid "When enabled, existing personal access tokens may be revoked. Leave blank for no limit."
......
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