make access token for real_user

access tokens cannot be created when masquerading a user on purpose,
because this would allow a user to get the real token and use it when
permissions for the user could change in the future. A commit was made
ee50eec4bd to create the access tokens
used when doing an lti launch on the real_user instead of the user, but
this breaks some tools that are not handling all the masquerade data.
c94b34348a reverted that change to create
them on the user again.

This commit is adding a column to access_token so we can audit usage of
the tokens created from an LTI launch. When a token is created while
masquerading we add the real_user_id to the token and make the token
expire in one hour.

test plan
 - masquerade as a user
 - launch an lti_tool that creates an access token
 - the tool should see the end users token
 - in a console verify the token is set to expire in an hour
 - verify that real_user_id is used on the token
 - the token should expire within an hour

fixes KNO-464
flag=none

Change-Id: I1f8913fc536f4e2c8539551efed69b27fbdb6b1a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/236443
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Davis Hyer <dhyer@instructure.com>
QA-Review: Davis Hyer <dhyer@instructure.com>
Product-Review: Davis Hyer <dhyer@instructure.com>
This commit is contained in:
Rob Orton 2020-05-06 11:21:54 -06:00
parent fc7ee5e5b3
commit 56f97d0f12
7 changed files with 72 additions and 23 deletions

View File

@ -26,7 +26,8 @@ class AccessToken < ActiveRecord::Base
attr_reader :full_token
attr_reader :plaintext_refresh_token
belongs_to :developer_key, counter_cache: :access_token_count
belongs_to :user
belongs_to :user, inverse_of: :access_tokens
belongs_to :real_user, inverse_of: :masquerade_tokens, class_name: 'User'
has_one :account, through: :developer_key
serialize :scopes, Array

View File

@ -100,9 +100,11 @@ class User < ActiveRecord::Base
has_many :current_groups, :through => :current_group_memberships, :source => :group
has_many :user_account_associations
has_many :associated_accounts, -> { order("user_account_associations.depth") }, source: :account, through: :user_account_associations
has_many :associated_root_accounts, -> { order("user_account_associations.depth").where(accounts: { parent_account_id: nil }) }, source: :account, through: :user_account_associations
has_many :associated_root_accounts, -> { order("user_account_associations.depth").
where(accounts: { parent_account_id: nil }) }, source: :account, through: :user_account_associations
has_many :developer_keys
has_many :access_tokens, -> { where(:workflow_state => "active").preload(:developer_key) }
has_many :access_tokens, -> { where(:workflow_state => "active").preload(:developer_key) }, inverse_of: :user
has_many :masquerade_tokens, -> { where(:workflow_state => "active").preload(:developer_key) }, class_name: 'AccessToken', inverse_of: :real_user
has_many :notification_endpoints, :through => :access_tokens
has_many :context_external_tools, -> { order(:name) }, as: :context, inverse_of: :context, dependent: :destroy
has_many :lti_results, inverse_of: :user, class_name: 'Lti::Result', dependent: :destroy

View File

@ -0,0 +1,32 @@
#
# Copyright (C) 2020 - 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 AddRealUserIdToAccessToken < ActiveRecord::Migration[5.2]
include MigrationHelpers::AddColumnAndFk
tag :predeploy
disable_ddl_transaction!
def up
add_column_and_fk :access_tokens, :real_user_id, :users
add_index :access_tokens, :real_user_id, algorithm: :concurrently, where: "real_user_id IS NOT NULL"
end
def down
remove_column :access_tokens, :real_user_id
end
end

View File

@ -138,6 +138,8 @@ module AuthenticationMethods
raise AccessTokenError unless @access_token.authorized_for_account?(account)
@current_user = @access_token.user
@real_current_user = @access_token.real_user
@real_current_pseudonym = SisPseudonym.for(@real_current_user, @domain_root_account, type: :implicit, require_sis: false) if @real_current_user
@current_pseudonym = SisPseudonym.for(@current_user, @domain_root_account, type: :implicit, require_sis: false)
unless @current_user && @current_pseudonym

View File

