Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Registration fails silently with long passwords #1443

Closed
bennypowers opened this issue Nov 10, 2024 · 3 comments · Fixed by #1455
Closed

Bug: Registration fails silently with long passwords #1443

bennypowers opened this issue Nov 10, 2024 · 3 comments · Fixed by #1455
Labels
🐛 Bug Something isn't working

Comments

@bennypowers
Copy link

Describe the bug
This bug is the root cause of #1417 (and #1425). Turns out my password manager's sensible length policies were to blame.

thanks to @ShimShtein for helping me debug this

To Reproduce
Steps to reproduce the behavior:

  1. create the following files
# ~/.config/containers/systemd/maybe-db.container
# maybe-db.container
[Unit]
Description=Maybe Personal Finance Database
Documentation=https://maybe.co
Requires=podman.socket

[Service]
Restart=always

[Container]
Image=docker.io/postgres:16
ContainerName=maybe-db
EnvironmentFile=%h/.config/maybe/.env
HealthCmd=pg_isready -U XXX -d XXX #replace with user and db
HealthInterval=5s
HealthRetries=5
HealthTimeout=5s
PublishPort=5432:5432
Network=maybe.network
Volume=maybe-db:/var/lib/postgresql/data
Volume=/run/user/1000/podman/podman.sock:/var/run/docker.sock
Notify=healthy

[Install]
WantedBy=multi-user.target
# ~/.config/containers/systemd/maybe-app.container
[Unit]
Description=Maybe Personal Finance Server
Documentation=https://maybe.co
Requires=podman.socket maybe-db.service
After=podman.socket maybe-db.service

[Service]
Restart=always

[Container]
ContainerName=maybe-server
Image=ghcr.io/maybe-finance/maybe:latest
AutoUpdate=registry
EnvironmentFile=%h/.config/maybe/.env
PublishPort=3000:3000
Network=maybe.network
Volume=maybe-server:/rails/storage
Volume=/run/user/1000/podman/podman.sock:/var/run/docker.sock

[Install]
WantedBy=multi-user.target default.target
# ~/.config/containers/systemd/maybe.network
[Network]
NetworkName=maybe
# ~/.config/containers/systemd/maybe.volume
[Volume]
VolumeName=maybe-db
[Volume]
VolumeName=maybe-server
# ~/.config/maybe/.env
DB_HOST=maybe-db
POSTGRES_DB=XXX
POSTGRES_USER=XXX
POSTGRES_PASSWORD=XXX
APP_DOMAIN=XXX.XXX.XXX
DISABLE_SSL=true
UPGRADES_ENABLED=false
UPGRADES_MODE=manual
UPGRADES_TARGET=release
RAILS_FORCE_SSL=false
RAILS_ASSUME_SSL=false
GOOD_JOB_EXECUTION_MODE=async
  1. systemctl --user start maybe-app
  2. localhost:3000
  3. enter a long password

Expected behavior
registration proceeds

Actual
422 unprocessable content, no error message

What version of Maybe are you using?
"Self-hosted". latest as of today

What operating system and browser are you using?
Linux/Firefox latest

Steps to debug

podman exec -it maybe-app bin/rails console
maybe(prod)> @user = User.new(email: "[email protected]", password: "q6d4oij8i0psojtlrxw4usbf1kmorc6lddy6m3uturiaj8y5ix2us7e6ix3lgmsw90qwqyy9kk5rlg4im7yzf065jlahhsdx2uid9t1pryzuu37bhhlgrqwpc3luhgho")
@user.save

notice DB rollback

maybe(prod)> @user.error

"password too long"

@bennypowers bennypowers added the 🐛 Bug Something isn't working label Nov 10, 2024
@tonyvince
Copy link
Contributor

has_secure_password supports passwords up to 72 bytes. You are using 128 byte password string

@bennypowers
Copy link
Author

the bug isn't that the server has pointless and needlessly insecure char limits

the but is that neither the server logs nor the response code, nor the response body, nor the html form validations say anything about the char limit. Users who innocently follow best practice, use pw managers to generate strong, 128 char passwords, will have no idea why the app isn't working, and end up spending a week tweaking cloudflare tunnel settings.

We need some HTML form validations, a proper response code, and a response body which explains what the problem is: i.e. password too long, passwords must be less than xyz characters

@tonyvince
Copy link
Contributor

tonyvince commented Nov 11, 2024

@bennypowers you are right. #1455 has a potential fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants