Add route scope enforcement to application controller

fixes PLAT-3176
fixes PLAT-3179
fixes PLAT-3181
fixes PLAT-3177

Test plan:
* Create a DeveloperKey
* Create an AccessToken
* Ensure that everything can be accessed as normal
* Set require_scopes to true on the DeveloperKey
* Ensure that nothing can be accessed
* Add some scopes to the AccessToken from the list of available scopes
    TokenScopes::SCOPES
* Ensure that the endpoints associated with those requests work but that
  others don't
* Ensure that HEAD requests work for GET endpoints
* Ensure all api endpoints behave normally when scopes are not turned on
  for  developer key

Change-Id: I0e7c1758ae2d51743490f243cfa21714255c8109
Reviewed-on: https://gerrit.instructure.com/143026
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Reviewed-by: Nathan Mills <nathanm@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Karl Lloyd <karl@instructure.com>
This commit is contained in:
Andrew Butterfield 2018-03-08 12:46:40 -08:00
parent 61f102711f
commit 66844d2366
13 changed files with 296 additions and 3 deletions

View File

@ -1419,6 +1419,8 @@ class ApplicationController < ActionController::Base
when AuthenticationMethods::AccessTokenError when AuthenticationMethods::AccessTokenError
add_www_authenticate_header add_www_authenticate_header
data = { errors: [{message: 'Invalid access token.'}] } data = { errors: [{message: 'Invalid access token.'}] }
when AuthenticationMethods::AccessTokenScopeError
data = { errors: [{message: 'Insufficient scopes on access token.'}] }
when ActionController::ParameterMissing when ActionController::ParameterMissing
data = { errors: [{message: "#{exception.param} is missing"}] } data = { errors: [{message: "#{exception.param} is missing"}] }
when BasicLTI::BasicOutcomes::Unauthorized, when BasicLTI::BasicOutcomes::Unauthorized,

View File

@ -45,8 +45,11 @@ class AccessToken < ActiveRecord::Base
scope :not_deleted, -> { where(:workflow_state => "active") } scope :not_deleted, -> { where(:workflow_state => "active") }
TOKEN_SIZE = 64 TOKEN_SIZE = 64
OAUTH2_SCOPE_NAMESPACE = '/auth/' OAUTH2_SCOPE_NAMESPACE = '/auth/'.freeze
ALLOWED_SCOPES = ["#{OAUTH2_SCOPE_NAMESPACE}userinfo"] ALLOWED_SCOPES = [
"#{OAUTH2_SCOPE_NAMESPACE}userinfo",
*TokenScopes::SCOPES # this will need to change once we start capturing scopes on developer keys
].freeze
before_create :generate_token before_create :generate_token
before_create :generate_refresh_token before_create :generate_refresh_token
@ -203,7 +206,20 @@ class AccessToken < ActiveRecord::Base
end end
end end
#Scoped token convenience method def url_scopes_for_method(method)
re = /^url:#{method}\|/
scopes.select { |scope| re =~ scope }.map do |scope|
path = scope.split('|').last
# build up the scope matching regexp from the route path
path = path.gsub(/:[^\/\)]+/, '[^/]+') # handle dynamic segments /courses/:course_id -> /courses/[^/]+
path = path.gsub(/\*[^\/\)]+/, '.+') # handle glob segments /files/*path -> /files/.+
path = path.gsub(/\(/, '(?:').gsub(/\)/, '|)') # handle optional segments /files(/[^/]+) -> /files(?:/[^/]+|)
path = "#{path}(?:\.[^/]+|)" # handle format segments /files(.:format) -> /files(\.[^/]+|)
Regexp.new("^#{path}$")
end
end
# Scoped token convenience method
def scoped_to?(req_scopes) def scoped_to?(req_scopes)
self.class.scopes_match?(scopes, req_scopes) self.class.scopes_match?(scopes, req_scopes)
end end

View File

@ -30,6 +30,7 @@ class DeveloperKey < ActiveRecord::Base
has_many :developer_key_account_bindings, inverse_of: :developer_key has_many :developer_key_account_bindings, inverse_of: :developer_key
has_one :tool_consumer_profile, :class_name => 'Lti::ToolConsumerProfile' has_one :tool_consumer_profile, :class_name => 'Lti::ToolConsumerProfile'
serialize :scopes, Array
before_create :generate_api_key before_create :generate_api_key
before_create :set_auto_expire_tokens before_create :set_auto_expire_tokens

View File

@ -42,6 +42,7 @@ module CanvasRails
require 'logging_filter' require 'logging_filter'
config.filter_parameters.concat LoggingFilter.filtered_parameters config.filter_parameters.concat LoggingFilter.filtered_parameters
config.action_dispatch.rescue_responses['AuthenticationMethods::AccessTokenError'] = 401 config.action_dispatch.rescue_responses['AuthenticationMethods::AccessTokenError'] = 401
config.action_dispatch.rescue_responses['AuthenticationMethods::AccessTokenScopeError'] = 401
config.action_dispatch.rescue_responses['AuthenticationMethods::LoggedOutError'] = 401 config.action_dispatch.rescue_responses['AuthenticationMethods::LoggedOutError'] = 401
config.action_dispatch.default_headers['X-UA-Compatible'] = "IE=Edge,chrome=1" config.action_dispatch.default_headers['X-UA-Compatible'] = "IE=Edge,chrome=1"
config.action_dispatch.default_headers.delete('X-Frame-Options') config.action_dispatch.default_headers.delete('X-Frame-Options')

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 AddScopesAndRequireScopesToDeveloperKeys < ActiveRecord::Migration[5.0]
tag :predeploy
def change
add_column :developer_keys, :scopes, :text
add_column :developer_keys, :require_scopes, :boolean, default: false, null: false
end
end

View File

@ -35,6 +35,9 @@ module AuthenticationMethods
class AccessTokenError < Exception class AccessTokenError < Exception
end end
class AccessTokenScopeError < StandardError
end
class LoggedOutError < Exception class LoggedOutError < Exception
end end
@ -82,6 +85,22 @@ module AuthenticationMethods
end end
end end
def validate_scopes
if @access_token && @domain_root_account.feature_enabled?(:api_token_scoping)
developer_key = @access_token.developer_key
request_method = request.method.casecmp('HEAD') == 0 ? 'GET' : request.method.upcase
if developer_key.try(:require_scopes)
if @access_token.url_scopes_for_method(request_method).any? { |scope| scope =~ request.path }
params.delete :include
params.delete :includes
else
raise AccessTokenScopeError
end
end
end
end
def load_pseudonym_from_access_token def load_pseudonym_from_access_token
return unless api_request? || return unless api_request? ||
(params[:controller] == 'oauth2_provider' && params[:action] == 'destroy') || (params[:controller] == 'oauth2_provider' && params[:action] == 'destroy') ||
@ -104,6 +123,8 @@ module AuthenticationMethods
unless @current_user && @current_pseudonym unless @current_user && @current_pseudonym
raise AccessTokenError raise AccessTokenError
end end
validate_scopes
@access_token.used! @access_token.used!
RequestContextGenerator.add_meta_header('at', @access_token.global_id) RequestContextGenerator.add_meta_header('at', @access_token.global_id)

View File

@ -663,6 +663,13 @@ END
applies_to: 'Course', applies_to: 'Course',
state: 'hidden', state: 'hidden',
beta: true beta: true
},
'api_token_scoping' => {
display_name: -> { I18n.t('API Token Scoping')},
description: -> { I18n.t('If enabled, scopes will be validated on API requests if the developer key being used requires scopes.') },
applies_to: 'RootAccount',
state: 'hidden',
development: true
} }
) )

33
lib/token_scopes.rb Normal file
View File

