From 927650971da40af17da3656136556dc82bab3867 Mon Sep 17 00:00:00 2001 From: James Coglan Date: Thu, 1 Aug 2013 13:51:27 +0100 Subject: [PATCH] Do not allow the user arg to Provider.access_token() to be nil. If you want implicit user lookup, pass :implicit as the user. This prevents accidental authorization in cases where the application developer does not check the result of their User.find() calls. --- README.rdoc | 4 ++-- example/application.rb | 2 +- lib/songkick/oauth2/provider/access_token.rb | 9 +++++++-- spec/songkick/oauth2/provider/access_token_spec.rb | 14 +++++++++++++- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/README.rdoc b/README.rdoc index 574d67e..1462393 100644 --- a/README.rdoc +++ b/README.rdoc @@ -362,10 +362,10 @@ determine whether to serve the request or not. It is also common to provide a dynamic resource for getting some basic data about a user by supplying their access token. This can be done by passing -nil as the resource owner: +:implicit as the resource owner: get '/me' do - token = Songkick::OAuth2::Provider.access_token(nil, [], env) + token = Songkick::OAuth2::Provider.access_token(:implicit, [], env) if token.valid? JSON.unparse('username' => token.owner.username) else diff --git a/example/application.rb b/example/application.rb index 9ea6985..de2e177 100644 --- a/example/application.rb +++ b/example/application.rb @@ -105,7 +105,7 @@ # Domain API get '/me' do - authorization = Songkick::OAuth2::Provider.access_token(nil, [], env) + authorization = Songkick::OAuth2::Provider.access_token(:implicit, [], env) headers authorization.response_headers status authorization.response_status diff --git a/lib/songkick/oauth2/provider/access_token.rb b/lib/songkick/oauth2/provider/access_token.rb index b7a47fb..ccba045 100644 --- a/lib/songkick/oauth2/provider/access_token.rb +++ b/lib/songkick/oauth2/provider/access_token.rb @@ -56,8 +56,13 @@ def validate! return @error = EXPIRED_TOKEN if @authorization.expired? return @error = INSUFFICIENT_SCOPE unless @authorization.in_scope?(@scopes) - if @resource_owner and @authorization.owner != @resource_owner - @error = INSUFFICIENT_SCOPE + case @resource_owner + when :implicit + # no error + when nil + @error = INVALID_TOKEN + else + @error = INSUFFICIENT_SCOPE if @authorization.owner != @resource_owner end end end diff --git a/spec/songkick/oauth2/provider/access_token_spec.rb b/spec/songkick/oauth2/provider/access_token_spec.rb index e834821..97e39d8 100644 --- a/spec/songkick/oauth2/provider/access_token_spec.rb +++ b/spec/songkick/oauth2/provider/access_token_spec.rb @@ -52,11 +52,23 @@ it_should_behave_like "valid token" end + describe "with an implicit user" do + let :token do + Songkick::OAuth2::Provider::AccessToken.new(:implicit, ['profile'], 'magic-key') + end + it_should_behave_like "valid token" + end + describe "with no user" do let :token do Songkick::OAuth2::Provider::AccessToken.new(nil, ['profile'], 'magic-key') end - it_should_behave_like "valid token" + it_should_behave_like "invalid token" + + it "returns an error response" do + token.response_headers['WWW-Authenticate'].should == "OAuth realm='Demo App', error='invalid_token'" + token.response_status.should == 401 + end end describe "with less scope than was granted" do