Merge branch '18697-uniqueness-key-validation' into 'master'

Remove Duplicated keys adding UNIQUE index to fingerprint

See merge request !4787

CHANGELOG

@@ -137,6 +137,7 @@ v 8.8.5 (unreleased)
   - Update tanuki logo highlight/loading colors
   - Use Git cached counters for branches and tags on project page
   - Filter parameters for request_uri value on instrumented transactions.
+  - Remove duplicated keys add UNIQUE index to keys fingerprint column
   - Cache user todo counts from TodoService
   - Ensure Todos counters doesn't count Todos for projects pending delete
 

app/models/key.rb

@@ -9,7 +9,7 @@ class Key < ActiveRecord::Base
   before_validation :strip_white_space, :generate_fingerprint
 
   validates :title, presence: true, length: { within: 0..255 }
-  validates :key, presence: true, length: { within: 0..5000 }, format: { with: /\A(ssh|ecdsa)-.*\Z/ }, uniqueness: true
+  validates :key, presence: true, length: { within: 0..5000 }, format: { with: /\A(ssh|ecdsa)-.*\Z/ }
   validates :key, format: { without: /\n|\r/, message: 'should be a single line' }
   validates :fingerprint, uniqueness: true, presence: { message: 'cannot be generated' }
 

db/migrate/20160616102642_remove_duplicated_keys.rb

+# rubocop:disable all
+class RemoveDuplicatedKeys < ActiveRecord::Migration
+  def up
+    select_all("SELECT fingerprint FROM #{quote_table_name(:keys)} GROUP BY fingerprint HAVING COUNT(*) > 1").each do |row|
+      fingerprint = connection.quote(row['fingerprint'])
+      execute(%Q{
+        DELETE FROM keys
+        WHERE fingerprint = #{fingerprint}
+        AND id != (
+          SELECT id FROM (
+            SELECT max(id) AS id
+            FROM keys
+            WHERE fingerprint = #{fingerprint}
+          ) max_ids
+        )
+      })
+    end
+  end
+end

db/migrate/20160616103005_remove_keys_fingerprint_index_if_exists.rb

+class RemoveKeysFingerprintIndexIfExists < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  # https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/250
+  # That MR was added on gitlab-ee so we need to check if the index
+  # already exists because we want to do is create an unique index instead.
+
+  def up
+    if index_exists?(:keys, :fingerprint)
+      remove_index :keys, :fingerprint
+    end
+  end
+
+  def down
+    unless index_exists?(:keys, :fingerprint)
+      add_concurrent_index :keys, :fingerprint
+    end
+  end
+end

db/migrate/20160616103948_add_unique_index_to_keys_fingerprint.rb

+class AddUniqueIndexToKeysFingerprint < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :keys, :fingerprint, unique: true
+  end
+
+  def down
+    remove_index :keys, :fingerprint
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160616084004) do
+ActiveRecord::Schema.define(version: 20160616103948) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -507,6 +507,7 @@ ActiveRecord::Schema.define(version: 20160616084004) do
   end
 
   add_index "keys", ["created_at", "id"], name: "index_keys_on_created_at_and_id", using: :btree
+  add_index "keys", ["fingerprint"], name: "index_keys_on_fingerprint", unique: true, using: :btree
   add_index "keys", ["user_id"], name: "index_keys_on_user_id", using: :btree
 
   create_table "label_links", force: :cascade do |t|

spec/models/key_spec.rb

@@ -26,7 +26,7 @@ describe Key, models: true do
     end
   end
 
-  context "validation of uniqueness" do
+  context "validation of uniqueness (based on fingerprint uniqueness)" do
     let(:user) { create(:user) }
 
     it "accepts the key once" do