Commit dc4230364eb201ec8803bf28bc17ceeec066db9f

Authored by Rodrigo Souto
2 parents bd528256 cc38c156

Merge remote branch 'mine/stoa-change-password'

app/controllers/public/account_controller.rb
... ... @@ -141,22 +141,34 @@ class AccountController < ApplicationController
141 141 end
142 142 end
143 143  
144   - # The user requests a password change. She forgot her old password.
145   - #
146   - # Posts back.
  144 + include ForgotPasswordHelper
  145 + helper :forgot_password
  146 +
147 147 def forgot_password
148 148 if @plugins.dispatch(:allow_password_recovery).include?(false)
149 149 redirect_back_or_default(:controller => 'home')
150 150 session[:notice] = _("This environment doesn't allow password recovery.")
151 151 end
152   - @change_password = ChangePassword.new(params[:change_password])
  152 +
  153 + @change_password = ChangePassword.new
153 154  
154 155 if request.post?
155 156 begin
156   - @change_password.save!
  157 + requestors = fetch_requestors(params[:value])
  158 + raise ActiveRecord::RecordNotFound if requestors.blank? || params[:value].blank?
  159 +
  160 + requestors.each do |requestor|
  161 + ChangePassword.create!(:requestor => requestor)
  162 + end
157 163 render :action => 'password_recovery_sent'
158   - rescue ActiveRecord::RecordInvalid => e
159   - nil # just pass and render at the end of the action
  164 + rescue ActiveRecord::RecordNotFound
  165 + if params[:value].blank?
  166 + @change_password.errors.add_to_base(_('Can not recover user password with blank value.'))
  167 + else
  168 + @change_password.errors.add_to_base(_('Could not find any user with %s equal to "%s".') % [fields_label, params[:value]])
  169 + end
  170 + rescue ActiveRecord::RecordInvald
  171 + @change_password.errors.add_to_base(_('Could not perform password recovery for the user.'))
160 172 end
161 173 end
162 174 end
... ...
app/helpers/forgot_password_helper.rb 0 → 100644
... ... @@ -0,0 +1,43 @@
  1 +module ForgotPasswordHelper
  2 + def plugins_options
  3 + @plugins.dispatch(:change_password_fields)
  4 + end
  5 +
  6 + def user_fields
  7 + %w[login email] + plugins_options.select {|options| options[:model].to_sym == :user }.map { |options| options[:field].to_s }
  8 + end
  9 +
  10 + def person_fields
  11 + %w[] + plugins_options.select {|options| options[:model].to_sym == :person }.map { |options| options[:field].to_s }
  12 + end
  13 +
  14 + def fields
  15 + user_fields + person_fields
  16 + end
  17 +
  18 + def fields_label
  19 + labels = [
  20 + _('Username'),
  21 + _('Email'),
  22 + ] + plugins_options.map { |options| options[:name] }
  23 +
  24 + last = labels.pop
  25 + label = labels.join(', ')
  26 + "#{label} #{_('or')} #{last}"
  27 + end
  28 +
  29 + def build_query(fields, value)
  30 + fields.map {|field| "#{field} = '#{value}'"}.join(' OR ')
  31 + end
  32 +
  33 + def fetch_requestors(value)
  34 + requestors = []
  35 + person_query = build_query(person_fields, value)
  36 + user_query = build_query(user_fields, value)
  37 +
  38 + requestors += Person.where(person_query).where(:environment_id => environment.id) if person_fields.present?
  39 + requestors += User.where(user_query).where(:environment_id => environment.id).map(&:person) if user_fields.present?
  40 + requestors
  41 + end
  42 +
  43 +end
... ...
app/models/change_password.rb
1 1 class ChangePassword < Task
2 2  
3   - attr_accessor :login, :email, :password, :password_confirmation, :environment_id
  3 + attr_accessor :password, :password_confirmation
