clients
clients copied to clipboard
Auth/PM-5501 - VaultTimeoutSettingsService State Provider Migration
Type of change
- [ ] Bug fix
- [X] New feature development
- [X] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
Resolve PM-5501 by migrating the vault timeout settings out of the StateService
into StateProvider
with the VaultTimeoutSettingsService
owning that state.
Code changes
Core VaultTimeoutSettingsService
changes
-
libs/common/src/platform/state/state-definitions.ts: Add new
VAULT_TIMEOUT_SETTINGS_DISK_LOCAL
state definition -
libs/common/src/services/vault-timeout/vault-timeout-settings.state.ts: Add
UserKeyDefinitions
for vault timeout action and vault timeout. -
libs/common/src/services/vault-timeout/vault-timeout-settings.state.spec.ts: Test deserialization of
UserKeyDefinitions
-
libs/common/src/services/vault-timeout/vault-timeout-settings.service.ts:
- Wire up
StateProvider
while significantly refactoring the retrieval logic for both the vault timeout action and vault timeout to be truly reactive and more single responsibility. - Also, moved towards a design which requires a user id for all sets and gets to be safer.
- Wire up
- libs/common/src/services/vault-timeout/vault-timeout-settings.service.spec.ts: Update tests
- libs/common/src/abstractions/vault-timeout/vault-timeout-settings.service.ts: Update abstraction
Dependency Updates
-
apps/cli/src/bw.ts: Move
vaultTimeoutSettingsService
instantiation + deps above newApiService
usage. AddVaultTimeoutSettingsService
toLoginStrategyService
-
apps/browser/src/background/main.background.ts: Move
vaultTimeoutSettingsService
instantiation + deps above newApiService
usage. AddVaultTimeoutSettingsService
toLoginStrategyService
andIdleBackground
- apps/browser/src/background/service-factories/vault-timeout-settings-service.factory.ts: Add new deps
-
apps/browser/src/auth/background/service-factories/login-strategy-service.factory.ts: Add
VaultTimeoutSettingsService
toLoginStrategyService
-
apps/browser/src/platform/background/service-factories/api-service.factory.ts: Add
vaultTimeoutSettingsServiceFactory
toApiServiceFactory
by replacingStateServiceFactory
-
apps/cli/src/platform/services/node-api.service.ts: Replace
StateService
dep withVaultTimeoutSettingsService
-
libs/angular/src/services/jslib-services.module.ts: Update
VaultTimeoutSettingsService
,ApiService
, andLoginStrategyService
deps
StateService
cleanup
- libs/common/src/platform/abstractions/state.service.ts: Remove vault timeout data setters and getters
- libs/common/src/platform/services/state.service.ts: Remove vault timeout data setters and getters
- libs/common/src/platform/models/domain/account.ts: Remove vault timeout data from account
- libs/common/src/platform/models/domain/global-state.ts: Remove vault timeout data from global state
User Data Migration
-
libs/common/src/state-migrations/migrations/59-migrate-vault-timeout-settings-svc-to-state-provider.ts: Migrate user vault timeout data into
StateProvider
. Note: we do not persist global vault timeout data as it was already almost fully deprecated. - libs/common/src/state-migrations/migrations/59-migrate-vault-timeout-settings-svc-to-state-provider.spec.ts: Test migration
- libs/common/src/state-migrations/migrate.ts: Register migration
Reference Updates (replacing StateService
methods with calls to VaultTimeoutSettingsService
)
- apps/browser/src/background/idle.background.ts
- apps/browser/src/popup/settings/settings.component.ts
- apps/desktop/src/app/app.component.ts
- apps/web/src/app/settings/preferences.component.ts
- libs/auth/src/common/login-strategies/login.strategy.ts: (requires updates to all login strategies)
- libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts
- libs/auth/src/common/login-strategies/login.strategy.spec.ts
- libs/auth/src/common/login-strategies/password-login.strategy.spec.ts
- libs/auth/src/common/login-strategies/password-login.strategy.ts
- libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts
- libs/auth/src/common/login-strategies/sso-login.strategy.ts
- libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts
- libs/auth/src/common/login-strategies/user-api-login.strategy.ts
- libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts
- libs/auth/src/common/login-strategies/webauthn-login.strategy.ts
- libs/auth/src/common/services/login-strategies/login-strategy.service.spec.ts
- libs/auth/src/common/services/login-strategies/login-strategy.service.ts
- libs/common/src/platform/services/crypto.service.spec.ts
- libs/common/src/platform/services/crypto.service.ts
- libs/common/src/platform/services/system.service.ts
- libs/common/src/services/api.service.ts
- libs/common/src/services/vault-timeout/vault-timeout.service.ts
- libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts
Screenshots
Note: I tested both vault timeout actions with 1 min timers on all 3 clients which support vault timeout settings (web, desktop, and browser), but didn't record as it would be 1 min of sitting per client per action.
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
Checkmarx One – Scan Summary & Details – 8fa92740-348f-40f8-aaab-4c177e489eff
No New Or Fixed Issues Found
Codecov Report
Attention: Patch coverage is 59.64912%
with 92 lines
in your changes are missing coverage. Please review.
Project coverage is 27.86%. Comparing base (
8e97c1c
) to head (fcae4f1
). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #8604 +/- ##
==========================================
+ Coverage 27.76% 27.86% +0.09%
==========================================
Files 2421 2424 +3
Lines 70097 70208 +111
Branches 13059 13085 +26
==========================================
+ Hits 19462 19563 +101
- Misses 49123 49126 +3
- Partials 1512 1519 +7
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Additional context for reviewers as to why we had to make more changes:
@justindbaur discovered that we had client specific default values on account initialization for the vault timeout. I had to rework the vault timeout retrieval to respect the client specific default. While doing so and while testing the changes, I realized that I needed to rework the default logic for the vault timeout action to ensure the previous default of lock was respected.
I tested the fresh load of a new account on web, desktop, and browser to ensure the new defaults were respected, and they were.
Further notes for reviewers: we had more work to do because we discovered that the CLI wasn't setting a vault timeout so we had to enable migrations to have access to the client type they were executing on so we could successfully migrate the CLI's undefined vault timeouts to the new "never"
value. I tested logging in on main
in the CLI, and swapping over to this branch, and I ensured that the migration was seamless with the user's vault still being unlocked properly.