Skip to content

Commit decd207

Browse files
committed
support for trusted developer keys
fixes CAT-66 Auto-authorize trusted developer keys during without prompting the end- user. This will allow for more seamless integrations with other in-house apps. Also fix remember-me access so it works when you re-auth into canvas (previously it only worked if you were already authenticated into canvas) Expand test coverage around oauth login scenarios test plan setup: * set up a web-based oauth integration with canvas * ensure your app doesn't currently pass force_login=1 during oauth * ensure your app doesn't currently delete access tokens during logout * for part 2, ensure your app uses the /auth/userinfo scope (for part 1 it doesn't matter) * for part 3, ensure your app does token request flow (not userinfo) test plan part 1 (trusted keys): 1. in the canvas console, set trusted=true on the app's developer key 2. do an oauth login as and end-user 3. confirm that you are authenticated into the app without being prompted to give it canvas access 4. log out of the app (but not canvas) 5. click to log in again 6. confirm that you are automagically logged in without any prompts test plan part 2 (remember access): 1. in the canvas console, set trusted=false on the app's developer key (or set up your app to use a different one) 2. do an oauth login as and end-user 3. confirm that you are prompted to authorize the app 4. check the box to remember access 5. log out of canvas and the app 6. do an oauth login again 7. confirm you are not prompted to authorize the app 8. log out of the app (but not canvas) 9. click to log in again 10. confirm that you are automagically logged in without any prompts test plan part 3 (untrusted key, not-userinfo) 1. in the canvas console, set trusted=false on the app's developer key (or set up your app to use a different one) 2. do an oauth login as and end-user 3. confirm that you are prompted to authorize the app 4. confirm there is no box to remember access 5. log out of canvas and the app 6. do an oauth login again 7. confirm you are prompted to authorize the app again 8. log out of the app (but not canvas) 9. click to log in again 10. confirm that are prompted to authorize the app again Change-Id: Ifb2eb29e4da163b595cb070455ebae21a4618ba4 Reviewed-on: https://gerrit.instructure.com/32926 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Jon Jensen <jon@instructure.com> Product-Review: Marc LeGendre <marc@instructure.com> QA-Review: Marc LeGendre <marc@instructure.com>
1 parent fdfff43 commit decd207

6 files changed

Lines changed: 129 additions & 20 deletions

File tree

app/controllers/pseudonym_sessions_controller.rb

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -567,8 +567,9 @@ def successful_login(user, pseudonym, otp_passed = false)
567567
session[:require_terms] = true if @domain_root_account.require_acceptance_of_terms?(@current_user)
568568

569569
respond_to do |format|
570-
if session[:oauth2]
571-
return redirect_to(oauth2_auth_confirm_url)
570+
if oauth = session[:oauth2]
571+
provider = Canvas::Oauth::Provider.new(oauth[:client_id], oauth[:redirect_uri], oauth[:scopes], oauth[:purpose])
572+
return oauth2_confirmation_redirect(provider)
572573
elsif session[:course_uuid] && user && (course = Course.find_by_uuid_and_workflow_state(session[:course_uuid], "created"))
573574
claim_session_course(course, user)
574575
format.html { redirect_to(course_url(course, :login_success => '1')) }
@@ -629,11 +630,7 @@ def oauth2_auth
629630
session[:oauth2][:state] = params[:state] if params.key?(:state)
630631

631632
if @current_pseudonym && !params[:force_login]
632-
if provider.authorized_token? @current_user
633-
final_oauth2_redirect(session[:oauth2][:redirect_uri], final_oauth2_redirect_params)
634-
elsif
635-
redirect_to oauth2_auth_confirm_url
636-
end
633+
oauth2_confirmation_redirect(provider)
637634
else
638635
redirect_to login_url(params.slice(:canvas_login, :pseudonym_session, :force_login))
639636
end
@@ -687,6 +684,15 @@ def oauth2_logout
687684
render :json => {}
688685
end
689686

687+
def oauth2_confirmation_redirect(provider)
688+
# skip the confirmation page if access is already (or automatically) granted
689+
if provider.authorized_token?(@current_user)
690+
final_oauth2_redirect(session[:oauth2][:redirect_uri], final_oauth2_redirect_params)
691+
else
692+
redirect_to oauth2_auth_confirm_url
693+
end
694+
end
695+
690696
def final_oauth2_redirect_params(options = {})
691697
options = {:scopes => session[:oauth2][:scopes], :remember_access => options[:remember_access], :purpose => session[:oauth2][:purpose]}
692698
code = Canvas::Oauth::Token.generate_code_for(@current_user.global_id, session[:oauth2][:client_id], options)

app/models/access_token.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ def visible_token
9797

9898
#Scoped token convenience method
9999
def scoped_to?(req_scopes)
100+
self.class.scopes_match?(scopes, req_scopes)
101+
end
102+
103+
def self.scopes_match?(scopes, req_scopes)
100104
return req_scopes.size == 0 if scopes.nil?
101105

102106
scopes.size == req_scopes.size &&
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
class AddTrustedToDeveloperKeys < ActiveRecord::Migration
2+
tag :predeploy
3+
4+
def self.up
5+
add_column :developer_keys, :trusted, :boolean
6+
end
7+
8+
def self.down
9+
remove_column :developer_keys, :trusted
10+
end
11+
end

lib/canvas/oauth/provider.rb

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,15 @@ def key
4242
@key ||= DeveloperKey.find_by_id(@client_id)
4343
end
4444

45-
#Checks to see if a token has already been issued to this client and if we can
46-
#reissue the same token to that client without asking for user permmission again.
45+
# Checks to see if a token has already been issued to this client and
46+
# if we can reissue the same token to that client without asking for
47+
# user permission again. If the developer key is trusted, access
48+
# tokens will be automatically authorized without prompting the end-
49+
# user
4750
def authorized_token?(user)
48-
token = nil
49-
5051
if !self.class.is_oob?(redirect_uri)
51-
token = Token.find_userinfo_access_token(user, key, scopes, purpose)
52-
return !token.nil? && token.remember_access?
52+
return true if Token.find_reusable_access_token(user, key, scopes, purpose)
53+
return true if key.trusted?
5354
end
5455

5556
return false

lib/canvas/oauth/token.rb

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def cached_code_entry
4747
end
4848

4949
def access_token
50-
@access_token ||= self.class.find_userinfo_access_token(user, key, scopes, purpose)
50+
@access_token ||= self.class.find_reusable_access_token(user, key, scopes, purpose)
5151

5252
if @access_token.nil?
5353
@access_token = user.access_tokens.create!({:developer_key => key, :remember_access => remember_access?, :scopes => scopes, :purpose => purpose})
@@ -57,17 +57,29 @@ def access_token
5757
@access_token
5858
end
5959

60+
def self.find_reusable_access_token(user, key, scopes, purpose)
61+
if key.trusted?
62+
find_access_token(user, key, scopes, purpose)
63+
elsif AccessToken.scopes_match?(scopes, ["userinfo"])
64+
find_userinfo_access_token(user, key, purpose)
65+
end
66+
end
67+
6068
def as_json(options={})
6169
{
6270
'access_token' => access_token.full_token,
6371
'user' => user.as_json(:only => [:id, :name], :include_root => false),
6472
}
6573
end
6674

67-
def self.find_userinfo_access_token(user, developer_key, scopes, purpose)
75+
def self.find_userinfo_access_token(user, developer_key, purpose)
76+
find_access_token(user, developer_key, ["userinfo"], purpose, {remember_access: true})
77+
end
78+
79+
def self.find_access_token(user, developer_key, scopes, purpose, conditions = {})
6880
user.shard.activate do
69-
user.access_tokens.active.where(:remember_access => true, :developer_key_id => developer_key, :purpose => purpose).detect do |token|
70-
token.scoped_to?(scopes) && token.scoped_to?(['userinfo'])
81+
user.access_tokens.active.where({:developer_key_id => developer_key, :purpose => purpose}.merge(conditions)).detect do |token|
82+
token.scoped_to?(scopes)
7183
end
7284
end
7385
end

spec/controllers/pseudonym_sessions_controller_spec.rb

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,55 @@ def cas_client.validate_service_ticket(st)
807807
assigns[:cc].should == cc
808808
end
809809
end
810+
811+
context "oauth" do
812+
before do
813+
user_with_pseudonym(:active_all => 1, :password => 'qwerty')
814+
815+
redis = stub('Redis')
816+
redis.stubs(:setex)
817+
redis.stubs(:hmget)
818+
redis.stubs(:del)
819+
Canvas.stubs(:redis => redis)
820+
end
821+
822+
let(:key) { DeveloperKey.create! :redirect_uri => 'https://example.com' }
823+
let(:params) { {:pseudonym_session => { :unique_id => @pseudonym.unique_id, :password => 'qwerty' } } }
824+
825+
it 'should redirect to the confirm url if the user has no token' do
826+
provider = Canvas::Oauth::Provider.new(key.id, key.redirect_uri, [], nil)
827+
828+
post :create, params, :oauth2 => provider.session_hash
829+
response.should redirect_to(oauth2_auth_confirm_url)
830+
end
831+
832+
it 'should redirect to the redirect uri if the user already has remember-me token' do
833+
@user.access_tokens.create!({:developer_key => key, :remember_access => true, :scopes => ['/auth/userinfo'], :purpose => nil})
834+
provider = Canvas::Oauth::Provider.new(key.id, key.redirect_uri, ['/auth/userinfo'], nil)
835+
836+
post :create, params, :oauth2 => provider.session_hash
837+
response.should be_redirect
838+
response.location.should match(/https:\/\/example.com/)
839+
end
840+
841+
it 'should not reuse userinfo tokens for other scopes' do
842+
@user.access_tokens.create!({:developer_key => key, :remember_access => true, :scopes => ['/auth/userinfo'], :purpose => nil})
843+
provider = Canvas::Oauth::Provider.new(key.id, key.redirect_uri, [], nil)
844+
845+
post :create, params, :oauth2 => provider.session_hash
846+
response.should redirect_to(oauth2_auth_confirm_url)
847+
end
848+
849+
it 'should redirect to the redirect uri if the developer key is trusted' do
850+
key.trusted = true
851+
key.save!
852+
provider = Canvas::Oauth::Provider.new(key.id, key.redirect_uri, [], nil)
853+
854+
post :create, params, :oauth2 => provider.session_hash
855+
response.should be_redirect
856+
response.location.should match(/https:\/\/example.com/)
857+
end
858+
end
810859
end
811860

812861
describe 'otp_login' do
@@ -1086,11 +1135,16 @@ def cas_client.validate_service_ticket(st)
10861135
end
10871136

10881137
context 'with a user logged in' do
1089-
before :each do
1090-
user_session user
1138+
before do
1139+
user_with_pseudonym(:active_all => 1, :password => 'qwerty')
1140+
user_session(@user)
1141+
1142+
redis = stub('Redis')
1143+
redis.stubs(:setex)
1144+
Canvas.stubs(:redis => redis)
10911145
end
10921146

1093-
it 'prompts the user to authorize this access' do
1147+
it 'should redirect to the confirm url if the user has no token' do
10941148
get :oauth2_auth, :client_id => key.id, :redirect_uri => Canvas::Oauth::Provider::OAUTH2_OOB_URI
10951149
response.should redirect_to(oauth2_auth_confirm_url)
10961150
end
@@ -1099,6 +1153,27 @@ def cas_client.validate_service_ticket(st)
10991153
get :oauth2_auth, :client_id => key.id, :redirect_uri => Canvas::Oauth::Provider::OAUTH2_OOB_URI, :force_login => 1
11001154
response.should redirect_to(login_url(:force_login => 1))
11011155
end
1156+
1157+
it 'should redirect to the redirect uri if the user already has remember-me token' do
1158+
@user.access_tokens.create!({:developer_key => key, :remember_access => true, :scopes => ['/auth/userinfo'], :purpose => nil})
1159+
get :oauth2_auth, :client_id => key.id, :redirect_uri => 'https://example.com', :scopes => '/auth/userinfo'
1160+
response.should be_redirect
1161+
response.location.should match(/https:\/\/example.com/)
1162+
end
1163+
1164+
it 'should not reuse userinfo tokens for other scopes' do
1165+
@user.access_tokens.create!({:developer_key => key, :remember_access => true, :scopes => ['/auth/userinfo'], :purpose => nil})
1166+
get :oauth2_auth, :client_id => key.id, :redirect_uri => 'https://example.com'
1167+
response.should redirect_to(oauth2_auth_confirm_url)
1168+
end
1169+
1170+
it 'should redirect to the redirect uri if the developer key is trusted' do
1171+
key.trusted = true
1172+
key.save!
1173+
get :oauth2_auth, :client_id => key.id, :redirect_uri => 'https://example.com', :scopes => '/auth/userinfo'
1174+
response.should be_redirect
1175+
response.location.should match(/https:\/\/example.com/)
1176+
end
11021177
end
11031178
end
11041179

0 commit comments

Comments
 (0)