4 4  
5 5 def self.human_attribute_name(attrib)
6 6 case attrib.to_sym
7   - when :login:
8   - _('Username')
9   - when :email
10   - _('e-mail')
11 7 when :password
12 8 _('Password')
13 9 when :password_confirmation
... ... @@ -17,29 +13,7 @@ class ChangePassword &lt; Task
17 13 end
18 14 end
19 15  
20   - ###################################################
21   - # validations for creating a ChangePassword task
22   -
23   - validates_presence_of :login, :email, :environment_id, :on => :create, :message => _('must be filled in')
24   -
25   - validates_format_of :email, :on => :create, :with => Noosfero::Constants::EMAIL_FORMAT, :if => (lambda { |obj| !obj.email.blank? })
26   -
27   - validates_each :login, :on => :create do |data,attr,value|
28   - unless data.login.blank? || data.email.blank?
29   - user = User.find_by_login_and_environment_id(data.login, data.environment_id)
30   - if user.nil?
31   - data.errors.add(:login, _('is invalid or user does not exists.'))
32   - else
33   - if user.email != data.email
34   - data.errors.add(:email, _('does not match the username you filled in'))
35   - end
36   - end
37   - end
38   - end
39   -
40   - before_validation_on_create do |change_password|
41   - change_password.requestor = Person.find_by_identifier_and_environment_id(change_password.login, change_password.environment_id)
42   - end
  16 + validates_presence_of :requestor
43 17  
44 18 ###################################################
45 19 # validations for updating a ChangePassword task
... ... @@ -49,6 +23,10 @@ class ChangePassword &lt; Task
49 23 validates_presence_of :password_confirmation, :on => :update, :if => lambda { |change| change.status != Task::Status::CANCELLED }
50 24 validates_confirmation_of :password, :if => lambda { |change| change.status != Task::Status::CANCELLED }
51 25  
  26 + def environment
  27 + requestor.environment unless requestor.nil?
  28 + end
  29 +
52 30 def title
53 31 _("Change password")
54 32 end
... ... @@ -87,12 +65,8 @@ class ChangePassword &lt; Task
87 65 url = url_for(:host => hostname, :controller => 'account', :action => 'new_password', :code => code)
88 66  
89 67 lambda do
90   - _("In order to change your password, please visit the following address:\n\n%s") % url
  68 + _("In order to change your password, please visit the following address:\n\n%s\n\nIf you did not required any change to your password just desconsider this email.") % url
91 69 end
92 70 end
93 71  
94   - def environment
95   - self.requestor.environment
96   - end
97   -
98 72 end
... ...
app/views/account/forgot_password.rhtml
... ... @@ -2,21 +2,13 @@
2 2  
3 3 <%= error_messages_for :change_password, :header_message => _('Instructions to password recovery could not be sent'), :message => nil %>
4 4  
5   -<% labelled_form_for :change_password, @change_password, :url => { :action => 'forgot_password' } do |f| %>
6   -
7   - <%= f.text_field :login,
8   - :onchange => 'this.value = convToValidUsername( this.value )' %>
9   -
10   - <%= f.text_field :email %>
11   -
12   - <%= f.hidden_field :environment_id, :value => environment.id %>
13   -
14   -<div>
15   - <% button_bar do %>
16   - <%= submit_button('send', _('Send instructions')) %>
17   - <% end %>
18   -</div>
19   -<%= content_tag(:small,_('After clicking the button above, you will receive an email with a link to a page where you will be able to create a new password.')) %>
20   -
  5 +<% form_tag do %>
  6 + <%= labelled_form_field fields_label, text_field_tag(:value) %>
  7 +
  8 + <div>
  9 + <% button_bar do %>
  10 + <%= submit_button('send', _('Send instructions')) %>
  11 + <% end %>
  12 + </div>
  13 + <%= content_tag(:small,_('After clicking the button above, you will receive an email with a link to a page where you will be able to create a new password.')) %>
21 14 <% end %>
22   -
... ...
lib/noosfero/plugin.rb
... ... @@ -431,6 +431,12 @@ class Noosfero::Plugin
431 431 def find_by_contents(asset, scope, query, paginate_options={}, options={})
432 432 end
433 433  
  434 + # -> Adds aditional fields for change_password
  435 + # returns = [{:field => 'field1', :name => 'field 1 name', :model => 'person'}, {...}]
  436 + def change_password_fields
  437 + nil
  438 + end
  439 +