@ -0,0 +1,33 @@
#
# 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 TokenScopes
def self.generate_scopes
api_routes = Rails.application.routes.routes.select { |route| /^\/api\/v1/ =~ route.path.spec.to_s }
api_route_hashes = api_routes.map { |route| { verb: route.verb, path: route.path.spec.to_s.gsub(/\(\.:format\)$/, '') } }
api_route_hashes += [
{ verb: 'GET', path: '/api/sis/accounts/:account_id/assignments' }.freeze,
{ verb: 'GET', path: '/api/sis/courses/:course_id/assignments' }.freeze,
{ verb: 'PUT', path: '/api/sis/courses/:course_id/disable_post_to_sis' }.freeze,
{ verb: 'GET', path: '/api/lti/courses/:course_id/membership_service' }.freeze,
{ verb: 'GET', path: '/api/lti/groups/:group_id/membership_service' }.freeze,
]
api_route_hashes.uniq.map { |route| "url:#{route[:verb]}|#{route[:path]}".freeze }
end
SCOPES = self.generate_scopes.freeze
end

View File

@ -33,6 +33,7 @@ describe ExternalToolsController, type: :request do
end end
it "should include allow_membership_service_access if feature flag enabled" do it "should include allow_membership_service_access if feature flag enabled" do
allow_any_instance_of(Account).to receive(:feature_enabled?).with(:api_token_scoping)
allow_any_instance_of(Account).to receive(:feature_enabled?).with(:membership_service_for_lti_tools).and_return(true) allow_any_instance_of(Account).to receive(:feature_enabled?).with(:membership_service_for_lti_tools).and_return(true)
et = tool_with_everything(@course, allow_membership_service_access: true) et = tool_with_everything(@course, allow_membership_service_access: true)
json = api_call(:get, "/api/v1/courses/#{@course.id}/external_tools/#{et.id}.json", json = api_call(:get, "/api/v1/courses/#{@course.id}/external_tools/#{et.id}.json",

View File

@ -20,6 +20,10 @@ require File.expand_path(File.dirname(__FILE__) + '/../api_spec_helper')
describe ScopesApiController, type: :request do describe ScopesApiController, type: :request do
describe "index" do describe "index" do
before :each do
allow_any_instance_of(Account).to receive(:feature_enabled?).with(:api_token_scoping)
end
context "with admin" do context "with admin" do
before :once do before :once do
@account = account_model @account = account_model

View File

@ -1157,4 +1157,112 @@ describe CoursesController do
expect(data['master_course_restrictions']).to eq({:content => true}) expect(data['master_course_restrictions']).to eq({:content => true})
end end
end end
context 'validate_scopes' do
let(:account_with_feature_enabled) do
account = double()
allow(account).to receive(:feature_enabled?).with(:api_token_scoping).and_return(true)
account
end
let(:account_with_feature_disabled) do
account = double()
allow(account).to receive(:feature_enabled?).with(:api_token_scoping).and_return(false)
account
end
context 'api_token_scoping feature enabled' do
before do
controller.instance_variable_set(:@domain_root_account, account_with_feature_enabled)
end
it 'does not affect session based api requests' do
allow(controller).to receive(:request).and_return(double({
params: {}
}))
expect(controller.send(:validate_scopes)).to be_nil
end
it 'does not affect api requests that use an access token with an unscoped developer key' do
user = user_model
developer_key = DeveloperKey.create!
token = AccessToken.create!(user: user, developer_key: developer_key)
controller.instance_variable_set(:@access_token, token)
allow(controller).to receive(:request).and_return(double({
params: {},
method: 'GET'
}))
expect(controller.send(:validate_scopes)).to be_nil
end
it 'raises AccessTokenScopeError if scopes do not match' do
user = user_model
developer_key = DeveloperKey.create!(require_scopes: true)
token = AccessToken.create!(user: user, developer_key: developer_key)
controller.instance_variable_set(:@access_token, token)
allow(controller).to receive(:request).and_return(double({
params: {},
method: 'GET'
}))
expect { controller.send(:validate_scopes) }.to raise_error(AuthenticationMethods::AccessTokenScopeError)
end
it 'allows adequately scoped requests through' do
user = user_model
developer_key = DeveloperKey.create!(require_scopes: true)
token = AccessToken.create!(user: user, developer_key: developer_key, scopes: ['url:GET|/api/v1/accounts'])
controller.instance_variable_set(:@access_token, token)
allow(controller).to receive(:request).and_return(double({
params: {},
method: 'GET',
path: '/api/v1/accounts'
}))
expect(controller.send(:validate_scopes)).to be_nil
end
it 'allows HEAD requests' do
user = user_model
developer_key = DeveloperKey.create!(require_scopes: true)
token = AccessToken.create!(user: user, developer_key: developer_key, scopes: ['url:GET|/api/v1/accounts'])
controller.instance_variable_set(:@access_token, token)
allow(controller).to receive(:request).and_return(double({
params: {},
method: 'HEAD',
path: '/api/v1/accounts'
}))
expect(controller.send(:validate_scopes)).to be_nil
end
it 'strips includes for adequately scoped requests' do
user = user_model
developer_key = DeveloperKey.create!(require_scopes: true)
token = AccessToken.create!(user: user, developer_key: developer_key, scopes: ['url:GET|/api/v1/accounts'])
controller.instance_variable_set(:@access_token, token)
allow(controller).to receive(:request).and_return(double({
method: 'GET',
path: '/api/v1/accounts'
}))
params = double()
expect(params).to receive(:delete).with(:include)
expect(params).to receive(:delete).with(:includes)
allow(controller).to receive(:params).and_return(params)
controller.send(:validate_scopes)
end
end
context 'api_token_scoping feature disabled' do
before do
controller.instance_variable_set(:@domain_root_account, account_with_feature_disabled)
end
after do
controller.instance_variable_set(:@domain_root_account, nil)
end
it "does nothing" do
expect(controller).not_to receive(:api_request?)
controller.send(:validate_scopes)
end
end
end
end end

View File

@ -0,0 +1,35 @@
#
# 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/>.
require File.expand_path('../spec_helper', File.dirname(__FILE__))
describe TokenScopes do
describe ".generate_scopes" do
it "formats the scopes with url:http_verb|api_path" do
TokenScopes.generate_scopes.sort.each do |scope|
expect(/^url:(?:GET|OPTIONS|POST|PUT|PATCH|DELETE)\|\/api\/.+/ =~ scope).not_to be_nil
end
end
it "does not include the optional format part of the route path" do
TokenScopes.generate_scopes.each do |scope|
expect(/\(\.:format\)/ =~ scope).to be_nil
end
end
end
end

View File

@ -199,6 +199,44 @@ describe AccessToken do
end end
end end
context "url scopes" do
let(:token) do
token = AccessToken.new
token.scopes = %w{
blah/scope
url:GET|/api/v1/accounts
url:POST|/api/v1/courses
url:PUT|/api/v1/courses/:id
url:DELETE|/api/v1/courses/:course_id/assignments/:id
}
token
end
it "returns regexes that correspond to the http method" do
expect(token.url_scopes_for_method('GET')).to match_array [/^\/api\/v1\/accounts(?:.[^\/]+|)$/]
end
it "accounts for format segments" do
token = AccessToken.new(scopes: %w{url:GET|/blah})
expect(token.url_scopes_for_method('GET')).to match_array [/^\/blah(?:.[^\/]+|)$/]
end
it "accounts for glob segments" do
token = AccessToken.new(scopes: %w{url:GET|/*blah})
expect(token.url_scopes_for_method('GET')).to match_array [/^\/.+(?:.[^\/]+|)$/]
end
it "accounts for dynamic segments" do
token = AccessToken.new(scopes: %w{url:GET|/courses/:id})
expect(token.url_scopes_for_method('GET')).to match_array [/^\/courses\/[^\/]+(?:.[^\/]+|)$/]
end
it "accounts for optional segments" do
token = AccessToken.new(scopes: %w{url:GET|/courses(/:course_id)(/*blah)})
expect(token.url_scopes_for_method('GET')).to match_array [/^\/courses(?:\/[^\/]+|)(?:\/.+|)(?:.[^\/]+|)$/]
end
end
describe "account scoped access" do describe "account scoped access" do
before :once do before :once do