diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 07f37ee13..15a41a395 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -32,23 +32,26 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController # 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 - if @user.save + 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? 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 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 - if @user.save - 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 - session["devise.#{provider}_data"] = auth - redirect_to new_user_registration_url - end + # If the failure is because something else happens, just present the "new user" form + session["devise.#{provider}_data"] = auth + redirect_to new_user_registration_url end end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index ad5f6abf7..0d32e725a 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -25,6 +25,7 @@ class Users::RegistrationsController < Devise::RegistrationsController def finish_signup current_user.registering_with_oauth = false + current_user.email = current_user.oauth_email if current_user.email.blank? current_user.validate end @@ -32,12 +33,12 @@ class Users::RegistrationsController < Devise::RegistrationsController current_user.registering_with_oauth = false if current_user.update(sign_up_params) - if current_user.confirmed_oauth_email != current_user.email + if current_user.oauth_email != current_user.email current_user.update(confirmed_at: nil) current_user.send_confirmation_instructions end - if current_user.confirmed_oauth_email.present? - current_user.update(confirmed_oauth_email: nil) + if current_user.oauth_email.present? + current_user.update(oauth_email: nil) end sign_in_and_redirect current_user, event: :authentication diff --git a/app/models/user.rb b/app/models/user.rb index 808b433bc..4d1aaf27e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -52,16 +52,16 @@ class User < ActiveRecord::Base # Get the existing user by email if the provider gives us a verified email. def self.first_or_initialize_for_oauth(auth) oauth_email = auth.info.email - confirmed_oauth_email = oauth_email if auth.info.verified || auth.info.verified_email - oauth_user = User.find_by(email: confirmed_oauth_email) if confirmed_oauth_email.present? + oauth_email_confirmed = auth.info.verified || auth.info.verified_email + oauth_user = User.find_by(email: oauth_email) if oauth_email_confirmed oauth_user || User.new( username: auth.info.name || auth.uid, email: oauth_email, - confirmed_oauth_email: confirmed_oauth_email, + oauth_email: oauth_email, password: Devise.friendly_token[0,20], terms_of_service: '1', - confirmed_at: confirmed_oauth_email.present? ? DateTime.now : nil + confirmed_at: oauth_email_confirmed ? DateTime.now : nil ) end @@ -174,11 +174,6 @@ class User < ActiveRecord::Base !erased? && !registering_with_oauth end - # Deactivates the email uniqueness validation when registering with oauth - def email_changed? - !registering_with_oauth && super - end - def has_official_email? domain = Setting['email_domain_for_officials'] !email.blank? && ( (email.end_with? "@#{domain}") || (email.end_with? ".#{domain}") ) diff --git a/db/migrate/20160126090634_rename_confirmed_oauth_email_to_oauth_email.rb b/db/migrate/20160126090634_rename_confirmed_oauth_email_to_oauth_email.rb new file mode 100644 index 000000000..3b3f027fe --- /dev/null +++ b/db/migrate/20160126090634_rename_confirmed_oauth_email_to_oauth_email.rb @@ -0,0 +1,5 @@ +class RenameConfirmedOauthEmailToOauthEmail < ActiveRecord::Migration + def change + rename_column :users, :confirmed_oauth_email, :oauth_email + end +end diff --git a/db/schema.rb b/db/schema.rb index 2fdd31706..a9cce3582 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160125100637) do +ActiveRecord::Schema.define(version: 20160126090634) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -398,7 +398,7 @@ ActiveRecord::Schema.define(version: 20160125100637) do t.integer "notifications_count", default: 0 t.string "locale" t.boolean "registering_with_oauth", default: false - t.string "confirmed_oauth_email" + t.string "oauth_email" end add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree