lucky icon indicating copy to clipboard operation
lucky copied to clipboard

"Invalid password size" ISE

Open carcinocron opened this issue 2 years ago • 3 comments

Describe the bug Attempting to register an account throws ISE "password size invalid".

To Reproduce Use a password of length 128 on a freshly generated lucky full. A password of length 64 worked. 86 did not.

Expected behavior

Other systems probably silently truncate or throw a 422 validation error.

Screenshots/code

web          | POST /sign_up ()
web          |  ▸ Handled by SignUps::Create
web          |  ▸  Crypto::Bcrypt::Error 
web          | 
web          |      Invalid password size
web          | 
web          |     Backtrace 
web          | 
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/crypto/bcrypt.cr:71:5 in 'initialize'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/crypto/bcrypt.cr:68:3 in 'new'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/crypto/bcrypt.cr:54:5 in 'hash_secret'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/crypto/bcrypt/password.cr:27:5 in 'create'
web          |      lib/authentic/src/authentic.cr:133:5 in 'generate_encrypted_password'
web          |      lib/authentic/src/authentic.cr:129:3 in 'generate_encrypted_password'
web          |      lib/authentic/src/authentic.cr:122:40 in 'copy_and_encrypt'
web          |      src/operations/sign_up_user.cr:12:1 in 'before_save'
web          |      lib/avram/src/avram/save_operation.cr:316:5 in 'save'
web          |      src/operations/sign_up_user.cr:1:1 in 'call'
web          |      lib/lucky/src/lucky/renderable.cr:136:16 in 'perform_action'
web          |      lib/lucky/src/lucky/route_handler.cr:10:7 in 'call'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
web          |      lib/lucky/src/lucky/remote_ip_handler.cr:15:5 in 'call'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
web          |      lib/lucky/src/lucky/error_handler.cr:14:5 in 'call'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
web          |      lib/lucky/src/lucky/log_handler.cr:34:9 in 'call'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
web          |      lib/lucky/src/lucky/http_method_override_handler.cr:11:5 in 'call'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
web          |      lib/lucky/src/lucky/force_ssl_handler.cr:37:7 in 'call'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
web          |      lib/lucky/src/lucky/request_id_handler.cr:24:5 in 'call'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/http/server/request_processor.cr:51:11 in 'process'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/http/server.cr:515:5 in 'handle_client'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/http/server.cr:468:13 in '->'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/primitives.cr:266:3 in 'run'
web          |      /home/forge/.asdf/installs/crystal/1.2.2/share/crystal/src/fiber.cr:92:34 in '->'
web          |      ???
web          | 
web          |  ▸ Sent 500 Internal Server Error (60.88ms) ()

Versions (please complete the following information):

  • Lucky version: 0.29.0
  • Crystal version 1.2.2:
  • OS: Ubuntu Mate 21.04

carcinocron avatar Feb 19 '22 18:02 carcinocron

That's interesting. You can only do a max of 72 since that's a limit of Bcrypt. Your operation should have caught that. Can you see if your fresh Lucky app has this line?

https://github.com/luckyframework/lucky_cli/blob/5649c9e78c9c359710347ef0e91fa87c04072ea6/src/base_authentication_app_skeleton/src/operations/mixins/password_validations.cr#L10

If it doesn't, then we need to see why not since this should be on all Lucky 0.29 apps.

Ref: https://github.com/crystal-lang/crystal/blob/584d0a273720609d7a9a8d2f69a93a1ae31e5938/src/crypto/bcrypt.cr#L38

jwoertink avatar Feb 19 '22 19:02 jwoertink

I modified the file to this:

module PasswordValidations
  macro included
    before_save run_password_validations
  end

  private def run_password_validations
    pp ({:line => __LINE__, :password => password})
    validate_required password, password_confirmation
    validate_confirmation_of password, with: password_confirmation
    # 72 is a limitation of BCrypt
    pp ({:line => __LINE__, :password => password})
    validate_size_of password, min: 6, max: 72
  end
end

and I did not see any output.

carcinocron avatar Feb 20 '22 01:02 carcinocron

so those validations aren't getting called? :raised_eyebrow: .... well that's definitely a bug, but I wonder why? :thinking:

jwoertink avatar Feb 20 '22 17:02 jwoertink

This is fixed by https://github.com/luckyframework/lucky_cli/pull/773 Basically, we don't short circuit the operations once a validation fails, it'll still go through the entire callback stack. So in this case it was doing a copy and encrypt with a bad password... As for why that macro wasn't being called.... I have no clue, but I'm unable to reproduce that in any current or upcoming Lucky release :man_shrugging: But at least the original error was caught and will be fixed in the next release.

jwoertink avatar Aug 28 '22 22:08 jwoertink