diff --git a/app/controllers/devise/registrations_controller.rb b/app/controllers/devise/registrations_controller.rb index a49506b8..e59feac2 100644 --- a/app/controllers/devise/registrations_controller.rb +++ b/app/controllers/devise/registrations_controller.rb @@ -47,6 +47,13 @@ class Devise::RegistrationsController < ApplicationController protected + # Build a devise resource passing in the session. Useful to move + # temporary session data to the newly created user. + def build_resource(hash=nil) + hash ||= params[resource_name] || {} + self.resource = resource_class.new_with_session(hash, session) + end + # Authenticates the current scope and gets a copy of the current resource. # We need to use a copy because we don't want actions like update changing # the current user in place. diff --git a/lib/devise.rb b/lib/devise.rb index ff83df23..a563050e 100644 --- a/lib/devise.rb +++ b/lib/devise.rb @@ -214,7 +214,7 @@ module Devise mapping = Devise::Mapping.new(resource, options) @@mappings[mapping.name] = mapping @@default_scope ||= mapping.name - @@helpers.each { |h| h.define_helpers(mapping.name) } + @@helpers.each { |h| h.define_helpers(mapping) } mapping end diff --git a/lib/devise/controllers/helpers.rb b/lib/devise/controllers/helpers.rb index 04059570..234ae6f0 100644 --- a/lib/devise/controllers/helpers.rb +++ b/lib/devise/controllers/helpers.rb @@ -34,6 +34,8 @@ module Devise # before_filter :authenticate_admin! # Tell devise to use :admin map # def self.define_helpers(mapping) #:nodoc: + mapping = mapping.name + class_eval <<-METHODS, __FILE__, __LINE__ + 1 def authenticate_#{mapping}! warden.authenticate!(:scope => :#{mapping}) diff --git a/lib/devise/controllers/internal_helpers.rb b/lib/devise/controllers/internal_helpers.rb index 4bfeefaf..656e0074 100644 --- a/lib/devise/controllers/internal_helpers.rb +++ b/lib/devise/controllers/internal_helpers.rb @@ -70,7 +70,7 @@ module Devise # Build a devise resource. def build_resource(hash=nil) hash ||= params[resource_name] || {} - self.resource = resource_class.new_with_session(hash, session) + self.resource = resource_class.new(hash) end # Helper for use in before_filters where no authentication is required. diff --git a/lib/devise/models/authenticatable.rb b/lib/devise/models/authenticatable.rb index df907d63..6c6fb227 100644 --- a/lib/devise/models/authenticatable.rb +++ b/lib/devise/models/authenticatable.rb @@ -78,12 +78,6 @@ module Devise http_authenticatable.include?(strategy) : http_authenticatable end - # By default discards all information sent by the session by calling - # new with params. - def new_with_session(params, session) - new(params) - end - # Find first record based on conditions given (ie by the sign in form). # Overwrite to add customized conditions, create a join, or maybe use a # namedscope to filter records while authenticating. diff --git a/lib/devise/models/registerable.rb b/lib/devise/models/registerable.rb index 9b51d4aa..f6da656c 100644 --- a/lib/devise/models/registerable.rb +++ b/lib/devise/models/registerable.rb @@ -3,6 +3,19 @@ module Devise # Registerable is responsible for everything related to registering a new # resource (ie user sign up). module Registerable + extend ActiveSupport::Concern + + module ClassMethods + # A convenience method that receives both parameters and session to + # initialize an user. This can be used by OAuth, for example, to send + # in the user token and be stored on initialization. + # + # By default discards all information sent by the session by calling + # new with params. + def new_with_session(params, session) + new(params) + end + end end end end diff --git a/lib/devise/oauth.rb b/lib/devise/oauth.rb index 2dcaa04c..11f68956 100644 --- a/lib/devise/oauth.rb +++ b/lib/devise/oauth.rb @@ -11,5 +11,16 @@ module Devise autoload :Helpers, "devise/oauth/helpers" autoload :InternalHelpers, "devise/oauth/internal_helpers" autoload :UrlHelpers, "devise/oauth/url_helpers" + autoload :TestHelpers, "devise/oauth/test_helpers" + + class << self + delegate :short_circuit_authorizers!, :unshort_circuit_authorizers!, :to => "Devise::Oauth::TestHelpers" + + def test_mode! + Faraday.default_adapter = :test + ActiveSupport.on_load(:action_controller) { include Devise::Oauth::TestHelpers } + ActiveSupport.on_load(:action_view) { include Devise::Oauth::TestHelpers } + end + 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 ca23c6c5..ac93aafb 100644 --- a/lib/devise/oauth/internal_helpers.rb +++ b/lib/devise/oauth/internal_helpers.rb @@ -122,7 +122,6 @@ module Devise clean_up_passwords(resource) render_for_oauth else - session[oauth_session_key] = access_token.token set_oauth_flash_message :alert, :skipped redirect_to after_oauth_skipped_path_for(resource_name) end diff --git a/lib/devise/oauth/test_helpers.rb b/lib/devise/oauth/test_helpers.rb new file mode 100644 index 00000000..9082bf63 --- /dev/null +++ b/lib/devise/oauth/test_helpers.rb @@ -0,0 +1,29 @@ +module Devise + module Oauth + module TestHelpers #:nodoc: + def self.short_circuit_authorizers! + module_eval <<-ALIASES, __FILE__, __LINE__ + 1 + def oauth_authorize_url(scope, provider) + oauth_callback_url(scope, provider, :code => "12345") + end + ALIASES + + Devise.mappings.each_value do |m| + next unless m.oauthable? + + module_eval <<-ALIASES, __FILE__, __LINE__ + 1 + def #{m.name}_oauth_authorize_url(provider) + #{m.name}_oauth_callback_url(provider, :code => "12345") + end + ALIASES + end + end + + def self.unshort_circuit_authorizers! + module_eval do + instance_methods.each { |m| remove_method(m) } + end + end + end + end +end \ No newline at end of file diff --git a/lib/devise/oauth/url_helpers.rb b/lib/devise/oauth/url_helpers.rb index eb245e28..49ac366b 100644 --- a/lib/devise/oauth/url_helpers.rb +++ b/lib/devise/oauth/url_helpers.rb @@ -2,10 +2,12 @@ module Devise module Oauth module UrlHelpers def self.define_helpers(mapping) + return unless mapping.oauthable? + class_eval <<-URL_HELPERS, __FILE__, __LINE__ + 1 - def #{mapping}_oauth_authorize_url(provider, options={}) + def #{mapping.name}_oauth_authorize_url(provider, options={}) if config = Devise.oauth_configs[provider.to_sym] - options[:redirect_uri] ||= #{mapping}_oauth_callback_url(provider.to_s) + options[:redirect_uri] ||= #{mapping.name}_oauth_callback_url(provider.to_s) config.authorize_url(options) else raise ArgumentError, "Could not find oauth provider \#{provider.inspect}" diff --git a/test/integration/authenticatable_test.rb b/test/integration/authenticatable_test.rb index 5ee55fad..267c89ab 100644 --- a/test/integration/authenticatable_test.rb +++ b/test/integration/authenticatable_test.rb @@ -1,15 +1,6 @@ require 'test_helper' class AuthenticationSanityTest < ActionController::IntegrationTest - - def setup - Devise.sign_out_all_scopes = false - end - - def teardown - Devise.sign_out_all_scopes = false - end - test 'home should be accessible without sign in' do visit '/' assert_response :success @@ -18,14 +9,12 @@ class AuthenticationSanityTest < ActionController::IntegrationTest test 'sign in as user should not authenticate admin scope' do sign_in_as_user - assert warden.authenticated?(:user) assert_not warden.authenticated?(:admin) end test 'sign in as admin should not authenticate user scope' do sign_in_as_admin - assert warden.authenticated?(:admin) assert_not warden.authenticated?(:user) end @@ -33,59 +22,61 @@ class AuthenticationSanityTest < ActionController::IntegrationTest test 'sign in as both user and admin at same time' do sign_in_as_user sign_in_as_admin - assert warden.authenticated?(:user) assert warden.authenticated?(:admin) end test 'sign out as user should not touch admin authentication if sign_out_all_scopes is false' do - sign_in_as_user - sign_in_as_admin - - get destroy_user_session_path - assert_not warden.authenticated?(:user) - assert warden.authenticated?(:admin) + swap Devise, :sign_out_all_scopes => false do + sign_in_as_user + sign_in_as_admin + get destroy_user_session_path + assert_not warden.authenticated?(:user) + assert warden.authenticated?(:admin) + end end test 'sign out as admin should not touch user authentication if sign_out_all_scopes is false' do - sign_in_as_user - sign_in_as_admin + swap Devise, :sign_out_all_scopes => false do + sign_in_as_user + sign_in_as_admin - get destroy_admin_session_path - assert_not warden.authenticated?(:admin) - assert warden.authenticated?(:user) + get destroy_admin_session_path + assert_not warden.authenticated?(:admin) + assert warden.authenticated?(:user) + end end test 'sign out as user should also sign out admin if sign_out_all_scopes is true' do - Devise.sign_out_all_scopes = true - sign_in_as_user - sign_in_as_admin + swap Devise, :sign_out_all_scopes => true do + sign_in_as_user + sign_in_as_admin - get destroy_user_session_path - assert_not warden.authenticated?(:user) - assert_not warden.authenticated?(:admin) + get destroy_user_session_path + assert_not warden.authenticated?(:user) + assert_not warden.authenticated?(:admin) + end end test 'sign out as admin should also sign out user if sign_out_all_scopes is true' do - Devise.sign_out_all_scopes = true - sign_in_as_user - sign_in_as_admin + swap Devise, :sign_out_all_scopes => true do + sign_in_as_user + sign_in_as_admin - get destroy_admin_session_path - assert_not warden.authenticated?(:admin) - assert_not warden.authenticated?(:user) + get destroy_admin_session_path + assert_not warden.authenticated?(:admin) + assert_not warden.authenticated?(:user) + end end test 'not signed in as admin should not be able to access admins actions' do get admins_path - assert_redirected_to new_admin_session_path assert_not warden.authenticated?(:admin) end test 'not signed in as admin should not be able to access private route restricted to admins' do get private_path - assert_redirected_to new_admin_session_path assert_not warden.authenticated?(:admin) end @@ -94,7 +85,6 @@ class AuthenticationSanityTest < ActionController::IntegrationTest sign_in_as_user assert warden.authenticated?(:user) assert_not warden.authenticated?(:admin) - get private_path assert_redirected_to new_admin_session_path end @@ -236,6 +226,25 @@ class AuthenticationSessionTest < ActionController::IntegrationTest get '/users' assert_equal "Cart", @controller.user_session[:cart] end + + test 'does not explode when invalid user class is stored in session' do + klass = User + paths = ActiveSupport::Dependencies.autoload_paths.dup + + begin + sign_in_as_user + assert warden.authenticated?(:user) + + Object.send :remove_const, :User + ActiveSupport::Dependencies.autoload_paths.clear + + visit "/users" + assert_not warden.authenticated?(:user) + ensure + Object.const_set(:User, klass) + ActiveSupport::Dependencies.autoload_paths.replace(paths) + end + end end class AuthenticationWithScopesTest < ActionController::IntegrationTest @@ -277,18 +286,6 @@ class AuthenticationWithScopesTest < ActionController::IntegrationTest end end end - - test 'uses the mapping from router' do - sign_in_as_user :visit => "/as/sign_in" - assert warden.authenticated?(:user) - assert_not warden.authenticated?(:admin) - end - - test 'uses the mapping from nested devise_for call' do - sign_in_as_user :visit => "/devise_for/sign_in" - assert warden.authenticated?(:user) - assert_not warden.authenticated?(:admin) - end end class AuthenticationOthersTest < ActionController::IntegrationTest @@ -317,28 +314,21 @@ class AuthenticationOthersTest < ActionController::IntegrationTest end end - test 'registration in xml format' do + test 'registration in xml format works when recognizing path' do assert_nothing_raised do post user_registration_path(:format => 'xml', :user => {:email => "test@example.com", :password => "invalid"} ) end end - test 'does not explode when invalid user class is stored in session' do - klass = User - paths = ActiveSupport::Dependencies.autoload_paths.dup + test 'uses the mapping from router' do + sign_in_as_user :visit => "/as/sign_in" + assert warden.authenticated?(:user) + assert_not warden.authenticated?(:admin) + end - begin - sign_in_as_user - assert warden.authenticated?(:user) - - Object.send :remove_const, :User - ActiveSupport::Dependencies.autoload_paths.clear - - visit "/users" - assert_not warden.authenticated?(:user) - ensure - Object.const_set(:User, klass) - ActiveSupport::Dependencies.autoload_paths.replace(paths) - end + test 'uses the mapping from nested devise_for call' do + sign_in_as_user :visit => "/devise_for/sign_in" + assert warden.authenticated?(:user) + assert_not warden.authenticated?(:admin) end end diff --git a/test/integration/oauthable_test.rb b/test/integration/oauthable_test.rb new file mode 100644 index 00000000..6a3e2120 --- /dev/null +++ b/test/integration/oauthable_test.rb @@ -0,0 +1,31 @@ +require 'test_helper' + +class OAuthableTest < ActionController::IntegrationTest + FACEBOOK_INFO = { + :username => 'usertest', + :email => 'user@test.com' + } + + ACCESS_TOKEN = { + :access_token => "plataformatec" + } + + stubs = Faraday::Adapter::Test::Stubs.new do |stub| + stub.post('/oauth/access_token') { [200, {}, ACCESS_TOKEN.to_json] } + stub.get('/me?access_token=plataformatec') { [200, {}, FACEBOOK_INFO.to_json] } + end + + User.oauth_configs[:facebook].client.connection.build do |b| + b.adapter :test, stubs + end + + setup { Devise::Oauth.short_circuit_authorizers! } + teardown { Devise::Oauth.unshort_circuit_authorizers! } + + test "omg" do + assert_difference "User.count", 1 do + get "/users/sign_up" + click_link "Sign in with Facebook" + end + end +end \ No newline at end of file diff --git a/test/rails_app/app/active_record/user.rb b/test/rails_app/app/active_record/user.rb index cee532e6..78db488d 100644 --- a/test/rails_app/app/active_record/user.rb +++ b/test/rails_app/app/active_record/user.rb @@ -1,7 +1,17 @@ class User < ActiveRecord::Base devise :database_authenticatable, :confirmable, :lockable, :recoverable, :registerable, :rememberable, :timeoutable, :token_authenticatable, - :trackable, :validatable, :oauthable, :oauth_providers => [:github, :twitter] + :trackable, :validatable, :oauthable, :oauth_providers => [:github, :facebook] attr_accessible :username, :email, :password, :password_confirmation + + def self.find_for_facebook_oauth(access_token, signed_in_resource=nil) + user = ActiveSupport::JSON.decode(access_token.get('/me')) + create_with_oauth(user) + end + + def self.create_with_oauth(user, &block) + User.create(:username => user["username"], :email => user["email"], + :password => Devise.friendly_token, :confirmed_at => Time.now, &block) + end end diff --git a/test/rails_app/app/mongoid/user.rb b/test/rails_app/app/mongoid/user.rb index 85fbb626..cbfd558f 100644 --- a/test/rails_app/app/mongoid/user.rb +++ b/test/rails_app/app/mongoid/user.rb @@ -6,5 +6,5 @@ class User devise :database_authenticatable, :confirmable, :lockable, :recoverable, :registerable, :rememberable, :timeoutable, :token_authenticatable, - :trackable, :validatable, :oauthable, :oauth_providers => [:github, :twitter] + :trackable, :validatable, :oauthable, :oauth_providers => [:github, :facebook] end diff --git a/test/rails_app/config/initializers/devise.rb b/test/rails_app/config/initializers/devise.rb index a0bde5e9..eea767c8 100644 --- a/test/rails_app/config/initializers/devise.rb +++ b/test/rails_app/config/initializers/devise.rb @@ -130,8 +130,9 @@ Devise.setup do |config| :access_token_path => '/login/oauth/access_token', :scope => 'user,public_repo' - config.oauth :twitter, 'APP_ID', 'APP_SECRET', - :site => 'http://twitter.com/' + config.oauth :facebook, 'APP_ID', 'APP_SECRET', + :site => 'https://graph.facebook.com/', + :scope => %w(email offline_access) # ==> Warden configuration # If you want to use other strategies, that are not (yet) supported by Devise, diff --git a/test/routes_test.rb b/test/routes_test.rb index f3fb01fe..cb97110f 100644 --- a/test/routes_test.rb +++ b/test/routes_test.rb @@ -87,11 +87,11 @@ class DefaultRoutingTest < ActionController::TestCase end test 'map oauth callbacks' do - assert_recognizes({:controller => 'devise/oauth_callbacks', :action => 'twitter'}, {:path => 'users/oauth/twitter/callback', :method => :get}) + assert_recognizes({:controller => 'devise/oauth_callbacks', :action => 'facebook'}, {:path => 'users/oauth/facebook/callback', :method => :get}) assert_recognizes({:controller => 'devise/oauth_callbacks', :action => 'github'}, {:path => 'users/oauth/github/callback', :method => :get}) assert_raise ActionController::RoutingError do - assert_recognizes({:controller => 'devise/oauth_callbacks', :action => 'facebook'}, {:path => 'users/oauth/facebook/callback', :method => :get}) + assert_recognizes({:controller => 'devise/oauth_callbacks', :action => 'twitter'}, {:path => 'users/oauth/twitter/callback', :method => :get}) end end diff --git a/test/test_helper.rb b/test/test_helper.rb index de3555b6..051029f6 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -16,7 +16,7 @@ Webrat.configure do |config| config.open_error_files = false end -Faraday.default_adapter = :test +Devise::Oauth.test_mode! # Add support to load paths so we can overwrite broken webrat setup $:.unshift File.expand_path('../support', __FILE__)