From 765ed9a96752546077f902bee5d43740b4b11b6a Mon Sep 17 00:00:00 2001 From: Mark Dodwell Date: Wed, 11 Dec 2013 23:49:47 -0800 Subject: [PATCH] remove support for canvas app flow --- lib/omniauth/strategies/facebook.rb | 81 ++++++-------------- test/test.rb | 113 ++-------------------------- 2 files changed, 30 insertions(+), 164 deletions(-) diff --git a/lib/omniauth/strategies/facebook.rb b/lib/omniauth/strategies/facebook.rb index 1bd257e..dd9e2e4 100644 --- a/lib/omniauth/strategies/facebook.rb +++ b/lib/omniauth/strategies/facebook.rb @@ -68,35 +68,22 @@ module OmniAuth end def callback_phase - super - rescue NoAuthorizationCodeError => e - fail!(:no_authorization_code, e) - rescue UnknownSignatureAlgorithmError => e - fail!(:unknown_signature_algoruthm, e) - end - - def request_phase - if signed_request_contains_access_token? - # If we already have an access token, we can just hit the callback URL directly and pass the signed request. - params = { :signed_request => raw_signed_request } - query = Rack::Utils.build_query(params) - - url = callback_url - url << "?" unless url.match(/\?/) - url << "&" unless url.match(/[\&\?]$/) - url << query - - redirect url - else - super + with_authorization_code! do + begin + super + rescue NoAuthorizationCodeError => e + fail!(:no_authorization_code, e) + rescue UnknownSignatureAlgorithmError => e + fail!(:unknown_signature_algoruthm, e) + end end end # NOTE If we're using code from the signed request then FB sets the redirect_uri to '' during the authorize # phase and it must match during the access_token phase: - # https://github.com/facebook/php-sdk/blob/master/src/base_facebook.php#L348 + # https://github.com/facebook/facebook-php-sdk/blob/master/src/base_facebook.php#L477 def callback_url - if @authorization_code_from_signed_request + if @authorization_code_from_signed_request_in_cookie '' else options[:callback_url] || super @@ -110,7 +97,7 @@ module OmniAuth # You can pass +display+, +scope+, or +auth_type+ params to the auth request, if you need to set them dynamically. # You can also set these options in the OmniAuth config :authorize_params option. # - # /auth/facebook?display=popup + # For example: /auth/facebook?display=popup def authorize_params super.tap do |params| %w[display scope auth_type].each do |v| @@ -123,63 +110,45 @@ module OmniAuth end end - # Parse signed request in order, from: - # - # 1. The request 'signed_request' param (server-side flow from canvas pages) or - # 2. A cookie (client-side flow via JS SDK) - def signed_request - @signed_request ||= raw_signed_request && parse_signed_request(raw_signed_request) - end - protected def build_access_token - if signed_request_contains_access_token? - hash = signed_request.clone - ::OAuth2::AccessToken.new( - client, - hash.delete('oauth_token'), - hash.merge!(access_token_options.merge(:expires_at => hash.delete('expires'))) - ) - else - with_authorization_code! { super }.tap do |token| - token.options.merge!(access_token_options) - end + super.tap do |token| + token.options.merge!(access_token_options) end end private - def raw_signed_request - request.params['signed_request'] || request.cookies["fbsr_#{client.id}"] + def signed_request_from_cookie + @signed_request_from_cookie ||= raw_signed_request_from_cookie && parse_signed_request(raw_signed_request_from_cookie) end - # If the signed_request comes from a FB canvas page and the user has already authorized your application, the JSON - # object will be contain the access token. - # - # https://developers.facebook.com/docs/authentication/canvas/ - def signed_request_contains_access_token? - signed_request && signed_request['oauth_token'] + def raw_signed_request_from_cookie + request.cookies["fbsr_#{client.id}"] end # Picks the authorization code in order, from: # # 1. The request 'code' param (manual callback from standard server-side flow) - # 2. A signed request (see #signed_request for more) + # 2. A signed request from cookie (passed from the client during the client-side flow) def with_authorization_code! if request.params.key?('code') yield - elsif code_from_signed_request = signed_request && signed_request['code'] + elsif code_from_signed_request = signed_request_from_cookie && signed_request_from_cookie['code'] request.params['code'] = code_from_signed_request - @authorization_code_from_signed_request = true + @authorization_code_from_signed_request_in_cookie = true + original_provider_ignores_state = options.provider_ignores_state + options.provider_ignores_state = true begin yield ensure request.params.delete('code') - @authorization_code_from_signed_request = false + @authorization_code_from_signed_request_in_cookie = false + options.provider_ignores_state = original_provider_ignores_state end else - raise NoAuthorizationCodeError, 'must pass either a `code` parameter or a signed request (via `signed_request` parameter or a `fbsr_XXX` cookie)' + raise NoAuthorizationCodeError, 'must pass either a `code` (via URL or by an `fbsr_XXX` signed request cookie)' end end diff --git a/test/test.rb b/test/test.rb index 79882ba..f46fabc 100644 --- a/test/test.rb +++ b/test/test.rb @@ -394,12 +394,11 @@ module SignedRequestTests class CookieAndParamNotPresentTest < TestCase test 'is nil' do - assert_nil strategy.send(:signed_request) + assert_nil strategy.send(:signed_request_from_cookie) end test 'throws an error on calling build_access_token' do - assert_equal 'must pass either a `code` parameter or a signed request (via `signed_request` parameter or a `fbsr_XXX` cookie)', - assert_raises(OmniAuth::Strategies::Facebook::NoAuthorizationCodeError) { strategy.send(:build_access_token) }.message + assert_raises(OmniAuth::Strategies::Facebook::NoAuthorizationCodeError) { strategy.send(:with_authorization_code!) {} } end end @@ -417,58 +416,12 @@ module SignedRequestTests end test 'parses the access code out from the cookie' do - assert_equal @payload, strategy.send(:signed_request) + assert_equal @payload, strategy.send(:signed_request_from_cookie) end test 'throws an error if the algorithm is unknown' do setup('UNKNOWN-ALGO') - assert_equal "unknown algorithm: UNKNOWN-ALGO", assert_raises(OmniAuth::Strategies::Facebook::UnknownSignatureAlgorithmError) { strategy.send(:signed_request) }.message - end - end - - class ParamPresentTest < TestCase - def setup(algo = nil) - super() - @payload = { - 'algorithm' => algo || 'HMAC-SHA256', - 'oauth_token' => 'XXX', - 'issued_at' => Time.now.to_i, - 'user_id' => '123456' - } - - @request.stubs(:params).returns({'signed_request' => signed_request(@payload, @client_secret)}) - end - - test 'parses the access code out from the param' do - assert_equal @payload, strategy.send(:signed_request) - end - - test 'throws an error if the algorithm is unknown' do - setup('UNKNOWN-ALGO') - assert_equal "unknown algorithm: UNKNOWN-ALGO", assert_raises(OmniAuth::Strategies::Facebook::UnknownSignatureAlgorithmError) { strategy.send(:signed_request) }.message - end - end - - class CookieAndParamPresentTest < TestCase - def setup - super - @payload_from_cookie = { - 'algorithm' => 'HMAC-SHA256', - 'from' => 'cookie' - } - - @request.stubs(:cookies).returns({"fbsr_#{@client_id}" => signed_request(@payload_from_cookie, @client_secret)}) - - @payload_from_param = { - 'algorithm' => 'HMAC-SHA256', - 'from' => 'param' - } - - @request.stubs(:params).returns({'signed_request' => signed_request(@payload_from_param, @client_secret)}) - end - - test 'picks param over cookie' do - assert_equal @payload_from_param, strategy.send(:signed_request) + assert_equal "unknown algorithm: UNKNOWN-ALGO", assert_raises(OmniAuth::Strategies::Facebook::UnknownSignatureAlgorithmError) { strategy.send(:signed_request_from_cookie) }.message end end @@ -479,63 +432,7 @@ module SignedRequestTests end test 'empty param' do - assert_equal nil, strategy.send(:signed_request) - end - end - -end - -class RequestPhaseWithSignedRequestTest < StrategyTestCase - include SignedRequestHelpers - - def setup - super - - payload = { - 'algorithm' => 'HMAC-SHA256', - 'oauth_token' => 'm4c0d3z' - } - @raw_signed_request = signed_request(payload, @client_secret) - @request.stubs(:params).returns("signed_request" => @raw_signed_request) - - strategy.stubs(:callback_url).returns('/') - end - - test 'redirects to callback passing along signed request' do - strategy.expects(:redirect).with("/?signed_request=#{Rack::Utils.escape(@raw_signed_request)}").once - strategy.request_phase - end -end - -module BuildAccessTokenTests - class TestCase < StrategyTestCase - include SignedRequestHelpers - end - - class ParamsContainSignedRequestWithAccessTokenTest < TestCase - def setup - super - - @payload = { - 'algorithm' => 'HMAC-SHA256', - 'oauth_token' => 'm4c0d3z', - 'expires' => Time.now.to_i - } - @raw_signed_request = signed_request(@payload, @client_secret) - @request.stubs(:params).returns({"signed_request" => @raw_signed_request}) - - strategy.stubs(:callback_url).returns('/') - end - - test 'returns a new access token from the signed request' do - result = strategy.send(:build_access_token) - assert_kind_of ::OAuth2::AccessToken, result - assert_equal @payload['oauth_token'], result.token - end - - test 'returns an access token with the correct expiry time' do - result = strategy.send(:build_access_token) - assert_equal @payload['expires'], result.expires_at + assert_equal nil, strategy.send(:signed_request_from_cookie) end end end