From 00c6f583e2350dd2de788b02164643e17c732d66 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@gmail.com>
Date: Fri, 16 Jul 2010 11:01:36 +0200
Subject: [PATCH] More OAuth tests.

---
 lib/devise/controllers/internal_helpers.rb    |  2 +-
 lib/devise/failure_app.rb                     |  6 ++
 lib/devise/oauth/internal_helpers.rb          |  4 +-
 test/controllers/helpers_test.rb              | 41 +------------
 test/controllers/internal_helpers_test.rb     | 18 +++---
 test/integration/oauthable_test.rb            | 57 ++++++++++++++++---
 test/integration/registerable_test.rb         |  6 +-
 test/rails_app/app/active_record/admin.rb     |  5 +-
 test/rails_app/app/active_record/user.rb      | 19 ++-----
 .../app/controllers/home_controller.rb        |  2 +-
 test/rails_app/app/mongoid/admin.rb           |  5 +-
 test/rails_app/app/mongoid/shim.rb            |  6 +-
 test/rails_app/app/mongoid/user.rb            |  9 +--
 test/rails_app/lib/shared_admin.rb            |  7 +++
 test/rails_app/lib/shared_user.rb             | 42 ++++++++++++++
 test/support/test_silencer.rb                 |  5 --
 16 files changed, 144 insertions(+), 90 deletions(-)
 create mode 100644 test/rails_app/lib/shared_admin.rb
 create mode 100644 test/rails_app/lib/shared_user.rb
 delete mode 100644 test/support/test_silencer.rb

diff --git a/lib/devise/controllers/internal_helpers.rb b/lib/devise/controllers/internal_helpers.rb
index 656e0074..36b4ad49 100644
--- a/lib/devise/controllers/internal_helpers.rb
+++ b/lib/devise/controllers/internal_helpers.rb
@@ -58,7 +58,7 @@ module Devise
       end
 
       def unknown_action!(msg)
-        logger.debug msg
+        logger.debug "[Devise] #{msg}" if logger
         raise ActionController::UnknownAction, msg
       end
 
diff --git a/lib/devise/failure_app.rb b/lib/devise/failure_app.rb
index 75b019c1..ada9d295 100644
--- a/lib/devise/failure_app.rb
+++ b/lib/devise/failure_app.rb
@@ -27,6 +27,7 @@ module Devise
       elsif warden_options[:recall]
         recall
       else
+        debug!
         redirect
       end
     end
@@ -52,6 +53,11 @@ module Devise
 
   protected
 
+    def debug!
+      return unless Rails.logger.try(:debug?)
+      Rails.logger.debug "[Devise] Could not sign in #{scope}: #{i18n_message.inspect}."
+    end
+
     def i18n_message(default = nil)
       message = warden.message || warden_options[:message] || default || :unauthenticated
 
diff --git a/lib/devise/oauth/internal_helpers.rb b/lib/devise/oauth/internal_helpers.rb
index e8269997..f62ae651 100644
--- a/lib/devise/oauth/internal_helpers.rb
+++ b/lib/devise/oauth/internal_helpers.rb
@@ -87,7 +87,7 @@ module Devise
         if error = params[:error] || params[:error_reason]
           # Some providers returns access-denied instead of access_denied.
           error = error.to_s.gsub("-", "_")
-          logger.warn "#{oauth_callback} oauth failed: #{error.inspect}."
+          logger.warn "[Devise] #{oauth_callback} oauth failed: #{error.inspect}."
 
           set_oauth_flash_message :alert, error[0,25], :default => :failure, :reason => error.humanize
           redirect_to after_oauth_failure_path_for(resource_name)
@@ -114,7 +114,7 @@ module Devise
         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 && resource.persisted?
+        if resource && resource.persisted? && resource.errors.empty?
           set_oauth_flash_message :notice, :success
           sign_in_and_redirect resource_name, resource, :event => :authentication
         elsif resource
diff --git a/test/controllers/helpers_test.rb b/test/controllers/helpers_test.rb
index be7da526..6dfb2092 100644
--- a/test/controllers/helpers_test.rb
+++ b/test/controllers/helpers_test.rb
@@ -1,51 +1,16 @@
 require 'test_helper'
 require 'ostruct'
 
-class MockController < ApplicationController
-  attr_accessor :env
-
-  def request
-    self
-  end
-
-  def path
-    ''
-  end
-
-  def index
-  end
-
-  def host_with_port
-    "test.host:3000"
-  end
-
-  def protocol
-    "http"
-  end
-
-  def script_name
-    ""
-  end
-
-  def symbolized_path_parameters
-    {}
-  end
-end
-
 class ControllerAuthenticableTest < ActionController::TestCase
-  tests MockController
+  tests ApplicationController
 
   def setup
     @mock_warden = OpenStruct.new
-    @controller.env = { 'warden' => @mock_warden }
-  end
-
-  test 'setup warden' do
-    assert_not_nil @controller.warden
+    @controller.request.env['warden'] = @mock_warden
   end
 
   test 'provide access to warden instance' do
-    assert_equal @controller.warden, @controller.env['warden']
+    assert_equal @mock_warden, @controller.warden
   end
 
   test 'proxy signed_in? to authenticated' do
diff --git a/test/controllers/internal_helpers_test.rb b/test/controllers/internal_helpers_test.rb
index fc08e6aa..cd334507 100644
--- a/test/controllers/internal_helpers_test.rb
+++ b/test/controllers/internal_helpers_test.rb
@@ -22,16 +22,16 @@ class HelpersTest < ActionController::TestCase
   end
 
   test 'get resource instance variable from env' do
-    @controller.instance_variable_set(:@user, admin = Admin.new)
-    assert_equal admin, @controller.resource
+    @controller.instance_variable_set(:@user, user = User.new)
+    assert_equal user, @controller.resource
   end
 
   test 'set resource instance variable from env' do
-    admin = @controller.send(:resource_class).new
-    @controller.send(:resource=, admin)
+    user = @controller.send(:resource_class).new
+    @controller.send(:resource=, user)
 
-    assert_equal admin, @controller.send(:resource)
-    assert_equal admin, @controller.instance_variable_get(:@user)
+    assert_equal user, @controller.send(:resource)
+    assert_equal user, @controller.instance_variable_get(:@user)
   end
 
   test 'resources methods are not controller actions' do
@@ -39,11 +39,15 @@ class HelpersTest < ActionController::TestCase
   end
 
   test 'require no authentication tests current mapping' do
-    @controller.expects(:resource_name).returns(:user).twice
     @mock_warden.expects(:authenticated?).with(:user).returns(true)
     @controller.expects(:redirect_to).with(root_path)
     @controller.send :require_no_authentication
   end
+
+  test 'signed in resource returns signed in resource for current scope' do
+    @mock_warden.expects(:authenticate).with(:scope => :user).returns(User.new)
+    assert_kind_of User, @controller.signed_in_resource
+  end
   
   test 'is a devise controller' do
     assert @controller.devise_controller?
diff --git a/test/integration/oauthable_test.rb b/test/integration/oauthable_test.rb
index 432e3e36..9e077696 100644
--- a/test/integration/oauthable_test.rb
+++ b/test/integration/oauthable_test.rb
@@ -12,11 +12,6 @@ class OAuthableTest < ActionController::IntegrationTest
 
   setup do
     Devise::Oauth.short_circuit_authorizers!
-
-    Devise::Oauth.stub!(:facebook) do |b|
-      b.post('/oauth/access_token') { [200, {}, ACCESS_TOKEN.to_json] }
-      b.get('/me?access_token=plataformatec') { [200, {}, FACEBOOK_INFO.to_json] }
-    end
   end
 
   teardown do
@@ -24,10 +19,54 @@ class OAuthableTest < ActionController::IntegrationTest
     Devise::Oauth.reset_stubs!
   end
 
-  test "omg" do
-    assert_difference "User.count", 1 do
-      get "/users/sign_in"
-      click_link "Sign in with Facebook"
+  def stub_facebook!(times=1)
+    data = (times == 1) ? FACEBOOK_INFO : FACEBOOK_INFO.except(:email)
+
+    Devise::Oauth.stub!(:facebook) do |b|
+      b.post('/oauth/access_token') { [200, {}, ACCESS_TOKEN.to_json] }
+      times.times {
+        b.get('/me?access_token=plataformatec') { [200, {}, data.to_json] }
+      }
     end
   end
