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

Add more restrictions for user's password (Auth) #88

Closed
matijaSos opened this issue Oct 16, 2020 · 8 comments · Fixed by #376
Closed

Add more restrictions for user's password (Auth) #88

matijaSos opened this issue Oct 16, 2020 · 8 comments · Fixed by #376
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@matijaSos
Copy link
Contributor

Currently we don't do any checks, but we should - length, special characters and everything else that determines the strength of the password.

@matijaSos matijaSos added enhancement New feature or request good first issue Good for newcomers labels Oct 16, 2020
@utkarsh4321
Copy link

Hii @matijaSos i can work on this

@matijaSos
Copy link
Contributor Author

Hi @utkarsh4321 thanks a lot for your interest and reaching out! I suggest you write here a short proposal on how would you implement this feature so we can review it together before you start.

Also, have you already checked our guide to contributing? Here you will find the instructions on how to compile Wasp locally and get started with adding new features!

Let us know if any questions! Also, I'd recommend joining our group on Discord for more conversational-style discussions: https://discord.com/invite/rzdnErX

@tom-f-hall
Copy link

I noticed that password hashes of the same input result in the same value in the backend. Should add salt

@Martinsos
Copy link
Member

@tom-f-hall thanks for letting us know about this!

We are using https://www.npmjs.com/package/secure-password for hashing the passwords, and from what I read and know, it does use salt when hashing the password. On my machine, when I save two same passwords, hash is different, not the same. However, I see that the start and end of each password is the same (some kind of padding I guess), so maybe that is what made you think they are always the same? If you are indeed getting completely same hashes, could you please let me know on which system are you running this so we can try to replicate it.

Btw we also do have a plan to revisit the security once again when we got closer to production, as per this issue: #127 .

@tom-f-hall
Copy link

You are correct, it's just the padding 🤦🏻‍♂️

@Martinsos
Copy link
Member

You are correct, it's just the padding 🤦🏻‍♂️

It confused me at first also, I needed to expand the content to figure it was more than padding! Atlhough I do wonder what is the purpose of all that padding hm.

shayneczyzewski pushed a commit that referenced this issue Dec 2, 2021
…d password

Adds an additional Prisma middleware for validation checks that the user
can disable and/or extend

Closes #88 and fixes #65
shayneczyzewski pushed a commit that referenced this issue Dec 2, 2021
…d password

Adds an additional Prisma middleware for validation checks that the user
can disable and/or extend

Closes #88 and fixes #65
shayneczyzewski pushed a commit that referenced this issue Dec 2, 2021
…d password

Adds an additional Prisma middleware for validation checks that the user
can disable and/or extend

Closes #88 and fixes #65
@shayneczyzewski
Copy link
Sponsor Contributor

shayneczyzewski commented Dec 2, 2021

I decided to add an additional middleware to do the validation, just before our hashing middleware. While technically a monkey patch of Prisma's create function just for User, it feels like the cleanest approach to ensure:

  • sane defaults always run
  • users can enable/disable
  • users can add their own custom validations

Something like this:

  prismaClient.$use(async (params, next) => {
    // Ensure strong plaintext password. Must come before hashing middleware.
    // Throws an EntityValidationError on the first validation that fails.
    if (params.model === '{= userEntityUpper =}' && params.action === 'create') {
        const data = params.args.data || {}

        const defaultValidations = [
          { name: 'email must be present', fn: data => !!data.email },
          { name: 'password must be present', fn: data => !!data.password },
          { name: 'password must be at least 8 characters', fn: data => data.password.length >= 8 },
          { name: 'password must contain a number', fn: data => /\d/.test(data.password) },
        ]

        let validations = params.args._waspSkipDefaultValidations ? [] : defaultValidations
        if (Array.isArray(params.args._waspCustomValidations)) {
          validations = validations.concat(params.args._waspCustomValidations)
        }

        for (const validation of validations) {
          if (!validation.fn(data)) {
            throw new EntityValidationError('{= userEntityUpper =}', validation.name)
          }
        }

        // Remove from downstream Prisma processing to avoid "Unknown arg" error
        delete params.args._waspSkipDefaultValidations
        delete params.args._waspCustomValidations
    }

    const result = next(params)

    return result
  })

Then, users can customize in the following ways:

// Runs just our default validations
const newUser = context.entities.User.create({
        data: { email: 'some@email.com', password: 'this will be hashed!' }
    })
// Runs no validations (previous behavior)
const newUser = context.entities.User.create({
        data: { email: 'some@email.com', password: 'this will be hashed!' },
        _waspSkipDefaultValidations: true, // defaults to false
    })
// Runs just their validations
const newUser = context.entities.User.create({
        data: { email: 'some@email.com', password: 'this will be hashed!' },
        _waspSkipDefaultValidations: true,
        _waspCustomValidations: [
          { name: 'password should not be "password"', fn: data => data.password !== 'password' },
          // More can be added below (note: it stops on first to return false)
        ]
    })

You could also run them all by omitting _waspSkipDefaultValidations in the final example, or setting it to false.

@shayneczyzewski
Copy link
Sponsor Contributor

shayneczyzewski commented Dec 3, 2021

⚠️ These code snippets are now out of date. Please see the PR for the latest versions.

shayneczyzewski pushed a commit that referenced this issue Dec 7, 2021
…d password

Adds an additional Prisma middleware for validation checks that the user
can disable and/or extend

Closes #88 and fixes #100
shayneczyzewski added a commit that referenced this issue Dec 7, 2021
…d password (#376)

Adds an additional Prisma middleware for User entity validation checks that the user can disable and/or extend

Closes #88 and fixes #100
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants