Merge branch '63502-encrypt-deploy-token' into 'master'

Resolve "Store deploy tokens encrypted"

Closes #63502

See merge request gitlab-org/gitlab-ce!30679

10 files changed, 125 insertions, 7 deletions

diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb
index 33f0be91632..85f5a2040c0 100644
--- a/app/models/deploy_token.rb
+++ b/app/models/deploy_token.rb
@@ -5,7 +5,7 @@ class DeployToken < ApplicationRecord
   include TokenAuthenticatable
   include PolicyActor
   include Gitlab::Utils::StrongMemoize
-  add_authentication_token_field :token
+  add_authentication_token_field :token, encrypted: :optional
 
   AVAILABLE_SCOPES = %i(read_repository read_registry).freeze
   GITLAB_DEPLOY_TOKEN_NAME = 'gitlab-deploy-token'.freeze
diff --git a/changelogs/unreleased/63502-encrypt-deploy-token.yml b/changelogs/unreleased/63502-encrypt-deploy-token.yml
new file mode 100644
index 00000000000..81ce1e6c3dd
--- /dev/null
+++ b/changelogs/unreleased/63502-encrypt-deploy-token.yml
@@ -0,0 +1,5 @@
+---
+title: Encrypt existing and new deploy tokens
+merge_request: 30679
+author:
+type: other
diff --git a/db/migrate/20190711200053_change_deploy_tokens_token_not_null.rb b/db/migrate/20190711200053_change_deploy_tokens_token_not_null.rb
new file mode 100644
index 00000000000..14ccf544d0b
--- /dev/null
+++ b/db/migrate/20190711200053_change_deploy_tokens_token_not_null.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class ChangeDeployTokensTokenNotNull < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    change_column_null :deploy_tokens, :token, true
+  end
+end
diff --git a/db/migrate/20190711200508_add_token_encrypted_to_deploy_tokens.rb b/db/migrate/20190711200508_add_token_encrypted_to_deploy_tokens.rb
new file mode 100644
index 00000000000..ea0956fdf7f
--- /dev/null
+++ b/db/migrate/20190711200508_add_token_encrypted_to_deploy_tokens.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class AddTokenEncryptedToDeployTokens < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :deploy_tokens, :token_encrypted, :string, limit: 255
+  end
+end
diff --git a/db/migrate/20190719174505_add_index_to_deploy_tokens_token_encrypted.rb b/db/migrate/20190719174505_add_index_to_deploy_tokens_token_encrypted.rb
new file mode 100644
index 00000000000..d58d1d8348c
--- /dev/null
+++ b/db/migrate/20190719174505_add_index_to_deploy_tokens_token_encrypted.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+class AddIndexToDeployTokensTokenEncrypted < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :deploy_tokens, :token_encrypted, unique: true, name: "index_deploy_tokens_on_token_encrypted"
+  end
+
+  def down
+    remove_concurrent_index_by_name :deploy_tokens, "index_deploy_tokens_on_token_encrypted"
+  end
+end
diff --git a/db/post_migrate/20190711201818_encrypt_deploy_tokens_tokens.rb b/db/post_migrate/20190711201818_encrypt_deploy_tokens_tokens.rb
new file mode 100644
index 00000000000..2eb8d1ee11c
--- /dev/null
+++ b/db/post_migrate/20190711201818_encrypt_deploy_tokens_tokens.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+class EncryptDeployTokensTokens < ActiveRecord::Migration[5.1]
+  DOWNTIME = false
+
+  class DeploymentTokens < ActiveRecord::Base
+    self.table_name = 'deploy_tokens'
+  end
+
+  def up
+    say_with_time("Encrypting tokens from deploy_tokens") do
+      DeploymentTokens.where('token_encrypted is NULL AND token IS NOT NULL').find_each(batch_size: 10000) do |deploy_token|
+        token_encrypted = Gitlab::CryptoHelper.aes256_gcm_encrypt(deploy_token.token)
+        deploy_token.update!(token_encrypted: token_encrypted)
+      end
+    end
+  end
+
+  def down
+    say_with_time("Decrypting tokens from deploy_tokens") do
+      DeploymentTokens.where('token_encrypted IS NOT NULL AND token IS NULL').find_each(batch_size: 10000) do |deploy_token|
+        token = Gitlab::CryptoHelper.aes256_gcm_decrypt(deploy_token.token_encrypted)
+        deploy_token.update!(token: token)
+      end
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 36bf2371994..f30dad3d030 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1121,10 +1121,12 @@ ActiveRecord::Schema.define(version: 2019_08_20_163320) do
     t.datetime_with_timezone "expires_at", null: false
     t.datetime_with_timezone "created_at", null: false
     t.string "name", null: false
