From d75f795cad0973ebb2969b61086ee2fa2ac0f630 Mon Sep 17 00:00:00 2001 From: aoh0x7DE Date: Tue, 5 Jul 2016 16:57:23 -0700 Subject: [PATCH] Fix issue #6847 (#6905) * Fix issue #6847 --- app/controllers/users_controller.rb | 49 +++++++++++++++-------------- app/models/user.rb | 20 ++++++++++-- spec/models/user_spec.rb | 16 ++++++++-- 3 files changed, 57 insertions(+), 28 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0e67c7b01..48d99c64b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -25,49 +25,50 @@ class UsersController < ApplicationController # change email notifications if u[:email_preferences] @user.update_user_preferences(u[:email_preferences]) - flash[:notice] = I18n.t 'users.update.email_notifications_changed' + flash[:notice] = I18n.t "users.update.email_notifications_changed" # change password elsif params[:change_password] if @user.update_with_password(u) password_changed = true - flash[:notice] = I18n.t 'users.update.password_changed' + flash[:notice] = I18n.t "users.update.password_changed" else - flash[:error] = I18n.t 'users.update.password_not_changed' + flash[:error] = I18n.t "users.update.password_not_changed" end elsif u[:show_community_spotlight_in_stream] || u[:getting_started] if @user.update_attributes(u) - flash[:notice] = I18n.t 'users.update.settings_updated' + flash[:notice] = I18n.t "users.update.settings_updated" else - flash[:notice] = I18n.t 'users.update.settings_not_updated' + flash[:notice] = I18n.t "users.update.settings_not_updated" end elsif u[:strip_exif] if @user.update_attributes(u) - flash[:notice] = I18n.t 'users.update.settings_updated' + flash[:notice] = I18n.t "users.update.settings_updated" else - flash[:notice] = I18n.t 'users.update.settings_not_updated' + flash[:notice] = I18n.t "users.update.settings_not_updated" end elsif u[:language] if @user.update_attributes(u) I18n.locale = @user.language - flash[:notice] = I18n.t 'users.update.language_changed' + flash[:notice] = I18n.t "users.update.language_changed" else - flash[:error] = I18n.t 'users.update.language_not_changed' + flash[:error] = I18n.t "users.update.language_not_changed" end elsif u[:email] @user.unconfirmed_email = u[:email] if @user.save @user.send_confirm_email if @user.unconfirmed_email - flash[:notice] = I18n.t 'users.update.unconfirmed_email_changed' + flash[:notice] = I18n.t "users.update.unconfirmed_email_changed" end else - flash[:error] = I18n.t 'users.update.unconfirmed_email_not_changed' + @user.reload # match user object with the database + flash[:error] = I18n.t "users.update.unconfirmed_email_not_changed" end elsif u[:auto_follow_back] if @user.update_attributes(u) - flash[:notice] = I18n.t 'users.update.follow_settings_changed' + flash[:notice] = I18n.t "users.update.follow_settings_changed" else - flash[:error] = I18n.t 'users.update.follow_settings_not_changed' + flash[:error] = I18n.t "users.update.follow_settings_not_changed" end elsif u[:color_theme] if @user.update_attributes(u) @@ -98,9 +99,9 @@ class UsersController < ApplicationController redirect_to(new_user_session_path(format: request[:format]), notice: I18n.t("users.destroy.success")) else if params[:user].present? && params[:user][:current_password].present? - flash[:error] = t 'users.destroy.wrong_password' + flash[:error] = t "users.destroy.wrong_password" else - flash[:error] = t 'users.destroy.no_password' + flash[:error] = t "users.destroy.no_password" end redirect_to :back end @@ -111,16 +112,16 @@ class UsersController < ApplicationController respond_to do |format| format.atom do @posts = Post.where(author_id: @user.person_id, public: true) - .order('created_at DESC') - .limit(25) - .map {|post| post.is_a?(Reshare) ? post.absolute_root : post } - .compact + .order("created_at DESC") + .limit(25) + .map {|post| post.is_a?(Reshare) ? post.absolute_root : post } + .compact end format.any { redirect_to person_path(@user.person) } end else - redirect_to stream_path, :error => I18n.t('users.public.does_not_exist', :username => params[:username]) + redirect_to stream_path, error: I18n.t("users.public.does_not_exist", username: params[:username]) end end @@ -141,7 +142,7 @@ class UsersController < ApplicationController def export_profile current_user.queue_export - flash[:notice] = I18n.t('users.edit.export_in_progress') + flash[:notice] = I18n.t("users.edit.export_in_progress") redirect_to edit_user_path end @@ -151,7 +152,7 @@ class UsersController < ApplicationController def export_photos current_user.queue_export_photos - flash[:notice] = I18n.t('users.edit.export_photos_in_progress') + flash[:notice] = I18n.t("users.edit.export_photos_in_progress") redirect_to edit_user_path end @@ -171,9 +172,9 @@ class UsersController < ApplicationController def confirm_email if current_user.confirm_email(params[:token]) - flash[:notice] = I18n.t('users.confirm_email.email_confirmed', :email => current_user.email) + flash[:notice] = I18n.t("users.confirm_email.email_confirmed", email: current_user.email) elsif current_user.unconfirmed_email.present? - flash[:error] = I18n.t('users.confirm_email.email_not_confirmed') + flash[:error] = I18n.t("users.confirm_email.email_not_confirmed") end redirect_to edit_user_path end diff --git a/app/models/user.rb b/app/models/user.rb index dc9723440..079ab71e7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -32,6 +32,8 @@ class User < ActiveRecord::Base validates :color_theme, inclusion: {in: AVAILABLE_COLOR_THEME_CODES}, allow_blank: true validates_format_of :unconfirmed_email, :with => Devise.email_regexp, :allow_blank => true + validate :unconfirmed_email_quasiuniqueness + validates_presence_of :person, :unless => proc {|user| user.invitation_token.present?} validates_associated :person validate :no_person_with_same_username @@ -83,6 +85,8 @@ class User < ActiveRecord::Base before_save :guard_unconfirmed_email + after_save :remove_invalid_unconfirmed_emails + def self.all_sharing_with_person(person) User.joins(:contacts).where(:contacts => {:person_id => person.id}) end @@ -484,6 +488,13 @@ class User < ActiveRecord::Base end + # Ensure that the unconfirmed email isn't already someone's email + def unconfirmed_email_quasiuniqueness + if User.exists?(["id != ? AND email = ?", id, unconfirmed_email]) + errors.add(:unconfirmed_email, I18n.t("errors.messages.taken")) + end + end + def guard_unconfirmed_email self.unconfirmed_email = nil if unconfirmed_email.blank? || unconfirmed_email == email @@ -492,11 +503,16 @@ class User < ActiveRecord::Base end end + # Whenever email is set, clear all unconfirmed emails which match + def remove_invalid_unconfirmed_emails + User.where(unconfirmed_email: email).update_all(unconfirmed_email: nil) if email_changed? + end + # Generate public/private keys for User and associated Person def generate_keys - key_size = (Rails.env == 'test' ? 512 : 4096) + key_size = (Rails.env == "test" ? 512 : 4096) - self.serialized_private_key = OpenSSL::PKey::RSA::generate(key_size).to_s if self.serialized_private_key.blank? + self.serialized_private_key = OpenSSL::PKey::RSA.generate(key_size).to_s if serialized_private_key.blank? if self.person && self.person.serialized_public_key.blank? self.person.serialized_public_key = OpenSSL::PKey::RSA.new(self.serialized_private_key).public_key.to_s diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8ba7a391e..9b551e95a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -305,6 +305,13 @@ describe User, :type => :model do alice.email = "somebody@anywhere" expect(alice).not_to be_valid end + + it "resets a matching unconfirmed_email on save" do + eve.update_attribute :unconfirmed_email, "new@example.com" + alice.update_attribute :email, "new@example.com" + eve.reload + expect(eve.unconfirmed_email).to eql(nil) + end end describe "of unconfirmed_email" do @@ -316,11 +323,16 @@ describe User, :type => :model do end it "does NOT require a unique unconfirmed_email address" do - eve.update_attribute :unconfirmed_email, "new@email.com" - alice.unconfirmed_email = "new@email.com" + eve.update_attribute :unconfirmed_email, "new@example.com" + alice.unconfirmed_email = "new@example.com" expect(alice).to be_valid end + it "requires an unconfirmed_email address which is not another user's email address" do + alice.unconfirmed_email = eve.email + expect(alice).not_to be_valid + end + it "requires a valid unconfirmed_email address" do alice.unconfirmed_email = "somebody@anywhere" expect(alice).not_to be_valid