diff --git a/app/password_reset.rb b/app/password_reset.rb index 9fd20299..863811fb 100644 --- a/app/password_reset.rb +++ b/app/password_reset.rb @@ -1,39 +1,43 @@ get '/password_reset' do + redirect '/' if signed_in? erb :'password_reset' end post '/send_password_reset' do + if params[:email].blank? + flash[:error] = 'You must enter a valid email address.' + redirect '/password_reset' + end + sites = Site.filter(email: params[:email]).all if sites.length > 0 token = SecureRandom.uuid.gsub('-', '') sites.each do |site| + next unless site.parent? site.update password_reset_token: token - end - body = <<-EOT -Hello! This is the Neocities cat, and I have received a password reset request for your e-mail address. Purrrr. + body = <<-EOT +Hello! This is the Neocities cat, and I have received a password reset request for your e-mail address. -Go to this URL to reset your password: http://neocities.org/password_reset_confirm?token=#{token} +Go to this URL to reset your password: http://neocities.org/password_reset_confirm?username=#{Rack::Utils.escape(site.username)}&token=#{token} -After clicking on this link, your password for all the sites registered to this email address will be changed to this token. - -Token: #{token} - -If you didn't request this reset, you can ignore it. Or hide under a bed. Or take a nap. Your call. +If you didn't request this password reset, you can ignore it. Or hide under a bed. Or take a nap. Your call. Meow, the Neocities Cat EOT - body.strip! + body.strip! - EmailWorker.perform_async({ - from: 'web@neocities.org', - to: params[:email], - subject: '[Neocities] Password Reset', - body: body - }) + EmailWorker.perform_async({ + from: 'web@neocities.org', + to: params[:email], + subject: '[Neocities] Password Reset', + body: body + }) + + end end flash[:success] = 'If your email was valid (and used by a site), the Neocities Cat will send an e-mail to your account with password reset instructions.' @@ -42,29 +46,22 @@ end get '/password_reset_confirm' do if params[:token].nil? || params[:token].strip.empty? - flash[:error] = 'Could not find a site with this token.' + flash[:error] = 'Token cannot be empty.' redirect '/' end - reset_site = Site[password_reset_token: params[:token]] + reset_site = Site.where(username: params[:username], password_reset_token: params[:token]).first if reset_site.nil? - flash[:error] = 'Could not find a site with this token.' + flash[:error] = 'Could not find a site with this username and token.' redirect '/' end - sites = Site.filter(email: reset_site.email).all + reset_site.password_reset_token = nil + reset_site.password_reset_confirmed = true + reset_site.save_changes - if sites.length > 0 - sites.each do |site| - site.password = reset_site.password_reset_token - site.save_changes - end + session[:id] = reset_site.id - flash[:success] = 'Your password for all sites with your email address has been changed to the token sent in your e-mail. Please login and change your password as soon as possible.' - else - flash[:error] = 'Could not find a site with this token.' - end - - redirect '/' + redirect '/settings#password' end diff --git a/app/settings.rb b/app/settings.rb index 816b6280..6abb67b9 100644 --- a/app/settings.rb +++ b/app/settings.rb @@ -248,7 +248,7 @@ end post '/settings/change_password' do require_login - if !Site.valid_login?(parent_site.username, params[:current_password]) + if !current_site.password_reset_confirmed && !Site.valid_login?(parent_site.username, params[:current_password]) flash[:error] = 'Your provided password does not match the current one.' redirect "/settings#password" end @@ -260,6 +260,9 @@ post '/settings/change_password' do parent_site.errors.add :password, 'New passwords do not match.' end + parent_site.password_reset_token = nil + parent_site.password_reset_confirmed = false + if parent_site.errors.empty? parent_site.save_changes flash[:success] = 'Successfully changed password.' diff --git a/migrations/087_password_reset_confirmed.rb b/migrations/087_password_reset_confirmed.rb new file mode 100644 index 00000000..feb39a6d --- /dev/null +++ b/migrations/087_password_reset_confirmed.rb @@ -0,0 +1,9 @@ +Sequel.migration do + up { + DB.add_column :sites, :password_reset_confirmed, :boolean, default: false + } + + down { + DB.drop_column :sites, :password_reset_confirmed + } +end diff --git a/tests/acceptance/password_reset_tests.rb b/tests/acceptance/password_reset_tests.rb new file mode 100644 index 00000000..81e4d10a --- /dev/null +++ b/tests/acceptance/password_reset_tests.rb @@ -0,0 +1,90 @@ +require_relative './environment.rb' + +describe '/password_reset' do + include Capybara::DSL + + before do + Capybara.reset_sessions! + EmailWorker.jobs.clear + end + + it 'should load the password reset page' do + visit '/password_reset' + page.body.must_match /Reset Password/ + end + + it 'should not load password reset if logged in' do + @site = Fabricate :site + page.set_rack_session id: @site.id + + visit '/password_reset' + URI.parse(page.current_url).path.must_equal '/' + end + + it 'errors for missing email' do + visit '/password_reset' + click_button 'Send Reset Token' + URI.parse(page.current_url).path.must_equal '/password_reset' + body.must_match /You must enter a valid email address/ + end + + it 'fails for invalid username or token' do + @site = Fabricate :site + visit '/password_reset' + fill_in 'email', with: @site.email + click_button 'Send Reset Token' + + @site.reload + + [ + {username: 'derp', token: @site.password_reset_token}, + {username: '', token: @site.password_reset_token}, + {username: @site.username, token: 'derp'}, + + {token: 'derp'}, + + ].each do |params| + visit "/password_reset_confirm?#{Rack::Utils.build_query params}" + page.must_have_content 'Could not find a site with this username and token' + @site.reload.password_reset_confirmed.must_equal false + end + + [ + {username: @site.username, token: ''}, + {username: @site.username}, + {username: '', token: ''} + ].each do |params| + visit "/password_reset_confirm?#{Rack::Utils.build_query params}" + page.must_have_content 'Token cannot be empty' + @site.reload.password_reset_confirmed.must_equal false + end + + end + + it 'works for valid username and token' do + @site = Fabricate :site + visit '/password_reset' + fill_in 'email', with: @site.email + click_button 'Send Reset Token' + + body.must_match /send an e-mail to your account with password reset instructions/ + @site.reload.password_reset_token.blank?.must_equal false + EmailWorker.jobs.first['args'].first['body'].must_match /#{Rack::Utils.build_query(username: @site.username, token: @site.password_reset_token)}/ + + visit "/password_reset_confirm?#{Rack::Utils.build_query username: @site.username, token: @site.reload.password_reset_token}" + + page.current_url.must_match /.+\/settings$/ + + fill_in 'new_password', with: 'n3wp4s$' + fill_in 'new_password_confirm', with: 'n3wp4s$' + click_button 'Change Password' + + page.current_url.must_match /.+\/settings$/ + page.must_have_content 'Successfully changed password' + Site.valid_login?(@site.username, 'n3wp4s$').must_equal true + page.get_rack_session['id'].must_equal @site.id + @site.reload.password_reset_token.must_equal nil + @site.password_reset_confirmed.must_equal false + end + +end diff --git a/views/password_reset.erb b/views/password_reset.erb index 3120ec4d..80489128 100644 --- a/views/password_reset.erb +++ b/views/password_reset.erb @@ -6,6 +6,14 @@
Note: If you provided your e-mail, you can reset your password. If you didn't, you will not be able to reset your password, and instead should create a new account. We cannot change a password without an entered email for security reasons.
diff --git a/views/settings/account/password.erb b/views/settings/account/password.erb index 10d16682..4867b74c 100644 --- a/views/settings/account/password.erb +++ b/views/settings/account/password.erb @@ -2,8 +2,10 @@ \ No newline at end of file +