Commit 016865c2 authored by huzaifaiftikhar1's avatar huzaifaiftikhar1 Committed by Huzaifa Iftikhar

Refactor code and remove redundant code blocks

Remove the method `ssh_key_max_expiry_date` and associated
tests. Add "validate_" prefix to methods used for validating
models.
parent 274be978
- max_date = ssh_key_max_expiry_date.to_date if ssh_key_expiration_policy_enabled? - max_date = ::Gitlab::CurrentSettings.max_ssh_key_lifetime_from_now.to_date if ssh_key_expiration_policy_enabled?
%div %div
= form_for [:profile, @key], html: { class: 'js-requires-input' } do |f| = form_for [:profile, @key], html: { class: 'js-requires-input' } do |f|
= form_errors(@key) = form_errors(@key)
......
...@@ -26,10 +26,6 @@ module EE ...@@ -26,10 +26,6 @@ module EE
License.feature_available?(:ssh_key_expiration_policy) && ::Feature.enabled?(:ff_limit_ssh_key_lifetime) License.feature_available?(:ssh_key_expiration_policy) && ::Feature.enabled?(:ff_limit_ssh_key_lifetime)
end end
def ssh_key_max_expiry_date
::Gitlab::CurrentSettings.max_ssh_key_lifetime_from_now
end
override :ssh_key_expiration_policy_enabled? 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)
......
...@@ -15,7 +15,7 @@ module EE ...@@ -15,7 +15,7 @@ module EE
validate :expiration, if: -> { ::Key.expiration_enforced? } validate :expiration, if: -> { ::Key.expiration_enforced? }
with_options if: :ssh_key_expiration_policy_enabled? do with_options if: :ssh_key_expiration_policy_enabled? do
validate :expires_at_before_max_expiry_date validate :validate_expires_at_before_max_expiry_date
end end
def expiration def expiration
...@@ -32,12 +32,12 @@ module EE ...@@ -32,12 +32,12 @@ module EE
errors.map(&:type).reject { |t| t.eql?(:expired_and_enforced) }.empty? errors.map(&:type).reject { |t| t.eql?(:expired_and_enforced) }.empty?
end end
def expires_at_before_max_expiry_date def validate_expires_at_before_max_expiry_date
return errors.add(:key, message: 'has no expiration date but an expiration date is required for SSH keys on this instance. Contact the instance administrator.') if expires_at.blank? return errors.add(:key, message: 'has no expiration date but an expiration date is required for SSH keys on this instance. Contact the instance administrator.') if expires_at.blank?
# when the key is not yet persisted the `created_at` field is nil # when the key is not yet persisted the `created_at` field is nil
key_creation_date = created_at.presence || Time.current max_expiry_date = (created_at.presence || Time.current) + ::Gitlab::CurrentSettings.max_ssh_key_lifetime.days
errors.add(:key, message: 'has an invalid expiration 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 errors.add(:key, message: 'has an invalid expiration date. Set a shorter lifetime for the key or contact the instance administrator.') if expires_at > max_expiry_date
end end
end end
......
...@@ -91,6 +91,7 @@ RSpec.describe ProfilesHelper do ...@@ -91,6 +91,7 @@ RSpec.describe ProfilesHelper do
before do before do
stub_feature_flags(ff_limit_ssh_key_lifetime: true) stub_feature_flags(ff_limit_ssh_key_lifetime: true)
end end
context 'when is licensed and used' do context 'when is licensed and used' do
before do before do
stub_licensed_features(ssh_key_expiration_policy: true) stub_licensed_features(ssh_key_expiration_policy: true)
...@@ -132,20 +133,4 @@ RSpec.describe ProfilesHelper do ...@@ -132,20 +133,4 @@ RSpec.describe ProfilesHelper do
end end
end end
end end
describe '#ssh_key_max_expiry_date' do
subject { helper.ssh_key_max_expiry_date }
context 'the instance has an expiry setting' do
before do
stub_application_setting(max_ssh_key_lifetime: 20)
end
it { is_expected.to be_like_time(20.days.from_now) }
end
context 'the instance does not have an expiry setting' do
it { is_expected.to be_nil }
end
end
end end
...@@ -819,7 +819,7 @@ RSpec.describe ApplicationSetting do ...@@ -819,7 +819,7 @@ RSpec.describe ApplicationSetting do
end end
end end
describe "#max_ssh_key_lifetime_from_now" do describe "#max_ssh_key_lifetime_from_now", :freeze_time do
subject { setting.max_ssh_key_lifetime_from_now } subject { setting.max_ssh_key_lifetime_from_now }
let(:days_from_now) { nil } let(:days_from_now) { nil }
......
...@@ -23,7 +23,7 @@ RSpec.describe Key do ...@@ -23,7 +23,7 @@ RSpec.describe Key do
end end
end end
describe '#expires_at_before_max_expiry_date' do describe '#validate_expires_at_before_max_expiry_date' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
context 'for a range of key expiry combinations' do context 'for a range of key expiry combinations' do
......
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