[PR #2366] [MERGED] [PS-2280] Retain app settings on logout #3522

Closed
opened 2025-11-26 23:33:53 -06:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/bitwarden/android/pull/2366
Author: @mpbw2
Created: 2/13/2023
Status: Merged
Merged: 2/15/2023
Merged by: @mpbw2

Base: masterHead: bugfix/retain-settings-logout


📝 Commits (3)

  • 8420666 [PS-2280] Retain app settings on logout
  • c6e58e6 adjustments
  • 7a63236 Merge branch 'master' into bugfix/retain-settings-logout

📊 Changes

8 files changed (+148 additions, -178 deletions)

View changed files

📝 src/Android/Services/DeviceActionService.cs (+0 -5)
📝 src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs (+0 -5)
📝 src/Core/Abstractions/IStateService.cs (+6 -6)
📝 src/Core/Constants.cs (+7 -3)
📝 src/Core/Models/Domain/Account.cs (+6 -2)
📝 src/Core/Services/StateMigrationService.cs (+87 -3)
📝 src/Core/Services/StateService.cs (+42 -135)
📝 src/Core/Utilities/CoreHelpers.cs (+0 -19)

📄 Description

Type of change

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

Objective

These changes allow the app to retain account-specific settings after a hard logout (previous would only retain on soft logout). In order to accomplish this without re-architecting the way soft logout works with multiple users, some stored settings (VaultTimeout, VaultTimeoutAction, and ScreenCaptureAllowed) had to be relocated from the state object to normal local storage with an account-specific pref name (follows existing pattern). This requires a new state migration, which resulted in a conflict with a previous migration with 3 values that were originally account-specific (DisableFavicon, Theme, and AutoDarkTheme but used trickery to apply globally to match other clients) requiring migrating those to true global values (remove the trickery).

Code changes

  • StateService.cs:
    • Update getter/setter for VaultTimeout, VaultTimeoutAction, and ScreenCaptureAllowed to support account-specific storage key
    • Update getter/setter for DisableFavicon, Theme, and AutoDarkTheme to remove account-specific storage key
    • Remove account-to-global trickery method
    • Remove deletion of app settings in RemoveAccountAsync
    • Remove DisableFavicon, Theme, and AutoDarkTheme getting/setting trickery in ScaffoldNewAccountAsync
    • Remove setting defaults for VaultTimeout, VaultTimeoutAction, and ScreenCaptureAllowed (using established pattern of getter fallbacks since these are no longer in the state object)
    • Remove purging of any existing data when logging into account that was previously logged in (sensitive data is already removed on logout and we want the settings to remain)
  • StateMigrationService.cs: Added new 3to4 migration block, details in Objective section above (realized I should not be referencing Constants for keys in 2to3 migration as those change over time and need to be self-contained in the service - will fix this in a subsequent PR since it adds a lot of noise)
  • CoreHelpers.cs: Removed ForceScreenCaptureEnabled as it made debugging the toggle impossible, now simply defaults to allowing screenshots in debug builds but still allows toggling (returns CoreHelpers.InDebugMode() as fallback default in StateService.GetScreenCaptureAllowedAsync(..)
  • Account.cs: Marked VaultTimeout, VaultTimeoutAction, and ScreenCaptureAllowed as [Obsolete] to allow older migrations to function, will eventually remove
  • Constants.cs: Changed VaultTimeout, VaultTimeoutAction, and ScreenCaptureAllowed to account specific keys, and DisableFavicon, Theme, and AutoDarkTheme to global keys

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • 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

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/bitwarden/android/pull/2366 **Author:** [@mpbw2](https://github.com/mpbw2) **Created:** 2/13/2023 **Status:** ✅ Merged **Merged:** 2/15/2023 **Merged by:** [@mpbw2](https://github.com/mpbw2) **Base:** `master` ← **Head:** `bugfix/retain-settings-logout` --- ### 📝 Commits (3) - [`8420666`](https://github.com/bitwarden/android/commit/8420666f3fbc0387483a14c079c901fabf1593e1) [PS-2280] Retain app settings on logout - [`c6e58e6`](https://github.com/bitwarden/android/commit/c6e58e62ab01deb2c3d4653a0b34a6f55a51b0ad) adjustments - [`7a63236`](https://github.com/bitwarden/android/commit/7a63236c122381c28545630ba1c6b2812e903a88) Merge branch 'master' into bugfix/retain-settings-logout ### 📊 Changes **8 files changed** (+148 additions, -178 deletions) <details> <summary>View changed files</summary> 📝 `src/Android/Services/DeviceActionService.cs` (+0 -5) 📝 `src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs` (+0 -5) 📝 `src/Core/Abstractions/IStateService.cs` (+6 -6) 📝 `src/Core/Constants.cs` (+7 -3) 📝 `src/Core/Models/Domain/Account.cs` (+6 -2) 📝 `src/Core/Services/StateMigrationService.cs` (+87 -3) 📝 `src/Core/Services/StateService.cs` (+42 -135) 📝 `src/Core/Utilities/CoreHelpers.cs` (+0 -19) </details> ### 📄 Description ## Type of change - [X] Bug fix - [ ] New feature development - [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ## Objective <!--Describe what the purpose of this PR is. For example: what bug you're fixing or what new feature you're adding--> These changes allow the app to retain account-specific settings after a hard logout (previous would only retain on soft logout). In order to accomplish this without re-architecting the way soft logout works with multiple users, some stored settings (`VaultTimeout`, `VaultTimeoutAction`, and `ScreenCaptureAllowed`) had to be relocated from the state object to normal local storage with an account-specific pref name (follows existing pattern). This requires a new state migration, which resulted in a conflict with a previous migration with 3 values that were originally account-specific (`DisableFavicon`, `Theme`, and `AutoDarkTheme` but used trickery to apply globally to match other clients) requiring migrating those to true global values (remove the trickery). ## Code changes <!--Explain the changes you've made to each file or major component. This should help the reviewer understand your changes--> <!--Also refer to any related changes or PRs in other repositories--> * **StateService.cs:** * Update getter/setter for `VaultTimeout`, `VaultTimeoutAction`, and `ScreenCaptureAllowed` to support account-specific storage key * Update getter/setter for `DisableFavicon`, `Theme`, and `AutoDarkTheme` to remove account-specific storage key * Remove account-to-global trickery method * Remove deletion of app settings in `RemoveAccountAsync` * Remove `DisableFavicon`, `Theme`, and `AutoDarkTheme` getting/setting trickery in `ScaffoldNewAccountAsync` * Remove setting defaults for `VaultTimeout`, `VaultTimeoutAction`, and `ScreenCaptureAllowed` (using established pattern of getter fallbacks since these are no longer in the state object) * Remove purging of any existing data when logging into account that was previously logged in (sensitive data is already removed on logout and we want the settings to remain) * **StateMigrationService.cs:** Added new 3to4 migration block, details in Objective section above (realized I should not be referencing `Constants` for keys in 2to3 migration as those change over time and need to be self-contained in the service - will fix this in a subsequent PR since it adds a lot of noise) * **CoreHelpers.cs:** Removed `ForceScreenCaptureEnabled` as it made debugging the toggle impossible, now simply defaults to allowing screenshots in debug builds but still allows toggling (returns `CoreHelpers.InDebugMode()` as fallback default in `StateService.GetScreenCaptureAllowedAsync(..)` * **Account.cs:** Marked `VaultTimeout`, `VaultTimeoutAction`, and `ScreenCaptureAllowed` as `[Obsolete]` to allow older migrations to function, will eventually remove * **Constants.cs:** Changed `VaultTimeout`, `VaultTimeoutAction`, and `ScreenCaptureAllowed` to account specific keys, and `DisableFavicon`, `Theme`, and `AutoDarkTheme` to global keys ## Before you submit - Please check for formatting errors (`dotnet format --verify-no-changes`) (required) - 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 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2025-11-26 23:33:53 -06:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/android#3522