diff --git a/app/assets/stylesheets/modules/shared.css b/app/assets/stylesheets/modules/shared.css index 5928ae378cc..3fa7523590b 100644 --- a/app/assets/stylesheets/modules/shared.css +++ b/app/assets/stylesheets/modules/shared.css @@ -152,5 +152,8 @@ a.page__heading { .push--bottom { margin-bottom: 60px; } +.push--bottom-s { + margin-bottom: 15px; } + .push--s { margin-top: 20px; } diff --git a/app/controllers/email_confirmations_controller.rb b/app/controllers/email_confirmations_controller.rb index 6f9f36ed36b..66c53fb38ce 100644 --- a/app/controllers/email_confirmations_controller.rb +++ b/app/controllers/email_confirmations_controller.rb @@ -24,6 +24,17 @@ def create redirect_to root_path, notice: t('.promise_resend') end + # used to resend confirmation mail for unconfirmed_email validation + def unconfirmed + if current_user.regenerate_confirmation_token && current_user.save + Mailer.delay.email_reset(current_user) + flash[:notice] = t('profiles.update.confirmation_mail_sent') + else + flash[:notice] = t('.try_again') + end + redirect_to edit_profile_path + end + private def confirmation_params diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index cee06db3beb..8d50820fb88 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -16,17 +16,13 @@ def show def update @user = current_user.clone if @user.update_attributes(params_user) - if @user.unconfirmed? + if @user.unconfirmed_email Mailer.delay.email_reset(current_user) - sign_out - flash[:notice] = "You will receive an email within the next few " \ - "minutes. It contains instructions for reconfirming " \ - "your account with your new email address." - redirect_to_root + flash[:notice] = t('.confirmation_mail_sent') else - flash[:notice] = "Your profile was updated." - redirect_to edit_profile_path + flash[:notice] = t('.updated') end + redirect_to edit_profile_path else current_user.reload render :edit diff --git a/app/mailers/mailer.rb b/app/mailers/mailer.rb index 7a4a10ebc4b..7220d876ebe 100644 --- a/app/mailers/mailer.rb +++ b/app/mailers/mailer.rb @@ -5,7 +5,7 @@ class Mailer < ActionMailer::Base def email_reset(user) @user = User.find(user['id']) mail from: Clearance.configuration.mailer_sender, - to: @user.email, + to: @user.unconfirmed_email, subject: I18n.t('mailer.confirmation_subject', default: 'Please confirm your email address with RubyGems.org') end diff --git a/app/models/user.rb b/app/models/user.rb index 02310a770c9..e2a38d65fa1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -23,7 +23,7 @@ class User < ActiveRecord::Base has_many :subscriptions has_many :web_hooks - before_validation :unconfirm_email, if: :email_changed?, on: :update + after_validation :set_unconfirmed_email, if: :email_changed?, on: :update before_create :generate_api_key, :regenerate_confirmation_token validates :handle, uniqueness: true, allow_nil: true @@ -40,6 +40,7 @@ class User < ActiveRecord::Base validates :twitter_username, length: { within: 0..20 }, allow_nil: true validates :password, length: { within: 10..200 }, allow_nil: true, unless: :skip_password_validation? + validate :unconfirmed_email_uniqueness def self.authenticate(who, password) user = find_by(email: who.downcase) || find_by(handle: who) @@ -101,8 +102,8 @@ def encode_with(coder) coder.map = payload end - def unconfirm_email - self.email_confirmed = false + def set_unconfirmed_email + self.attributes = { unconfirmed_email: email, email: email_was } regenerate_confirmation_token end @@ -123,6 +124,7 @@ def total_rubygems_count end def confirm_email! + update_email! if unconfirmed_email update!(email_confirmed: true, confirmation_token: nil) end @@ -139,4 +141,19 @@ def regenerate_confirmation_token def unconfirmed? !email_confirmed end + + private + + def update_email! + self.attributes = { email: unconfirmed_email, unconfirmed_email: nil } + save!(validate: false) + end + + def unconfirmed_email_uniqueness + errors.add(:email, I18n.t('errors.messages.taken')) if unconfirmed_email_exists? + end + + def unconfirmed_email_exists? + User.where(unconfirmed_email: email).exists? + end end diff --git a/app/views/profiles/edit.html.erb b/app/views/profiles/edit.html.erb index ac17f0e5a5e..b989749b353 100644 --- a/app/views/profiles/edit.html.erb +++ b/app/views/profiles/edit.html.erb @@ -35,6 +35,15 @@ + <% if current_user.unconfirmed_email %> +
+

+ <%= t('.email_awaiting_confirmation', unconfirmed_email: current_user.unconfirmed_email) %> +

+ <%= link_to "Resend confirmation", unconfirmed_email_confirmations_path, method: :patch, class: 'form__field__instructions t-link' %> +
+ <% end %> +
<%= form.label :email, :class => 'form__label' %> <%= form.email_field :email, :class => 'form__input' %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 50da05675e2..bf4d1b6a77b 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -94,6 +94,7 @@ en: profiles: edit: title: Edit profile + email_awaiting_confirmation: Please confirm your new email address %{unconfirmed_email} hide_email: Hide email in public profile optional_twitter_username: Optional Twitter username. Will be displayed publicly enter_password: Please enter your account's password @@ -109,6 +110,8 @@ en: reset: Reset my API key confirm_reset: Are you sure? This cannot be undone. update: + confirmation_mail_sent: You will receive an email within the next few minutes. It contains instructions for confirming your new email address. + updated: Your profile was updated. request_denied: This request was denied. We could not verify your password. rubygems: @@ -254,3 +257,5 @@ en: submit: Resend Confirmation create: promise_resend: We will email you confirmation link to activate your account if one exists. + unconfirmed_email: + try_again: "Something went wrong. Please try again." diff --git a/config/routes.rb b/config/routes.rb index 38983c7e486..cadf8f7bd1d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -148,6 +148,7 @@ resource :email_confirmations, only: [:new, :create] do get 'confirm/:token', to: 'email_confirmations#update', as: :update + patch 'unconfirmed' end # login path is "/session" => "session#create" diff --git a/db/migrate/20161231080902_add_unconfirmed_email_to_users.rb b/db/migrate/20161231080902_add_unconfirmed_email_to_users.rb new file mode 100644 index 00000000000..6dcb7eea1ab --- /dev/null +++ b/db/migrate/20161231080902_add_unconfirmed_email_to_users.rb @@ -0,0 +1,5 @@ +class AddUnconfirmedEmailToUsers < ActiveRecord::Migration + def change + add_column :users, :unconfirmed_email, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 9b783a4bf3a..b2f94b40ae8 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: 20160929104437) do +ActiveRecord::Schema.define(version: 20161231080902) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -143,6 +143,7 @@ t.string "handle" t.boolean "hide_email" t.string "twitter_username" + t.string "unconfirmed_email" end add_index "users", ["email"], name: "index_users_on_email", using: :btree diff --git a/test/functional/profiles_controller_test.rb b/test/functional/profiles_controller_test.rb index 896595787bc..a0c5e4192d2 100644 --- a/test/functional/profiles_controller_test.rb +++ b/test/functional/profiles_controller_test.rb @@ -142,6 +142,30 @@ class ProfilesControllerTest < ActionController::TestCase assert_equal @handle, @user.handle end end + + context "updating email with existing email" do + setup do + create(:user, email: "cannotchange@tothis.com") + put :update, user: { email: "cannotchange@tothis.com", password: @user.password } + end + + should "not set unconfirmed_email" do + assert page.has_content? "Email address has already been taken" + refute_equal "cannotchange@tothis.com", @user.unconfirmed_email + end + end + + context "updating email with existing unconfirmed_email" do + setup do + create(:user, unconfirmed_email: "cannotchange@tothis.com") + put :update, user: { email: "cannotchange@tothis.com", password: @user.password } + end + + should "not set unconfirmed_email" do + assert page.has_content? "Email address has already been taken" + refute_equal "cannotchange@tothis.com", @user.unconfirmed_email + end + end end end diff --git a/test/integration/profile_test.rb b/test/integration/profile_test.rb index e9c04244bed..31dd9627a73 100644 --- a/test/integration/profile_test.rb +++ b/test/integration/profile_test.rb @@ -53,7 +53,7 @@ def sign_in assert page.has_link?("nick1", href: "/profiles/nick1") end - test "changing email signs out user and asks to confirm email" do + test "changing email does not change email and asks to confirm email" do sign_in visit profile_path("nick1") click_link "Edit Profile" @@ -62,17 +62,18 @@ def sign_in fill_in "Password", with: "password12345" click_button "Update" - assert page.has_content? "Sign in" + assert page.has_selector? "input[value='nick@example.com']" assert page.has_selector? '#flash_notice', text: "You will receive "\ "an email within the next few minutes. It contains instructions "\ - "for reconfirming your account with your new email address." + "for confirming your new email address." link = last_email_link assert_not_nil link visit link - assert page.has_content? "Sign out" assert page.has_selector? "#flash_notice", text: "Your email address has been verified" + visit edit_profile_path + assert page.has_selector? "input[value='nick2@example.com']" end test "disabling email on profile" do diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 891ee5e1991..4d1b5c92d14 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -176,8 +176,8 @@ def assert_resetting_email_changes(attr_name) assert_resetting_email_changes :confirmation_token end - should "unconfirm email" do - assert_resetting_email_changes :unconfirmed? + should "store unconfirm email" do + assert_resetting_email_changes :unconfirmed_email end should "reset token_expires_at" do