@ -64,13 +64,13 @@ module Canvas::Oauth
# user permission again. If the developer key is trusted, access
# tokens will be automatically authorized without prompting the end-
# user
def authorized_token?(user)
if !self.class.is_oob?(redirect_uri)
return true if Token.find_reusable_access_token(user, key, scopes, purpose)
def authorized_token?(user, real_user: nil)
unless self.class.is_oob?(redirect_uri)
return true if Token.find_reusable_access_token(user, key, scopes, purpose, real_user: real_user)
return true if key.trusted?
end
return false
false
end
def token_for(code)
@ -109,7 +109,7 @@ module Canvas::Oauth
def self.confirmation_redirect(controller, provider, current_user, real_user=nil)
# skip the confirmation page if access is already (or automatically) granted
if provider.authorized_token?(current_user)
if provider.authorized_token?(current_user, real_user: real_user)
final_redirect(controller, final_redirect_params(controller.session[:oauth2], current_user, real_user))
else
controller.oauth2_auth_confirm_url

View File

@ -78,19 +78,22 @@ module Canvas::Oauth
end
def create_access_token_if_needed(replace_tokens = false)
@access_token ||= self.class.find_reusable_access_token(user, key, scopes, purpose)
@access_token ||= self.class.find_reusable_access_token(user, key, scopes, purpose, real_user: real_user)
if @access_token.nil?
# Clear other tokens issued under the same developer key if requested
user.access_tokens.where(developer_key_id: key).destroy_all if replace_tokens || key.replace_tokens
# Then create a new one
@access_token = user.access_tokens.create!({
:developer_key => key,
:remember_access => remember_access?,
:scopes => scopes,
:purpose => purpose
})
@access_token = user.access_tokens.new({
developer_key: key,
remember_access: remember_access?,
scopes: scopes,
purpose: purpose
})
@access_token.real_user = real_user if real_user && real_user != user
@access_token.save!
@access_token.clear_full_token! if @access_token.scoped_to?(['userinfo'])
@access_token.clear_plaintext_refresh_token! if @access_token.scoped_to?(['userinfo'])
@ -102,11 +105,11 @@ module Canvas::Oauth
@access_token
end
def self.find_reusable_access_token(user, key, scopes, purpose)
def self.find_reusable_access_token(user, key, scopes, purpose, real_user: nil)
if key.force_token_reuse
find_access_token(user, key, scopes, purpose)
find_access_token(user, key, scopes, purpose, real_user: real_user)
elsif AccessToken.scopes_match?(scopes, ["userinfo"])
find_userinfo_access_token(user, key, purpose)
find_userinfo_access_token(user, key, purpose, real_user: real_user)
end
end
@ -138,13 +141,14 @@ module Canvas::Oauth
json
end
def self.find_userinfo_access_token(user, developer_key, purpose)
find_access_token(user, developer_key, ["userinfo"], purpose, {remember_access: true})
def self.find_userinfo_access_token(user, developer_key, purpose, real_user: nil)
find_access_token(user, developer_key, ["userinfo"], purpose, { remember_access: true }, real_user: real_user)
end
def self.find_access_token(user, developer_key, scopes, purpose, conditions = {})
def self.find_access_token(user, developer_key, scopes, purpose, conditions = {}, real_user: nil)
user.shard.activate do
user.access_tokens.active.where({:developer_key_id => developer_key, :purpose => purpose}.merge(conditions)).detect do |token|
real_user = nil if real_user == user
user.access_tokens.active.where({ developer_key_id: developer_key, purpose: purpose, real_user: real_user }.merge(conditions)).detect do |token|
token.scoped_to?(scopes)
end
end

View File

@ -199,13 +199,14 @@ module Canvas::Oauth
end
it 'puts real_user in the json when masquerading' do
real_user = User.new
real_user = User.create!
allow(token).to receive(:real_user).and_return(real_user)
expect(json['real_user']).to eq({
'id' => real_user.id,
'name' => real_user.name,
'global_id' => real_user.global_id.to_s
})
expect(user.access_tokens.where(real_user: real_user).count).to eq 1
end
it 'does not put real_user in the json when not masquerading' do
@ -255,6 +256,13 @@ module Canvas::Oauth
expect(token.access_token.expires_at).to be_nil
end
it 'gives short expiration for real_users' do
real_user = User.create!
token2 = Token.new(key, 'real_user_code')
allow(token2).to receive(:real_user).and_return(real_user)
expect(token.access_token.expires_at).to be <= 1.hour.from_now
end
it 'Tokens wont expire if the dev key has auto_expire_tokens set to false' do
allow(DateTime).to receive(:now).and_return(Time.zone.parse('2015-06-29T23:01:00Z'))
key = token.key