major improvements and testing for password reset

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
+      body = <<-EOT
-Hello! This is the Neocities cat, and I have received a password reset request for your e-mail address. Purrrr.
+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.
+If you didn't request this password reset, you can ignore it. Or hide under a bed. Or take a nap. Your call.
-
-Token: #{token}
-
-If you didn't request this 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({
+      EmailWorker.perform_async({
-      from: 'web@neocities.org',
+        from: 'web@neocities.org',
-      to: params[:email],
+        to: params[:email],
-      subject: '[Neocities] Password Reset',
+        subject: '[Neocities] Password Reset',
-      body: body
+        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
+  session[:id] = reset_site.id
-    sites.each do |site|
-      site.password = reset_site.password_reset_token
-      site.save_changes
-    end
 
-    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.'
+  redirect '/settings#password'
-  else
-    flash[:error] = 'Could not find a site with this token.'
-  end
-
-  redirect '/'
 end

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.'

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

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

views/password_reset.erb

@@ -6,6 +6,14 @@
 
 <div class="content single-Col misc-page">
 
+  <% if flash.keys.length > 0 %>
+    <div class="alert alert-block txt-Center">
+      <% flash.keys.each do |key| %>
+        <%== flash[key] %>
+      <% end %>
+    </div>
+  <% end %>
+
   <p>
     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.
   </p>

views/settings/account/password.erb

@@ -2,8 +2,10 @@
 <form method="POST" action="/settings/change_password">
   <%== csrf_token_input_html %>
 
-  <p>Current Password:</p>
+  <% unless current_site.password_reset_confirmed %>
-  <input class="input-Area" name="current_password" type="password">
+    <p>Current Password:</p>
+    <input class="input-Area" name="current_password" type="password">
+  <% end %>
 
   <p>New Password:</p>
   <input class="input-Area" name="new_password" type="password">
@@ -12,4 +14,4 @@
   <input class="input-Area" name="new_password_confirm" type="password">
 
   <input class="btn-Action" type="submit" value="Change Password">
-</form>
+</form>