From 33d2f3bd5a23e5b5e0181ed49974dbfcda86f3c8 Mon Sep 17 00:00:00 2001 From: Jenny Shen Date: Mon, 2 May 2022 22:37:07 +0300 Subject: [PATCH] Disable gem scope selector unless applicable scope is selected --- app/assets/javascripts/api_key_form.js | 36 ++++++++++++++ app/models/api_key.rb | 1 + test/integration/api_keys_test.rb | 69 +++++++++++++++++++++++--- 3 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 app/assets/javascripts/api_key_form.js diff --git a/app/assets/javascripts/api_key_form.js b/app/assets/javascripts/api_key_form.js new file mode 100644 index 00000000000..ddbbf5de90d --- /dev/null +++ b/app/assets/javascripts/api_key_form.js @@ -0,0 +1,36 @@ +$(function() { + var enableGemScopeCheckboxes = $("#push_rubygem, #yank_rubygem, #add_owner, #remove_owner"); + var hiddenRubygemId = "hidden_api_key_rubygem_id"; + toggleGemSelector(); + + enableGemScopeCheckboxes.click(function() { + toggleGemSelector(); + }); + + function toggleGemSelector() { + var isApplicableGemScopeSelected = enableGemScopeCheckboxes.is(":checked"); + var gemScopeSelector = $("#api_key_rubygem_id"); + + if (isApplicableGemScopeSelected) { + gemScopeSelector.removeAttr("disabled"); + removeHiddenRubygemField(); + } else { + gemScopeSelector.val(""); + gemScopeSelector.prop("disabled", true); + addHiddenRubygemField(); + } + } + + function addHiddenRubygemField() { + $("").attr({ + type: "hidden", + id: hiddenRubygemId, + name: "api_key[rubygem_id]", + value: "" + }).appendTo(".t-body form"); + } + + function removeHiddenRubygemField() { + $("#" + hiddenRubygemId + ":hidden").remove(); + } +}); diff --git a/app/models/api_key.rb b/app/models/api_key.rb index a1403b67265..14e0f4afad6 100644 --- a/app/models/api_key.rb +++ b/app/models/api_key.rb @@ -1,5 +1,6 @@ class ApiKey < ApplicationRecord API_SCOPES = %i[index_rubygems push_rubygem yank_rubygem add_owner remove_owner access_webhooks show_dashboard].freeze + APPLICABLE_GEM_API_SCOPES = %i[push_rubygem yank_rubygem add_owner remove_owner].freeze belongs_to :user has_one :api_key_rubygem_scope, dependent: :destroy diff --git a/test/integration/api_keys_test.rb b/test/integration/api_keys_test.rb index 5dfd964bbe4..488125c34a7 100644 --- a/test/integration/api_keys_test.rb +++ b/test/integration/api_keys_test.rb @@ -30,8 +30,8 @@ class ApiKeysTest < SystemTest visit_profile_api_keys_path fill_in "api_key[name]", with: "test" - check "api_key[index_rubygems]" - assert page.has_select? "api_key_rubygem_id", selected: nil + check "api_key[push_rubygem]" + assert page.has_select? "api_key_rubygem_id", selected: "All Gems" page.select @ownership.rubygem.name click_button "Create" @@ -39,12 +39,35 @@ class ApiKeysTest < SystemTest assert_equal @ownership.rubygem, @user.api_keys.last.rubygem end + (ApiKey::API_SCOPES - ApiKey::APPLICABLE_GEM_API_SCOPES).each do |scope| + test "creating new api key cannot set gem scope with #{scope} scope selected" do + visit_profile_api_keys_path + check "api_key[#{scope}]" + assert page.has_select? "api_key_rubygem_id", selected: "All Gems", disabled: true + end + end + + ApiKey::APPLICABLE_GEM_API_SCOPES.each do |scope| + test "creating new api key scoped to a gem with #{scope} scope" do + visit_profile_api_keys_path + fill_in "api_key[name]", with: "test" + check "api_key[#{scope}]" + + assert page.has_select? "api_key_rubygem_id", selected: "All Gems" + page.select @ownership.rubygem.name + click_button "Create" + + assert page.has_content? "Note that we won't be able to show the key to you again. New API key:" + assert_equal @ownership.rubygem, @user.api_keys.last.rubygem + end + end + test "creating new api key scoped to gem that the user does not own" do visit_profile_api_keys_path fill_in "api_key[name]", with: "test" - check "api_key[index_rubygems]" - assert page.has_select? "api_key_rubygem_id", selected: nil + check "api_key[push_rubygem]" + assert page.has_select? "api_key_rubygem_id", selected: "All Gems" page.select @ownership.rubygem.name @ownership.destroy! @@ -97,7 +120,7 @@ class ApiKeysTest < SystemTest end test "update api key gem scope" do - api_key = create(:api_key, user: @user, ownership: @ownership) + api_key = create(:api_key, push_rubygem: true, user: @user, ownership: @ownership) visit_profile_api_keys_path click_button "Edit" @@ -110,8 +133,40 @@ class ApiKeysTest < SystemTest assert_nil api_key.reload.rubygem end + test "update gem scoped api key with applicable scopes removed" do + api_key = create(:api_key, push_rubygem: true, user: @user, ownership: @ownership) + + visit_profile_api_keys_path + click_button "Edit" + + assert page.has_content? "Edit API key" + page.check "api_key[index_rubygems]" + page.uncheck "api_key[push_rubygem]" + assert page.has_select? "api_key_rubygem_id", selected: "All Gems", disabled: true + click_button "Update" + + assert_nil api_key.reload.rubygem + end + + test "update gem scoped api key to another applicable scope" do + api_key = create(:api_key, push_rubygem: true, user: @user, ownership: @ownership) + + visit_profile_api_keys_path + click_button "Edit" + + assert page.has_content? "Edit API key" + page.uncheck "api_key[push_rubygem]" + assert page.has_select? "api_key_rubygem_id", selected: "All Gems", disabled: true + + page.check "api_key[yank_rubygem]" + page.select @ownership.rubygem.name + click_button "Update" + + assert_equal api_key.reload.rubygem, @ownership.rubygem + end + test "update api key gem scope to a gem the user does not own" do - api_key = create(:api_key, user: @user, ownership: @ownership) + api_key = create(:api_key, push_rubygem: true, user: @user, ownership: @ownership) @another_ownership = create(:ownership, user: @user, rubygem: create(:rubygem, name: "another_gem")) visit_profile_api_keys_path @@ -184,7 +239,7 @@ class ApiKeysTest < SystemTest end test "gem ownership removed displays api key as invalid" do - api_key = create(:api_key, user: @user, ownership: @ownership) + api_key = create(:api_key, push_rubygem: true, user: @user, ownership: @ownership) visit_profile_api_keys_path refute page.has_css? ".owners__row__invalid"