Clear other access tokens under the same key when requested
Fixes PLAT-991 Test plan: - Run through an OAuth flow in the usual manner with a given developer key - Run through it again, this time giving replace_tokens=1 to the call to /login/oauth2/token - Ensure that the old token was nuked and only the new one remains - Set replace_tokens to true on the developer key you're using - Run through the OAuth flow again, without giving replace_tokens=1 to /login/oauth2/token, and ensure that the old token was once again nuked Change-Id: Id2b40bfd16918ab3d0852dabc13ed7b500277dc9 Reviewed-on: https://gerrit.instructure.com/52286 Reviewed-by: Brad Humphrey <brad@instructure.com> QA-Review: August Thornton <august@instructure.com> Tested-by: Jenkins Product-Review: Alex Boyd <aboyd@instructure.com>
This commit is contained in:
parent
0c4a819f15
commit
a52af1503f
|
@ -76,6 +76,8 @@ class Oauth2ProviderController < ApplicationController
|
|||
token = provider.token_for(params[:code])
|
||||
return render(:status => 400, :json => { :message => "invalid code" }) unless token.is_for_valid_code?
|
||||
|
||||
token.create_access_token_if_needed(value_to_boolean(params[:replace_tokens]))
|
||||
|
||||
Canvas::Oauth::Token.expire_code(params[:code])
|
||||
|
||||
render :json => token
|
||||
|
|
|
@ -0,0 +1,7 @@
|
|||
class AddReplaceTokensToDeveloperKeys < ActiveRecord::Migration
|
||||
tag :predeploy
|
||||
|
||||
def change
|
||||
add_column :developer_keys, :replace_tokens, :boolean
|
||||
end
|
||||
end
|
|
@ -46,14 +46,22 @@ module Canvas::Oauth
|
|||
Canvas.redis.get("#{REDIS_PREFIX}#{code}").presence || "{}"
|
||||
end
|
||||
|
||||
def access_token
|
||||
def create_access_token_if_needed(replace_tokens = false)
|
||||
@access_token ||= self.class.find_reusable_access_token(user, key, scopes, purpose)
|
||||
|
||||
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.clear_full_token! if @access_token.scoped_to?(['userinfo'])
|
||||
end
|
||||
end
|
||||
|
||||
def access_token
|
||||
create_access_token_if_needed
|
||||
@access_token
|
||||
end
|
||||
|
||||
|
|
|
@ -129,6 +129,22 @@ describe Oauth2ProviderController do
|
|||
expect(response).to be_success
|
||||
expect(JSON.parse(response.body).keys.sort).to eq ['access_token', 'user']
|
||||
end
|
||||
|
||||
it 'deletes existing tokens for the same key when replace_tokens=1' do
|
||||
old_token = user.access_tokens.create! :developer_key => key
|
||||
Canvas.stubs(:redis => redis)
|
||||
get :token, :client_id => key.id, :client_secret => key.api_key, :code => valid_code, :replace_tokens => '1'
|
||||
expect(response).to be_success
|
||||
expect(AccessToken.exists?(old_token.id)).to be(false)
|
||||
end
|
||||
|
||||
it 'does not delete existing tokens without replace_tokens' do
|
||||
old_token = user.access_tokens.create! :developer_key => key
|
||||
Canvas.stubs(:redis => redis)
|
||||
get :token, :client_id => key.id, :client_secret => key.api_key, :code => valid_code
|
||||
expect(response).to be_success
|
||||
expect(AccessToken.exists?(old_token.id)).to be(true)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'POST accept' do
|
||||
|
|
|
@ -122,6 +122,20 @@ module Canvas::Oauth
|
|||
end
|
||||
end
|
||||
|
||||
describe '#create_access_token_if_needed' do
|
||||
it 'deletes existing tokens for the same key when requested' do
|
||||
old_token = user.access_tokens.create! :developer_key => key
|
||||
token.create_access_token_if_needed(true)
|
||||
expect(AccessToken.exists?(old_token.id)).to be(false)
|
||||
end
|
||||
|
||||
it 'does not delete existing tokens for the same key when not requested' do
|
||||
old_token = user.access_tokens.create! :developer_key => key
|
||||
token.create_access_token_if_needed
|
||||
expect(AccessToken.exists?(old_token.id)).to be(true)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#as_json' do
|
||||
let(:json) { token.as_json }
|
||||
|
||||
|
|
Loading…
Reference in New Issue