allow creating developer keys that only work against test clusters

Some production installations have a beta or test environment that is
refreshed with data from production, and is used as a place to test
integrations or new features. In that case, you may want to create
a developer key that only works against this test instance, which has
traditionally meant making the following tradeoff:
- Create the key in the test instance directly, which means it will be
  removed the next time the data is refreshed
- Create the key in production, which means the key works against the
  production instance as well

This new functionality allows the best of both worlds: create the key in
production for persistance, but only allow it to function against a test
cluster.

To enable test cluster functionality, you need a plugin that overrides
`ApplicationController.test_cluster?` to return appropriately for the
environment.

To see the functionality, you need to set:
  `Setting.set("dev_key_test_cluster_checks_enabled", true)`

closes PLAT-3392
[ci no-db-snapshot]

test plan:
- First ensure that all existing developer key functionality works and
no new functionality appears without any action taken
- Then set Setting.set("dev_key_test_cluster_checks_enabled", true), you
should see the new option available in the new dev key UI
- Create a key with and without the new option checked. Access tokens
from the key without it check should still work normally. Tokens from
the key with it checked should not work
- Now manually override `ApplicationController.test_cluster?` to be
true.
- Tokens from both keys should now work

Change-Id: I5bbb46782d19c26a7b703834aaa507b0cb10039a
Reviewed-on: https://gerrit.instructure.com/153035
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Weston Dransfield <wdransfield@instructure.com>
Tested-by: Jenkins
QA-Review: August Thornton <august@instructure.com>
Product-Review: Karl Lloyd <karl@instructure.com>
This commit is contained in:
Simon Williams 2018-06-07 15:33:55 -06:00 committed by August Thornton
parent 5c37a5a165
commit 87a5b6032f
13 changed files with 147 additions and 36 deletions

View File

@ -29,7 +29,10 @@ class DeveloperKeysController < ApplicationController
respond_to do |format|
format.html do
set_navigation
js_env(accountEndpoint: api_v1_account_developer_keys_path(@context))
js_env(
accountEndpoint: api_v1_account_developer_keys_path(@context),
enableTestClusterChecks: DeveloperKey.test_cluster_checks_enabled?
)
if use_new_dev_key_features?
render :index_react
@ -147,6 +150,7 @@ class DeveloperKeysController < ApplicationController
:redirect_uris,
:vendor_code,
:visible,
:test_cluster_only,
:require_scopes,
scopes: []
)

View File

@ -25,7 +25,7 @@ module Lti::Ims::AccessTokenHelper
def validate_access_token!
access_token.validate!
raise Lti::Oauth2::InvalidTokenError 'Developer Key is not active' if developer_key && !developer_key.active?
raise Lti::Oauth2::InvalidTokenError 'Developer Key is not active or available in this environment' if developer_key && !developer_key.usable?
rescue Lti::Oauth2::InvalidTokenError
raise
rescue StandardError => e

View File