434 440 # -> Adds additional blocks to profiles and environments.
435 441 # Your plugin must implements a class method called 'extra_blocks'
436 442 # that returns a hash with the following syntax.
... ...
plugins/stoa/lib/stoa_plugin.rb
... ... @@ -120,4 +120,8 @@ class StoaPlugin &lt; Noosfero::Plugin
120 120 true
121 121 end
122 122  
  123 + def change_password_fields
  124 + {:field => :usp_id, :name => _('USP Number'), :model => 'person'}
  125 + end
  126 +
123 127 end
... ...
plugins/stoa/test/functional/account_controller_test.rb
... ... @@ -66,4 +66,8 @@ class AccountControllerTest &lt; ActionController::TestCase
66 66 assert_equal @user.login, assigns(:current_user).login
67 67 end
68 68  
  69 + should 'be able to recover password with usp_id' do
  70 + post :forgot_password, :value => '87654321'
  71 + assert_template 'password_recovery_sent'
  72 + end
69 73 end
... ...
test/functional/account_controller_test.rb
... ... @@ -215,12 +215,18 @@ class AccountControllerTest &lt; ActionController::TestCase
215 215 assert_response :success
216 216 end
217 217  
218   - should 'respond to forgotten password change request' do
  218 + should 'respond to forgotten password change request with login' do
  219 + create_user('test')
  220 +
  221 + post :forgot_password, :value => 'test'
  222 + assert_template 'password_recovery_sent'
  223 + end
  224 +
  225 + should 'respond to forgotten password change request with email' do
219 226 change = ChangePassword.new
220   - ChangePassword.expects(:new).with('login' => 'test', 'email' => 'test@localhost.localdomain').returns(change)
221   - change.expects(:save!).returns(true)
  227 + create_user('test', :email => 'test@localhost.localdomain')
222 228  
223   - post :forgot_password, :change_password => { :login => 'test', :email => 'test@localhost.localdomain' }
  229 + post :forgot_password, :value => 'test@localhost.localdomain'
224 230 assert_template 'password_recovery_sent'
225 231 end
226 232  
... ... @@ -267,7 +273,7 @@ class AccountControllerTest &lt; ActionController::TestCase
267 273  
268 274 should 'require password confirmation correctly to enter new pasword' do
269 275 user = create_user('testuser', :email => 'testuser@example.com', :password => 'test', :password_confirmation => 'test')
270   - change = ChangePassword.create!(:login => 'testuser', :email => 'testuser@example.com', :environment_id => Environment.default.id)
  276 + change = ChangePassword.create!(:requestor => user.person)
271 277  
272 278 post :new_password, :code => change.code, :change_password => { :password => 'onepass', :password_confirmation => 'another_pass' }
273 279 assert_response :success
... ... @@ -853,7 +859,7 @@ class AccountControllerTest &lt; ActionController::TestCase
853 859 assert_response :redirect
854 860  
855 861 #Redirect on post action
856   - post :forgot_password, :change_password => { :login => 'test', :email => 'test@localhost.localdomain' }
  862 + post :forgot_password, :value => 'test'
857 863 assert_response :redirect
858 864 end
859 865  
... ...
test/integration/forgot_password_test.rb
... ... @@ -6,7 +6,7 @@ class ForgotPasswordTest &lt; ActionController::IntegrationTest
6 6 ActionController::Integration::Session.any_instance.stubs(:https?).returns(true)
7 7 end
8 8  
9   - def test_forgot_password
  9 + def test_forgot_password_with_login
