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

[PM-2903] fix : don't set pin if not provided #5769

Merged
merged 5 commits into from
Nov 13, 2023
Merged

[PM-2903] fix : don't set pin if not provided #5769

merged 5 commits into from
Nov 13, 2023

Conversation

arnabrahman
Copy link
Contributor

@arnabrahman arnabrahman commented Jul 9, 2023

Type of change

- [X] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

We don't want to set a pin when a user doesn't provide one. This PR addresses #5726

Code changes

When the pin is not given, the modal gets closed but the remaining code is getting executed. So an invalid pin was getting set. Just return from submit function if the pin is not given.

Before

before.webm

After

after.webm

Before you submit

  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team
  • Ensure that all UI additions follow WCAG AA requirements

@bitwarden-bot
Copy link

Thank you for your contribution! We've added this to our internal Community PR board for review.
ID: PM-2903

@bitwarden-bot bitwarden-bot changed the title fix : don't set pin if not provided [PM-2903] fix : don't set pin if not provided Jul 9, 2023
@djsmith85 djsmith85 linked an issue Jul 9, 2023 that may be closed by this pull request
1 task
@arnabrahman arnabrahman marked this pull request as ready for review July 16, 2023 05:24
@arnabrahman arnabrahman requested a review from a team as a code owner July 16, 2023 05:24
@arnabrahman
Copy link
Contributor Author

Hi @djsmith85 , this has been open for some time. Let me know if i need to do anything.

@eliykat eliykat requested a review from a team September 29, 2023 03:23
eliykat
eliykat previously approved these changes Oct 17, 2023
Copy link
Member

@eliykat eliykat left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! I've passed on to our QA team to verify before merging.

@eliykat eliykat merged commit 84f93ed into bitwarden:master Nov 13, 2023
44 of 52 checks passed
@arnabrahman arnabrahman deleted the fix-5726 branch November 18, 2023 12:52
BlackDex pushed a commit to BlackDex/bitwarden-clients that referenced this pull request Nov 21, 2023
Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
Co-authored-by: Thomas Rittson <trittson@bitwarden.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't allow establishing 0 length PINs by accident
5 participants