Auth/PM-3275 - Changes to support TDE User without MP being able to Set a Password + misc refactoring

[JaredSnider-Bitwarden]

Type of change

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

Objective

Resolve PM-3275 where a TDE User without a MP who obtains the ability to reset other user passwords must set a password but they could not without these changes.

Associated clients PR here.

Notes for Reviewers

• This is a large change set. Please reach out if any further clarification needed, and I'll be happy to jump on a call to clarify any of these.

Code changes

Originally, the scope of these this branch was to simply add a new endpoint for retrieving just the Master Password policy options for a user (which is done on the set-password.component client side), but it quickly grew as more changes and refactorings were required to support this new flow while meeting our latest code standards.

New MP Policy Endpoint + Existing Endpoint Deprecation

• src/Api/Controllers/PoliciesController.cs
  • Add new, secured GetMasterPasswordPolicy(...) endpoint
  • Marked existing, anonymous GetByInvitedUser(...) endpoint as deprecated with associated tech debt ticket to clean it up later
    • We do not need an anonymous endpoint for this logic. In all known scenarios (user JIT provisioned via SSO and TDE user w/out MP), a user will be authenticated.
• test/Api.Test/Controllers/PoliciesControllerTests.cs
  • There were no tests for the PoliciesController so I created a test file and tested the new GetMasterPasswordPolicy(...) method. I did not test all existing methods as that would be even more scope creep, but now we have a starting point for future tests.

Service to Command refactors per ADR-0008

• Refactor UserService.SetPasswordAsync(...) into SetInitialMasterPasswordCommand

  • src/Core/Services/Implementations/UserService.cs
  • src/Core/Services/IUserService.cs
    • Remove SetPasswordAsync(...)
    • Convert UpdatePasswordHash(...) from private to public (+ add to interface) for consumption in new SetInitialMasterPasswordCommand
  • src/Core/Auth/UserFeatures/UserMasterPassword/Interfaces/ISetInitialMasterPasswordCommand.cs
    • Create interface for new command
  • src/Core/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommand.cs
    • Create new command
      • New Behavior: As TDE Users are generally already confirmed when obtaining elevated permissions, we had to add a check which only attempts to accept a user if the user is in the invited status.
  • test/Core.Test/Auth/UserFeatures/UserMasterPassword/SetInitialMasterPasswordCommandTests.cs
    • Add tests for new command
  • src/Core/Auth/UserFeatures/UserServiceCollectionExtensions.cs
    • Add new UserServiceCollectionExtensions file which matching existing code patterns and bundles the addition of the existing UserService along with new commands like the SetInitialMasterPasswordCommand
  • src/SharedWeb/Utilities/ServiceCollectionExtensions.cs
    • Leverage new UserServiceCollectionExtensions to add UserService and any user commands
  • src/Api/Controllers/AccountsController.cs
    • Replace usages of _userService.SetPasswordAsync(...) with SetInitialMasterPasswordCommand
  • test/Api.Test/Controllers/AccountsControllerTests.cs
    • Mock SetInitialMasterPasswordCommand and pass to controller constructor

• Refactor OrganizationService.AcceptUserAsync(...) into AcceptOrgUserCommand

  • As the new SetInitialMasterPasswordCommand leveraged the OrganizationService.AcceptUserAsync(...) method, I continued the command refactoring efforts at @justindbaur's request.
  • src/Core/Services/Implementations/OrganizationService.cs
  • src/Core/Services/IOrganizationService.cs
    • Remove all public overloads for AcceptUserAsync(...) and private helper AcceptUserAsync(...)
  • src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IAcceptOrgUserCommand.cs
    • Create interface for new command
    • Replace method overloads with deliberately named methods for improved readability / clarity.
  • src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs
    • Create new command
      • Note: I was careful to persist the changes introduced in [AC-244] Consider a user's email as verified when they accept an organization invitation via the email link #3199 by by @r-tome
  • test/Core.Test/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs
    • Test all AcceptOrgUser scenarios
  • test/Core.Test/Services/OrganizationServiceTests.cs
    • Moved the new AcceptOrgUser test scenarios introduced in [AC-244] Consider a user's email as verified when they accept an organization invitation via the email link #3199 by @r-tome out of the OrganizationServiceTests into the AcceptOrgUserCommandTests.
  • test/Core.Test/Services/UserServiceTests.cs
  • test/Api.Test/Controllers/OrganizationUsersControllerTests.cs
    • Replace IOrganizationService with IAcceptOrgUserCommand