10 10  
11 11 User.destroy_all
12 12 Profile.destroy_all
... ... @@ -19,7 +19,40 @@ class ForgotPasswordTest &lt; ActionController::IntegrationTest
19 19 assert_response :success
20 20 assert_tag :tag => 'form', :attributes => { :action => '/account/forgot_password', :method => 'post' }
21 21  
22   - post '/account/forgot_password', :change_password => { :login => 'forgotten', :email => 'forgotten@localhost.localdomain', :environment_id => Environment.default.id }
  22 + post '/account/forgot_password', :change_password => { :field => 'login', :value => 'forgotten', :environment_id => Environment.default.id }
  23 +
  24 + assert_response :success
  25 + assert_template 'password_recovery_sent'
  26 +
  27 + assert_equal 1, ChangePassword.count
  28 + code = ChangePassword.find(:first).code
  29 +
  30 + get "/account/new_password/#{code}"
  31 + assert_response :success
  32 + assert_tag :tag => 'form', :attributes => { :action => "/account/new_password/#{code}" }
  33 +
  34 + post "/account/new_password/#{code}", :change_password => { :password => 'newpass', :password_confirmation => 'newpass'}
  35 + assert_response :success
  36 + assert_template 'new_password_ok'
  37 + assert_tag :tag => 'a', :attributes => { :href => "/account/login" }
  38 +
  39 + login('forgotten', 'newpass')
  40 + end
  41 +
  42 + def test_forgot_password_with_email
  43 +
  44 + User.destroy_all
  45 + Profile.destroy_all
  46 + ChangePassword.destroy_all
  47 +
  48 + create_user('forgotten', :password => 'test', :password_confirmation => 'test', :email => 'forgotten@localhost.localdomain').activate
  49 +
  50 + get '/account/forgot_password'
  51 +
  52 + assert_response :success
  53 + assert_tag :tag => 'form', :attributes => { :action => '/account/forgot_password', :method => 'post' }
  54 +
  55 + post '/account/forgot_password', :change_password => { :field => 'email', :value => 'forgotten@localhost.localdomain', :environment_id => Environment.default.id }
23 56  
24 57 assert_response :success
25 58 assert_template 'password_recovery_sent'
... ...
test/unit/change_password_test.rb
... ... @@ -4,69 +4,15 @@ class ChangePasswordTest &lt; ActiveSupport::TestCase
4 4  
5 5 fixtures :environments
6 6  
7   - should 'validate' do
8   - data = ChangePassword.new
9   - assert !data.valid?
  7 + def setup
  8 + @user = create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com')
  9 + @person = @user.person
10 10 end
11   -
12   - should 'refuse invalid username' do
13   - User.destroy_all
14   -
15   - data = ChangePassword.new
16   - data.login = 'unexisting'
17   - data.email = 'example@example.com'
18   - data.environment_id = Environment.default.id
19   - data.valid?
20   - assert data.errors.invalid?(:login)
21   - end
22   -
23   - should 'require a valid username' do
24   - User.destroy_all
25   - create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com')
26   -
27   - data = ChangePassword.new
28   - data.login = 'testuser'
29   - data.valid?
30   - assert !data.errors.invalid?(:login)
31   - end
32   -
33   - should 'refuse incorrect e-mail address' do
34   - User.destroy_all
35   - create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com')
36   -
37   - data = ChangePassword.new
38   - data.login = 'testuser'
39   - data.email = 'wrong@example.com'
40   - data.environment_id = Environment.default.id
41 11  
42   - data.valid?
43   - assert !data.errors.invalid?(:login)
44   - assert data.errors.invalid?(:email)
45   - end
46   -
47   - should 'require the correct e-mail address' do
48   - User.destroy_all
49   - create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com')
50   -
51   - data = ChangePassword.new
52   - data.login = 'testuser'
53   - data.email = 'test@example.com'
54   - data.environment_id = Environment.default.id
55   -
56   - data.valid?
57   - assert !data.errors.invalid?(:login)
58   - assert !data.errors.invalid?(:email)
59   - end
  12 + attr_accessor :user, :person
60 13  
61 14 should 'require correct passsword confirmation' do
62   - create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com')
63   -
64   - change = ChangePassword.new
65   - change.login = 'testuser'
66   - change.email = 'test@example.com'
67   - change.environment_id = Environment.default.id
68   - change.save!
69   -
  15 + change = ChangePassword.create!(:requestor => person)
