Move the key restriction validation to its own class

This commit is contained in:
Nick Thomas 2017-08-28 21:58:36 +01:00
parent b84ca08e35
commit eb05bdc6f5
4 changed files with 33 additions and 10 deletions

View file

@ -15,13 +15,9 @@ class ApplicationSetting < ActiveRecord::Base
# Setting a key restriction to `-1` means that all keys of this type are # Setting a key restriction to `-1` means that all keys of this type are
# forbidden. # forbidden.
FORBIDDEN_KEY_VALUE = -1 FORBIDDEN_KEY_VALUE = KeyRestrictionValidator::FORBIDDEN
SUPPORTED_KEY_TYPES = %i[rsa dsa ecdsa ed25519].freeze SUPPORTED_KEY_TYPES = %i[rsa dsa ecdsa ed25519].freeze
def self.supported_key_restrictions(type)
[0, *Gitlab::SSHPublicKey.supported_sizes(type), FORBIDDEN_KEY_VALUE]
end
serialize :restricted_visibility_levels # rubocop:disable Cop/ActiveRecordSerialize serialize :restricted_visibility_levels # rubocop:disable Cop/ActiveRecordSerialize
serialize :import_sources # rubocop:disable Cop/ActiveRecordSerialize serialize :import_sources # rubocop:disable Cop/ActiveRecordSerialize
serialize :disabled_oauth_sign_in_sources, Array # rubocop:disable Cop/ActiveRecordSerialize serialize :disabled_oauth_sign_in_sources, Array # rubocop:disable Cop/ActiveRecordSerialize
@ -156,9 +152,7 @@ class ApplicationSetting < ActiveRecord::Base
numericality: { greater_than_or_equal_to: 0 } numericality: { greater_than_or_equal_to: 0 }
SUPPORTED_KEY_TYPES.each do |type| SUPPORTED_KEY_TYPES.each do |type|
validates :"#{type}_key_restriction", validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
presence: true,
inclusion: { in: ApplicationSetting.supported_key_restrictions(type) }
end end
validates_each :restricted_visibility_levels do |record, attr, value| validates_each :restricted_visibility_levels do |record, attr, value|

View file

@ -0,0 +1,29 @@
class KeyRestrictionValidator < ActiveModel::EachValidator
FORBIDDEN = -1
def self.supported_sizes(type)
Gitlab::SSHPublicKey.supported_sizes(type)
end
def self.supported_key_restrictions(type)
[0, *supported_sizes(type), FORBIDDEN]
end
def validate_each(record, attribute, value)
unless valid_restriction?(value)
record.errors.add(attribute, "must be forbidden, allowed, or one of these sizes: #{supported_sizes_message}")
end
end
private
def supported_sizes_message
sizes = self.class.supported_sizes(options[:type])
sizes.to_sentence(last_word_connector: ', or ', two_words_connector: ' or ')
end
def valid_restriction?(value)
choices = self.class.supported_key_restrictions(options[:type])
choices.include?(value)
end
end

View file

@ -125,7 +125,7 @@ module API
ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type| ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
optional :"#{type}_key_restriction", optional :"#{type}_key_restriction",
type: Integer, type: Integer,
values: ApplicationSetting.supported_key_restrictions(type), values: KeyRestrictionValidator.supported_key_restrictions(type),
desc: "Restrictions on the complexity of uploaded #{type.upcase} keys. A value of #{ApplicationSetting::FORBIDDEN_KEY_VALUE} disables all #{type.upcase} keys." desc: "Restrictions on the complexity of uploaded #{type.upcase} keys. A value of #{ApplicationSetting::FORBIDDEN_KEY_VALUE} disables all #{type.upcase} keys."
end end

View file

@ -85,7 +85,7 @@ describe ApplicationSetting do
let(:field) { :"#{type}_key_restriction" } let(:field) { :"#{type}_key_restriction" }
it { is_expected.to validate_presence_of(field) } it { is_expected.to validate_presence_of(field) }
it { is_expected.to allow_value(*described_class.supported_key_restrictions(type)).for(field) } it { is_expected.to allow_value(*KeyRestrictionValidator.supported_key_restrictions(type)).for(field) }
it { is_expected.not_to allow_value(128).for(field) } it { is_expected.not_to allow_value(128).for(field) }
end end
end end