@ -16,6 +16,7 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import Checkbox from '@instructure/ui-forms/lib/components/Checkbox'
import FormFieldGroup from '@instructure/ui-forms/lib/components/FormFieldGroup'
import Grid, {GridCol, GridRow} from '@instructure/ui-layout/lib/components/Grid'
import I18n from 'i18n!react_developer_keys'
@ -33,7 +34,8 @@ export default class DeveloperKeyFormFields extends React.Component {
const { developerKey } = props
this.state = {
requireScopes: developerKey && developerKey.require_scopes
requireScopes: developerKey && developerKey.require_scopes,
testClusterOnly: developerKey && developerKey.test_cluster_only
}
}
@ -45,6 +47,10 @@ export default class DeveloperKeyFormFields extends React.Component {
return this.state.requireScopes
}
get testClusterOnly () {
return this.state.testClusterOnly
}
setKeyFormRef = node => { this.keyFormRef = node }
fieldValue(field, defaultValue) {
@ -59,6 +65,23 @@ export default class DeveloperKeyFormFields extends React.Component {
this.setState({ requireScopes: !this.state.requireScopes })
}
handleTestClusterOnlyChange = () => {
this.setState({ testClusterOnly: !this.state.testClusterOnly })
}
renderTestClusterOnlyCheckbox() {
if (ENV.enableTestClusterChecks) {
return (
<Checkbox
label={I18n.t('Test Cluster Only')}
name="developer_key[test_cluster_only]"
checked={this.state.testClusterOnly}
onChange={this.handleTestClusterOnlyChange}
/>
)
}
}
render() {
return (
<form ref={this.setKeyFormRef}>
@ -107,6 +130,7 @@ export default class DeveloperKeyFormFields extends React.Component {
defaultValue={this.fieldValue('notes')}
resize="both"
/>
{this.renderTestClusterOnlyCheckbox()}
</FormFieldGroup>
</GridCol>
<GridCol width={8}>

View File

@ -52,6 +52,10 @@ export default class DeveloperKeyModal extends React.Component {
return this.newForm && this.newForm.requireScopes
}
get testClusterOnly () {
return this.newForm && this.newForm.testClusterOnly
}
submitForm = () => {
const method = this.developerKey() ? 'put' : 'post'
const formData = new FormData(this.submissionForm)
@ -70,6 +74,11 @@ export default class DeveloperKeyModal extends React.Component {
})
formData.append('developer_key[require_scopes]', true)
}
if (!this.testClusterOnly) {
formData.append('developer_key[test_cluster_only]', false)
}
this.props.store.dispatch(
this.props.actions.createOrEditDeveloperKey(formData, this.developerKeyUrl(), method)
)

View File

@ -88,13 +88,13 @@ class AccessToken < ActiveRecord::Base
def usable?(token_key = :crypted_token)
# true if
# developer key is active AND
# developer key is usable AND
# there is a user id AND
# its not expired OR Its a refresh token
# since you need a refresh token to
# refresh expired tokens
if !developer_key_id || cached_developer_key.try(:active?)
if !developer_key_id || cached_developer_key.try(:usable?)
# we are a stand alone token, or a token with an active developer key
# make sure we
# - have a user id

View File

@ -65,6 +65,12 @@ class DeveloperKey < ActiveRecord::Base
self.save
end
def usable?
return false if DeveloperKey.test_cluster_checks_enabled? &&
test_cluster_only? && !ApplicationController.test_cluster?
active?
end
def redirect_uri=(value)
super(value.presence)
end
@ -141,6 +147,10 @@ class DeveloperKey < ActiveRecord::Base
@sns
end
def test_cluster_checks_enabled?
Setting.get("dev_key_test_cluster_checks_enabled", nil).present?
end
def find_cached(id)
global_id = Shard.global_id_for(id)
MultiCache.fetch("developer_key/#{global_id}") do

View File

@ -0,0 +1,26 @@
#
# Copyright (C) 2018 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
class AddTestClusterOnlyToDeveloperKeys < ActiveRecord::Migration[5.1]
tag :predeploy
disable_ddl_transaction!
def change
# small enough table that we can set not null / default in line
add_column :developer_keys, :test_cluster_only, :boolean, null: false, default: false
end
end

View File

@ -31,12 +31,14 @@ module Api::V1::DeveloperKey
def developer_key_json(key, user, session, context, show_bindings=false, inherited: false)
context ||= Account.site_admin
account_binding = key.account_binding_for(context) if show_bindings.present?
keys_to_show = if inherited
keys_to_show = if inherited
INHERITED_DEVELOPER_KEY_JSON_ATTRS
else
DEVELOPER_KEY_JSON_ATTRS
end
keys_to_show += ['test_cluster_only'] if DeveloperKey.test_cluster_checks_enabled?
api_json(key, user, session, :only => keys_to_show).tap do |hash|
if (context.grants_right?(user, session, :manage_developer_keys) || user.try(:id) == key.user_id) && !inherited
hash['api_key'] = key.api_key

View File

@ -62,7 +62,7 @@ module Lti
tp = ToolProxy.where(guid: unverified_jwt[:sub], workflow_state: 'active').first
return nil unless tp.present?
developer_key = tp.product_family.developer_key
raise InvalidAuthJwt, "the Developer Key is not active" if developer_key.present? && !developer_key.active?
raise InvalidAuthJwt, "the Developer Key is not active or available in this environment" if developer_key.present? && !developer_key.usable?
ims_tool_proxy = IMS::LTI::Models::ToolProxy.from_json(tp.raw_data)
if (ims_tool_proxy.enabled_capabilities & ['Security.splitSecret', 'OAuth.splitSecret']).blank?
raise InvalidAuthJwt, "the Tool Proxy must be using a split secret"

View File

@ -75,7 +75,31 @@ describe DeveloperKeysController, type: :request do
format: 'json',
account_id: sa_id.to_s
})
end
it 'does not include `test_cluster_only` by default' do
admin_session
key = DeveloperKey.create!
json = api_call(:get, "/api/v1/accounts/#{sa_id}/developer_keys.json", {
controller: 'developer_keys',
action: 'index',
format: 'json',
account_id: sa_id.to_s
})
expect(json.first.keys).not_to be_include('test_cluster_only')
end
it 'does include `test_cluster_only` when enabled' do
Setting.set("dev_key_test_cluster_checks_enabled", true)
admin_session
key = DeveloperKey.create!
json = api_call(:get, "/api/v1/accounts/#{sa_id}/developer_keys.json", {
controller: 'developer_keys',
action: 'index',
format: 'json',
account_id: sa_id.to_s
})
expect(json.first.keys).to be_include('test_cluster_only')
end
describe 'developer key account bindings' do
@ -240,7 +264,6 @@ describe DeveloperKeysController, type: :request do
end
expect(json.include?(key_to_hash(key))).to be true
end
def key_to_hash(key)

View File

@ -19,6 +19,7 @@
import React from 'react'
import TestUtils from 'react-addons-test-utils'
import DeveloperKeyFormFields from 'jsx/developer_keys/NewKeyForm'
import fakeENV from 'helpers/fakeENV'
QUnit.module('NewKeyForm')
@ -38,69 +39,72 @@ const developerKey = {
user_id: '53532',
user_name: 'billy bob',
vendor_code: 'b3w9w9bf',
workflow_state: 'active'
workflow_state: 'active',
test_cluster_only: true
}
function formFieldInputs(devKey) {
function formFieldOfTypeAndName(devKey, fieldType, name) {
const component = TestUtils.renderIntoDocument(
<DeveloperKeyFormFields
availableScopes={{}}
availableScopesPending={false}
developerKey={devKey}
store={ {dispatch: () => {}} }
dispatch={ () => {} }
listDeveloperKeyScopesSet={ () => {} }
/>
)
return TestUtils.scryRenderedDOMComponentsWithTag(component, 'input')
}
function formFieldTextAreas(devKey) {
const component = TestUtils.renderIntoDocument(
<DeveloperKeyFormFields
availableScopes={{}}
availableScopesPending={false}
developerKey={devKey}
store={ {dispatch: () => {}} }
/>
)
return TestUtils.scryRenderedDOMComponentsWithTag(component, 'textarea')
return TestUtils.scryRenderedDOMComponentsWithTag(component, fieldType).
find((elem) => elem.name == `developer_key[${name}]`);
}
test('populates the key name', () => {
const [input] = formFieldInputs(developerKey)
const input = formFieldOfTypeAndName(developerKey, 'input', 'name')
equal(input.value, developerKey.name)
})
test('defaults name to "Unnamed Tool"', () => {
const [input] = formFieldInputs({id: 123})
const input = formFieldOfTypeAndName({id: 123}, 'input', 'name')
equal(input.value, 'Unnamed Tool')
})
test('populates the key owner email', () => {
const [, input] = formFieldInputs(developerKey)
const input = formFieldOfTypeAndName(developerKey, 'input', 'email')
equal(input.value, developerKey.email)
})
test('populates the key legacy redirect uri', () => {
const [, , input] = formFieldInputs(developerKey)
const input = formFieldOfTypeAndName(developerKey, 'input', 'redirect_uri')
equal(input.value, developerKey.redirect_uri)
})
test('populates the key redirect uris', () => {
const [input] = formFieldTextAreas(developerKey)
equal(input.value, developerKey.redirect_uris)
const textarea = formFieldOfTypeAndName(developerKey, 'textarea', 'redirect_uris')
equal(textarea.value, developerKey.redirect_uris)
})
test('populates the key vendor code', () => {
const [, , , input] = formFieldInputs(developerKey)
const input = formFieldOfTypeAndName(developerKey, 'input', 'vendor_code')
equal(input.value, developerKey.vendor_code)
})
test('populates the key icon URL', () => {
const [, , , , input] = formFieldInputs(developerKey)
const input = formFieldOfTypeAndName(developerKey, 'input', 'icon_url')
equal(input.value, developerKey.icon_url)
})
test('populates the key notes', () => {
const [, input] = formFieldTextAreas(developerKey)
equal(input.value, developerKey.notes)
const textarea = formFieldOfTypeAndName(developerKey, 'textarea', 'notes')
equal(textarea.value, developerKey.notes)
})
test('does not populates the key test_cluster_only without ENV set', () => {
const input = formFieldOfTypeAndName(developerKey, 'input', 'test_cluster_only')
equal(input, undefined)
})
test('populates the key test_cluster_only', () => {
fakeENV.setup({ enableTestClusterChecks: true })
const input = formFieldOfTypeAndName(developerKey, 'input', 'test_cluster_only')
equal(input.defaultChecked, developerKey.test_cluster_only)
fakeENV.teardown()
})

View File

@ -49,7 +49,8 @@ const developerKey = {
user_id: '53532',
user_name: 'billy bob',
vendor_code: 'b3w9w9bf',
workflow_state: 'active'
workflow_state: 'active',
test_cluster_only: false
}
const createDeveloperKeyState = {
@ -347,6 +348,7 @@ test('it sends the contents of the form saving', () => {
equal(sentFormData.get('developer_key[icon_url]'), developerKey.icon_url)
equal(sentFormData.get('developer_key[notes]'), developerKey.notes)
equal(sentFormData.get('developer_key[require_scopes]'), 'true')
equal(sentFormData.get('developer_key[test_cluster_only]'), 'false')
wrapper.unmount()
})

View File

@ -246,7 +246,14 @@ module Lti
it "requires an active developer_key" do
allow(dev_key).to receive(:active?).and_return false
expect { auth_validator.tool_proxy }.to raise_error Lti::Oauth2::AuthorizationValidator::InvalidAuthJwt,
"the Developer Key is not active"
"the Developer Key is not active or available in this environment"
end
it "requires a developer key that is usable for the cluster" do
allow(DeveloperKey).to receive(:test_cluster_checks_enabled?).and_return true
allow(dev_key).to receive(:test_cluster_only?).and_return true
expect { auth_validator.tool_proxy }.to raise_error Lti::Oauth2::AuthorizationValidator::InvalidAuthJwt,
"the Developer Key is not active or available in this environment"
end
end