summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaco Guzman <pacoguzmanp@gmail.com>2016-06-16 12:53:32 +0200
committerPaco Guzman <pacoguzmanp@gmail.com>2016-06-20 13:29:04 +0200
commitca01c4c6484aa135234028e5e1ca5829adad1a50 (patch)
treec8033b1de6db640c7fab80d2a8f7a800a9d943d8
parent98cede7ebeae9dac994b35b66be6aab14eb932b3 (diff)
downloadgitlab-ce-ca01c4c6484aa135234028e5e1ca5829adad1a50.tar.gz
Remove Duplicated keys add UNIQUE index to fingerprint18697-uniqueness-key-validation
-rw-r--r--CHANGELOG1
-rw-r--r--app/models/key.rb2
-rw-r--r--db/migrate/20160616102642_remove_duplicated_keys.rb19
-rw-r--r--db/migrate/20160616103005_remove_keys_fingerprint_index_if_exists.rb21
-rw-r--r--db/migrate/20160616103948_add_unique_index_to_keys_fingerprint.rb13
-rw-r--r--db/schema.rb3
-rw-r--r--spec/models/key_spec.rb2
7 files changed, 58 insertions, 3 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 44e6a194745..b5b224760a0 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -130,6 +130,7 @@ v 8.9.0 (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
diff --git a/app/models/key.rb b/app/models/key.rb
index 0532e84f47d..b9bc38a0436 100644
--- a/app/models/key.rb
+++ b/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' }
diff --git a/db/migrate/20160616102642_remove_duplicated_keys.rb b/db/migrate/20160616102642_remove_duplicated_keys.rb
new file mode 100644
index 00000000000..00a45d7fe73
--- /dev/null
+++ b/db/migrate/20160616102642_remove_duplicated_keys.rb
@@ -0,0 +1,19 @@
+# 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
diff --git a/db/migrate/20160616103005_remove_keys_fingerprint_index_if_exists.rb b/db/migrate/20160616103005_remove_keys_fingerprint_index_if_exists.rb
new file mode 100644
index 00000000000..4bb4204cebd
--- /dev/null
+++ b/db/migrate/20160616103005_remove_keys_fingerprint_index_if_exists.rb
@@ -0,0 +1,21 @@
+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
diff --git a/db/migrate/20160616103948_add_unique_index_to_keys_fingerprint.rb b/db/migrate/20160616103948_add_unique_index_to_keys_fingerprint.rb
new file mode 100644
index 00000000000..e35af38aac3
--- /dev/null
+++ b/db/migrate/20160616103948_add_unique_index_to_keys_fingerprint.rb
@@ -0,0 +1,13 @@
+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
diff --git a/db/schema.rb b/db/schema.rb
index 5a27e9d5cdc..4c1eb8cc1b3 100644
--- a/db/schema.rb
+++ b/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|
diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb
index 26fbedbef2f..49cf3d8633a 100644
--- a/spec/models/key_spec.rb
+++ b/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