-    t.string "token", null: false
+    t.string "token"
     t.string "username"
+    t.string "token_encrypted", limit: 255
     t.index ["token", "expires_at", "id"], name: "index_deploy_tokens_on_token_and_expires_at_and_id", where: "(revoked IS FALSE)"
     t.index ["token"], name: "index_deploy_tokens_on_token", unique: true
+    t.index ["token_encrypted"], name: "index_deploy_tokens_on_token_encrypted", unique: true
   end
 
   create_table "deployments", id: :serial, force: :cascade do |t|
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index e17a096ef19..6769bd95c2b 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -198,12 +198,10 @@ module Gitlab
         end.uniq
       end
 
-      # rubocop: disable CodeReuse/ActiveRecord
       def deploy_token_check(login, password)
         return unless password.present?
 
-        token =
-          DeployToken.active.find_by(token: password)
+        token = DeployToken.active.find_by_token(password)
 
         return unless token && login
         return if login != token.username
@@ -214,7 +212,6 @@ module Gitlab
           Gitlab::Auth::Result.new(token, token.project, :deploy_token, scopes)
         end
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       def lfs_token_check(login, encoded_token, project)
         deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/)
diff --git a/spec/factories/deploy_tokens.rb b/spec/factories/deploy_tokens.rb
index a96258f5cbe..99486acc2ab 100644
--- a/spec/factories/deploy_tokens.rb
+++ b/spec/factories/deploy_tokens.rb
@@ -2,7 +2,8 @@
 
 FactoryBot.define do
   factory :deploy_token do
-    token { SecureRandom.hex(50) }
+    token nil
+    token_encrypted { Gitlab::CryptoHelper.aes256_gcm_encrypt( SecureRandom.hex(50) ) }
     sequence(:name) { |n| "PDT #{n}" }
     read_repository true
     read_registry true
diff --git a/spec/migrations/encrypt_deploy_tokens_tokens_spec.rb b/spec/migrations/encrypt_deploy_tokens_tokens_spec.rb
new file mode 100644
index 00000000000..a398e079731
--- /dev/null
+++ b/spec/migrations/encrypt_deploy_tokens_tokens_spec.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require Rails.root.join('db', 'post_migrate', '20190711201818_encrypt_deploy_tokens_tokens.rb')
+
+describe EncryptDeployTokensTokens, :migration do
+  let(:migration) { described_class.new }
+  let(:deployment_tokens) { table(:deploy_tokens) }
+  let(:plaintext) { "secret-token" }
+  let(:expires_at) { DateTime.now + 1.year }
+  let(:ciphertext) { Gitlab::CryptoHelper.aes256_gcm_encrypt(plaintext) }
+
+  describe '#up' do
+    it 'keeps plaintext token the same and populates token_encrypted if not present' do
+      deploy_token = deployment_tokens.create!(
+        name: 'test_token',
+        read_repository: true,
+        expires_at: expires_at,
+        username: 'gitlab-token-1',
+        token: plaintext
+      )
+
+      migration.up
+
+      expect(deploy_token.reload.token).to eq(plaintext)
+      expect(deploy_token.reload.token_encrypted).to eq(ciphertext)
+    end
+  end
+
+  describe '#down' do
+    it 'decrypts encrypted token and saves it' do
+      deploy_token = deployment_tokens.create!(
+        name: 'test_token',
+        read_repository: true,
+        expires_at: expires_at,
+        username: 'gitlab-token-1',
+        token_encrypted: ciphertext
+      )
+
+      migration.down
+
+      expect(deploy_token.reload.token).to eq(plaintext)
+      expect(deploy_token.reload.token_encrypted).to eq(ciphertext)
+    end
+  end
+end