Browse Source

Fixes issue #2907 - Password strength settings are ignored when creating new customer accounts. Make login available to verified users only.

Rolf Schmidt 4 years ago
parent
commit
4014839242

+ 17 - 40
app/assets/javascripts/app/controllers/email_verify.coffee

@@ -1,7 +1,6 @@
 class Index extends App.Controller
   constructor: ->
     super
-    @authenticateCheckRedirect()
     @verifyCall()
 
   verifyCall: =>
@@ -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')

+ 29 - 18
app/assets/javascripts/app/controllers/signup.coffee

@@ -2,6 +2,7 @@ class Index extends App.ControllerContent
   events:
     'submit form': 'submit'
     'click .submit': 'submit'
+    'click .js-submitResend': 'resend'
     'click .cancel': 'cancel'
 
   constructor: ->
@@ -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

+ 28 - 0
app/assets/javascripts/app/controllers/user_profile.coffee

@@ -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) =>
@@ -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 = [
       {
@@ -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

+ 0 - 82
app/assets/javascripts/app/controllers/widget/user_signup_check.coffee

@@ -1,82 +0,0 @@
-class Widget extends App.Controller
-  constructor: ->
-
-    # for browser test
-    App.Event.bind('user_signup_verify', (user) ->
-      new Modal(user: user)
-      'user_signup_verify'
-    )
-
-    App.Event.bind('auth:login', (user) =>
-      return if !user
-      @verifyLater(user.id)
-      'user_signup_verify'
-    )
-    user = App.User.current()
-    @verifyLater(user.id) if user?
-
-  verifyLater: (userId) =>
-    delay = =>
-      @verify(userId)
-    @delay(delay, 5000, 'user_signup_verify_dialog')
-
-  verify: (userId) ->
-    return if !userId
-    return if !App.User.exists(userId)
-    user = App.User.find(userId)
-    return if user.source isnt 'signup'
-    return if user.verified is true
-    currentTime = new Date().getTime()
-    createdAt = Date.parse(user.created_at)
-    diff = currentTime - createdAt
-    max = 1000 * 60 * 30 # show message if account is older then 30 minutes
-    return if diff < max
-    new Modal(user: user)
-
-class Modal extends App.ControllerModal
-  backdrop: false
-  keyboard: false
-  head: 'Account not verified'
-  small: true
-  buttonClose: false
-  buttonCancel: false
-  buttonSubmit: 'Resend verification email'
-
-  constructor: ->
-    super
-
-  content: =>
-    if !@sent
-      return App.i18n.translateContent('Your account has not been verified. Please click the link in the verification email.')
-    content = App.i18n.translateContent('We\'ve sent an email to _%s_. Click the link in the email to verify your account.', @user.email)
-    content += '<br><br>'
-    content += App.i18n.translateContent('If you don\'t see the email, check other places it might be, like your junk, spam, social, or other folders.')
-    content
-
-  onSubmit: =>
-    @ajax(
-      id:          'email_verify_send'
-      type:        'POST'
-      url:         @apiPath + '/users/email_verify_send'
-      data:        JSON.stringify(email: @user.email)
-      processData: true
-      success:     @success
-      error:       @error
-    )
-
-  success: (data) =>
-    @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.Config.set('user_signup', Widget, 'Widgets')

+ 16 - 0
app/assets/javascripts/app/views/signup/verify.jst.eco

@@ -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>

+ 16 - 10
app/controllers/users_controller.rb

@@ -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
@@ -140,6 +140,15 @@ class UsersController < ApplicationController
       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
@@ -499,6 +508,8 @@ curl http://localhost/api/v1/users/email_verify -v -u #{login}:#{password} -H "C
     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
 
@@ -527,17 +538,12 @@ curl http://localhost/api/v1/users/email_verify_send -v -u #{login}:#{password}
     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)
@@ -1029,13 +1035,13 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content
 
   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

+ 1 - 1
app/models/user.rb

@@ -586,7 +586,7 @@ returns
     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

+ 5 - 1
lib/auth/internal.rb

@@ -12,7 +12,11 @@ class Auth
         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

+ 8 - 47
test/browser/signup_password_change_and_reset_test.rb

@@ -23,65 +23,26 @@ class SignupPasswordChangeAndResetTest < TestCase
     )
     set(
       css:   'input[name="password"]',
-      value: 'some-pass',
+      value: 'some-pass-123',
     )
     set(
       css:   'input[name="password_confirm"]',
-      value: 'some-pass',
+      value: 'some-pass-123',
     )
     click(css: 'button.js-submit')
 
-    watch_for_disappear(
-      css:     '.signup',
-      timeout: 10,
-    )
-
-    match(
-      css:       '.user-menu .user a',
-      value:     signup_user_email,
-      attribute: 'title',
-    )
-
-    # check email verify
-    location(url: "#{browser_url}#email_verify/not_existing")
     watch_for(
       css:   '#content',
-      value: 'Unable to verify email',
+      value: 'Registration successful!',
     )
-    logout()
 
-    login(
-      username: signup_user_email,
-      password: 'some-pass',
-      url:      "#{browser_url}#email_verify/not_existing2",
-    )
-    watch_for(
-      css:   '#content',
-      value: 'Unable to verify email',
-    )
-    execute(
-      js: 'App.Event.trigger("user_signup_verify", App.Session.get())',
-    )
-    modal_ready()
-    click(css: '.modal .js-submit')
+    # auto login via token trick in dev mode
+    click(css: '.signup .js-submitResend')
 
-    execute(
-      js: 'App.Auth.logout()',
-    )
-    sleep 6
     watch_for(
-      css: '#login',
-    )
-    login(
-      username: signup_user_email,
-      password: 'some-pass',
-    )
-    watch_for(
-      css:   '#content',
-      value: 'Your email address has been verified',
+      css:   '.content.active',
+      value: 'Welcome!',
     )
-    modal_disappear()
-    sleep 2
 
     # change password
     click(css: '.navbar-items-personal .user a')
@@ -109,7 +70,7 @@ class SignupPasswordChangeAndResetTest < TestCase
 
     set(
       css:   'input[name="password_old"]',
-      value: 'some-pass',
+      value: 'some-pass-123',
     )
     set(
       css:   'input[name="password_new_confirm"]',