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

"password_hash can't be blank" error when trying to register email with invalid format #320

Closed
christophweegen opened this issue Nov 2, 2019 · 2 comments · Fixed by #391

Comments

@christophweegen
Copy link

Hello everybody,

just followed the guides to setup API authentication.

I played a bit around to see how errors are handled.

When I try to register a user with an invalid email like infoexample.com, I get following JSON response:

{
  "error": {
    "errors": {
      "email": [
        "has invalid format"
      ],
      "password_hash": [
        "can't be blank"
      ]
    },
    "message": "Couldn't create user",
    "status": 500
  }
}

While it makes sense to give out email has invalid format as error description, password_hash can't be blank isn't a useful error message in client applications, since a user can't do anything about this because setting the password hash is a "private" implementation detail and out of the user's control. Returning this error will only confuse users I guess.

Didn't look into the source yet and how it can be be easily avoided to return this error, just wanted to point that out and would like to hear your feedback.

Thanks a lot!

@danschultzer
Copy link
Collaborator

Yeah, it's because the final check in the changeset is whether the password hash exists: https://github.com/danschultzer/pow/blob/v1.0.14/lib/pow/ecto/schema/changeset.ex#L84

I think it makes more sense to have that check in maybe_put_password_hash/2 instead. I'll open a PR.

@danschultzer
Copy link
Collaborator

Fixed in #391 finally 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants