From a6707fbfa2afc4a8561414db7af67546af40c448 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 14 Feb 2021 04:06:39 -0600 Subject: [PATCH] api keys: allow users to have multiple API keys. This is useful if you have multiple programs and want to give them different API keys, or if you want to rotate keys for a single program. --- app/models/api_key.rb | 1 - app/models/user.rb | 5 +++-- ...21_remove_uniqueness_constraint_from_api_keys.rb | 6 ++++++ db/structure.sql | 5 +++-- test/functional/api_keys_controller_test.rb | 6 +++--- test/functional/application_controller_test.rb | 13 +++++++++++++ test/unit/api_key_test.rb | 2 +- 7 files changed, 29 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20210214095121_remove_uniqueness_constraint_from_api_keys.rb diff --git a/app/models/api_key.rb b/app/models/api_key.rb index 869051d4f..61621ac5b 100644 --- a/app/models/api_key.rb +++ b/app/models/api_key.rb @@ -1,6 +1,5 @@ class ApiKey < ApplicationRecord belongs_to :user - validates_uniqueness_of :user_id validates_uniqueness_of :key has_secure_token :key diff --git a/app/models/user.rb b/app/models/user.rb index 8b93c5f1a..a9fcb7707 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -134,9 +134,9 @@ class User < ApplicationRecord has_many :user_events, dependent: :destroy has_one :recent_ban, -> {order("bans.id desc")}, :class_name => "Ban" - has_one :api_key has_one :token_bucket has_one :email_address, dependent: :destroy + has_many :api_keys, dependent: :destroy has_many :note_versions, :foreign_key => "updater_id" has_many :dmails, -> {order("dmails.id desc")}, :foreign_key => "owner_id" has_many :saved_searches @@ -208,6 +208,7 @@ class User < ApplicationRecord end def authenticate_api_key(key) + api_key = api_keys.find_by(key: key) api_key.present? && ActiveSupport::SecurityUtils.secure_compare(api_key.key, key) && self end @@ -560,7 +561,7 @@ class User < ApplicationRecord end def api_token - api_key.try(:key) + api_keys.first.try(:key) end end diff --git a/db/migrate/20210214095121_remove_uniqueness_constraint_from_api_keys.rb b/db/migrate/20210214095121_remove_uniqueness_constraint_from_api_keys.rb new file mode 100644 index 000000000..61d325896 --- /dev/null +++ b/db/migrate/20210214095121_remove_uniqueness_constraint_from_api_keys.rb @@ -0,0 +1,6 @@ +class RemoveUniquenessConstraintFromApiKeys < ActiveRecord::Migration[6.1] + def change + remove_index :api_keys, :user_id, unique: true + add_index :api_keys, :user_id, unique: false + end +end diff --git a/db/structure.sql b/db/structure.sql index 8b19ec044..a82f535b0 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4856,7 +4856,7 @@ CREATE UNIQUE INDEX index_api_keys_on_key ON public.api_keys USING btree (key); -- Name: index_api_keys_on_user_id; Type: INDEX; Schema: public; Owner: - -- -CREATE UNIQUE INDEX index_api_keys_on_user_id ON public.api_keys USING btree (user_id); +CREATE INDEX index_api_keys_on_user_id ON public.api_keys USING btree (user_id); -- @@ -7956,6 +7956,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20210115015308'), ('20210123112752'), ('20210127000201'), -('20210127012303'); +('20210127012303'), +('20210214095121'); diff --git a/test/functional/api_keys_controller_test.rb b/test/functional/api_keys_controller_test.rb index 38be2a8cb..b433be149 100644 --- a/test/functional/api_keys_controller_test.rb +++ b/test/functional/api_keys_controller_test.rb @@ -45,7 +45,7 @@ class ApiKeysControllerTest < ActionDispatch::IntegrationTest post_auth user_api_keys_path(@user.id), @user assert_redirected_to user_api_keys_path(@user.id) - assert_equal(true, @user.api_key.present?) + assert_equal(true, @user.api_keys.last.present?) end end @@ -58,14 +58,14 @@ class ApiKeysControllerTest < ActionDispatch::IntegrationTest delete_auth api_key_path(@api_key.id), @user assert_redirected_to user_api_keys_path(@user.id) - assert_nil(@user.reload.api_key) + assert_raise(ActiveRecord::RecordNotFound) { @api_key.reload } end should "not allow deleting another user's API key" do delete_auth api_key_path(@api_key.id), create(:user) assert_response 403 - assert_not_nil(@user.reload.api_key) + assert_not_nil(@api_key.reload) end end end diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb index 9792d42ed..8b39e994c 100644 --- a/test/functional/application_controller_test.rb +++ b/test/functional/application_controller_test.rb @@ -55,6 +55,13 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest assert_response :success end + should "succeed when the user has multiple api keys" do + @api_key2 = create(:api_key, user: @user) + basic_auth_string = "Basic #{::Base64.encode64("#{@user.name}:#{@api_key2.key}")}" + get edit_user_path(@user), headers: { HTTP_AUTHORIZATION: basic_auth_string } + assert_response :success + end + should "fail for api key mismatches" do basic_auth_string = "Basic #{::Base64.encode64("#{@user.name}:badpassword")}" get profile_path, as: :json, headers: { HTTP_AUTHORIZATION: basic_auth_string } @@ -76,6 +83,12 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest assert_response :success end + should "succeed when the user has multiple api keys" do + @api_key2 = create(:api_key, user: @user) + get edit_user_path(@user), params: { login: @user.name, api_key: @api_key2.key } + assert_response :success + end + should "fail for api key mismatches" do get profile_path, as: :json, params: { login: @user.name } assert_response 401 diff --git a/test/unit/api_key_test.rb b/test/unit/api_key_test.rb index 98e237f1e..6122114e9 100644 --- a/test/unit/api_key_test.rb +++ b/test/unit/api_key_test.rb @@ -25,7 +25,7 @@ class ApiKeyTest < ActiveSupport::TestCase should "have the same limits whether or not they have an api key" do assert_no_difference(["@user.reload.api_regen_multiplier", "@user.reload.api_burst_limit"]) do - @user.api_key.destroy + @api_key.destroy end end end