Org Email Invite Token Creation Process Refactor

While building the AcceptOrgUserCommand.AcceptOrgUserByTokenAsync(...) method, due to the existing DataProtectionProvider implementation, I had to instantiate the command's data protector with the same purpose as the OrganizationService:

_dataProtector = dataProtectionProvider.CreateProtector("OrganizationServiceDataProtector").

In order to be able to decrypt the token included in an org user invite email generated in the OrganizationService, this wasn't ideal as the whole point of the command refactor was to move towards standalone commands. So, I decided to refactor the Org User Invite email token creation process to use our Tokenable framework instead.

This change affects the following flows:

• Existing User clicks Org User invite email - logs in and is accepted into the org
  • Policies are retrieved on login by token
  • Accept user logic validates token before accepting user
• New User clicks Org User invite email - registers and is accepted into the org
  • When a self hosted org has disabled open registration, we only allow account registration via email invite (we validate this using the token)
  • Accept user logic validates token before accepting user

Code changes:

• src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenable.cs
  • Created new tokenable for use in org user invite emails
  • Created multiple methods of validation + static methods to reduce code duplication in validating tokens and avoid hitting DB for retrieving an Org User
• src/Core/Auth/Models/Business/Tokenables/OrgUserInviteTokenableFactory.cs
• src/Core/Auth/Models/Business/Tokenables/IOrgUserInviteTokenableFactory.cs
  • The old, manually created org user invite email token honored the _globalSettings.OrganizationInviteExpirationHours as part of its validation process in CoreHelpers.UserInviteTokenIsValid(...).
  • In order to achieve feature parity in the new tokenable implementation, I had to create a factory for creating OrgUserInviteTokenables so that the expiration could be set accordingly.
• test/Core.Test/Auth/Models/Business/Tokenables/OrgUserInviteTokenableTests.cs
  • Add tests for OrgUserInviteTokenable
• src/Core/Services/Implementations/OrganizationService.cs
  • Refactor manual token creation process to use new OrgUserInviteTokenable
• src/Core/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommand.cs
  • Update AcceptOrgUserByTokenAsync to be backwards compatible with old and new tokens (backwards compatibility is required as invites can already have been emailed out when this server change goes live).
  • Added PM-4142 for cleaning up backwards compatibility in future.
• src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs
  • Register new command in services.
• src/Api/Controllers/PoliciesController.cs
  • Update GetByToken(...) to have new token and old token validation
• src/Core/Services/Implementations/UserService.cs
  • Update RegisterUserAsync(...) to have new token and old token validation
• test/Common/Fakes/FakeDataProtectorTokenFactory.cs
  • Create fake implementation of the IDataProtectorTokenFactory which can be used to mock validation of any tokenable
• test/Core.Test/Services/OrganizationServiceTests.cs
  • Update OrganizationServiceTests to reflect use of new OrgUserInviteTokenable

Org Invite working with old token:

PM-3275.-.Org.Invite.Refactor.Backwards.compatibility.proof.with.old.token.mov

Org Invite working with new token:

PM-3275.-.Org.Invite.working.with.new.token.mov

AccountsController PostSetPasswordAsync(...) Refactor to support TDE user setting a password

• src/Api/Auth/Models/Request/Accounts/SetPasswordRequestModel.cs
  • Refactor SetPasswordRequestModel to allow user asymmetric keys to be null; a TDE user will have already generated a user asymmetric key pair, so we should allow initial password setting without requiring new user keys.
• src/Api/Controllers/AccountsController.cs
  • Add new lines
• test/Api.Test/Controllers/AccountsControllerTests.cs
  • Add tests for PostSetPasswordAsync(...) method to guarantee that the change to allow null will not break anything

Misc Refactors

• src/Core/Tokens/ExpiringTokenable.cs
  • Add summary documentation to clarify the intents behind the ExpiringTokenable class props / methods.
• src/Core/Auth/Models/Business/Tokenables/SsoEmail2faSessionTokenable.cs
  • Removed unnecessary new lines

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

[bitwarden-bot]

Checkmarx One – Scan Summary & Details – 709e9dcd-7ea7-4f5d-a3b2-b6e6c87d2f71

No New Or Fixed Issues Found

[JaredSnider-Bitwarden]

Passed QA. Awaiting final stamps for merging!