Fixes #3116 - Verification tokens questionable behavior in developer mode

app/assets/javascripts/app/controllers/_plugin/user_signup_check.coffee

@@ -63,13 +63,6 @@ class Modal extends App.ControllerModal
     @sent = true
     @update()
 
-    # if in developer mode, redirect to verify
-    if data.token && @Config.get('developer_mode') is true
-      redirect = =>
-        @close()
-        @navigate "#email_verify/#{data.token}"
-      App.Delay.set(redirect, 4000)
-
   error: =>
     @contentInline = App.i18n.translateContent('Unable to send verify email.')
     @update()

app/assets/javascripts/app/controllers/password_reset.coffee

@@ -58,12 +58,6 @@ class PasswordReset extends App.ControllerFullPage
     )
 
   success: (data) =>
-
-    # if in developer mode, redirect to set new password
-    if data.token && @Config.get('developer_mode') is true
-      redirect = =>
-        @navigate "#password_reset_verify/#{data.token}"
-      @delay(redirect, 2000)
     @html(App.view('password/reset_sent')())
 
 App.Config.set('password_reset', PasswordReset, 'Routes')

app/assets/javascripts/app/controllers/signup.coffee

@@ -98,8 +98,6 @@ class Signup extends App.ControllerFullPage
           removeAll: true
         )
 
-        if data.token && @Config.get('developer_mode') is true
-          @navigate "#email_verify/#{data.token}"
       error: @error
     )
 

app/controllers/users_controller.rb

@@ -411,12 +411,6 @@ curl http://localhost/api/v1/users/email_verify_send -v -u #{login}:#{password}
         objects:  result
       )
 
-      # only if system is in develop mode, send token back to browser for browser tests
-      if Setting.get('developer_mode') == true
-        render json: { message: 'ok', token: result[:token].name }, status: :ok
-        return
-      end
-
       # token sent to user, send ok to browser
       render json: { message: 'ok' }, status: :ok
       return
@@ -466,12 +460,6 @@ curl http://localhost/api/v1/users/password_reset -v -u #{login}:#{password} -H
         user:     result[:user],
         objects:  result
       )
-
-      # only if system is in develop mode, send token back to browser for browser tests
-      if Setting.get('developer_mode') == true
-        render json: { message: 'ok', token: result[:token].name }, status: :ok
-        return
-      end
     end
 
     # result is always positive to avoid leaking of existing user accounts

script/build/test_slice_tests.sh

@@ -74,7 +74,6 @@ if [ "$LEVEL" == '1' ]; then
   rm test/browser/preferences_token_access_test.rb
   rm test/browser/reporting_test.rb
   rm test/browser/setting_test.rb
-  # test/browser/signup_password_change_and_reset_test.rb
   # test/browser/swich_to_user_test.rb
   # test/browser/taskbar_session_test.rb
   # test/browser/taskbar_task_test.rb
@@ -151,7 +150,6 @@ elif [ "$LEVEL" == '2' ]; then
   rm test/browser/preferences_token_access_test.rb
   rm test/browser/reporting_test.rb
   rm test/browser/setting_test.rb
-  rm test/browser/signup_password_change_and_reset_test.rb
   rm test/browser/switch_to_user_test.rb
   rm test/browser/taskbar_session_test.rb
   rm test/browser/taskbar_task_test.rb
@@ -228,7 +226,6 @@ elif [ "$LEVEL" == '3' ]; then
   rm test/browser/preferences_token_access_test.rb
   rm test/browser/reporting_test.rb
   rm test/browser/setting_test.rb
-  rm test/browser/signup_password_change_and_reset_test.rb
   rm test/browser/switch_to_user_test.rb
   rm test/browser/taskbar_session_test.rb
   rm test/browser/taskbar_task_test.rb
@@ -305,7 +302,6 @@ elif [ "$LEVEL" == '4' ]; then
   rm test/browser/preferences_token_access_test.rb
   rm test/browser/reporting_test.rb
   rm test/browser/setting_test.rb
-  rm test/browser/signup_password_change_and_reset_test.rb
   rm test/browser/switch_to_user_test.rb
   rm test/browser/taskbar_session_test.rb
   rm test/browser/taskbar_task_test.rb
@@ -381,7 +377,6 @@ elif [ "$LEVEL" == '5' ]; then
   rm test/browser/preferences_token_access_test.rb
   rm test/browser/reporting_test.rb
   rm test/browser/setting_test.rb
-  rm test/browser/signup_password_change_and_reset_test.rb
   rm test/browser/switch_to_user_test.rb
   rm test/browser/taskbar_session_test.rb
   rm test/browser/taskbar_task_test.rb
@@ -460,7 +455,6 @@ elif [ "$LEVEL" == '6' ]; then
   # test/browser/preferences_token_access_test.rb
   # test/browser/reporting_test.rb
   # test/browser/setting_test.rb
-  rm test/browser/signup_password_change_and_reset_test.rb
   rm test/browser/switch_to_user_test.rb
   rm test/browser/taskbar_session_test.rb
   rm test/browser/taskbar_task_test.rb

spec/factories/user.rb

@@ -48,10 +48,18 @@ FactoryBot.define do
       roles { Role.where(name: %w[Admin Agent]) }
     end
 
+    trait :with_valid_password do
+      password { generate :password_valid }
+    end
+
     # make given password accessible for e.g. authentication logic
     before(:create) do |user|
       password_plain = user.password
       user.define_singleton_method(:password_plain, -> { password_plain })
     end
   end
+
+  sequence(:password_valid) do |n|
+    "SOme-pass#{n}"
+  end
 end

spec/system/password_reset_spec.rb

@@ -0,0 +1,33 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'Password Reset', type: :system do
+  before do
+    visit 'password_reset'
+  end
+
+  it 'logged in user cannot open password reset' do
+    expect(page).to have_no_text 'password'
+  end
+
+  context 'when not logged in', authenticated_as: false do
+    it 'proceeds with non-existant user' do
+      fill_in 'username', with: 'nonexisting'
+
+      click '.reset_password .btn--primary'
+
+      expect(page).to have_text 'sent password reset instructions'
+    end
+
+    it 'proceeds with an actual user' do
+      user = create(:agent)
+
+      fill_in 'username', with: user.email
+
+      click '.reset_password .btn--primary'
+
+      expect(page).to have_text 'sent password reset instructions'
+    end
+  end
+end

spec/system/password_reset_verify_spec.rb

@@ -0,0 +1,52 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'Password Reset verify', type: :system, authenticated_as: false do
+  context 'with a valid token' do
+    let(:user)  { create(:agent) }
+    let(:token) { User.password_reset_new_token(user.email)[:token].name }
+
+    before do
+      visit "password_reset_verify/#{token}"
+    end
+
+    it 'resetting password with non matching passwords fail' do
+      fill_in 'password', with: 'some'
+      fill_in 'password_confirm', with: 'some2'
+
+      click '.js-passwordForm .js-submit'
+
+      expect(page).to have_text 'passwords do not match'
+    end
+
+    it 'resetting password with weak password fail' do
+      fill_in 'password', with: 'some'
+      fill_in 'password_confirm', with: 'some'
+
+      click '.js-passwordForm .js-submit'
+
+      expect(page).to have_text 'Invalid password'
+    end
+
+    it 'successfully resets password and logs in' do
+      new_password = generate :password_valid
+
+      fill_in 'password', with: new_password
+      fill_in 'password_confirm', with: new_password
+
+      click '.js-passwordForm .js-submit'
+
+      expect(page).to have_text('Your password has been changed')
+        .and have_selector(".user-menu .user a[title=#{user.login}")
+    end
+  end
+
+  context 'without a valid token' do
+    it 'error shown if opened with a not existing token' do
+      visit 'password_reset_verify/not_existing_token'
+
+      expect(page).to have_text 'Token is invalid'
+    end
+  end
+end

spec/system/profile/password_spec.rb

@@ -0,0 +1,53 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'rails_helper'
+
+RSpec.describe 'Profile > Password', type: :system, authenticated_as: :user do
+  before do
+    visit 'profile/password'
+  end
+
+  let(:user) { create(:agent, :with_valid_password) }
+
+  it 'when current password is wrong, show error' do
+    fill_in 'password_old',         with: 'nonexisting'
+    fill_in 'password_new',         with: 'some'
+    fill_in 'password_new_confirm', with: 'some'
+
+    click '.btn--primary'
+
+    expect(page).to have_text 'Current password is wrong'
+  end
+
+  it 'when new passwords do not match, show error' do
+    fill_in 'password_old',         with: user.password_plain
+    fill_in 'password_new',         with: 'some'
+    fill_in 'password_new_confirm', with: 'some2'
+
+    click '.btn--primary'
+
+    expect(page).to have_text 'passwords do not match'
+  end
+
+  it 'when new password is invalid, show error' do
+    fill_in 'password_old',         with: user.password_plain
+    fill_in 'password_new',         with: 'some'
+    fill_in 'password_new_confirm', with: 'some'
+
+    click '.btn--primary'
+
+    expect(page).to have_text 'Invalid password'
+  end
+
+  it 'allows to change password' do
+    new_password = generate :password_valid
+
+    fill_in 'password_old',         with: user.password_plain
+    fill_in 'password_new',         with: new_password
+    fill_in 'password_new_confirm', with: new_password
+
+    click '.btn--primary'
+
+    expect(page).to have_text 'Password changed successfully!'
+  end
+end

spec/system/signup_spec.rb

@@ -3,9 +3,23 @@
 require 'rails_helper'
 
 RSpec.describe 'Signup', type: :system, authenticated_as: false do
-  it 'shows password strength error' do
+  before do
     visit 'signup'
+  end
 
+  it 'creates an accoutn successfully' do
+    fill_in 'firstname',        with: 'Test'
+    fill_in 'lastname',         with: 'Test'
+    fill_in 'email',            with: 'test@example.com'
+    fill_in 'password',         with: 'SOme-pass1'
+    fill_in 'password_confirm', with: 'SOme-pass1'
+
+    click '.js-submit'
+
+    expect(page).to have_css '.signup', text: 'Registration successful!'
+  end
+
+  it 'with a weak password show password strength error' do
     fill_in 'firstname',        with: 'Test'
     fill_in 'lastname',         with: 'Test'
     fill_in 'email',            with: 'test@example.com'

test/browser/signup_password_change_and_reset_test.rb

