Merge branch 'deploy_keys_nonunique' of https://github.com/miks/gitlabhq into mi…

…ks-deploy_keys_nonunique Added/fixed specs Update spec/factory to allow Factory#new without opts Conflicts: app/models/key.rb

Merge branch 'deploy_keys_nonunique' of https://github.com/miks/gitlabhq into mi…
…ks-deploy_keys_nonunique Added/fixed specs Update spec/factory to allow Factory#new without opts Conflicts: app/models/key.rb
Ariejan de Vroom
2 parents bfe0906f f5a16663
Showing 4 changed files with 65 additions and 8 deletions Show diff stats
app/models/key.rb
db/schema.rb
spec/factory.rb
spec/models/key_spec.rb
+require 'digest/md5'
+
 class Key < ActiveRecord::Base
   belongs_to :user
   belongs_to :project
@@ -8,17 +10,30 @@ class Key &lt; ActiveRecord::Base
  
   validates :key,
             :presence => true,
-            :uniqueness => true,
             :length   => { :within => 0..5000 }
  
   before_save :set_identifier
+  before_validation :strip_white_space
   after_save :update_repository
   after_destroy :repository_delete_key
   delegate :name, :email, :to => :user, :prefix => true
+  validate :unique_key
+
+  def strip_white_space
+    self.key = self.key.strip unless self.key.blank?
+  end
+
+  def unique_key
+    query = Key.where('key = ?', key)
+    query = query.where('(project_id IS NULL OR project_id = ?)', project_id) if project_id
+    if (query.count > 0)
+      errors.add :key, 'already exist.'
+    end
+  end
  
   def set_identifier
     if is_deploy_key
-      self.identifier = "deploy_#{project.code}_#{Time.now.to_i}"
+      self.identifier = "deploy_" + Digest::MD5.hexdigest(key)
     else
       self.identifier = "#{user.identifier}_#{Time.now.to_i}"
     end
@@ -33,11 +48,14 @@ class Key &lt; ActiveRecord::Base
  
   def repository_delete_key
     Gitlabhq::GitHost.system.new.configure do |c|
-      c.delete_key(identifier)
+      #delete key file is there is no identically deploy keys
+      if !is_deploy_key || Key.where(:identifier => identifier).count() == 0
+        c.delete_key(identifier)
+      end
       c.update_projects(projects)
     end
   end
-  
+
   def is_deploy_key
     true if project_id
   end
@@ -163,6 +163,13 @@ ActiveRecord::Schema.define(:version =&gt; 20120228134252) do
     t.integer  "project_access", :default => 0, :null => false
   end
  
+  create_table "web_hook_urls", :force => true do |t|
+    t.string   "url"
+    t.integer  "project_id"
+    t.datetime "created_at"
+    t.datetime "updated_at"
+  end
+
   create_table "web_hooks", :force => true do |t|
     t.string   "url"
     t.integer  "project_id"
@@ -10,8 +10,8 @@ class Factory
       new(name, opts).tap(&:save!)
     end
  
-    def new(name, opts)
-      factory = @factories[name]
+    def new(name, opts = {})
+      factory= @factories[name]
       factory[0].new.tap do |obj|
         factory[1].call(obj)
       end.tap do |obj|
@@ -14,8 +14,40 @@ describe Key do
     it { should respond_to :projects }
   end
  
-  it { Factory.create(:key,
-                      :user => Factory(:user)).should be_valid }
+  context "validation of uniqueness" do
+
+    context "as a deploy key" do
+      let(:project) { Factory.create(:project, path: 'alpha', code: 'alpha') }
+      let(:another_project) { Factory.create(:project, path: 'beta', code: 'beta') }
+
+      before do
+        deploy_key = Factory.create(:key, project: project)
+      end
+
+      it "does not accept the same key twice for a project" do
+        key = Factory.new(:key, project: project)
+        key.should_not be_valid
+      end
+
+      it "does accept the same key for another project" do
+        key = Factory.new(:key, project: another_project)
+        key.should be_valid
+      end
+    end
+
+    context "as a personal key" do
+      let(:user) { Factory.create(:user) }
+
+      it "accepts the key once" do
+        Factory.new(:key, user: user).should be_valid
+      end
+
+      it "does not accepts the key twice" do
+        Factory.create(:key, user: user)
+        Factory.new(:key, user: user).should_not be_valid
+      end
+    end
+  end
 end
 # == Schema Information
 #
	1	+require 'digest/md5'
	2	+
