clients icon indicating copy to clipboard operation
clients copied to clipboard

[PM-5614] introduce SecretState wrapper

Open audreyality opened this issue 1 year ago • 6 comments

Type of change

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

Objective

This PR introduces a state storage wrapper that encapsulates the encryption and decryption of stored state. The wrapper partitions an object into secret and disclosed fields. It encrypts the secret fields before writing the encrypted secret and disclosed fields to a backing store.

This provider will be leveraged to store secrets connected to the username generator's forwarder, password history, and reference events. It could be extended to support send storage, but that decision is out of scope of this PR.

Code changes

Platform:

  • libs/common/src/platform/state/index.ts: exposed the StateUpdateOptions from the state module so that SecretState can mimic state update calls.

State:

  • libs/common/src/tools/generator/state/secret-classifier.ts: Introduces the secret classifier. This configures SecretState by showing which properties of a type should be encrypted and which should be stored in plaintext.
  • libs/common/src/tools/generator/state/secret-state.ts: Introduces the secret state. This transparently encrypts and decrypts data on its way to backing storage.
  • libs/common/src/tools/generator/state/user-encryptor.abstraction.ts: Allows a user-based cryptographic scheme to power a SecretState.
  • libs/common/src/tools/generator/state/user-key-encryptor-options.ts: Configuration for the user key encryptor.
  • libs/common/src/tools/generator/state/user-key-encryptor.ts: Introduces the UserKeyEncryptor, a data protection strategy that smashes all secrets into an encrypted string by jsonifying them.

Logic from the following files was merged into the UserKeyEncryptor:

  • libs/common/src/tools/generator/username/options/utilities.spec.ts
  • libs/common/src/tools/generator/username/options/utilities.ts

Unit tests:

  • libs/common/src/tools/generator/state/secret-classifier.spec.ts
  • libs/common/src/tools/generator/state/secret-state.spec.ts
  • libs/common/src/tools/generator/state/user-key-encryptor.spec.ts

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

audreyality avatar Feb 05 '24 22:02 audreyality

Logo Checkmarx One – Scan Summary & Detailse2292619-d02f-4c44-b424-3b4aa41e0a98

No New Or Fixed Issues Found

bitwarden-bot avatar Feb 06 '24 18:02 bitwarden-bot

Codecov Report

Attention: Patch coverage is 99.23664% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 24.81%. Comparing base (7674a3f) to head (11307f5).

Files Patch % Lines
...s/common/src/tools/generator/state/secret-state.ts 97.56% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7823      +/-   ##
==========================================
+ Coverage   24.70%   24.81%   +0.10%     
==========================================
  Files        2224     2230       +6     
  Lines       65370    65463      +93     
  Branches    12356    12358       +2     
==========================================
+ Hits        16149    16243      +94     
+ Misses      47895    47894       -1     
  Partials     1326     1326              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 06 '24 21:02 codecov[bot]

@eliykat - Could you please take a look at the UserKeyEncryptor to make sure I didn't misuse anything?

audreyality avatar Feb 13 '24 20:02 audreyality

@justindbaur, @MGibson1 - Could y'all take a look at the SecretState to ensure that it's not breaking anything obvious with the state provider services?

Also, major thank you to both of you for writing the Classifier's type magic for me.

audreyality avatar Feb 13 '24 20:02 audreyality

If there's a good person to review the UserKeyEncryptor, please add them to the review. @eliykat indicated he's not the SME for that work.

audreyality avatar Feb 14 '24 21:02 audreyality

@MGibson1 - I think I've addressed everything.

audreyality avatar Feb 23 '24 00:02 audreyality