Skip to content

Commit

Permalink
Fixes issue #2907 - Password strength settings are ignored when creat…
Browse files Browse the repository at this point in the history
…ing new customer accounts. Make login available to verified users only.
  • Loading branch information
rolfschmidt authored and thorsteneckel committed Jun 10, 2020
1 parent a336d52 commit 4014839
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 199 deletions.
57 changes: 17 additions & 40 deletions app/assets/javascripts/app/controllers/email_verify.coffee
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
class Index extends App.Controller
constructor: ->
super
@authenticateCheckRedirect()
@verifyCall()

verifyCall: =>
Expand All @@ -11,45 +10,23 @@ class Index extends App.Controller
url: "#{@apiPath}/users/email_verify"
data: JSON.stringify(token: @token)
processData: true
success: @success
error: @error
)

success: =>
new Success(el: @el)

error: =>
new Fail(el: @el)

class Success extends App.ControllerContent
constructor: ->
super
@render()

# rerender view, e. g. on language change
@bind 'ui:rerender', =>
@render()

render: =>
@renderScreenSuccess(
detail: 'Woo hoo! Your email address has been verified!'
)
delay = =>
@navigate '#'
@delay(delay, 20500)

class Fail extends App.ControllerContent
constructor: ->
super
@render()

# rerender view, e. g. on language change
@bind 'ui:rerender', =>
@render()

render: =>
@renderScreenError(
detail: 'Unable to verify email. Please contact your administrator.'
success: (data, status, xhr) =>
App.Auth.loginCheck()
@navigate '#'

@notify
type: 'success'
msg: App.i18n.translateContent('Woo hoo! Your email address has been verified!')
removeAll: true
timeout: 2000

error: (data, status, xhr) =>
@navigate '#'

@notify
type: 'error'
msg: App.i18n.translateContent('Unable to verify email. Please contact your administrator.')
removeAll: true
)

App.Config.set('email_verify/:token', Index, 'Routes')
47 changes: 29 additions & 18 deletions app/assets/javascripts/app/controllers/signup.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ class Index extends App.ControllerContent
events:
'submit form': 'submit'
'click .submit': 'submit'
'click .js-submitResend': 'resend'
'click .cancel': 'cancel'

