diff --git a/app/models/key.rb b/app/models/key.rb index 11c01242b1562..902a7c5911654 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -21,11 +21,11 @@ class Key < ActiveRecord::Base attr_accessible :key, :title - before_validation :strip_white_space + before_validation :strip_white_space, :generate_fingerpint validates :title, presence: true, length: { within: 0..255 } validates :key, presence: true, length: { within: 0..5000 }, format: { with: /\A(ssh|ecdsa)-.*\Z/ }, uniqueness: true - validate :fingerprintable_key + validates :fingerprint, uniqueness: true, presence: { message: 'cannot be generated' } delegate :name, :email, to: :user, prefix: true @@ -33,15 +33,6 @@ def strip_white_space self.key = key.strip unless key.blank? end - def fingerprintable_key - return true unless key # Don't test if there is no key. - - unless generate_fingerpint - errors.add(:key, "can't be fingerprinted") - false - end - end - # projects that has this key def projects user.authorized_projects @@ -54,26 +45,21 @@ def shell_id private def generate_fingerpint + self.fingerprint = nil + return unless key.present? + cmd_status = 0 cmd_output = '' - file = Tempfile.new('gitlab_key_file') - - begin + Tempfile.open('gitlab_key_file') do |file| file.puts key file.rewind cmd_output, cmd_status = popen("ssh-keygen -lf #{file.path}", '/tmp') - ensure - file.close - file.unlink # deletes the temp file end if cmd_status.zero? cmd_output.gsub /([\d\h]{2}:)+[\d\h]{2}/ do |match| self.fingerprint = match end - true - else - false end end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 6c06f5268b37c..f4dd726331aef 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -42,17 +42,22 @@ build(:key, user: user).should be_valid end - it "does not accepts the key twice" do + it "does not accept the exact same key twice" do create(:key, user: user) build(:key, user: user).should_not be_valid end + + it "does not accept a duplicate key with a different comment" do + create(:key, user: user) + duplicate = build(:key, user: user) + duplicate.key << ' extra comment' + duplicate.should_not be_valid + end end context "validate it is a fingerprintable key" do - let(:user) { create(:user) } - it "accepts the fingerprintable key" do - build(:key, user: user).should be_valid + build(:key).should be_valid end it "rejects the unfingerprintable key (contains space in middle)" do