mirror of
https://github.com/heartcombo/devise.git
synced 2022-11-09 12:18:31 -05:00
Remove skipped handling from OAuth in favor of exceptions and rescue_from syntax.
This commit is contained in:
parent
a707423d23
commit
c31b1f2146
5 changed files with 39 additions and 88 deletions
57
Gemfile.lock
57
Gemfile.lock
|
@ -9,16 +9,15 @@ GIT
|
|||
tzinfo (~> 0.3.22)
|
||||
will_paginate (~> 3.0.pre)
|
||||
|
||||
GIT
|
||||
remote: git://github.com/rails/rails.git
|
||||
revision: dd61a81
|
||||
PATH
|
||||
remote: /Users/jose/Work/github/rails
|
||||
specs:
|
||||
actionmailer (3.0.0.beta4)
|
||||
actionpack (= 3.0.0.beta4)
|
||||
actionmailer (3.0.0.rc)
|
||||
actionpack (= 3.0.0.rc)
|
||||
mail (~> 2.2.5)
|
||||
actionpack (3.0.0.beta4)
|
||||
activemodel (= 3.0.0.beta4)
|
||||
activesupport (= 3.0.0.beta4)
|
||||
actionpack (3.0.0.rc)
|
||||
activemodel (= 3.0.0.rc)
|
||||
activesupport (= 3.0.0.rc)
|
||||
builder (~> 2.1.2)
|
||||
erubis (~> 2.6.6)
|
||||
i18n (~> 0.4.1)
|
||||
|
@ -26,30 +25,30 @@ GIT
|
|||
rack-mount (~> 0.6.9)
|
||||
rack-test (~> 0.5.4)
|
||||
tzinfo (~> 0.3.22)
|
||||
activemodel (3.0.0.beta4)
|
||||
activesupport (= 3.0.0.beta4)
|
||||
activemodel (3.0.0.rc)
|
||||
activesupport (= 3.0.0.rc)
|
||||
builder (~> 2.1.2)
|
||||
i18n (~> 0.4.1)
|
||||
activerecord (3.0.0.beta4)
|
||||
activemodel (= 3.0.0.beta4)
|
||||
activesupport (= 3.0.0.beta4)
|
||||
activerecord (3.0.0.rc)
|
||||
activemodel (= 3.0.0.rc)
|
||||
activesupport (= 3.0.0.rc)
|
||||
arel (~> 0.4.0)
|
||||
tzinfo (~> 0.3.22)
|
||||
activeresource (3.0.0.beta4)
|
||||
activemodel (= 3.0.0.beta4)
|
||||
activesupport (= 3.0.0.beta4)
|
||||
activesupport (3.0.0.beta4)
|
||||
rails (3.0.0.beta4)
|
||||
actionmailer (= 3.0.0.beta4)
|
||||
actionpack (= 3.0.0.beta4)
|
||||
activerecord (= 3.0.0.beta4)
|
||||
activeresource (= 3.0.0.beta4)
|
||||
activesupport (= 3.0.0.beta4)
|
||||
bundler (>= 1.0.0.beta.10)
|
||||
railties (= 3.0.0.beta4)
|
||||
railties (3.0.0.beta4)
|
||||
actionpack (= 3.0.0.beta4)
|
||||
activesupport (= 3.0.0.beta4)
|
||||
activeresource (3.0.0.rc)
|
||||
activemodel (= 3.0.0.rc)
|
||||
activesupport (= 3.0.0.rc)
|
||||
activesupport (3.0.0.rc)
|
||||
rails (3.0.0.rc)
|
||||
actionmailer (= 3.0.0.rc)
|
||||
actionpack (= 3.0.0.rc)
|
||||
activerecord (= 3.0.0.rc)
|
||||
activeresource (= 3.0.0.rc)
|
||||
activesupport (= 3.0.0.rc)
|
||||
bundler (>= 1.0.0.rc.1)
|
||||
railties (= 3.0.0.rc)
|
||||
railties (3.0.0.rc)
|
||||
actionpack (= 3.0.0.rc)
|
||||
activesupport (= 3.0.0.rc)
|
||||
rake (>= 0.8.3)
|
||||
thor (~> 0.14.0)
|
||||
|
||||
|
@ -87,9 +86,9 @@ GEM
|
|||
mongo (1.0.5)
|
||||
bson (>= 1.0.4)
|
||||
multi_json (0.0.4)
|
||||
nokogiri (1.4.2)
|
||||
nokogiri (1.4.2-java)
|
||||
weakling (>= 0.0.3)
|
||||
nokogiri (1.4.2)
|
||||
oauth2 (0.0.10)
|
||||
faraday (~> 0.4.1)
|
||||
multi_json (>= 0.0.4)
|
||||
|
|
17
README.rdoc
17
README.rdoc
|
@ -299,20 +299,17 @@ This link will send the user straight to Github. After the user authorizes your
|
|||
access_token = github_config.access_token_by_code(params[:code])
|
||||
@user = User.find_for_github_oauth(access_token, signed_in_resource)
|
||||
|
||||
if @user && @user.persisted?
|
||||
if @user.persisted? && @user.errors.empty?
|
||||
sign_in @user
|
||||
set_oauth_flash_message :notice, :success
|
||||
redirect_to after_oauth_success_path_for(@user) #=> redirects to user_root_path or root_path
|
||||
elsif @user
|
||||
else
|
||||
session[:user_github_oauth_token] = access_token.token
|
||||
render_for_auth #=> renders sign up view by default
|
||||
else
|
||||
set_oauth_flash_message :alert, :skipped
|
||||
redirect_to after_oauth_skipped_path_for(:user) #=> redirects to user_new_session_path
|
||||
end
|
||||
end
|
||||
|
||||
In other words, Devise does all the work for you but it expects you to implement the +find_for_github_oauth+ method in your model that receives two arguments: the first is an +access_token+ object from OAuth2 library (http://github.com/intridea/oauth2) and the second is the signed in resource which we will ignore for this while. Depending on what this method returns, Devise act in a different way as seen above.
|
||||
In other words, Devise does all the work for you but it expects you to implement the +find_for_github_oauth+ method in your model that receives two arguments: the first is an +access_token+ object from OAuth2 library (http://github.com/intridea/oauth2) and the second is the signed in resource, which we will ignore for this while. Depending on what this method returns, Devise will act in a different way.
|
||||
|
||||
A basic implementation for +find_for_github_oauth+ would be:
|
||||
|
||||
|
@ -328,7 +325,7 @@ A basic implementation for +find_for_github_oauth+ would be:
|
|||
end
|
||||
end
|
||||
|
||||
First, notice the +access_token+ object allows you to make requests to the provider using get/post/put/delete methods to retrieve user information. Next, our method above has two branches and both of them returns a persisted user. So, if we go back to our github action above, we will see that after returning a persisted record, it will sign in the returned user and redirect to the configured +after_oauth_success_path_for+ with a flash message. This flash message is retrieved from I18n and looks like this:
|
||||
First, notice the given +access_token+ object allows you to make requests to the provider using get/post/put/delete methods to retrieve user information. Next, our method above has two branches and both of them returns a persisted user. So, if we go back to our github action above, we will see that after returning a persisted record, it will sign in the returned user and redirect to the configured +after_oauth_success_path_for+ with a flash message. This flash message is retrieved from I18n and looks like this:
|
||||
|
||||
en:
|
||||
devise:
|
||||
|
@ -345,9 +342,11 @@ First, notice the +access_token+ object allows you to make requests to the provi
|
|||
# With lower priority
|
||||
success: 'Successfully authorized from %{kind} account.'
|
||||
|
||||
Our basic implementation assumes that all information retrieved from Github is enough for us to create an user, however this may not be true for all providers. That said, Devise allows +find_for_github_oauth+ to have different outcomes. For instance, if it returns a record which was not persisted (usually a new record with errors), it will render the sign up views from the registrations controller and show all error messages. On the other hand, if you decide to return nil from +find_for_github_oauth+, Devise will consider that you decided to skip the authentication and will redirect to +after_oauth_skipped_path_for+ (defaults to the sign in page) with the skipped flash message.
|
||||
Our basic implementation assumes that all information retrieved from Github is enough for us to create an user, however this may not be true for all providers. That said, Devise allows +find_for_github_oauth+ to return a non persisted or an invalid record and it will render the sign up view from the registrations controller and show all error messages.
|
||||
|
||||
All these methods +after_oauth_skipped_path_for+, +render_for_oauth+ and so on can be customized and overwritten in your application by inheriting from Devise::OauthCallbacksController as we have seen above in the "Configuring controllers" section.
|
||||
All these methods +after_oauth_success_path_for+, +render_for_oauth+ and so on can be customized and overwritten in your application by inheriting from Devise::OauthCallbacksController as we have seen above in the "Configuring controllers" section.
|
||||
|
||||
If you need to provide another path besides signing in the persisted user or showing the signup page for invalid one due to some exceptional scenario, it is recommended that you raise an error from your +find_for_github_oauth+ method and rescue and handle it in the controller using Rails' +rescue_from+ syntax.
|
||||
|
||||
For last but not least, Devise also supports linking accounts. The setup discussed above only uses Github for sign up and assumes that after the user signs up, there will not have any interaction with Github at all. However, this is not true for some applications.
|
||||
|
||||
|
|
|
@ -33,7 +33,6 @@ en:
|
|||
oauth_callbacks:
|
||||
success: 'Successfully authorized from %{kind} account.'
|
||||
failure: 'Could not authorize you from %{kind} because "%{reason}".'
|
||||
skipped: 'Skipped Oauth authorization for %{kind}.'
|
||||
mailer:
|
||||
confirmation_instructions:
|
||||
subject: 'Confirmation instructions'
|
||||
|
|
|
@ -9,7 +9,7 @@ module Devise
|
|||
end
|
||||
|
||||
included do
|
||||
helpers = %w(oauth_config oauth_callback)
|
||||
helpers = %w(oauth_config)
|
||||
hide_action *helpers
|
||||
helper_method *helpers
|
||||
before_filter :valid_oauth_callback?, :oauth_error_happened?
|
||||
|
@ -114,16 +114,13 @@ 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? && resource.errors.empty?
|
||||
if resource.persisted? && resource.errors.empty?
|
||||
set_oauth_flash_message :notice, :success
|
||||
sign_in_and_redirect resource_name, resource, :event => :authentication
|
||||
elsif resource
|
||||
else
|
||||
session[oauth_session_key] = access_token.token
|
||||
clean_up_passwords(resource)
|
||||
render_for_oauth
|
||||
else
|
||||
set_oauth_flash_message :alert, :skipped
|
||||
redirect_to after_oauth_skipped_path_for(resource_name)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -171,11 +168,6 @@ module Devise
|
|||
after_sign_in_path_for(resource_or_scope)
|
||||
end
|
||||
|
||||
# The default hook used by oauth to specify the redirect url for skip.
|
||||
def after_oauth_skipped_path_for(scope)
|
||||
new_session_path(scope)
|
||||
end
|
||||
|
||||
# The default hook used by oauth to specify the redirect url for failure.
|
||||
def after_oauth_failure_path_for(scope)
|
||||
new_session_path(scope)
|
||||
|
|
|
@ -118,21 +118,6 @@ class OAuthableIntegrationTest < ActionController::IntegrationTest
|
|||
assert_equal "plataformatec", user.reload.facebook_token
|
||||
end
|
||||
|
||||
test "[BASIC] setup skipping oauth callback" do
|
||||
stub_github!
|
||||
|
||||
assert_no_difference "User.count" do
|
||||
visit "/users/sign_in"
|
||||
click_link "Sign in with Github"
|
||||
end
|
||||
|
||||
assert_current_url "/users/sign_in"
|
||||
assert_contain "Skipped Oauth authorization for Github."
|
||||
|
||||
assert_not warden.authenticated?(:user)
|
||||
assert_not warden.authenticated?(:admin)
|
||||
end
|
||||
|
||||
test "[SESSION CLEANUP] ensures session is cleaned up after sign up" do
|
||||
stub_facebook!(2)
|
||||
|
||||
|
@ -207,29 +192,6 @@ class OAuthableIntegrationTest < ActionController::IntegrationTest
|
|||
end
|
||||
end
|
||||
|
||||
test "[I18N] scopes messages based on oauth callback for skipped" do
|
||||
stub_github!
|
||||
|
||||
store_translations :en, :devise => { :oauth_callbacks => {
|
||||
:github => { :skipped => "Skipped github" } } } do
|
||||
visit "/users/sign_in"
|
||||
click_link "Sign in with Github"
|
||||
assert_contain "Skipped github"
|
||||
end
|
||||
end
|
||||
|
||||
test "[I18N] scopes messages based on oauth callback and resource name for skipped" do
|
||||
stub_github!
|
||||
|
||||
store_translations :en, :devise => { :oauth_callbacks => {
|
||||
:user => { :github => { :skipped => "Skipped github user" } },
|
||||
:github => { :skipped => "Skipped github" } } } do
|
||||
visit "/users/sign_in"
|
||||
click_link "Sign in with Github"
|
||||
assert_contain "Skipped github user"
|
||||
end
|
||||
end
|
||||
|
||||
test "[FAILURE] shows 404 if no code or error are given as params" do
|
||||
assert_raise AbstractController::ActionNotFound do
|
||||
visit "/users/oauth/facebook/callback"
|
||||
|
|
Loading…
Reference in a new issue