constructor: ->
Expand Down Expand Up @@ -61,31 +62,41 @@ class Index extends App.ControllerContent
# save user
user.save(
done: (r) =>
App.Auth.login(
data:
username: @params.login
password: @params.password
success: @success
error: @error
@html App.view('signup/verify')(
email: @params.email
)
fail: (settings, details) =>
@formEnable(e)
@form.showAlert(details.error_human || details.error || 'Unable to update object!')
if _.isArray(details.error)
@form.showAlert( App.i18n.translateInline( details.error[0], details.error[1] ) )
else
@form.showAlert(details.error_human || details.error || 'Unable to update object!')
)

success: (data, status, xhr) =>
resend: (e) =>
e.preventDefault()
@formDisable(e)
@resendParams = @formParam(e.target)

@ajax(
id: 'email_verify_send'
type: 'POST'
url: @apiPath + '/users/email_verify_send'
data: JSON.stringify(email: @resendParams.email)
processData: true
success: (data, status, xhr) =>
@formEnable(e)

# login check
App.Auth.loginCheck()
# add notify
@notify
type: 'success'
msg: App.i18n.translateContent('Email sent to "%s". Please verify your email address.', @params.email)
removeAll: true

# add notify
@notify
type: 'success'
msg: App.i18n.translateContent('Thanks for joining. Email sent to "%s". Please verify your email address.', @params.email)
removeAll: true

# redirect to #
@navigate '#'
if data.token && @Config.get('developer_mode') is true
@navigate "#email_verify/#{data.token}"
error: @error
)

error: (xhr, statusText, error) =>
detailsRaw = xhr.responseText
Expand Down
28 changes: 28 additions & 0 deletions app/assets/javascripts/app/controllers/user_profile.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ class App.UserProfile extends App.Controller
class ActionRow extends App.ObserverActionRow
model: 'User'
observe:
verified: true
source: true
organization_id: true

showHistory: (user) =>
Expand All @@ -100,6 +102,25 @@ class ActionRow extends App.ObserverActionRow
newTicket: (user) =>
@navigate("ticket/create/customer/#{user.id}")

resendVerificationEmail: (user) =>
@ajax(
id: 'email_verify_send'
type: 'POST'
url: @apiPath + '/users/email_verify_send'
data: JSON.stringify(email: user.email)
processData: true
success: (data, status, xhr) =>
@notify
type: 'success'
msg: App.i18n.translateContent('Email sent to "%s". Please let the user verify his email address.', user.email)
removeAll: true
error: (data, status, xhr) =>
@notify
type: 'error'
msg: App.i18n.translateContent('Failed to sent Email "%s". Please contact an administrator.', user.email)
removeAll: true
)

actions: (user) =>
actions = [
{
Expand All @@ -121,6 +142,13 @@ class ActionRow extends App.ObserverActionRow
callback: @editUser
}

if user.verified isnt true && user.source is 'signup'
actions.push({
name: 'resend_verification_email'
title: 'Resend verification email'
callback: @resendVerificationEmail
})

actions

class Object extends App.ObserverController
Expand Down

This file was deleted.

16 changes: 16 additions & 0 deletions app/assets/javascripts/app/views/signup/verify.jst.eco
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<div class="signup fullscreen">
<div class="fullscreen-center">
<div class="hero-unit fullscreen-body">
<h1><%- @T('Registration successful!') %></h1>
<p><%- @T('Thanks for joining. Email sent to "%s".', @email) %></p>
<p><%- @T('Please click the link in the verification email.') %> <%- @T('If you don\'t see the email, check other places it might be, like your junk, spam, social, or other folders.') %></p>
<form>
<input type="hidden" name="email" value="<%= @email %>">
<div class="form-controls">
<a class="btn btn--text btn--subtle js-cancel" href="#login"><%- @T('Go Back') %></a>
<button class="btn btn--primary js-submitResend align-right"><%- @T('Resend verification email') %></button>
</div>
</form>
</div>
</div>
</div>
26 changes: 16 additions & 10 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class UsersController < ApplicationController
include ChecksUserAttributesByCurrentUserPermission

prepend_before_action -> { authorize! }, only: %i[import_example import_start search history]
prepend_before_action :authentication_check, except: %i[create password_reset_send password_reset_verify image]
prepend_before_action :authentication_check, except: %i[create password_reset_send password_reset_verify image email_verify email_verify_send]
prepend_before_action :authentication_check_only, only: [:create]

# @path [GET] /users
Expand Down Expand Up @@ -140,6 +140,15 @@ def create
exists = User.exists?(email: clean_params[:email].downcase.strip)
raise Exceptions::UnprocessableEntity, "Email address '#{clean_params[:email].downcase.strip}' is already used for other user." if exists

# check password policy
if clean_params[:password].present?
result = password_policy(clean_params[:password])
if result != true
render json: { error: result }, status: :unprocessable_entity
return
end
end

user = User.new(clean_params)
user.associations_from_param(params)
user.updated_by_id = 1
Expand Down Expand Up @@ -499,6 +508,8 @@ def email_verify
user = User.signup_verify_via_token(params[:token], current_user)
raise Exceptions::UnprocessableEntity, 'Invalid token!' if !user

current_user_set(user)

render json: { message: 'ok', user_email: user.email }, status: :ok
end

Expand Down Expand Up @@ -527,17 +538,12 @@ def email_verify_send
raise Exceptions::UnprocessableEntity, 'No email!' if !params[:email]

user = User.find_by(email: params[:email].downcase)
if !user
if !user || user.verified == true
# result is always positive to avoid leaking of existing user accounts
render json: { message: 'ok' }, status: :ok
return
end

#if user.verified == true
# render json: { error: 'Already verified!' }, status: :unprocessable_entity
# return
#end

Token.create(action: 'Signup', user_id: user.id)

result = User.signup_new_token(user)
Expand Down Expand Up @@ -1029,13 +1035,13 @@ def import_start

def password_policy(password)
if Setting.get('password_min_size').to_i > password.length
return ["Can\'t update password, it must be at least %s characters long!", Setting.get('password_min_size')]
return ['Invalid password, it must be at least %s characters long!', Setting.get('password_min_size')]
end
if Setting.get('password_need_digit').to_i == 1 && password !~ /\d/
return ["Can't update password, it must contain at least 1 digit!"]
return ['Invalid password, it must contain at least 1 digit!']
end
if Setting.get('password_min_2_lower_2_upper_characters').to_i == 1 && ( password !~ /[A-Z].*[A-Z]/ || password !~ /[a-z].*[a-z]/ )
return ["Can't update password, it must contain at least 2 lowercase and 2 uppercase characters!"]
return ['Invalid password, it must contain at least 2 lowercase and 2 uppercase characters!']
end

true
Expand Down
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ def self.password_reset_via_token(token, password)
return if !user

# reset password
user.update!(password: password)
user.update!(password: password, verified: true)

# delete token
Token.find_by(action: 'PasswordReset', name: token).destroy
Expand Down
6 changes: 5 additions & 1 deletion lib/auth/internal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ def valid?(user, password)
return true
end

PasswordHash.verified?(user.password, password)
password_verified = PasswordHash.verified?(user.password, password)

raise Exceptions::NotAuthorized, 'Please verify your account before you can login!' if !user.verified && user.source == 'signup' && password_verified

password_verified
end

private
Expand Down
Loading

1 comment on commit 4014839

@thorsteneckel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.