Skip to content

[AC-1070] Enforce master password policy on login #2714

Merged
shane-melton merged 18 commits intomasterfrom
EC-1070-expand-master-pass-reqs
Apr 17, 2023
Merged

[AC-1070] Enforce master password policy on login #2714
shane-melton merged 18 commits intomasterfrom
EC-1070-expand-master-pass-reqs

Conversation

@shane-melton
Copy link
Member

@shane-melton shane-melton commented Feb 17, 2023

Type of change

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

Objective

Provides the server requirements for the client PR for AC-1070.

  • Add a new GET /policies endpoint that returns all organization policies that apply to the authorized user. It is used by the CLI when a weak master password is detected during login and they are used to validate that the new password meets requirements.
  • Add a new MasterPasswordPolicy field as a custom response to the Identity Server token and 2FA responses. It is used by clients to evaluate a user's master password client side while the password is still in working memory of the login component/command.
  • Add a VerifyMasterPasswordResponseModel response to the POST /accounts/verify-password endpoint that includes any enabled master password policy for the user. To be used when an authenticated user is unlocking their vault and avoids making an additional network request for master password policies for every vault unlock.

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • If making database changes - make sure you also update Entity Framework queries and/or migrations
  • 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

The additional API endpoint is required to avoid forcing a full sync call before every login for master password policy enforcement on login.
@shane-melton shane-melton force-pushed the EC-1070-expand-master-pass-reqs branch from 3d49ccd to 728bd87 Compare February 21, 2023 19:58
@shane-melton shane-melton force-pushed the EC-1070-expand-master-pass-reqs branch from 728bd87 to f639a8e Compare March 1, 2023 16:16
@shane-melton shane-melton force-pushed the EC-1070-expand-master-pass-reqs branch from f639a8e to bdfe85a Compare March 1, 2023 16:26
@shane-melton shane-melton changed the title [EC-1070] Enforce master password policy on login [AC-1070] Enforce master password policy on login Mar 1, 2023
@shane-melton shane-melton marked this pull request as ready for review March 1, 2023 16:38
@shane-melton shane-melton requested a review from a team March 1, 2023 16:39
- Update BaseRequestValidator
- Update AccountsController for /verify-password endpoint
- Update VerifyMasterPasswordResponseModel to accept MasterPasswordPolicyData
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.

Great work on this!

- Use User object instead of Guid
- Remove TODO message
- Use `PolicyRepository.GetManyByTypeApplicableToUserIdAsync` instead of filtering locally
- Remove default values from both models
- Add missing `RequireLower`
- Fix mismatched properties in `CombineWith` method
- Make properties nullable in response model
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.

LGTM but I'll wait until I've reviewed the client PR before approving.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

eliykat
eliykat previously approved these changes Apr 7, 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.

Re-approving after updating from master.

@shane-melton shane-melton merged commit f2fad55 into master Apr 17, 2023
@shane-melton shane-melton deleted the EC-1070-expand-master-pass-reqs branch April 17, 2023 14:35
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 this pull request may close these issues.

2 participants