1	3	class Key < ActiveRecord::Base
2	4	belongs_to :user
3	5	belongs_to :project
...	...	@@ -8,17 +10,30 @@ class Key < ActiveRecord::Base
8	10
9	11	validates :key,
10	12	:presence => true,
11		- :uniqueness => true,
12	13	:length => { :within => 0..5000 }
13	14
14	15	before_save :set_identifier
	16	+ before_validation :strip_white_space
15	17	after_save :update_repository
16	18	after_destroy :repository_delete_key
17	19	delegate :name, :email, :to => :user, :prefix => true
	20	+ validate :unique_key
	21	+
	22	+ def strip_white_space
	23	+ self.key = self.key.strip unless self.key.blank?
	24	+ end
	25	+
	26	+ def unique_key
	27	+ query = Key.where('key = ?', key)
	28	+ query = query.where('(project_id IS NULL OR project_id = ?)', project_id) if project_id
	29	+ if (query.count > 0)
	30	+ errors.add :key, 'already exist.'
	31	+ end
	32	+ end
18	33
19	34	def set_identifier
20	35	if is_deploy_key
21		- self.identifier = "deploy_#{project.code}_#{Time.now.to_i}"
	36	+ self.identifier = "deploy_" + Digest::MD5.hexdigest(key)
22	37	else
23	38	self.identifier = "#{user.identifier}_#{Time.now.to_i}"
24	39	end
...	...	@@ -33,11 +48,14 @@ class Key < ActiveRecord::Base
33	48
34	49	def repository_delete_key
35	50	Gitlabhq::GitHost.system.new.configure do \|c\|
36		- c.delete_key(identifier)
	51	+ #delete key file is there is no identically deploy keys
	52	+ if !is_deploy_key \|\| Key.where(:identifier => identifier).count() == 0
	53	+ c.delete_key(identifier)
	54	+ end
37	55	c.update_projects(projects)
38	56	end
39	57	end
40		-
	58	+
41	59	def is_deploy_key
42	60	true if project_id
43	61	end
...	...
...	...	@@ -163,6 +163,13 @@ ActiveRecord::Schema.define(:version => 20120228134252) do
163	163	t.integer "project_access", :default => 0, :null => false
164	164	end
165	165
	166	+ create_table "web_hook_urls", :force => true do \|t\|
	167	+ t.string "url"
	168	+ t.integer "project_id"
	169	+ t.datetime "created_at"
	170	+ t.datetime "updated_at"
	171	+ end
	172	+
166	173	create_table "web_hooks", :force => true do \|t\|
167	174	t.string "url"
168	175	t.integer "project_id"
...	...
...	...	@@ -10,8 +10,8 @@ class Factory
10	10	new(name, opts).tap(&:save!)
11	11	end
12	12
13		- def new(name, opts)
14		- factory = @factories[name]
	13	+ def new(name, opts = {})
	14	+ factory= @factories[name]
15	15	factory[0].new.tap do \|obj\|
16	16	factory[1].call(obj)
17	17	end.tap do \|obj\|
...	...
...	...	@@ -14,8 +14,40 @@ describe Key do
14	14	it { should respond_to :projects }
15	15	end
16	16
17		- it { Factory.create(:key,
18		- :user => Factory(:user)).should be_valid }
	17	+ context "validation of uniqueness" do
	18	+
	19	+ context "as a deploy key" do
	20	+ let(:project) { Factory.create(:project, path: 'alpha', code: 'alpha') }
	21	+ let(:another_project) { Factory.create(:project, path: 'beta', code: 'beta') }
	22	+
	23	+ before do
	24	+ deploy_key = Factory.create(:key, project: project)
	25	+ end
	26	+
	27	+ it "does not accept the same key twice for a project" do
	28	+ key = Factory.new(:key, project: project)
	29	+ key.should_not be_valid
	30	+ end
	31	+
	32	+ it "does accept the same key for another project" do
	33	+ key = Factory.new(:key, project: another_project)
	34	+ key.should be_valid
	35	+ end
	36	+ end
	37	+
	38	+ context "as a personal key" do
	39	+ let(:user) { Factory.create(:user) }
	40	+
	41	+ it "accepts the key once" do
	42	+ Factory.new(:key, user: user).should be_valid
	43	+ end
	44	+
	45	+ it "does not accepts the key twice" do
	46	+ Factory.create(:key, user: user)
	47	+ Factory.new(:key, user: user).should_not be_valid
	48	+ end
	49	+ end
	50	+ end
19	51	end
20	52	# == Schema Information
21	53	#
...	...