70 16 change.status = Task::Status::FINISHED
71 17 change.password = 'right'
72 18 change.password_confirmation = 'wrong'
... ... @@ -79,17 +25,7 @@ class ChangePasswordTest &lt; ActiveSupport::TestCase
79 25 end
80 26  
81 27 should 'actually change password' do
82   - User.destroy_all
83   - person = create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com').person
84   -
85   - change = ChangePassword.new
86   - change.login = 'testuser'
87   - change.email = 'test@example.com'
88   - change.environment_id = Environment.default.id
89   - change.save!
90   -
91   - change.expects(:requestor).returns(person).at_least_once
92   -
  28 + change = ChangePassword.create!(:requestor => person)
93 29 change.password = 'newpass'
94 30 change.password_confirmation = 'newpass'
95 31 change.finish
... ... @@ -98,15 +34,7 @@ class ChangePasswordTest &lt; ActiveSupport::TestCase
98 34 end
99 35  
100 36 should 'not require password and password confirmation when cancelling' do
101   - User.destroy_all
102   - person = create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com').person
103   -
104   - change = ChangePassword.new
105   - change.login = 'testuser'
106   - change.email = 'test@example.com'
107   - change.environment_id = Environment.default.id
108   - change.save!
109   -
  37 + change = ChangePassword.create!(:requestor => person)
110 38 assert_nothing_raised do
111 39 change.cancel
112 40 end
... ... @@ -125,26 +53,20 @@ class ChangePasswordTest &lt; ActiveSupport::TestCase
125 53 p1 = create_user('sample-user', :password => 'test', :password_confirmation => 'test', :email => 'sample-user@test.com', :environment => e1).person
126 54 p2 = create_user('sample-user', :password => 'test', :password_confirmation => 'test', :email => 'sample-user@test.com', :environment => e2).person
127 55  
128   - c1 = ChangePassword.create!(:login => 'sample-user', :email => 'sample-user@test.com', :environment_id => e1.id)
129   - c2 = ChangePassword.create!(:login => 'sample-user', :email => 'sample-user@test.com', :environment_id => e2.id)
  56 + c1 = ChangePassword.create!(:requestor => p1)
  57 + c2 = ChangePassword.create!(:requestor => p2)
130 58  
131 59 assert_equal c1.requestor, p1
132 60 assert_equal c2.requestor, p2
133 61 end
134 62  
135 63 should 'have target notification description' do
136   - person = fast_create(Person, :identifier => 'testuser')
137   -
138   - change = ChangePassword.create(:login => 'testuser', :email => 'test@example.com', :environment_id => Environment.default.id)
139   -
  64 + change = ChangePassword.create!(:requestor => person)
