Merge branch 'miks-deploy_keys_nonunique'

Ariejan de Vroom
2 parents bfe0906f b0ce61c4
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
 #
	@@ -163,6 +163,13 @@ ActiveRecord::Schema.define(:version => 20120228134252) do		@@ -163,6 +163,13 @@ ActiveRecord::Schema.define(:version => 20120228134252) do
163	t.integer "project_access", :default => 0, :null => false	163	t.integer "project_access", :default => 0, :null => false
164	end	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	create_table "web_hooks", :force => true do \|t\|	173	create_table "web_hooks", :force => true do \|t\|
167	t.string "url"	174	t.string "url"
168	t.integer "project_id"	175	t.integer "project_id"
	@@ -14,8 +14,40 @@ describe Key do		@@ -14,8 +14,40 @@ describe Key do
14	it { should respond_to :projects }	14	it { should respond_to :projects }
15	end	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	end	51	end
20	# == Schema Information	52	# == Schema Information
21	#	53	#