From b31d60ce7ce843451fa9f5540553ded1b668a40c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 14 Jul 2010 10:01:34 +0200 Subject: [PATCH] Implement error handling for OAuth. --- config/locales/en.yml | 3 +- lib/devise/controllers/internal_helpers.rb | 4 +- lib/devise/oauth/config.rb | 4 +- lib/devise/oauth/helpers.rb | 14 +--- lib/devise/oauth/internal_helpers.rb | 83 ++++++++++++++++++---- lib/devise/oauth/url_helpers.rb | 2 +- 6 files changed, 77 insertions(+), 33 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index b4e15188..352efa29 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -31,7 +31,8 @@ en: send_instructions: 'You will receive an email with instructions about how to unlock your account in a few minutes.' unlocked: 'Your account was successfully unlocked. You are now signed in.' oauth_callbacks: - default: 'Successfully authorized from %{kind} account.' + success: 'Successfully authorized from %{kind} account.' + failure: 'Could not authorize you from %{kind} because "%{reason}".' mailer: confirmation_instructions: subject: 'Confirmation instructions' diff --git a/lib/devise/controllers/internal_helpers.rb b/lib/devise/controllers/internal_helpers.rb index e5005b15..4bfeefaf 100644 --- a/lib/devise/controllers/internal_helpers.rb +++ b/lib/devise/controllers/internal_helpers.rb @@ -97,9 +97,9 @@ module Devise # available. def set_flash_message(key, kind, options={}) #:nodoc: options[:scope] = "devise.#{controller_name}" - options[:default] = Array(options[:default]).unshift(kind) + options[:default] = Array(options[:default]).unshift(kind.to_sym) options[:resource_name] = resource_name - flash[key] = I18n.t(:"#{resource_name}.#{kind}", options) + flash[key] = I18n.t("#{resource_name}.#{kind}", options) end def clean_up_passwords(object) #:nodoc: diff --git a/lib/devise/oauth/config.rb b/lib/devise/oauth/config.rb index 5b7f24d3..e0d9d80f 100644 --- a/lib/devise/oauth/config.rb +++ b/lib/devise/oauth/config.rb @@ -16,8 +16,8 @@ module Devise client.web_server.authorize_url(options) end - def access_token_by_code(code) - client.web_server.get_access_token(code) + def access_token_by_code(code, redirect_uri=nil) + client.web_server.get_access_token(code, :redirect_uri => redirect_uri) end def access_token_by_token(token) diff --git a/lib/devise/oauth/helpers.rb b/lib/devise/oauth/helpers.rb index d00bf9df..fa85baad 100644 --- a/lib/devise/oauth/helpers.rb +++ b/lib/devise/oauth/helpers.rb @@ -12,19 +12,7 @@ module Devise def oauth_callback nil end - - protected - - def render_for_oauth - render_with_scope oauth_callback - rescue ActionView::MissingTemplate - render_with_scope :new, devise_mapping.controllers[:registrations] - end - - # The default hook used by oauth to specify the redirect url. - def after_oauth_sign_in_path_for(resource_or_scope) - after_sign_in_path_for(resource_or_scope) - end + alias :oauth_provider :oauth_callback end end end \ No newline at end of file diff --git a/lib/devise/oauth/internal_helpers.rb b/lib/devise/oauth/internal_helpers.rb index dab49367..c0f73e1d 100644 --- a/lib/devise/oauth/internal_helpers.rb +++ b/lib/devise/oauth/internal_helpers.rb @@ -12,54 +12,109 @@ module Devise helpers = %w(oauth_config) hide_action *helpers helper_method *helpers - before_filter :is_oauth_callback? + before_filter :valid_oauth_callback?, :error_happened? end + # Returns the oauth_callback (also aliases oauth_provider) as a symbol. + # For example: :github. def oauth_callback @oauth_callback ||= action_name.to_sym end + alias :oauth_provider :oauth_callback + # Returns the configuration object for this oauth callback. def oauth_config @oauth_client ||= resource_class.oauth_configs[oauth_callback] end protected - def is_oauth_callback? - unless params[:code] - unknown_action! "Skipping OAuth #{outh_callback.inspect} callback because code was not sent." + # This method checks three things: + # + # * If the URL being access is a valid provider for the given scope; + # * If code or error was streamed back from the server; + # * If the resource class implements the required hook; + # + def valid_oauth_callback? #:nodoc: + unless oauth_config + unknown_action! "Skipping #{oauth_callback} OAuth because configuration " << + "could not be found for model #{resource_name}." end - unless oauth_config - unknown_action! "Skipping OAuth #{outh_callback.inspect} callback because provider " << - "could not be found in model #{resource_name.inspect}." + unless params[:code] || params[:error] || params[:error_reason] + unknown_action! "Skipping #{oauth_callback} OAuth because code nor error were sent." end unless resource_class.respond_to?(oauth_model_callback) - raise "#{resource_class.name} does not respond to to OAuth callback #{oauth_model_callback.inspect}. " << + raise "#{resource_class.name} does not respond to #{oauth_model_callback}. " << "Check the OAuth section in the README for more information." end end - def oauth_model_callback + # Check if an error was sent by the authorizer. + # + # TODO: Currently, Facebook is returning error_reason=user_defined when + # the user denies, but the specification defines error=access_denied instead. + def error_happened? #:nodoc: + if error = params[:error] || params[:error_reason] + logger.warn "#{oauth_callback} OAuth failed: #{error.inspect}." + + # Some providers returns access-denied instead of access_denied. + error = error.to_s.gsub("-", "_") + set_flash_message :alert, error[0,25], :default => :failure, :reason => error.titleize + redirect_to after_oauth_failure_path_for(resource_name) + end + end + + # The model method used as hook. + def oauth_model_callback #:nodoc: "find_for_#{oauth_callback}_oauth" end + # The session key to store the token. + def oauth_session_key #:nodoc: + "#{resource_name}_#{oauth_callback}_token" + end + + # The callback redirect uri. Used to request the access token. + def oauth_redirect_uri #:nodoc: + oauth_callback_url(resource_name, oauth_callback) + end + + # This is the implementation for all actions in this controller. def callback_action - access_token = oauth_config.access_token_by_code(params[:code]) + access_token = oauth_config.access_token_by_code(params[:code], oauth_redirect_uri) self.resource = resource_class.send(oauth_model_callback, access_token, signed_in_resource) if resource.persisted? - set_flash_message :notice, oauth_callback, :default => :default, :kind => oauth_callback.to_s.titleize + set_flash_message :notice, oauth_callback, :default => :success sign_in_and_redirect resource_name, resource, :event => :authentication else - session[oauth_session_scope] = access_token.token + session[oauth_session_key] = access_token.token render_for_oauth end end - def oauth_session_scope - "#{resource_name}_#{oauth_callback}_token" + # Overwrite to automatically add kind to messages. + def set_flash_message(key, kind, options={}) #:nodoc: + options[:kind] = oauth_callback.to_s.titleize + super + end + + # Choose which template to render when a not persisted resource is + # returned in the find_for_x_oauth. By default, it renders registrations/new. + def render_for_oauth + render_with_scope :new, devise_mapping.controllers[:registrations] + end + + # The default hook used by oauth to specify the redirect url. + def after_oauth_sign_in_path_for(resource_or_scope) + after_sign_in_path_for(resource_or_scope) + end + + # A callback to redirect your user to the proper location after create. + def after_oauth_failure_path_for(scope) + root_path end # Overwrite redirect_for_sign_in so it takes uses after_oauth_sign_in_path_for. diff --git a/lib/devise/oauth/url_helpers.rb b/lib/devise/oauth/url_helpers.rb index 9f9659e9..eb245e28 100644 --- a/lib/devise/oauth/url_helpers.rb +++ b/lib/devise/oauth/url_helpers.rb @@ -21,7 +21,7 @@ module Devise def oauth_callback_url(resource_or_scope, *args) scope = Devise::Mapping.find_scope!(resource_or_scope) - send("#{scope}_oauth_callback_path", *args) + send("#{scope}_oauth_callback_url", *args) end def oauth_callback_path(resource_or_scope, *args)