diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index f5682d97b..ca0dfc667 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -30,29 +30,31 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController identity = Identity.first_or_create_from_oauth(auth) @user = current_user || identity.user || User.first_or_initialize_for_oauth(auth) - # If there are no problems with the email/username, then they were provided by oauth or they - # correspond to an existing user. Associate the identity and sign in - unless @user.save - # If either the username or email have provoked a failure, we save the user anyway (but marked for revision) - # This mark will be detected by applicationcontroller and the user will be redirected to finish_signup - @user.registering_with_oauth = true - unless @user.save - # If we still can't save the user, the email might be invalidating devise's validatable "unique" - # constraint. Set email to nil and try again (we'll reset later using oauth_email) - @user.email = nil - @user.save - end - end - - if @user.persisted? + if save_user(@user) identity.update(user: @user) sign_in_and_redirect @user, event: :authentication set_flash_message(:notice, :success, kind: "#{provider}".capitalize) if is_navigational_format? else - # If the failure is because something else happens, just present the "new user" form + # If saving the user was not possible (weird errors, etc) just present the "new user" form session["devise.#{provider}_data"] = auth redirect_to new_user_registration_url end end + def save_user(user) + # If there are no problems with the email/username, then they were provided by oauth or they + # correspond to an existing user. Associate the identity and sign in + return true if @user.save + + # If either the username or email have provoked a failure, we save the user anyway (but marked for revision) + # This mark will be detected by applicationcontroller and the user will be redirected to finish_signup + @user.registering_with_oauth = true + return true if @user.save + + # If we still can't save the user, the email might be invalidating devise's validatable "unique" + # constraint. Set email to nil and try again (we'll reset later using oauth_email) + @user.email = nil + @user.save + end + end diff --git a/spec/features/users_auth_spec.rb b/spec/features/users_auth_spec.rb index 0105a8dbd..c757c1c90 100644 --- a/spec/features/users_auth_spec.rb +++ b/spec/features/users_auth_spec.rb @@ -47,12 +47,11 @@ feature 'Users' do context 'OAuth authentication' do context 'Twitter' do - let(:twitter_hash){ {'provider' => 'twitter', 'uid' => '12345', 'info' => { 'name' => 'manuela' }} } - let(:twitter_hash_with_email){ { 'provider' => 'twitter', 'uid' => '12345', 'info' => {'name' => 'manuela', 'email' => 'manuelacarmena@example.com' } } } - let(:twitter_hash_with_verified_email) { { 'provider' => 'twitter', - 'uid' => '12345', - 'info' => { 'name' => 'manuela' , 'email' => 'manuelacarmena@example.com', 'verified' => '1' } } - } + let(:twitter_hash){ {provider: 'twitter', uid: '12345', info: {name: 'manuela'}} } + let(:twitter_hash_with_email){ {provider: 'twitter', uid: '12345', info: {name: 'manuela', email: 'manuelacarmena@example.com'}} } + let(:twitter_hash_with_verified_email){ {provider: 'twitter', + uid: '12345', + info: {name: 'manuela', email: 'manuelacarmena@example.com', verified: '1'}} } scenario 'Sign up when Oauth provider has a verified email' do