prevent disabling default key

why: the default key should never be set to anything besides "on" and
     we discovered the hard way that we needed to guard for this on the
     backend. This commit doesn't make any changes to the front end and
     it's ok for the user to get a "Request failed with status code 500"
     toast message

closes: INTEROP-8632
flag=none

test plan:
- specs pass
- on Site Admin > Developer Keys, try to set the default key to "off"
  - you should see the toast message
  - verify that the key binding didn't change

Change-Id: I8443e3d05946dec75e4cd55efdf3d9aee77e267a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/354064
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Evan Battaglia <ebattaglia@instructure.com>
QA-Review: Evan Battaglia <ebattaglia@instructure.com>
Product-Review: Alexis Nast <alexis.nast@instructure.com>
This commit is contained in:
Steve McGee 2024-07-31 15:34:33 -06:00 committed by Steven McGee
parent ada60e861c
commit fe97567e11
6 changed files with 41 additions and 6 deletions

View File

@ -100,6 +100,8 @@ class DeveloperKey < ActiveRecord::Base
state :deleted
end
DEFAULT_KEY_NAME = "User-Generated"
# https://stackoverflow.com/a/2500819
alias_method :referenced_tool_configuration, :tool_configuration
@ -176,7 +178,7 @@ class DeveloperKey < ActiveRecord::Base
class << self
def default
get_special_key("User-Generated")
get_special_key(DeveloperKey::DEFAULT_KEY_NAME)
end
def get_special_key(default_key_name)

View File

@ -34,6 +34,8 @@ class DeveloperKeyAccountBinding < ApplicationRecord
validates :account, :developer_key, presence: true
before_save :set_root_account
before_create :enable_default_key
before_update :protect_default_key_binding
after_save :update_lti_registration_account_binding
after_update :clear_cache_if_site_admin
after_update :update_tools!
@ -121,6 +123,18 @@ class DeveloperKeyAccountBinding < ApplicationRecord
private
# DeveloperKey.default is for user-generated tokens and must always be ON
def enable_default_key
if developer_key.name == DeveloperKey::DEFAULT_KEY_NAME && developer_key == DeveloperKey.default
self.workflow_state = :on
end
end
# DeveloperKey.default is for user-generated tokens and must always be ON
def protect_default_key_binding
raise "Please don't turn off the default developer key" if developer_key.name == DeveloperKey::DEFAULT_KEY_NAME && developer_key == DeveloperKey.default && !on?
end
def update_lti_registration_account_binding
if skip_lime_sync
self.skip_lime_sync = false

View File

@ -865,21 +865,21 @@ describe "Student reports" do
"never",
"never",
DeveloperKey.default.id,
"User-Generated"],
DeveloperKey::DEFAULT_KEY_NAME],
[@user2.id,
"Bolton, Michael",
@at2.token_hint.gsub(/.+~/, ""),
@at2.permanent_expires_at.iso8601,
@at2.last_used_at.iso8601,
DeveloperKey.default.id,
"User-Generated"],
DeveloperKey::DEFAULT_KEY_NAME],
[@user1.id,
"Clair, John St.",
@at1.token_hint.gsub(/.+~/, ""),
@at1.permanent_expires_at.iso8601,
"never",
DeveloperKey.default.id,
"User-Generated"]
DeveloperKey::DEFAULT_KEY_NAME]
]
end

View File

@ -779,7 +779,7 @@ describe "Users API", type: :request do
json.each { |j| expect(j["url"]).to eq "http://www.example.com/courses/1" }
expect(json[0]["created_at"]).to be > json[1]["created_at"]
expect(json[0]["app_name"]).to be_nil
expect(json[1]["app_name"]).to eq "User-Generated"
expect(json[1]["app_name"]).to eq DeveloperKey::DEFAULT_KEY_NAME
expect(response.headers["Link"]).to match(/next/)
response.headers["Link"].split(",").find { |l| l =~ /<([^>]+)>.+next/ }
url = $1

View File

@ -345,7 +345,7 @@ describe AccessToken do
@at = AccessToken.create!(user: user_model, developer_key: @dk)
@dk_without_account = DeveloperKey.create!
@at_without_account = AccessToken.create!(user: user_model, developer_key: @dk2)
@at_without_account = AccessToken.create!(user: user_model, developer_key: @dk_without_account)
end
it "account should be set" do

View File

@ -34,6 +34,10 @@ RSpec.describe DeveloperKeyAccountBinding do
let(:root_account_key) { DeveloperKey.create!(account:, **params) }
let(:root_account_binding) { root_account_key.developer_key_account_bindings.first }
before do
DeveloperKey.default # create default before any other needed keys
end
describe "validations and callbacks" do
it "requires an account" do
dev_key_binding.account = nil
@ -75,6 +79,21 @@ RSpec.describe DeveloperKeyAccountBinding do
expect(lrab.updated_by).to eq(user2)
end
context "for default key" do
let(:developer_key) { DeveloperKey.default }
let(:account) { Account.site_admin }
it "does not allow default key to be set to off" do
binding = DeveloperKeyAccountBinding.where(developer_key_id: developer_key.id).first
expect { binding.update!(workflow_state: "off") }.to raise_error("Please don't turn off the default developer key")
end
it "does not allow default key to be set to allow" do
binding = DeveloperKeyAccountBinding.where(developer_key_id: developer_key.id).first
expect { binding.update!(workflow_state: "allow") }.to raise_error("Please don't turn off the default developer key")
end
end
describe "workflow state" do
it 'allows "off"' do
dev_key_binding.workflow_state = "off"