clients icon indicating copy to clipboard operation
clients copied to clipboard

[AC-2008] [AC-2122] [Pt 1] Transition PolicyService to use StateProvider

Open eliykat opened this issue 1 year ago • 3 comments
trafficstars

Type of change

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

Objective

Start the work to transition PolicyService to use StateProvider.

This PR adds observables and getter methods. The previous logic to determine whether a policy is enforced was quite hard to reason about so that has also been rewritten using observable-first logic, which fits nicer with the stateProvider observable interface.

None of this is wired up so it's not currently exposed. The next PR will wire up the "write" methods and migrate existing data. Thanks to @addisonbeck for inspiration on this approach.

Code changes

PolicyService

  • add activeUserPolicies$ and wire up to StateService
  • add new getter methods, each of which replaces their non-vNext equivalent:
    • get$_vNext
    • getAll$_vNext
    • policyAppliesToActiveUser_vNext$
  • rewrite the enforced policy logic in enforcedPolicyFilter method

StateDefinitions Delete the memory state definition which is not required. Policies are stored on disk unencrypted (i.e. there is nothing to encrypt) and StateProvider handles memory caching for us.

Other Update dependencies, add tests.

Screenshots

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

eliykat avatar Feb 15 '24 02:02 eliykat

@justindbaur I'd appreciate it if you can cast your eye over StateProvider use in PolicyService as well, thanks.

eliykat avatar Feb 15 '24 02:02 eliykat

Codecov Report

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

Project coverage is 24.69%. Comparing base (9968231) to head (de76955).

Files Patch % Lines
...rc/admin-console/services/policy/policy.service.ts 80.64% 4 Missing and 2 partials :warning:
...ommon/src/admin-console/models/data/policy.data.ts 0.00% 2 Missing :warning:
apps/browser/src/background/main.background.ts 0.00% 1 Missing :warning:
apps/browser/src/popup/services/services.module.ts 0.00% 1 Missing :warning:
apps/cli/src/bw.ts 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7959      +/-   ##
==========================================
+ Coverage   24.66%   24.69%   +0.02%     
==========================================
  Files        2225     2225              
  Lines       65379    65407      +28     
  Branches    12352    12356       +4     
==========================================
+ Hits        16125    16149      +24     
- Misses      47930    47932       +2     
- Partials     1324     1326       +2     

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

codecov[bot] avatar Feb 15 '24 02:02 codecov[bot]

Logo Checkmarx One – Scan Summary & Detailsb170f08d-489a-4982-96d7-963fbd7cb625

No New Or Fixed Issues Found

bitwarden-bot avatar Feb 15 '24 03:02 bitwarden-bot