+
+  test "basic setup with persisted user" do
+    stub_facebook!
+
+    assert_difference "User.count", 1 do
+      visit "/users/sign_in"
+      click_link "Sign in with Facebook"
+    end
+
+    assert_current_url "/"
+    assert_contain "Successfully authorized from Facebook account."
+
+    assert warden.authenticated?(:user)
+    assert_not warden.authenticated?(:admin)
+  end
+
+  test "basic setup with not persisted user and follow up" do
+    stub_facebook!(2)
+
+    assert_no_difference "User.count" do
+      visit "/users/sign_in"
+      click_link "Sign in with Facebook"
+    end
+
+    assert_contain "1 error prohibited this user from being saved"
+    assert_contain "Email can't be blank"
+
+    assert_not warden.authenticated?(:user)
+    assert_not warden.authenticated?(:admin)
+
+    fill_in "Email", :with => "user.form@test.com"
+    click_button "Sign up"
+
+    assert_current_url "/"
+    assert_contain "You have signed up successfully."
+    assert_contain "Hello User user.form@test.com"
+
+    assert warden.authenticated?(:user)
+    assert_not warden.authenticated?(:admin)
+  end
 end
\ No newline at end of file
diff --git a/test/integration/registerable_test.rb b/test/integration/registerable_test.rb
index f256bcab..2ef47339 100644
--- a/test/integration/registerable_test.rb
+++ b/test/integration/registerable_test.rb
@@ -153,13 +153,13 @@ class RegistrationTest < ActionController::IntegrationTest
 
   test 'a user should be able to cancel sign up by deleting data in the session' do
     get "/set"
-    assert_equal "something", @request.session["user_facebook_oauth_token"]
+    assert_equal "something", @request.session["user_provider_oauth_token"]
 
     get "/users/sign_up"
-    assert_equal "something", @request.session["user_facebook_oauth_token"]
+    assert_equal "something", @request.session["user_provider_oauth_token"]
 
     get "/users/cancel"
-    assert_nil @request.session["user_facebook_oauth_token"]
+    assert_nil @request.session["user_provider_oauth_token"]
     assert_redirected_to new_user_registration_path
   end
 end
diff --git a/test/rails_app/app/active_record/admin.rb b/test/rails_app/app/active_record/admin.rb
index 5b36afa6..124bc905 100644
--- a/test/rails_app/app/active_record/admin.rb
+++ b/test/rails_app/app/active_record/admin.rb
@@ -1,3 +1,6 @@
+require 'shared_admin'
+
 class Admin < ActiveRecord::Base
-  devise :database_authenticatable, :registerable, :timeoutable, :recoverable, :lockable, :unlock_strategy => :time
+  include Shim
+  include SharedAdmin
 end
diff --git a/test/rails_app/app/active_record/user.rb b/test/rails_app/app/active_record/user.rb
index 78db488d..ccb119e2 100644
--- a/test/rails_app/app/active_record/user.rb
+++ b/test/rails_app/app/active_record/user.rb
@@ -1,17 +1,8 @@
+require 'shared_user'
+
 class User < ActiveRecord::Base
-  devise :database_authenticatable, :confirmable, :lockable, :recoverable,
-         :registerable, :rememberable, :timeoutable, :token_authenticatable,
-         :trackable, :validatable, :oauthable, :oauth_providers => [:github, :facebook]
+  include Shim
+  include SharedUser
 
-  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
+  attr_accessible :username, :email, :password, :password_confirmation, :remember_me
 end
diff --git a/test/rails_app/app/controllers/home_controller.rb b/test/rails_app/app/controllers/home_controller.rb
index 039065aa..28412b83 100644
--- a/test/rails_app/app/controllers/home_controller.rb
+++ b/test/rails_app/app/controllers/home_controller.rb
@@ -6,7 +6,7 @@ class HomeController < ApplicationController
   end
 
   def set
-    session["user_facebook_oauth_token"] = "something"
+    session["user_provider_oauth_token"] = "something"
     head :ok
   end
 end
