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

fix: update password requirements #4107

Merged
merged 5 commits into from
May 23, 2024
Merged

fix: update password requirements #4107

merged 5 commits into from
May 23, 2024

Conversation

emilyjablonski
Copy link
Collaborator

@emilyjablonski emilyjablonski commented May 22, 2024

Pull Request Template

Issue Overview

This PR addresses #4101

  • This change addresses the issue in full

Description

We had a user in AC who was trying to change their password and was receiving "passwordTooWeak" but their password did meet our rules. After testing various secure passwords, I saw that while our regex did allow special characters, it did not allow special characters to be the first character. This is the only failure I could find in the regex but I can't guarantee it was this user's issue.

After chatting w product, as this was going to require a change to the regex anyway, we also upped the security requirements from 8 characters / one number to 12 characters / one lowercase / one uppercase / one number / one special character. This should only apply to new passwords, which can be created on the public side by creating an account, updating your password in your account settings, or resetting your password, and on the parter side by confirming an account invitation.

I kept admin@example.com as abcdef to test the flow of a user with an existing password under the old rules. All other seeded users like jurisdiction-admin@example.com are Abcdef12345!.

I also added loading states on all the forms I touched if they didn't exist because it was making it hard to test.

How Can This Be Tested/Reviewed?

  • Try out passwords to see the regex is working as expected
  • Try to login as seeded user admin@example.com and abcdef which should already have a currently-weak password to ensure existing users are okay
  • Update a password w the four flows above and see the regex working

Checklist:

  • My code follows the style guidelines of this project
  • I have added QA notes to the issue with applicable URLs
  • I have performed a self-review of my own code
  • I have reviewed the changes in a desktop view
  • I have reviewed the changes in a mobile view
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have assigned reviewers
  • I have run yarn generate:client and/or created a migration if I made backend changes that require them
  • My commit message(s) is/are polished, and any breaking changes are indicated in the message and are well-described
  • Commits made across packages purposefully have the same commit message/version change, else are separated into different commits

Reviewer Notes:

Steps to review a PR:

  • Read and understand the issue, and ensure the author has added QA notes
  • Review the code itself from a style point of view
  • Pull the changes down locally and test that the acceptance criteria is met
  • Also review the acceptance criteria on the Netlify deploy preview (noting that these do not yet include any backend changes made in the PR)
  • Either explicitly ask a clarifying question, request changes, or approve the PR if there are small remaining changes but the PR is otherwise good to go

On Merge:

If you have one commit and message, squash. If you need each message to be applied, rebase and merge.

Copy link

netlify bot commented May 22, 2024

Deploy Preview for bloom-exygy-dev ready!

Name Link
🔨 Latest commit 70c112b
🔍 Latest deploy log https://app.netlify.com/sites/bloom-exygy-dev/deploys/664e8b589b2a540008655198
😎 Deploy Preview https://deploy-preview-4107--bloom-exygy-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@emilyjablonski emilyjablonski added the 2 reviews needed Requires 2 more review before ready to merge label May 23, 2024
Copy link
Collaborator

@YazeedLoonat YazeedLoonat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@YazeedLoonat YazeedLoonat added 1 review needed Requires 1 more review before ready to merge and removed 2 reviews needed Requires 2 more review before ready to merge labels May 23, 2024
@mcgarrye mcgarrye added ready to merge Should be applied when a PR has been reviewed and approved and removed 1 review needed Requires 1 more review before ready to merge labels May 23, 2024
@emilyjablonski emilyjablonski merged commit 1f96fbb into main May 23, 2024
20 checks passed
emilyjablonski added a commit to housingbayarea/bloom that referenced this pull request May 23, 2024
emilyjablonski added a commit to housingbayarea/bloom that referenced this pull request May 29, 2024
emilyjablonski added a commit to metrotranscom/doorway that referenced this pull request May 29, 2024
emilyjablonski added a commit to metrotranscom/doorway that referenced this pull request Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Should be applied when a PR has been reviewed and approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants