From 732e31528efda517e71167eb512cca6f674dd1e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 8 Feb 2010 23:14:03 +0100 Subject: [PATCH] More changes in update_with_password. --- app/controllers/registrations_controller.rb | 1 + app/controllers/sessions_controller.rb | 8 ++---- app/views/registrations/edit.html.erb | 13 +++++---- devise.gemspec | 13 ++++++--- lib/devise/locales/en.yml | 2 ++ lib/devise/models/authenticatable.rb | 31 +++++++++++++++++---- test/models/authenticatable_test.rb | 26 +++++++++++++++-- 7 files changed, 71 insertions(+), 23 deletions(-) diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 5f6af555..eefaa791 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -41,6 +41,7 @@ class RegistrationsController < ApplicationController # DELETE /resource def destroy self.resource.destroy + set_flash_message :notice, :destroyed sign_out_and_redirect(self.resource) end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 9f8dff41..1eb57c17 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -22,7 +22,7 @@ class SessionsController < ApplicationController sign_in_and_redirect(resource_name, resource, true) else set_now_flash_message :alert, (warden.message || :invalid) - clean_up_password_methods(build_resource) + clean_up_passwords(build_resource) render_with_scope :new end end @@ -39,9 +39,7 @@ class SessionsController < ApplicationController flash[:"#{resource_name}_signed_up"] end - def clean_up_password_methods(object) - [:password=, :password_confirmation=].each do |method| - object.send(method, nil) if object.respond_to?(method) - end + def clean_up_passwords(object) + object.clean_up_passwords if object.respond_to?(:clean_up_passwords) end end diff --git a/app/views/registrations/edit.html.erb b/app/views/registrations/edit.html.erb index 072fd4fb..58f3b49f 100644 --- a/app/views/registrations/edit.html.erb +++ b/app/views/registrations/edit.html.erb @@ -1,24 +1,25 @@

Edit <%= resource_name.to_s.humanize %>

-<% form_for resource_name, resource, :url => registration_path(resource_name) do |f| -%> +<% form_for resource_name, resource, :url => registration_path(resource_name), :html => { :method => :put } do |f| -%> <%= f.error_messages %> -

Change your settings

<%= f.label :email %>

<%= f.text_field :email %>

-

Change your password (leave blank if you don't want to)

-

<%= f.label :password %>

+

<%= f.label :password %> (leave blank if you don't want to change it)

<%= f.password_field :password %>

<%= f.label :password_confirmation %>

<%= f.password_field :password_confirmation %>

-

We need your current password to confirm your changes

-

<%= f.label :current_password %>

+

<%= f.label :current_password %> (we need your current password to confirm your changes)

<%= f.password_field :current_password %>

<%= f.submit "Update" %>

<% end -%> +

Cancel your account

+ +

Unhappy? <%= link_to "Cancel your account", registration_path(resource_name), :confirm => "Are you sure?", :method => :delete %>

+ <%= render :partial => "shared/devise_links" %> diff --git a/devise.gemspec b/devise.gemspec index 56415caa..723d620c 100644 --- a/devise.gemspec +++ b/devise.gemspec @@ -5,11 +5,11 @@ Gem::Specification.new do |s| s.name = %q{devise} - s.version = "0.9.2" + s.version = "1.0.0" s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version= s.authors = ["Jos\303\251 Valim", "Carlos Ant\303\264nio"] - s.date = %q{2010-02-05} + s.date = %q{2010-02-08} s.description = %q{Flexible authentication solution for Rails with Warden} s.email = %q{contact@plataformatec.com.br} s.extra_rdoc_files = [ @@ -24,6 +24,7 @@ Gem::Specification.new do |s| "TODO", "app/controllers/confirmations_controller.rb", "app/controllers/passwords_controller.rb", + "app/controllers/registrations_controller.rb", "app/controllers/sessions_controller.rb", "app/controllers/unlocks_controller.rb", "app/models/devise_mailer.rb", @@ -33,6 +34,8 @@ Gem::Specification.new do |s| "app/views/devise_mailer/unlock_instructions.html.erb", "app/views/passwords/edit.html.erb", "app/views/passwords/new.html.erb", + "app/views/registrations/edit.html.erb", + "app/views/registrations/new.html.erb", "app/views/sessions/new.html.erb", "app/views/shared/_devise_links.erb", "app/views/unlocks/new.html.erb", @@ -49,7 +52,6 @@ Gem::Specification.new do |s| "generators/devise_views/devise_views_generator.rb", "init.rb", "lib/devise.rb", - "lib/devise/controllers/common.rb", "lib/devise/controllers/helpers.rb", "lib/devise/controllers/internal_helpers.rb", "lib/devise/controllers/url_helpers.rb", @@ -73,6 +75,7 @@ Gem::Specification.new do |s| "lib/devise/models/confirmable.rb", "lib/devise/models/lockable.rb", "lib/devise/models/recoverable.rb", + "lib/devise/models/registerable.rb", "lib/devise/models/rememberable.rb", "lib/devise/models/timeoutable.rb", "lib/devise/models/token_authenticatable.rb", @@ -87,6 +90,7 @@ Gem::Specification.new do |s| "lib/devise/schema.rb", "lib/devise/strategies/authenticatable.rb", "lib/devise/strategies/base.rb", + "lib/devise/strategies/http_authenticatable.rb", "lib/devise/strategies/rememberable.rb", "lib/devise/strategies/token_authenticatable.rb", "lib/devise/test_helpers.rb", @@ -106,8 +110,10 @@ Gem::Specification.new do |s| "test/failure_app_test.rb", "test/integration/authenticatable_test.rb", "test/integration/confirmable_test.rb", + "test/integration/http_authenticatable_test.rb", "test/integration/lockable_test.rb", "test/integration/recoverable_test.rb", + "test/integration/registerable_test.rb", "test/integration/rememberable_test.rb", "test/integration/timeoutable_test.rb", "test/integration/token_authenticatable_test.rb", @@ -150,7 +156,6 @@ Gem::Specification.new do |s| "test/routes_test.rb", "test/support/assertions_helper.rb", "test/support/integration_tests_helper.rb", - "test/support/model_tests_helper.rb", "test/support/test_silencer.rb", "test/support/tests_helper.rb", "test/test_helper.rb", diff --git a/lib/devise/locales/en.yml b/lib/devise/locales/en.yml index 2c163a0a..3428bf06 100644 --- a/lib/devise/locales/en.yml +++ b/lib/devise/locales/en.yml @@ -22,6 +22,8 @@ en: registrations: link: 'Sign up' signed_up: 'You have signed up successfully.' + updated: 'You updated your account successfully.' + destroyed: 'Bye! Your account was successfully cancelled. We hope to see you again soon.' unlocks: link: "Didn't receive unlock instructions?" send_instructions: 'You will receive an email with instructions about how to unlock your account in a few minutes.' diff --git a/lib/devise/models/authenticatable.rb b/lib/devise/models/authenticatable.rb index 6da79b39..77b8f47a 100644 --- a/lib/devise/models/authenticatable.rb +++ b/lib/devise/models/authenticatable.rb @@ -37,6 +37,7 @@ module Devise end end + # TODO Remove me in next release def old_password ActiveSupport::Deprecation.warn "old_password is deprecated, please use current_password instead", caller @old_password @@ -69,17 +70,35 @@ module Devise valid_password?(attributes[:password]) end - # Update record attributes when :old_password matches, otherwise returns - # error on :old_password. - def update_with_password(params={}) - params[:current_password] ||= params[:old_password] if params[:old_password] + # Set password and password confirmation to nil + def clean_up_passwords + self.password = self.password_confirmation = nil + end - if valid_password?(params[:current_password]) + # Update record attributes when :current_password matches, otherwise returns + # error on :current_password. It also automatically rejects :password and + # :password_confirmation if they are blank. + def update_with_password(params={}) + # TODO Remove me in next release + if params[:old_password].present? + params[:current_password] ||= params[:old_password] + ActiveSupport::Deprecation.warn "old_password is deprecated, please use current_password instead", caller + end + + params.delete(:password) if params[:password].blank? + params.delete(:password_confirmation) if params[:password_confirmation].blank? + + result = if valid_password?(params[:current_password]) update_attributes(params) else - self.class.add_error_on(self, :current_password, :invalid, false) + message = params[:current_password].blank? ? :blank : :invalid + self.class.add_error_on(self, :current_password, message, false) + self.attributes = params false end + + clean_up_passwords unless result + result end protected diff --git a/test/models/authenticatable_test.rb b/test/models/authenticatable_test.rb index 1ae3c9ae..8ec09728 100644 --- a/test/models/authenticatable_test.rb +++ b/test/models/authenticatable_test.rb @@ -134,14 +134,14 @@ class AuthenticatableTest < ActiveSupport::TestCase assert new_user.respond_to?(:current_password) end - test 'should update password with valid old password' do + test 'should update password with valid current password' do user = create_user assert user.update_with_password(:current_password => '123456', :password => 'pass321', :password_confirmation => 'pass321') assert user.reload.valid_password?('pass321') end - test 'should add an error to old password when it is invalid' do + test 'should add an error to current password when it is invalid' do user = create_user assert_not user.update_with_password(:current_password => 'other', :password => 'pass321', :password_confirmation => 'pass321') @@ -149,10 +149,32 @@ class AuthenticatableTest < ActiveSupport::TestCase assert_match /invalid/, user.errors[:current_password] end + test 'should add an error to current password when it is blank' do + user = create_user + assert_not user.update_with_password(:password => 'pass321', + :password_confirmation => 'pass321') + assert user.reload.valid_password?('123456') + assert_match /blank/, user.errors[:current_password] + end + + test 'should ignore password and its confirmation if they are blank' do + user = create_user + assert user.update_with_password(:current_password => '123456', :email => "new@email.com") + assert_equal "new@email.com", user.email + end + test 'should not update password with invalid confirmation' do user = create_user assert_not user.update_with_password(:current_password => '123456', :password => 'pass321', :password_confirmation => 'other') assert user.reload.valid_password?('123456') end + + test 'should clean up password fields on failure' do + user = create_user + assert_not user.update_with_password(:current_password => '123456', + :password => 'pass321', :password_confirmation => 'other') + assert user.password.blank? + assert user.password_confirmation.blank? + end end