Commit aa0473d0eb581a1c4d20be22c53ec4002a3801e0
1 parent
656d800f
Exists in
master
and in
4 other branches
Validate fingerprint uniqueness
Showing
2 changed files
with
13 additions
and
15 deletions
Show diff stats
app/models/key.rb
@@ -21,11 +21,11 @@ class Key < ActiveRecord::Base | @@ -21,11 +21,11 @@ class Key < ActiveRecord::Base | ||
21 | 21 | ||
22 | attr_accessible :key, :title | 22 | attr_accessible :key, :title |
23 | 23 | ||
24 | - before_validation :strip_white_space | 24 | + before_validation :strip_white_space, :generate_fingerpint |
25 | 25 | ||
26 | validates :title, presence: true, length: { within: 0..255 } | 26 | validates :title, presence: true, length: { within: 0..255 } |
27 | validates :key, presence: true, length: { within: 0..5000 }, format: { with: /\A(ssh|ecdsa)-.*\Z/ }, uniqueness: true | 27 | validates :key, presence: true, length: { within: 0..5000 }, format: { with: /\A(ssh|ecdsa)-.*\Z/ }, uniqueness: true |
28 | - validate :fingerprintable_key | 28 | + validates :fingerprint, uniqueness: true, presence: { message: 'cannot be generated' } |
29 | 29 | ||
30 | delegate :name, :email, to: :user, prefix: true | 30 | delegate :name, :email, to: :user, prefix: true |
31 | 31 | ||
@@ -33,15 +33,6 @@ class Key < ActiveRecord::Base | @@ -33,15 +33,6 @@ class Key < ActiveRecord::Base | ||
33 | self.key = key.strip unless key.blank? | 33 | self.key = key.strip unless key.blank? |
34 | end | 34 | end |
35 | 35 | ||
36 | - def fingerprintable_key | ||
37 | - return true unless key # Don't test if there is no key. | ||
38 | - | ||
39 | - unless generate_fingerpint | ||
40 | - errors.add(:key, "can't be fingerprinted") | ||
41 | - false | ||
42 | - end | ||
43 | - end | ||
44 | - | ||
45 | # projects that has this key | 36 | # projects that has this key |
46 | def projects | 37 | def projects |
47 | user.authorized_projects | 38 | user.authorized_projects |
@@ -54,6 +45,9 @@ class Key < ActiveRecord::Base | @@ -54,6 +45,9 @@ class Key < ActiveRecord::Base | ||
54 | private | 45 | private |
55 | 46 | ||
56 | def generate_fingerpint | 47 | def generate_fingerpint |
48 | + self.fingerprint = nil | ||
49 | + return unless key.present? | ||
50 | + | ||
57 | cmd_status = 0 | 51 | cmd_status = 0 |
58 | cmd_output = '' | 52 | cmd_output = '' |
59 | Tempfile.open('gitlab_key_file') do |file| | 53 | Tempfile.open('gitlab_key_file') do |file| |
@@ -66,9 +60,6 @@ class Key < ActiveRecord::Base | @@ -66,9 +60,6 @@ class Key < ActiveRecord::Base | ||
66 | cmd_output.gsub /([\d\h]{2}:)+[\d\h]{2}/ do |match| | 60 | cmd_output.gsub /([\d\h]{2}:)+[\d\h]{2}/ do |match| |
67 | self.fingerprint = match | 61 | self.fingerprint = match |
68 | end | 62 | end |
69 | - true | ||
70 | - else | ||
71 | - false | ||
72 | end | 63 | end |
73 | end | 64 | end |
74 | end | 65 | end |
spec/models/key_spec.rb
@@ -42,10 +42,17 @@ describe Key do | @@ -42,10 +42,17 @@ describe Key do | ||
42 | build(:key, user: user).should be_valid | 42 | build(:key, user: user).should be_valid |
43 | end | 43 | end |
44 | 44 | ||
45 | - it "does not accepts the key twice" do | 45 | + it "does not accept the exact same key twice" do |
46 | create(:key, user: user) | 46 | create(:key, user: user) |
47 | build(:key, user: user).should_not be_valid | 47 | build(:key, user: user).should_not be_valid |
48 | end | 48 | end |
49 | + | ||
50 | + it "does not accept a duplicate key with a different comment" do | ||
51 | + create(:key, user: user) | ||
52 | + duplicate = build(:key, user: user) | ||
53 | + duplicate.key << ' extra comment' | ||
54 | + duplicate.should_not be_valid | ||
55 | + end | ||
49 | end | 56 | end |
50 | 57 | ||
51 | context "validate it is a fingerprintable key" do | 58 | context "validate it is a fingerprintable key" do |