From e3e24b732841ea8233da4836c61caaae8b4e2949 Mon Sep 17 00:00:00 2001 From: nbrustein Date: Fri, 31 Jul 2015 14:21:33 -0400 Subject: [PATCH] feat(improved-omniauth): add support for sameWindow and inAppBrowser omniauth flows --- CHANGELOG.md | 10 + Gemfile | 1 + Gemfile.lock | 8 +- README.md | 7 + .../devise_token_auth/CHANGELOG.md | 10 + .../omniauth_callbacks_controller.rb | 165 ++++++--- app/models/devise_token_auth/concerns/user.rb | 15 +- .../omniauth_external_window.html.erb | 38 ++ .../omniauth_failure.html.erb | 2 - .../omniauth_success.html.erb | 12 - app/views/layouts/omniauth_response.html.erb | 31 -- config/routes.rb | 1 + lib/devise_token_auth.rb | 1 + lib/devise_token_auth/engine.rb | 35 ++ lib/devise_token_auth/url.rb | 15 + lib/devise_token_auth/version.rb | 2 +- ...stom_omniauth_callbacks_controller_test.rb | 3 +- .../omniauth_callbacks_controller_test.rb | 333 ++++++++++-------- .../omniauth_callbacks_controller_test.rb | 3 +- .../app/controllers/auth_origin_controller.rb | 5 + test/dummy/config/routes.rb | 3 + test/lib/devise_token_auth/url_test.rb | 11 + test/models/user_test.rb | 8 - 23 files changed, 459 insertions(+), 260 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 app/controllers/devise_token_auth/CHANGELOG.md create mode 100644 app/views/devise_token_auth/omniauth_external_window.html.erb delete mode 100644 app/views/devise_token_auth/omniauth_failure.html.erb delete mode 100644 app/views/devise_token_auth/omniauth_success.html.erb delete mode 100644 app/views/layouts/omniauth_response.html.erb create mode 100644 lib/devise_token_auth/url.rb create mode 100644 test/dummy/app/controllers/auth_origin_controller.rb create mode 100644 test/lib/devise_token_auth/url_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 000000000..a9c4380f9 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,10 @@ + +# 0.1.33 (2015-08-09) + +## Features + +- **Improved OAuth Flow**: Supports new OAuth window flows, allowing options for `sameWindow`, `newWindow`, and `inAppBrowser` + +## Breaking Changes + +- The new OmniAuth callback behavior now defaults to `sameWindow` mode, whereas the previous implementation mimicked the functionality of `newWindow`. This was changed due to limitations with the `postMessage` API support in popular browsers, as well as feedback from user-experience testing. \ No newline at end of file diff --git a/Gemfile b/Gemfile index 0822904c2..f826f53ff 100644 --- a/Gemfile +++ b/Gemfile @@ -34,6 +34,7 @@ group :development, :test do gem 'guard-minitest' gem 'faker' gem 'fuzz_ball' + gem 'mocha' end # code coverage, metrics diff --git a/Gemfile.lock b/Gemfile.lock index f5447bbdd..4fdbc7b0b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -33,7 +33,7 @@ GIT PATH remote: . specs: - devise_token_auth (0.1.32) + devise_token_auth (0.1.33) devise (~> 3.3) rails (~> 4.2) @@ -128,6 +128,7 @@ GEM lumberjack (1.0.9) mail (2.6.3) mime-types (>= 1.16, < 3) + metaclass (0.0.4) method_source (0.8.2) mime-types (2.6.1) mini_portile (0.6.2) @@ -142,10 +143,12 @@ GEM builder minitest (>= 5.0) ruby-progressbar + mocha (1.1.0) + metaclass (~> 0.0.1) multi_json (1.11.2) multi_xml (0.5.5) multipart-post (2.0.0) - mysql2 (0.3.18) + mysql2 (0.3.19) nenv (0.2.0) nokogiri (1.6.6.2) mini_portile (~> 0.6.0) @@ -245,6 +248,7 @@ DEPENDENCIES minitest-focus minitest-rails minitest-reporters + mocha mysql2 omniauth-facebook! omniauth-github! diff --git a/README.md b/README.md index 2e3be0b72..a5a6e8aec 100644 --- a/README.md +++ b/README.md @@ -846,5 +846,12 @@ To run the test suite do the following: The last command will open the [guard](https://github.com/guard/guard) test-runner. Guard will re-run each test suite when changes are made to its corresponding files. +To run just one test: +1. Clone this repo +2. Run `bundle install` +3. Run `rake db:migrate` +4. Run `RAILS_ENV=test rake db:migrate` +5. See this link for various ways to run a single file or a single test: http://flavio.castelli.name/2010/05/28/rails_execute_single_test/ + # License This project uses the WTFPL diff --git a/app/controllers/devise_token_auth/CHANGELOG.md b/app/controllers/devise_token_auth/CHANGELOG.md new file mode 100644 index 000000000..a7c7ff0d3 --- /dev/null +++ b/app/controllers/devise_token_auth/CHANGELOG.md @@ -0,0 +1,10 @@ ++ ++# 0.1.33 (2015-??-??) ++ ++## Features ++ ++- **Improved OAuth Flow**: Supports new OAuth window flows, allowing options for `sameWindow`, `newWindow`, and `inAppBrowser` ++ ++## Breaking Changes ++ ++- The new OAuth redirect behavior now defaults to `sameWindow` mode, whereas the previous implementation mimicked the functionality of `newWindow`. This was changed due to limitations with the `postMessage` API support in popular browsers, as well as feedback from user-experience testing. \ No newline at end of file diff --git a/app/controllers/devise_token_auth/omniauth_callbacks_controller.rb b/app/controllers/devise_token_auth/omniauth_callbacks_controller.rb index 3c12c2523..64abc8b4f 100644 --- a/app/controllers/devise_token_auth/omniauth_callbacks_controller.rb +++ b/app/controllers/devise_token_auth/omniauth_callbacks_controller.rb @@ -1,11 +1,14 @@ module DeviseTokenAuth class OmniauthCallbacksController < DeviseTokenAuth::ApplicationController + + attr_reader :auth_params skip_before_filter :set_user_by_token skip_after_filter :update_auth_header # intermediary route for successful omniauth authentication. omniauth does # not support multiple models, so we must resort to this terrible hack. def redirect_callbacks + # derive target redirect route from 'resource_class' param, which was set # before authentication. devise_mapping = request.env['omniauth.params']['resource_class'].underscore.to_sym @@ -19,49 +22,113 @@ def redirect_callbacks redirect_to redirect_route end - def omniauth_success + def get_resource_from_auth_hash # find or create user by provider and provider uid @resource = resource_class.where({ uid: auth_hash['uid'], provider: auth_hash['provider'] }).first_or_initialize - @oauth_registration = @resource.new_record? + if @resource.new_record? + @oauth_registration = true + set_random_password + end + + # sync user info with provider, update/generate auth token + assign_provider_attrs(@resource, auth_hash) + + # assign any additional (whitelisted) attributes + extra_params = whitelisted_params + @resource.assign_attributes(extra_params) if extra_params + + @resource + end + + def set_random_password + # set crazy password for new oauth users. this is only used to prevent + # access via email sign-in. + p = SecureRandom.urlsafe_base64(nil, false) + @resource.password = p + @resource.password_confirmation = p + end + + def create_token_info # create token info @client_id = SecureRandom.urlsafe_base64(nil, false) @token = SecureRandom.urlsafe_base64(nil, false) @expiry = (Time.now + DeviseTokenAuth.token_lifespan).to_i @config = omniauth_params['config_name'] + end - auth_origin_url_params = { - token: @token, + def create_auth_params + @auth_params = { + auth_token: @token, client_id: @client_id, uid: @resource.uid, expiry: @expiry, config: @config } - auth_origin_url_params.merge!(oauth_registration: true) if @oauth_registration - @auth_origin_url = generate_url(omniauth_params['auth_origin_url'], auth_origin_url_params) - - # set crazy password for new oauth users. this is only used to prevent - # access via email sign-in. - unless @resource.id - p = SecureRandom.urlsafe_base64(nil, false) - @resource.password = p - @resource.password_confirmation = p - end + @auth_params.merge!(oauth_registration: true) if @oauth_registration + @auth_params + end + def set_token_on_resource @resource.tokens[@client_id] = { token: BCrypt::Password.create(@token), expiry: @expiry } + end - # sync user info with provider, update/generate auth token - assign_provider_attrs(@resource, auth_hash) + def render_data(message, data) + @data = data.merge({ + message: message + }) + render :layout => nil, :template => "devise_token_auth/omniauth_external_window" + end - # assign any additional (whitelisted) attributes - extra_params = whitelisted_params - @resource.assign_attributes(extra_params) if extra_params + def render_data_or_redirect(message, data) + + # We handle inAppBrowser and newWindow the same, but it is nice + # to support values in case people need custom implementations for each case + # (For example, nbrustein does not allow new users to be created if logging in with + # an inAppBrowser) + # + # See app/views/devise_token_auth/omniauth_external_window.html.erb to understand + # why we can handle these both the same. The view is setup to handle both cases + # at the same time. + if ['inAppBrowser', 'newWindow'].include?(omniauth_window_type) + render_data(message, data) + + elsif auth_origin_url # default to same-window implementation, which forwards back to auth_origin_url + + # build and redirect to destination url + redirect_to DeviseTokenAuth::Url.generate(auth_origin_url, data) + else + + # there SHOULD always be an auth_origin_url, but if someone does something silly + # like coming straight to this url or refreshing the page at the wrong time, there may not be one. + # In that case, just render in plain text the error message if there is one or otherwise + # a generic message. + fallback_render data[:error] || 'An error occurred' + end + end + + def fallback_render(text) + render inline: %Q| + + + + + #{text} + + | + end + + def omniauth_success + get_resource_from_auth_hash + create_token_info + set_token_on_resource + create_auth_params if resource_class.devise_modules.include?(:confirmable) # don't send confirmation email!!! @@ -74,8 +141,7 @@ def omniauth_success yield if block_given? - # render user info to javascript postMessage communication window - render :layout => "layouts/omniauth_response", :template => "devise_token_auth/omniauth_success" + render_data_or_redirect('deliverCredentials', @resource.as_json.merge(@auth_params.as_json)) end @@ -92,7 +158,7 @@ def assign_provider_attrs(user, auth_hash) def omniauth_failure @error = params[:message] - render :layout => "layouts/omniauth_response", :template => "devise_token_auth/omniauth_failure" + render_data_or_redirect('authFailure', {error: @error}) end @@ -109,10 +175,13 @@ def whitelisted_params } end - # pull resource class from omniauth return def resource_class(mapping = nil) - if omniauth_params + if omniauth_params['resource_class'] omniauth_params['resource_class'].constantize + elsif params['resource_class'] + params['resource_class'].constantize + else + raise "No resource_class found" end end @@ -126,14 +195,37 @@ def resource_name # request.env variable. this variable is then persisted thru the redirect # using our own dta.omniauth.params session var. the omniauth_success # method will access that session var and then destroy it immediately - # after use. + # after use. In the failure case, finally, the omniauth params + # are added as query params in our monkey patch to OmniAuth in engine.rb def omniauth_params - if request.env['omniauth.params'] - request.env['omniauth.params'] - else - @_omniauth_params ||= session.delete('dta.omniauth.params') - @_omniauth_params + if !defined?(@_omniauth_params) + if request.env['omniauth.params'] && request.env['omniauth.params'].any? + @_omniauth_params = request.env['omniauth.params'] + elsif session['dta.omniauth.params'] && session['dta.omniauth.params'].any? + @_omniauth_params ||= session.delete('dta.omniauth.params') + @_omniauth_params + elsif params['omniauth_window_type'] + @_omniauth_params = params.slice('omniauth_window_type', 'auth_origin_url', 'resource_class', 'origin') + else + @_omniauth_params = {} + end end + @_omniauth_params + + end + + def omniauth_window_type + omniauth_params['omniauth_window_type'] + end + + def auth_origin_url + omniauth_params['auth_origin_url'] || omniauth_params['origin'] + end + + # in the success case, omniauth_window_type is in the omniauth_params. + # in the failure case, it is in a query param. See monkey patch above + def omniauth_window_type + omniauth_params.nil? ? params['omniauth_window_type'] : omniauth_params['omniauth_window_type'] end # this sesison value is set by the redirect_callbacks method. its purpose @@ -159,18 +251,5 @@ def devise_mapping end end - def generate_url(url, params = {}) - auth_url = url - - # ensure that hash-bang is present BEFORE querystring for angularjs - unless url.match(/#/) - auth_url += '#' - end - - # add query AFTER hash-bang - auth_url += "?#{params.to_query}" - - return auth_url - end end end diff --git a/app/models/devise_token_auth/concerns/user.rb b/app/models/devise_token_auth/concerns/user.rb index baa336f99..8ca4c7ae9 100644 --- a/app/models/devise_token_auth/concerns/user.rb +++ b/app/models/devise_token_auth/concerns/user.rb @@ -198,7 +198,7 @@ def build_auth_url(base_url, args) args[:uid] = self.uid args[:expiry] = self.tokens[args[:client_id]]['expiry'] - generate_url(base_url, args) + DeviseTokenAuth::Url.generate(base_url, args) end @@ -222,19 +222,6 @@ def token_validation_response protected - - def generate_url(url, params = {}) - uri = URI(url) - - res = "#{uri.scheme}://#{uri.host}" - res += ":#{uri.port}" if (uri.port and uri.port != 80 and uri.port != 443) - res += "#{uri.path}" if uri.path - res += "?#{params.to_query}" - res += "##{uri.fragment}" if uri.fragment - - return res - end - # only validate unique email among users that registered by email def unique_email_user if provider == 'email' and self.class.where(provider: 'email', email: email).count > 0 diff --git a/app/views/devise_token_auth/omniauth_external_window.html.erb b/app/views/devise_token_auth/omniauth_external_window.html.erb new file mode 100644 index 000000000..c81af76ba --- /dev/null +++ b/app/views/devise_token_auth/omniauth_external_window.html.erb @@ -0,0 +1,38 @@ + + + + + + + 
+
+ + \ No newline at end of file diff --git a/app/views/devise_token_auth/omniauth_failure.html.erb b/app/views/devise_token_auth/omniauth_failure.html.erb deleted file mode 100644 index e483d3018..000000000 --- a/app/views/devise_token_auth/omniauth_failure.html.erb +++ /dev/null @@ -1,2 +0,0 @@ -message: "authFailure", -error: "<%= @error %>" diff --git a/app/views/devise_token_auth/omniauth_success.html.erb b/app/views/devise_token_auth/omniauth_success.html.erb deleted file mode 100644 index 5536ede5e..000000000 --- a/app/views/devise_token_auth/omniauth_success.html.erb +++ /dev/null @@ -1,12 +0,0 @@ -<% @resource.as_json.each do |attr, val| %> - "<%= attr %>": <%= val.to_json.html_safe %>, -<% end %> - -"auth_token": "<%= @token %>", -"message": "deliverCredentials", -"client_id": "<%= @client_id %>", -"expiry": "<%= @expiry %>", -<% if @oauth_registration %> -"oauth_registration": "true", -<% end %> -"config": "<%= @config %>" \ No newline at end of file diff --git a/app/views/layouts/omniauth_response.html.erb b/app/views/layouts/omniauth_response.html.erb deleted file mode 100644 index 2b3d6d3a2..000000000 --- a/app/views/layouts/omniauth_response.html.erb +++ /dev/null @@ -1,31 +0,0 @@ - - - - - - - 
-      Redirecting...
-
- - diff --git a/config/routes.rb b/config/routes.rb index d50f7e6b0..a0aee7715 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,6 @@ Rails.application.routes.draw do if defined?(::OmniAuth) get "#{DeviseTokenAuth.omniauth_prefix}/:provider/callback", to: "devise_token_auth/omniauth_callbacks#redirect_callbacks" + get "#{DeviseTokenAuth.omniauth_prefix}/failure", to: "devise_token_auth/omniauth_callbacks#omniauth_failure" end end diff --git a/lib/devise_token_auth.rb b/lib/devise_token_auth.rb index 3a7865ab3..7ba990e61 100644 --- a/lib/devise_token_auth.rb +++ b/lib/devise_token_auth.rb @@ -2,6 +2,7 @@ require "devise_token_auth/engine" require "devise_token_auth/controllers/helpers" require "devise_token_auth/controllers/url_helpers" +require "devise_token_auth/url" module DeviseTokenAuth end diff --git a/lib/devise_token_auth/engine.rb b/lib/devise_token_auth/engine.rb index baf126332..b40a4d66b 100644 --- a/lib/devise_token_auth/engine.rb +++ b/lib/devise_token_auth/engine.rb @@ -33,6 +33,41 @@ def self.setup(&block) Rails.application.config.after_initialize do if defined?(::OmniAuth) ::OmniAuth::config.path_prefix = Devise.omniauth_path_prefix = self.omniauth_prefix + + + # Omniauth currently does not pass along omniauth.params upon failure redirect + # see also: https://github.com/intridea/omniauth/issues/626 + OmniAuth::FailureEndpoint.class_eval do + def redirect_to_failure + message_key = env['omniauth.error.type'] + origin_query_param = env['omniauth.origin'] ? "&origin=#{CGI.escape(env['omniauth.origin'])}" : "" + strategy_name_query_param = env['omniauth.error.strategy'] ? "&strategy=#{env['omniauth.error.strategy'].name}" : "" + extra_params = env['omniauth.params'] ? "&#{env['omniauth.params'].to_query}" : "" + new_path = "#{env['SCRIPT_NAME']}#{OmniAuth.config.path_prefix}/failure?message=#{message_key}#{origin_query_param}#{strategy_name_query_param}#{extra_params}" + Rack::Response.new(["302 Moved"], 302, 'Location' => new_path).finish + end + end + + + # Omniauth currently removes omniauth.params during mocked requests + # see also: https://github.com/intridea/omniauth/pull/812 + OmniAuth::Strategy.class_eval do + def mock_callback_call + setup_phase + @env['omniauth.origin'] = session.delete('omniauth.origin') + @env['omniauth.origin'] = nil if env['omniauth.origin'] == '' + @env['omniauth.params'] = session.delete('omniauth.params') || {} + mocked_auth = OmniAuth.mock_auth_for(name.to_s) + if mocked_auth.is_a?(Symbol) + fail!(mocked_auth) + else + @env['omniauth.auth'] = mocked_auth + OmniAuth.config.before_callback_phase.call(@env) if OmniAuth.config.before_callback_phase + call_app! + end + end + end + end end end diff --git a/lib/devise_token_auth/url.rb b/lib/devise_token_auth/url.rb new file mode 100644 index 000000000..172a22fd6 --- /dev/null +++ b/lib/devise_token_auth/url.rb @@ -0,0 +1,15 @@ +module DeviseTokenAuth::Url + + def self.generate(url, params = {}) + uri = URI(url) + + res = "#{uri.scheme}://#{uri.host}" + res += ":#{uri.port}" if (uri.port and uri.port != 80 and uri.port != 443) + res += "#{uri.path}" if uri.path + res += "?#{params.to_query}" + res += "##{uri.fragment}" if uri.fragment + + return res + end + +end \ No newline at end of file diff --git a/lib/devise_token_auth/version.rb b/lib/devise_token_auth/version.rb index 1725586a4..aa2634345 100644 --- a/lib/devise_token_auth/version.rb +++ b/lib/devise_token_auth/version.rb @@ -1,3 +1,3 @@ module DeviseTokenAuth - VERSION = "0.1.32" + VERSION = "0.1.33" end diff --git a/test/controllers/custom/custom_omniauth_callbacks_controller_test.rb b/test/controllers/custom/custom_omniauth_callbacks_controller_test.rb index edf785cb1..950bc7820 100644 --- a/test/controllers/custom/custom_omniauth_callbacks_controller_test.rb +++ b/test/controllers/custom/custom_omniauth_callbacks_controller_test.rb @@ -19,7 +19,8 @@ class Custom::OmniauthCallbacksControllerTest < ActionDispatch::IntegrationTest test "yield resource to block on omniauth_sucess success" do @redirect_url = "http://ng-token-auth.dev/" get_via_redirect '/nice_user_auth/facebook', { - auth_origin_url: @redirect_url + auth_origin_url: @redirect_url, + omniauth_window_type: 'newWindow' } assert @controller.omniauth_success_block_called?, "omniauth_success failed to yield resource to provided block" end diff --git a/test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb b/test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb index c85e543c1..e11bee030 100644 --- a/test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb +++ b/test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb @@ -1,4 +1,5 @@ require 'test_helper' +require 'mocha/test_unit' # was the web request successful? # was the user redirected to the right page? @@ -9,148 +10,91 @@ class OmniauthTest < ActionDispatch::IntegrationTest setup do OmniAuth.config.test_mode = true - OmniAuth.config.mock_auth[:facebook] = OmniAuth::AuthHash.new({ - :provider => 'facebook', - :uid => '123545', - :info => { - name: 'chong', - email: 'chongbong@aol.com' - } - }) end before do @redirect_url = "http://ng-token-auth.dev/" end - describe 'default user model' do - describe 'from api to provider' do - before do - get_via_redirect '/auth/facebook', { - auth_origin_url: @redirect_url - } + describe 'success callback' do - @resource = assigns(:resource) - end - - test 'status should be success' do - assert_equal 200, response.status - end + setup do + OmniAuth.config.mock_auth[:facebook] = OmniAuth::AuthHash.new({ + :provider => 'facebook', + :uid => '123545', + :info => { + name: 'chong', + email: 'chongbong@aol.com' + } + }) + end - test 'request should determine the correct resource_class' do - assert_equal 'User', controller.omniauth_params['resource_class'] - end + test 'request should pass correct redirect_url' do + get_success + assert_equal @redirect_url, controller.omniauth_params['auth_origin_url'] + end - test 'request should pass correct redirect_url' do - assert_equal @redirect_url, controller.omniauth_params['auth_origin_url'] - end + test 'user should have been created' do + get_success + assert @resource + end - test 'user should have been created' do - assert @resource - end + test 'user should be assigned info from provider' do + get_success + assert_equal 'chongbong@aol.com', @resource.email + end - test 'user should be assigned info from provider' do - assert_equal 'chongbong@aol.com', @resource.email - end + test 'user should be assigned token' do + get_success + client_id = controller.auth_params[:client_id] + token = controller.auth_params[:auth_token] + expiry = controller.auth_params[:expiry] - test 'user should be of the correct class' do - assert_equal User, @resource.class - end + # the expiry should have been set + assert_equal expiry, @resource.tokens[client_id][:expiry] + # the token sent down to the client should now be valid + assert @resource.valid_token?(token, client_id) + end - test 'response contains all serializable attributes for user' do - post_message = JSON.parse(/postMessage\((?.*), '\*'\);/m.match(response.body)[:data]) + test 'session vars have been cleared' do + get_success + refute request.session['dta.omniauth.auth'] + refute request.session['dta.omniauth.params'] + end + test 'sign_in was called' do + User.any_instance.expects(:sign_in) + get_success + end - ['id', 'email', 'uid', 'name', - 'favorite_color', 'tokens', 'password' - ].each do |key| - assert_equal post_message[key], @resource.as_json[key], "Unexpected value for #{key.inspect}" - end - - assert_equal "deliverCredentials", post_message["message"] - assert post_message["auth_token"] - assert post_message["client_id"] - assert post_message["expiry"] - assert post_message["config"] + describe 'with default user model' do + before do + get_success end - - test 'session vars have been cleared' do - refute request.session['dta.omniauth.auth'] - refute request.session['dta.omniauth.params'] + test 'request should determine the correct resource_class' do + assert_equal 'User', controller.omniauth_params['resource_class'] end - describe 'trackable' do - test 'sign_in_count incrementns' do - assert @resource.sign_in_count > 0 - end - - test 'current_sign_in_at is updated' do - assert @resource.current_sign_in_at - end - - test 'last_sign_in_at is updated' do - assert @resource.last_sign_in_at - end - - test 'sign_in_ip is updated' do - assert @resource.current_sign_in_ip - end - - test 'last_sign_in_ip is updated' do - assert @resource.last_sign_in_ip - end + test 'user should be of the correct class' do + assert_equal User, @resource.class end - end - describe "oauth_registration attr" do - - def stub_resource - relation = {} - def relation.first_or_initialize - @resource ||= User.new - def @resource.save!; end # prevent validation error - @resource - end - User.stub(:where, relation) do - yield(relation.first_or_initialize) - end + describe 'with alternate user model' do + before do + get_via_redirect '/mangs/facebook', { + auth_origin_url: @redirect_url, + omniauth_window_type: 'newWindow' + } + assert_equal 200, response.status + @resource = assigns(:resource) end - - test 'response contains oauth_registration attr with new user' do - - stub_resource do |resource| - def resource.new_record? - true - end - get_via_redirect '/auth/facebook', { - auth_origin_url: @redirect_url - } - - post_message = JSON.parse(/postMessage\((?.*), '\*'\);/m.match(response.body)[:data]) - assert post_message['oauth_registration'] - assert_match 'oauth_registration', @controller.instance_variable_get(:@auth_origin_url) - end + test 'request should determine the correct resource_class' do + assert_equal 'Mang', controller.omniauth_params['resource_class'] end - - test 'response does not contain oauth_registration attr with existing user' do - - stub_resource do |resource| - def resource.new_record? - false - end - get_via_redirect '/auth/facebook', { - auth_origin_url: @redirect_url - } - - post_message = JSON.parse(/postMessage\((?.*), '\*'\);/m.match(response.body)[:data]) - refute post_message['oauth_registration'] - assert_no_match 'oauth_registration', @controller.instance_variable_get(:@auth_origin_url) - end + test 'user should be of the correct class' do + assert_equal Mang, @resource.class end - - - end describe 'pass additional params' do @@ -160,7 +104,8 @@ def resource.new_record? get_via_redirect '/auth/facebook', { auth_origin_url: @redirect_url, favorite_color: @fav_color, - name: @unpermitted_param + name: @unpermitted_param, + omniauth_window_type: 'newWindow' } @resource = assigns(:resource) @@ -179,11 +124,54 @@ def resource.new_record? end end + describe "oauth registration attr" do + + after do + User.any_instance.unstub(:new_record?) + end + + describe 'with new user' do + + before do + User.any_instance.expects(:new_record?).returns(true).at_least_once + end + + test 'response contains oauth_registration attr' do + + get_via_redirect '/auth/facebook', { + auth_origin_url: @redirect_url, + omniauth_window_type: 'newWindow' + } + + assert_equal true, controller.auth_params[:oauth_registration] + end + end + + describe 'with existing user' do + + before do + User.any_instance.expects(:new_record?).returns(false).at_least_once + end + + test 'response does not contain oauth_registration attr' do + + get_via_redirect '/auth/facebook', { + auth_origin_url: @redirect_url, + omniauth_window_type: 'newWindow' + } + + assert_equal false, controller.auth_params.key?(:oauth_registration) + end + + end + + end describe 'using namespaces' do before do get_via_redirect '/api/v1/auth/facebook', { - auth_origin_url: @redirect_url + auth_origin_url: @redirect_url, + omniauth_window_type: 'newWindow' } @resource = assigns(:resource) @@ -201,43 +189,98 @@ def resource.new_record? assert_equal User, @resource.class end end - end + describe 'with omniauth_window_type=inAppBrowser' do - describe 'alternate user model' do - describe 'from api to provider' do - before do - get_via_redirect '/mangs/facebook', { - auth_origin_url: @redirect_url - } + test 'response contains all expected data' do + get_success(omniauth_window_type: 'inAppBrowser') + assert_expected_data_in_new_window + end - @resource = assigns(:resource) + end + + describe 'with omniauth_window_type=newWindow' do + + test 'response contains all expected data' do + get_success(omniauth_window_type: 'newWindow') + assert_expected_data_in_new_window end + end + + def assert_expected_data_in_new_window + data_json = @response.body.match(/var data \= (.+)\;/)[1] + data = ActiveSupport::JSON.decode(data_json) + expected_data = @resource.as_json.merge(controller.auth_params.as_json) + expected_data = ActiveSupport::JSON.decode(expected_data.to_json) + assert_equal(expected_data.merge("message" => "deliverCredentials"), data) + end - test 'status should be success' do + describe 'with omniauth_window_type=sameWindow' do + + test 'redirects to auth_origin_url with all expected query params' do + get_via_redirect '/auth/facebook', { + auth_origin_url: '/auth_origin', + omniauth_window_type: 'sameWindow' + } assert_equal 200, response.status - end - test 'request should determine the correct resource_class' do - assert_equal 'Mang', controller.omniauth_params['resource_class'] - end + # We have been forwarded to a url with all the expected + # data in the query params. - test 'request should pass correct redirect_url' do - assert_equal @redirect_url, controller.omniauth_params['auth_origin_url'] - end + # Assert that a uid was passed along. We have to assume + # that the rest of the values were as well, as we don't + # have access to @resource in this test anymore + assert(uid = controller.params['uid'], "No uid found") - test 'user should have been created' do - assert @resource + # check that all the auth stuff is there + [:auth_token, :client_id, :uid, :expiry, :config].each do |key| + assert(controller.params.key?(key), "No value for #{key.inspect}") + end end + end - test 'user should be assigned info from provider' do - assert_equal 'chongbong@aol.com', @resource.email - end + def get_success(params = {}) + get_via_redirect '/auth/facebook', { + auth_origin_url: @redirect_url, + omniauth_window_type: 'newWindow' + }.merge(params) + assert_equal 200, response.status + @resource = assigns(:resource) + end - test 'user should be of the correct class' do - assert_equal Mang, @resource.class - end + + + end + + describe 'failure callback' do + + + setup do + OmniAuth.config.mock_auth[:facebook] = :invalid_credentials + OmniAuth.config.on_failure = Proc.new { |env| + OmniAuth::FailureEndpoint.new(env).redirect_to_failure + } end + + test 'renders expected data' do + get_via_redirect '/auth/facebook', { + auth_origin_url: @redirect_url, + omniauth_window_type: 'newWindow' + } + assert_equal 200, response.status + + data_json = @response.body.match(/var data \= (.+)\;/)[1] + data = ActiveSupport::JSON.decode(data_json) + + assert_equal({"error"=>"invalid_credentials", "message"=>"authFailure"}, data) + end + + test 'renders somethign with no auth_origin_url' do + get_via_redirect '/auth/facebook' + assert_equal 200, response.status + assert_select "body", "invalid_credentials" + end + end describe 'User with only :database_authenticatable and :registerable included' do @@ -249,4 +292,4 @@ def resource.new_record? } end end -end +end \ No newline at end of file diff --git a/test/controllers/overrides/omniauth_callbacks_controller_test.rb b/test/controllers/overrides/omniauth_callbacks_controller_test.rb index 65265f0ef..640446202 100644 --- a/test/controllers/overrides/omniauth_callbacks_controller_test.rb +++ b/test/controllers/overrides/omniauth_callbacks_controller_test.rb @@ -23,7 +23,8 @@ class Overrides::OmniauthCallbacksControllerTest < ActionDispatch::IntegrationTe get_via_redirect '/evil_user_auth/facebook', { auth_origin_url: Faker::Internet.url, - favorite_color: @favorite_color + favorite_color: @favorite_color, + omniauth_window_type: 'newWindow' } @resource = assigns(:resource) diff --git a/test/dummy/app/controllers/auth_origin_controller.rb b/test/dummy/app/controllers/auth_origin_controller.rb new file mode 100644 index 000000000..6df99d122 --- /dev/null +++ b/test/dummy/app/controllers/auth_origin_controller.rb @@ -0,0 +1,5 @@ +class AuthOriginController < ApplicationController + def redirected + render :nothing => true + end +end \ No newline at end of file diff --git a/test/dummy/config/routes.rb b/test/dummy/config/routes.rb index 24c3c984f..c189223bb 100644 --- a/test/dummy/config/routes.rb +++ b/test/dummy/config/routes.rb @@ -49,4 +49,7 @@ # routes within this block will authorize visitors using the Mang or User class get 'demo/members_only_group', to: 'demo_group#members_only' + + # we need a route for omniauth_callback_controller to redirect to in sameWindow case + get 'auth_origin', to: 'auth_origin#redirected' end diff --git a/test/lib/devise_token_auth/url_test.rb b/test/lib/devise_token_auth/url_test.rb new file mode 100644 index 000000000..012dbf076 --- /dev/null +++ b/test/lib/devise_token_auth/url_test.rb @@ -0,0 +1,11 @@ +require 'test_helper' + +class DeviseTokenAuth::UrlTest < ActiveSupport::TestCase + describe "DeviseTokenAuth::Url#generate" do + test 'URI fragment should appear at the end of URL' do + params = {client_id: 123} + url = 'http://example.com#fragment' + assert_equal DeviseTokenAuth::Url.send(:generate, url, params), "http://example.com?client_id=123#fragment" + end + end +end \ No newline at end of file diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 93a127c9e..0e745ce91 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -99,13 +99,5 @@ class UserTest < ActiveSupport::TestCase assert @resource.save end end - - describe "#generate_url" do - test 'URI fragment should appear at the end of URL' do - params = {client_id: 123} - url = 'http://example.com#fragment' - assert_equal @resource.send(:generate_url, url, params), "http://example.com?client_id=123#fragment" - end - end end end