Skip to content

[Bug?]: dbAuth handler resePasword does not validate password format as signup does #10724

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

Closed
1 task
irg1008 opened this issue Jun 4, 2024 · 4 comments
Closed
1 task
Assignees
Labels
bug/confirmed We have confirmed this is a bug topic/auth

Comments

@irg1008
Copy link
Contributor

irg1008 commented Jun 4, 2024

What's not working?

dbAuth handler resePasword does not validate password format as signup does

Basically on signUp handler we can set a custom password validation like for exmaple:

...
    passwordValidation: (password) => {
      validate(password, 'password', {
        length: { minimum: 8 },
        format: {
          pattern: /^(?=.*[a-z])(?=.*[A-Z])(?=.*\d).{8,}$/,
          message:
            'Password must contain at least 1 uppercase letter, 1 lowercase letter, and 1 number',
        },
      })

      return true
    },
...

But then in the "resetPassword" handler the password is set to the database as it comes from the client, be that a simple number....

We should be able to validate the password somehow, maybe using the same passwordValidation as sign up since that makes sense

How do we reproduce the bug?

No response

What's your environment? (If it applies)

No response

Are you interested in working on this?

  • I'm interested in working on this
@irg1008 irg1008 added the bug/needs-info More information is needed for reproduction label Jun 4, 2024
@dac09 dac09 added bug/confirmed We have confirmed this is a bug and removed bug/needs-info More information is needed for reproduction labels Jun 5, 2024
@dac09
Copy link
Contributor

dac09 commented Jun 5, 2024

Hello @irg1008 thanks for raising this, and I can confirm that you cannot currenly customise the validation function on the server side for reset password.

@cannikin I looked through the dbAuth handler, and it would be relatively straightforward (I think!) to call the options - (this.options.signup as SignupFlowOptions).passwordValidation in the forgot password flow too.

If you have time, do you think you could look at this one please?

@irg1008
Copy link
Contributor Author

irg1008 commented Jun 5, 2024

I think I can do this no problem

@irg1008
Copy link
Contributor Author

irg1008 commented Jun 5, 2024

Okay I just created de PR. Check it out if you find the time. Could this mean breaking changes since could affect some users bussiness logic?

cannikin added a commit that referenced this issue Jun 5, 2024
…10734)

Related to #10724. This pull request adds password validation on
resetPassword handler using the one from the signup handler.

This could affect some users that "expect" no validation from this
handler and some new errors and failed tests may be raised for them, but
I think this feature is primary concerning security and to mantain
validation logic between handlers

---------

Co-authored-by: Rob Cameron <[email protected]>
@Josh-Walker-GM
Copy link
Contributor

This was closed by #10734 or is there more work to be done on this?

@irg1008 irg1008 closed this as completed Jun 6, 2024
Josh-Walker-GM pushed a commit that referenced this issue Jun 6, 2024
…10734)

Related to #10724. This pull request adds password validation on
resetPassword handler using the one from the signup handler.

This could affect some users that "expect" no validation from this
handler and some new errors and failed tests may be raised for them, but
I think this feature is primary concerning security and to mantain
validation logic between handlers

---------

Co-authored-by: Rob Cameron <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/confirmed We have confirmed this is a bug topic/auth
Projects
None yet
Development

No branches or pull requests

4 participants