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