FEATURE: Hash user API keys in the database (#9344)

The 'key' column will be dropped in a future commit.

app/controllers/user_api_keys_controller.rb

@@ -71,7 +71,6 @@ class UserApiKeysController < ApplicationController
       client_id: params[:client_id],
       user_id: current_user.id,
       push_url: params[:push_url],
-      key: SecureRandom.hex,
       scopes: scopes
     )
 
@@ -146,7 +145,7 @@ class UserApiKeysController < ApplicationController
     revoke_key = find_key if params[:id]
 
     if current_key = request.env['HTTP_USER_API_KEY']
-      request_key = UserApiKey.find_by(key: current_key)
+      request_key = UserApiKey.with_key(current_key).first
       revoke_key ||= request_key
       if request_key && request_key.id != revoke_key.id && !request_key.scopes.include?("write")
         raise Discourse::InvalidAccess

app/models/user_api_key.rb

@@ -20,6 +20,28 @@ class UserApiKey < ActiveRecord::Base
 
   belongs_to :user
 
+  scope :active, -> { where(revoked_at: nil) }
+  scope :with_key, ->(key) { where(key_hash: ApiKey.hash_key(key)) }
+
+  after_initialize :generate_key
+
+  def generate_key
+    if !self.key_hash
+      @key ||= SecureRandom.hex
+      self.key = @key
+      self.key_hash = ApiKey.hash_key(@key)
+    end
+  end
+
+  def key
+    raise ApiKey::KeyAccessError.new "API key is only accessible immediately after creation" unless key_available?
+    @key
+  end
+
+  def key_available?
+    @key.present?
+  end
+
   def self.allowed_scopes
     Set.new(SiteSetting.allow_user_api_key_scopes.split("|"))
   end
@@ -88,6 +110,7 @@ end
 #  revoked_at       :datetime
 #  scopes           :text             default([]), not null, is an Array
 #  last_used_at     :datetime         not null
+#  key_hash         :string           not null
 #
 # Indexes
 #

db/migrate/20200403100259_add_key_hash_to_user_api_key.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+class AddKeyHashToUserApiKey < ActiveRecord::Migration[6.0]
+  def up
+    add_column :user_api_keys, :key_hash, :string
+
+    batch_size = 500
+    loop do
+      rows = DB
+        .query("SELECT id, key FROM user_api_keys WHERE key_hash IS NULL LIMIT #{batch_size}")
+        .map { |row| { id: row.id, key_hash: Digest::SHA256.hexdigest(key) } }
+
+      break if rows.size == 0
+
+      data_string = rows.map { |r| "(#{r[:id]}, '#{r[:key_hash]}')" }.join(",")
+      execute <<~SQL
+        UPDATE user_api_keys
+        SET key_hash = data.key_hash
+        FROM (VALUES #{data_string}) AS data(id, key_hash)
+        WHERE user_api_keys.id = data.id
+      SQL
+
+      break if rows.size < batch_size
+    end
+
+    change_column_null :user_api_keys, :key_hash, false
+  end
+
+  def down
+    drop_column :user_api_keys, :key_hash, :string
+  end
+end

lib/auth/default_current_user_provider.rb

@@ -260,7 +260,7 @@ class Auth::DefaultCurrentUserProvider
   protected
 
   def lookup_user_api_user_and_update_key(user_api_key, client_id)
-    if api_key = UserApiKey.where(key: user_api_key, revoked_at: nil).includes(:user).first
+    if api_key = UserApiKey.active.with_key(user_api_key).includes(:user).first
       unless api_key.allow?(@env)
         raise Discourse::InvalidAccess
       end

spec/integration/user_api_keys_spec.rb

@@ -3,12 +3,12 @@
 require 'rails_helper'
 
 describe 'user api keys integration' do
-  fab!(:user_api_key) { Fabricate(:readonly_user_api_key) }
-
   it 'updates last used time on use' do
     freeze_time
 
+    user_api_key = Fabricate(:readonly_user_api_key)
     user_api_key.update_columns(last_used_at: 7.days.ago)
+
     get '/session/current.json', headers: {
       HTTP_USER_API_KEY: user_api_key.key,
     }

spec/requests/user_api_keys_controller_spec.rb

@@ -205,7 +205,7 @@ describe UserApiKeysController do
       expect(parsed["nonce"]).to eq(args[:nonce])
       expect(parsed["push"]).to eq(true)
 
-      api_key = UserApiKey.find_by(key: parsed["key"])
+      api_key = UserApiKey.with_key(parsed["key"]).first
 
       expect(api_key.user_id).to eq(user.id)
       expect(api_key.scopes.sort).to eq(["push", "message_bus", "notifications", "session_info", "one_time_password"].sort)
@@ -242,7 +242,7 @@ describe UserApiKeysController do
       encrypted = Base64.decode64(payload)
       key = OpenSSL::PKey::RSA.new(private_key)
       parsed = JSON.parse(key.private_decrypt(encrypted))
-      api_key = UserApiKey.find_by(key: parsed["key"])
+      api_key = UserApiKey.with_key(parsed["key"]).first
       expect(api_key.user_id).to eq(user.id)
     end
 
@@ -259,7 +259,7 @@ describe UserApiKeysController do
       encrypted = Base64.decode64(payload)
       key = OpenSSL::PKey::RSA.new(private_key)
       parsed = JSON.parse(key.private_decrypt(encrypted))
-      api_key = UserApiKey.find_by(key: parsed["key"])
+      api_key = UserApiKey.with_key(parsed["key"]).first
       expect(api_key.user_id).to eq(user.id)
 
     end