140 65 assert_match(/#{change.requestor.name} wants to change its password/, change.target_notification_description)
141 66 end
142 67  
143 68 should 'deliver task created message' do
144   - person = fast_create(Person, :identifier => 'testuser')
145   -
146   - task = ChangePassword.create(:login => 'testuser', :email => 'test@example.com', :environment_id => Environment.default.id)
147   -
  69 + task = ChangePassword.create!(:requestor => person)
148 70 email = TaskMailer.deliver_task_created(task)
149 71 assert_match(/#{task.requestor.name} wants to change its password/, email.subject)
150 72 end
... ...
test/unit/forgot_password_helper_test.rb 0 → 100644
... ... @@ -0,0 +1,129 @@
  1 +require File.dirname(__FILE__) + '/../test_helper'
  2 +
  3 +class ForgotPasswordHelperTest < ActionView::TestCase
  4 + include ForgotPasswordHelper
  5 +
  6 + def setup
  7 + @environment = Environment.default
  8 + @plugins = Noosfero::Plugin::Manager.new(@environment, self)
  9 + end
  10 +
  11 + attr_accessor :environment
  12 +
  13 + should 'allow extra fields provided by plugins' do
  14 + class Plugin1 < Noosfero::Plugin
  15 + def change_password_fields
  16 + {:field => 'f1', :name => 'F1', :model => 'person'}
  17 + end
  18 + end
  19 + class Plugin2 < Noosfero::Plugin
  20 + def change_password_fields
  21 + [{:field => 'f2', :name => 'F2', :model => 'person'},
  22 + {:field => 'f3', :name => 'F3', :model => 'person'}]
  23 + end
  24 + end
  25 +
  26 + environment.enable_plugin(Plugin1)
  27 + environment.enable_plugin(Plugin2)
  28 +
  29 + assert_includes fields, 'f1'
  30 + assert_includes fields, 'f2'
  31 + assert_includes fields, 'f3'
  32 + end
  33 +
  34 + should 'allow extra person fields provided by plugins' do
  35 + class Plugin1 < Noosfero::Plugin
  36 + def change_password_fields
  37 + {:field => 'f1', :name => 'F1', :model => 'person'}
  38 + end
  39 + end
  40 + class Plugin2 < Noosfero::Plugin
  41 + def change_password_fields
  42 + [{:field => 'f2', :name => 'F2', :model => 'user'},
  43 + {:field => 'f3', :name => 'F3', :model => 'person'}]
  44 + end
  45 + end
  46 +
  47 + environment.enable_plugin(Plugin1)
  48 + environment.enable_plugin(Plugin2)
  49 +
  50 + assert_includes person_fields, 'f1'
  51 + assert_not_includes person_fields, 'f2'
  52 + assert_includes person_fields, 'f3'
  53 + end
  54 +
  55 + should 'allow extra user fields provided by plugins' do
  56 + class Plugin1 < Noosfero::Plugin
  57 + def change_password_fields
  58 + {:field => 'f1', :name => 'F1', :model => 'user'}
  59 + end
  60 + end
  61 + class Plugin2 < Noosfero::Plugin
  62 + def change_password_fields
  63 + [{:field => 'f2', :name => 'F2', :model => 'person'},
  64 + {:field => 'f3', :name => 'F3', :model => 'user'}]
  65 + end
  66 + end
  67 +
  68 + environment.enable_plugin(Plugin1)
  69 + environment.enable_plugin(Plugin2)
  70 +
  71 + assert_includes user_fields, 'f1'
  72 + assert_not_includes user_fields, 'f2'
  73 + assert_includes user_fields, 'f3'
  74 + end
  75 +
  76 + should 'add plugins fields labels to final label' do
  77 + class Plugin1 < Noosfero::Plugin
  78 + def change_password_fields
  79 + {:field => 'f1', :name => 'F1', :model => 'user'}
  80 + end
  81 + end
  82 + class Plugin2 < Noosfero::Plugin
  83 + def change_password_fields
  84 + [{:field => 'f2', :name => 'F2', :model => 'person'},
  85 + {:field => 'f3', :name => 'F3', :model => 'user'}]
  86 + end
  87 + end
  88 +
  89 + environment.enable_plugin(Plugin1)
  90 + environment.enable_plugin(Plugin2)
  91 +
  92 + assert_match /F1/, fields_label
  93 + assert_match /F2/, fields_label
  94 + assert_match /F3/, fields_label
  95 + end
  96 +
  97 + should 'fetch requestors based on fields available' do
  98 + p1 = create_user('person1').person
  99 + p2 = create_user('person2').person
  100 +
  101 + requestors = fetch_requestors('person1')
  102 + assert_includes requestors, p1
  103 + assert_not_includes requestors, p2
  104 +
  105 + p3 = create_user('person3').person
  106 + p3.address = 'some address'
  107 + p3.save!
  108 + p4 = create_user('person4').person
  109 + p4.address = 'some address'
  110 + p4.save!
  111 + p5 = create_user('person5').person
  112 + p5.address = 'another address'
  113 + p5.save!
  114 +
  115 + self.stubs(:person_fields).returns(%w[address])
  116 + requestors = fetch_requestors('some address')
  117 + assert_includes requestors, p3
  118 + assert_includes requestors, p4
  119 + end
  120 +
  121 + should 'not fetch people from other environments' do
  122 + p1 = create_user('person', :environment => fast_create(Environment)).person
  123 + p2 = create_user('person').person
  124 +
  125 + requestors = fetch_requestors('person')
  126 + assert_not_includes requestors, p1
  127 + assert_includes requestors, p2
  128 + end
  129 +end
... ...