From a98a04738d032ac1ac943bac0ac5e978e6b77070 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Tue, 14 May 2013 14:02:22 -0500 Subject: [PATCH] * VRFS-312 in-app password updates --- lib/jam_ruby/app/mailers/user_mailer.rb | 2 +- .../user_mailer/password_changed.html.erb | 12 +++-- .../user_mailer/password_changed.text.erb | 2 +- .../user_mailer/updated_email.text.erb | 2 +- lib/jam_ruby/models/user.rb | 52 +++++++++++++------ lib/jam_ruby/models/user_observer.rb | 2 + spec/jam_ruby/models/user_spec.rb | 30 ++++++++--- 7 files changed, 71 insertions(+), 31 deletions(-) diff --git a/lib/jam_ruby/app/mailers/user_mailer.rb b/lib/jam_ruby/app/mailers/user_mailer.rb index 3da683b70..b1ed5041b 100644 --- a/lib/jam_ruby/app/mailers/user_mailer.rb +++ b/lib/jam_ruby/app/mailers/user_mailer.rb @@ -16,7 +16,7 @@ module JamRuby default :from => DEFAULT_SENDER sendgrid_category :use_subject_lines - sendgrid_enable :opentrack, :clicktrack + #sendgrid_enable :opentrack, :clicktrack # this makes our emails creepy, imo (seth) sendgrid_unique_args :env => Environment.mode def welcome_message(user, signup_confirm_url) diff --git a/lib/jam_ruby/app/views/jam_ruby/user_mailer/password_changed.html.erb b/lib/jam_ruby/app/views/jam_ruby/user_mailer/password_changed.html.erb index f9a712566..e2fe554ae 100644 --- a/lib/jam_ruby/app/views/jam_ruby/user_mailer/password_changed.html.erb +++ b/lib/jam_ruby/app/views/jam_ruby/user_mailer/password_changed.html.erb @@ -1,5 +1,7 @@ - - -You Just Changed Your Password! <%= @user.email %> - - + +

Jamkazam Password Changed

+

You just changed your password!

+
+ + + diff --git a/lib/jam_ruby/app/views/jam_ruby/user_mailer/password_changed.text.erb b/lib/jam_ruby/app/views/jam_ruby/user_mailer/password_changed.text.erb index 125193f24..7dec8f658 100644 --- a/lib/jam_ruby/app/views/jam_ruby/user_mailer/password_changed.text.erb +++ b/lib/jam_ruby/app/views/jam_ruby/user_mailer/password_changed.text.erb @@ -1 +1 @@ -You Just Changed Your Password! <%= @user.email %> +You just changed your password! \ No newline at end of file diff --git a/lib/jam_ruby/app/views/jam_ruby/user_mailer/updated_email.text.erb b/lib/jam_ruby/app/views/jam_ruby/user_mailer/updated_email.text.erb index 808d8eb93..d85bfcca8 100644 --- a/lib/jam_ruby/app/views/jam_ruby/user_mailer/updated_email.text.erb +++ b/lib/jam_ruby/app/views/jam_ruby/user_mailer/updated_email.text.erb @@ -1 +1 @@ -do it \ No newline at end of file +<%= @user.email %> is confirmed! \ No newline at end of file diff --git a/lib/jam_ruby/models/user.rb b/lib/jam_ruby/models/user.rb index 7ac8f8b32..053e8434d 100644 --- a/lib/jam_ruby/models/user.rb +++ b/lib/jam_ruby/models/user.rb @@ -6,11 +6,13 @@ module JamRuby #devise: for later: :trackable devise :database_authenticatable, - :recoverable, :rememberable, :validatable + :recoverable, :rememberable - attr_accessible :first_name, :last_name, :email, :password, :password_confirmation, :city, :state, :country, :subscribe_email, :terms_of_service - attr_accessor :updating_password, :updating_email, :updated_email, :update_email_confirmation_url, :administratively_created, :password_validation + attr_accessible :first_name, :last_name, :email, :city, :password, :password_confirmation, :state, :country, :subscribe_email, :terms_of_service + + # updating_password corresponds to a lost_password + attr_accessor :updating_password, :updating_email, :updated_email, :update_email_confirmation_url, :administratively_created, :current_password, :setting_password, :confirm_current_password # authorizations (for facebook, etc -- omniauth) has_many :user_authorizations, :class_name => "JamRuby::UserAuthorization" @@ -116,28 +118,29 @@ module JamRuby uniqueness: {case_sensitive: false} validates :update_email, presence: true, format: {with: VALID_EMAIL_REGEX}, uniqueness: {case_sensitive: false}, :if => :updating_email - validates_length_of :password, minimum: 6, maximum: 100, :if => :should_validate_password? + validates_length_of :password, minimum: 6, maximum: 100, :if => :should_validate_password? validates_presence_of :password_confirmation, :if => :should_validate_password? validates_confirmation_of :password, :if => :should_validate_password? + validates :terms_of_service, :acceptance => {:accept => true, :on => :create, :allow_nil => false } validates :subscribe_email, :inclusion => {:in => [nil, true, false]} validate :validate_musician_instruments - validate :check_password_valid - validate :check_update_email + validate :validate_current_password + validate :validate_update_email def validate_musician_instruments errors.add(:musician_instruments, ValidationMessages::INSTRUMENT_MINIMUM_NOT_MET) if !administratively_created && musician && musician_instruments.length == 0 errors.add(:musician_instruments, ValidationMessages::INSTRUMENT_LIMIT_EXCEEDED) if !administratively_created && musician && musician_instruments.length > 5 end - def check_password_valid + def validate_current_password # checks if the user put in their current password (used when changing your email, for instance) - errors.add(:password_validation, ValidationMessages::NOT_YOUR_PASSWORD) if should_confirm_existing_password? && !valid_password?(self.password_validation) + errors.add(:current_password, ValidationMessages::NOT_YOUR_PASSWORD) if should_confirm_existing_password? && !valid_password?(self.current_password) end - def check_update_email + def validate_update_email if updating_email && self.update_email == self.email errors.add(:update_email, ValidationMessages::EMAIL_MATCHES_CURRENT) elsif updating_email && User.find_by_email(self.update_email) != nil @@ -177,7 +180,7 @@ module JamRuby end def should_confirm_existing_password? - updating_email + confirm_current_password end def end_user_created? @@ -259,9 +262,26 @@ module JamRuby end def set_password(old_password, new_password, new_password_confirmation) - raise JamRuby::JamArgumentError unless valid_password? old_password - change_password(new_password, new_password_confirmation) - save + + # so that UserObserver knows to send a confirmation email on success + self.setting_password = true + # so that should_validate_password? fires + self.updating_password = true + + attributes = { :password => new_password, :password_confirmation => new_password_confirmation } + + # taken liberally from Devise::DatabaseAuthenticatable.update_with_password + + if valid_password?(old_password) + update_attributes(attributes) + else + self.assign_attributes(attributes) + self.valid? + self.errors.add(:current_password, current_password.blank? ? :blank : :invalid) + end + + #clean_up_passwords + end def self.set_password_from_token(email, token, new_password, new_password_confirmation) @@ -438,14 +458,14 @@ module JamRuby return user end - def begin_update_email(email, password_validation, confirmation_url) + def begin_update_email(email, current_password, confirmation_url) # sets the user model in a state such that it's expecting to have it's email updated # two columns matter for this; 'update_email_token' and 'update_email' # confirmation_link is odd in the sense that it can likely only come from www.jamkazam.com (jam-web) # an observer should be set up to send an email based on this activity - self.updating_email = true - self.password_validation = password_validation + self.updating_email = self.confirm_current_password = true + self.current_password = current_password self.update_email = email self.update_email_token = SecureRandom.urlsafe_base64 self.update_email_confirmation_url = "#{confirmation_url}#{self.update_email_token}" diff --git a/lib/jam_ruby/models/user_observer.rb b/lib/jam_ruby/models/user_observer.rb index c71989f49..5a031340a 100644 --- a/lib/jam_ruby/models/user_observer.rb +++ b/lib/jam_ruby/models/user_observer.rb @@ -8,6 +8,8 @@ module JamRuby UserMailer.updating_email(user).deliver elsif user.updated_email && !user.errors.any? UserMailer.updated_email(user).deliver + elsif user.setting_password && !user.errors.any? + UserMailer.password_changed(user).deliver end end end diff --git a/spec/jam_ruby/models/user_spec.rb b/spec/jam_ruby/models/user_spec.rb index aa8ea7d0e..1964e8961 100644 --- a/spec/jam_ruby/models/user_spec.rb +++ b/spec/jam_ruby/models/user_spec.rb @@ -130,25 +130,42 @@ describe User do describe "set_password" do before do @user.confirm_email! + @user.save.should be_true + UserMailer.deliveries.clear end it "setting a new password should work" do - @user.set_password("foobar", "newpassword", "newpassword") - User.authenticate(@user.email, "newpassword").should_not be_nil + @user.set_password("foobar", "newpassword", "newpassword") + User.authenticate(@user.email, "newpassword").should_not be_nil + UserMailer.deliveries.length.should == 1 end it "setting a new password should fail if old one doesnt match" do - expect { @user.set_password("wrongold", "newpassword", "newpassword") }.to raise_error + @user.set_password("wrongold", "newpassword", "newpassword") + @user.errors.any?.should be_true + @user.errors[:current_password].length.should == 1 + UserMailer.deliveries.length.should == 0 end it "setting a new password should fail if new ones dont match" do @user.set_password("foobar", "newpassword", "newpassword2") - User.authenticate(@user.email, "newpassword").should be_nil + @user.errors.any?.should be_true + @user.errors[:password].length.should == 1 + UserMailer.deliveries.length.should == 0 end it "setting a new password should fail if new one doesnt validate" do @user.set_password("foobar", "a", "a") - User.authenticate(@user.email, "newpassword").should be_nil + @user.errors.any?.should be_true + @user.errors[:password].length.should == 1 + UserMailer.deliveries.length.should == 0 + end + + it "setting a new password should fail if the new one is null" do + @user.set_password("foobar", nil, nil) + @user.errors.any?.should be_true + @user.errors[:password].length.should == 1 + UserMailer.deliveries.length.should == 0 end end @@ -268,7 +285,6 @@ describe User do end # useful to see contents of email without actually running the app and sending it - # it { puts UserMailer.deliveries[0].parts[0].body.decoded } it { @user.errors.any?.should be_false } it { @user.update_email.should == "somenewemail@blah.com" } it { @user.update_email_confirmation_url.should == "http://www.jamkazam.com/confirm_email_update?token=#{@user.update_email_token}" } @@ -285,7 +301,7 @@ describe User do it "bad password validation" do @user.begin_update_email("somenewemail@blah.com", "wrong password", "http://www.jamkazam.com/confirm_email_update?token=") - @user.errors[:password_validation][0].should == ValidationMessages::NOT_YOUR_PASSWORD + @user.errors[:current_password][0].should == ValidationMessages::NOT_YOUR_PASSWORD end it "matches current email" do