From a52af1503f5648a58d9b157fe2cf87d768707d0a Mon Sep 17 00:00:00 2001 From: Alex Boyd Date: Wed, 15 Apr 2015 13:49:31 -0600 Subject: [PATCH] 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 QA-Review: August Thornton Tested-by: Jenkins Product-Review: Alex Boyd --- app/controllers/oauth2_provider_controller.rb | 2 ++ ...91548_add_replace_tokens_to_developer_keys.rb | 7 +++++++ lib/canvas/oauth/token.rb | 10 +++++++++- .../oauth2_provider_controller_spec.rb | 16 ++++++++++++++++ spec/lib/canvas/oauth/token_spec.rb | 14 ++++++++++++++ 5 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20150415191548_add_replace_tokens_to_developer_keys.rb diff --git a/app/controllers/oauth2_provider_controller.rb b/app/controllers/oauth2_provider_controller.rb index 54a5c93d435..25c6ca9c5c8 100644 --- a/app/controllers/oauth2_provider_controller.rb +++ b/app/controllers/oauth2_provider_controller.rb @@ -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 diff --git a/db/migrate/20150415191548_add_replace_tokens_to_developer_keys.rb b/db/migrate/20150415191548_add_replace_tokens_to_developer_keys.rb new file mode 100644 index 00000000000..89b759d44c6 --- /dev/null +++ b/db/migrate/20150415191548_add_replace_tokens_to_developer_keys.rb @@ -0,0 +1,7 @@ +class AddReplaceTokensToDeveloperKeys < ActiveRecord::Migration + tag :predeploy + + def change + add_column :developer_keys, :replace_tokens, :boolean + end +end diff --git a/lib/canvas/oauth/token.rb b/lib/canvas/oauth/token.rb index 4d41c691abf..963e1ad88fe 100644 --- a/lib/canvas/oauth/token.rb +++ b/lib/canvas/oauth/token.rb @@ -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 diff --git a/spec/controllers/oauth2_provider_controller_spec.rb b/spec/controllers/oauth2_provider_controller_spec.rb index ba3db77a1c5..5c8fc14a363 100644 --- a/spec/controllers/oauth2_provider_controller_spec.rb +++ b/spec/controllers/oauth2_provider_controller_spec.rb @@ -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 diff --git a/spec/lib/canvas/oauth/token_spec.rb b/spec/lib/canvas/oauth/token_spec.rb index 2166060b59b..e6f263a2269 100644 --- a/spec/lib/canvas/oauth/token_spec.rb +++ b/spec/lib/canvas/oauth/token_spec.rb @@ -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 }