@@ -1,269 +0,0 @@
-# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
-
-require 'browser_test_helper'
-
-class SignupPasswordChangeAndResetTest < TestCase
-  def test_signup
-    signup_user_email = "signup-test-#{rand(999_999)}@example.com"
-    @browser = browser_instance
-    location(url: browser_url)
-    click(css: 'a[href="#signup"]')
-    exists(css: '.signup')
-
-    # signup
-    set(
-      css:   'input[name="firstname"]',
-      value: 'Signup Firstname',
-    )
-    set(
-      css:   'input[name="lastname"]',
-      value: 'Signup Lastname',
-    )
-    set(
-      css:   'input[name="email"]',
-      value: signup_user_email,
-    )
-    set(
-      css:   'input[name="password"]',
-      value: 'SOme-pass1',
-    )
-    set(
-      css:   'input[name="password_confirm"]',
-      value: 'SOme-pass1',
-    )
-    click(css: 'button.js-submit')
-
-    watch_for(
-      css:   '.signup',
-      value: 'Registration successful!',
-    )
-
-    # auto login via token trick in dev mode
-    click(css: '.signup .js-submitResend')
-
-    watch_for(
-      css: '#login',
-    )
-    login(
-      username: signup_user_email,
-      password: 'SOme-pass1',
-    )
-
-    watch_for(
-      css:   '.content.active',
-      value: 'Welcome!',
-    )
-
-    # change password
-    click(css: '.navbar-items-personal .user a')
-    sleep 1
-    click(css: 'a[href="#profile"]')
-    click(css: 'a[href="#profile/password"]')
-    set(
-      css:   'input[name="password_old"]',
-      value: 'nonexisiting',
-    )
-    set(
-      css:   'input[name="password_new"]',
-      value: 'some',
-    )
-    set(
-      css:   'input[name="password_new_confirm"]',
-      value: 'some',
-    )
-    click(css: '.content .btn--primary')
-
-    watch_for(
-      css:   'body',
-      value: 'current password is wrong',
-    )
-
-    set(
-      css:   'input[name="password_old"]',
-      value: 'SOme-pass1',
-    )
-    set(
-      css:   'input[name="password_new_confirm"]',
-      value: 'some2',
-    )
-    click(css: '.content .btn--primary')
-    watch_for(
-      css:   'body',
-      value: 'passwords do not match',
-    )
-
-    set(
-      css:   'input[name="password_new"]',
-      value: 'SOme-1',
-    )
-    set(
-      css:   'input[name="password_new_confirm"]',
-      value: 'SOme-1',
-    )
-    click(css: '.content .btn--primary')
-
-    watch_for(
-      css:   'body',
-      value: 'it must be at least',
-    )
-
-    set(
-      css:   'input[name="password_new"]',
-      value: 'SOme-pass-new',
-    )
-    set(
-      css:   'input[name="password_new_confirm"]',
-      value: 'SOme-pass-new',
-    )
-    click(css: '.content .btn--primary')
-
-    watch_for(
-      css:   'body',
-      value: 'must contain at least 1 digit',
-    )
-
-    set(
-      css:   'input[name="password_new"]',
-      value: 'SOme-pass-new2',
-    )
-    set(
-      css:   'input[name="password_new_confirm"]',
-      value: 'SOme-pass-new2',
-    )
-    click(css: '.content .btn--primary')
-
-    watch_for(
-      css:   'body',
-      value: 'Password changed successfully',
-    )
-    logout()
-
-    # check login with new pw
-    login(
-      username: signup_user_email,
-      password: 'SOme-pass-new2',
-    )
-    logout()
-
-    # reset password (not possible)
-    location(url: "#{browser_url}/#password_reset_verify/not_existing_token")
-
-    watch_for(
-      css:   'body',
-      value: 'Token is invalid',
-    )
-
-    # reset password (with valid session - should not be possible)
-    login(
-      username: signup_user_email,
-      password: 'SOme-pass-new2',
-      url:      browser_url,
-    )
-
-    location(url: "#{browser_url}/#password_reset")
-    sleep 1
-
-    match_not(
-      css:   'body',
-      value: 'password',
-    )
-    logout()
-
-    # reset password (correct way)
-    click(css: 'a[href="#password_reset"]')
-
-    set(
-      css:   'input[name="username"]',
-      value: 'nonexisiting',
-    )
-    click(css: '.reset_password .btn--primary')
-    watch_for(
-      css:   'body',
-      value: 'sent password reset instructions',
-    )
-
-    click(css: '.reset_password .btn--primary')
-
-    set(
-      css:   'input[name="username"]',
-      value: signup_user_email,
-    )
-    click(css: '.reset_password .btn--primary')
-    watch_for(
-      css:   'body',
-      value: 'sent password reset instructions',
-    )
-
-    # redirect to "#password_reset_verify/#{token}" url by app, because of "developer_mode"
-    watch_for(
-      css:   'body',
-      value: 'Choose your new password',
-    )
-
-    # set new password
-    set(
-      css:   'input[name="password"]',
-      value: 'some',
-    )
-    set(
-      css:   'input[name="password_confirm"]',
-      value: 'some2',
-    )
-    click(css: '.js-passwordForm .js-submit')
-    watch_for(
-      css:   'body',
-      value: 'passwords do not match',
-    )
-
-    set(
-      css:   'input[name="password"]',
-      value: 'SOme-1',
-    )
-    set(
-      css:   'input[name="password_confirm"]',
-      value: 'SOme-1',
-    )
-    click(css: '.js-passwordForm .js-submit')
-    watch_for(
-      css:   'body',
-      value: 'it must be at least',
-    )
-
-    set(
-      css:   'input[name="password"]',
-      value: 'SOme-pass-new',
-    )
-    set(
-      css:   'input[name="password_confirm"]',
-      value: 'SOme-pass-new',
-    )
-    click(css: '.js-passwordForm .js-submit')
-    watch_for(
-      css:   'body',
-      value: 'must contain at least 1 digit',
-    )
-
-    set(
-      css:   'input[name="password"]',
-      value: 'SOme-pass-new2',
-    )
-    set(
-      css:   'input[name="password_confirm"]',
-      value: 'SOme-pass-new2',
-    )
-    click(css: '.js-passwordForm .js-submit')
-    watch_for(
-      css:   'body',
-      value: 'Your password has been changed',
-    )
-
-    # check if user is logged in
-    sleep 5
-    match(
-      css:       '.user-menu .user a',
-      value:     signup_user_email,
-      attribute: 'title',
-    )
-
-  end
-end