diff --git a/test/rails_app/app/mongoid/admin.rb b/test/rails_app/app/mongoid/admin.rb
index 76a5bfd0..acbe2f74 100644
--- a/test/rails_app/app/mongoid/admin.rb
+++ b/test/rails_app/app/mongoid/admin.rb
@@ -1,6 +1,7 @@
+require 'shared_admin'
+
 class Admin
   include Mongoid::Document
   include Shim
-
-  devise :database_authenticatable, :timeoutable, :registerable, :recoverable, :lockable, :unlock_strategy => :time
+  include SharedAdmin
 end
diff --git a/test/rails_app/app/mongoid/shim.rb b/test/rails_app/app/mongoid/shim.rb
index fb01ec0d..5d16fd31 100644
--- a/test/rails_app/app/mongoid/shim.rb
+++ b/test/rails_app/app/mongoid/shim.rb
@@ -1,6 +1,10 @@
 module Shim
   extend ::ActiveSupport::Concern
-  include ::Mongoid::Timestamps
+
+  included do
+    include ::Mongoid::Timestamps
+    field :created_at, :type => DateTime
+  end
 
   module ClassMethods
     def last(options={})
diff --git a/test/rails_app/app/mongoid/user.rb b/test/rails_app/app/mongoid/user.rb
index cbfd558f..1a75f930 100644
--- a/test/rails_app/app/mongoid/user.rb
+++ b/test/rails_app/app/mongoid/user.rb
@@ -1,10 +1,7 @@
+require 'shared_user'
+
 class User
   include Mongoid::Document
   include Shim
-
-  field :created_at, :type => DateTime
-
-  devise :database_authenticatable, :confirmable, :lockable, :recoverable,
-         :registerable, :rememberable, :timeoutable, :token_authenticatable,
-         :trackable, :validatable, :oauthable, :oauth_providers => [:github, :facebook]
+  include SharedUser
 end
diff --git a/test/rails_app/lib/shared_admin.rb b/test/rails_app/lib/shared_admin.rb
new file mode 100644
index 00000000..baabfc06
--- /dev/null
+++ b/test/rails_app/lib/shared_admin.rb
@@ -0,0 +1,7 @@
+module SharedAdmin
+  extend ActiveSupport::Concern
+
+  included do
+    devise :database_authenticatable, :registerable, :timeoutable, :recoverable, :lockable, :unlock_strategy => :time
+  end
+end
\ No newline at end of file
diff --git a/test/rails_app/lib/shared_user.rb b/test/rails_app/lib/shared_user.rb
new file mode 100644
index 00000000..880e1fb6
--- /dev/null
+++ b/test/rails_app/lib/shared_user.rb
@@ -0,0 +1,42 @@
+module SharedUser
+  extend ActiveSupport::Concern
+
+  included do
+    devise :database_authenticatable, :confirmable, :lockable, :recoverable,
+           :registerable, :rememberable, :timeoutable, :token_authenticatable,
+           :trackable, :validatable, :oauthable, :oauth_providers => [:github, :facebook]
+
+    # They need to be included after Devise is called.
+    extend ExtendMethods
+  end
+
+  module ExtendMethods
+    def find_for_facebook_oauth(access_token, signed_in_resource=nil)
+      User.create { |u| u.update_with_facebook_oauth(access_token) }
+    end
+
+    def new_with_session(params, session)
+      super.tap do |user|
+        if session[:user_facebook_oauth_token]
+          access_token = oauth_access_token(:facebook, session[:user_facebook_oauth_token])
+          user.update_with_facebook_oauth(access_token)
+        end
+      end
+    end
+  end
+
+  def update_with_facebook_oauth(access_token)
+    data = ActiveSupport::JSON.decode(access_token.get('/me'))
+
+    self.username = data["username"] unless username.present?
+    self.email    = data["email"] unless email.present?
+    self.confirmed_at ||= Time.now
+
+    unless password.present?
+      self.password = Devise.friendly_token
+      self.password_confirmation = nil
+    end
+
+    yield self if block_given?    
+  end
+end
diff --git a/test/support/test_silencer.rb b/test/support/test_silencer.rb
deleted file mode 100644
index 274a0aa9..00000000
--- a/test/support/test_silencer.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-module Devise
-  module TestSilencer
-    def test(*args, &block); end
-  end
-end
\ No newline at end of file