Account Switching by mpbw2 · Pull Request #1720 · bitwarden/android

mpbw2 · 2022-01-18T04:18:44Z

Type of change

Objective

Added support for using multiple simultaneous accounts in the mobile client.

Code changes

StateService.cs/State.cs: The official replacement for UserService and the new gateway for all storage queries. While this started out as a 1:1 mirror of the version in jslib, it morphed into something slightly different to accommodate mobile environments by only storing the info necessary to decrypt each user's data and performing said decryption upon switching accounts, only keeping the current account data in cache. Practically speaking this means unlimited accounts, though the loading/saving of the account list (state) would likely start to suffer after a fashion (varies by device, maybe 40-60 accounts or so). That can be optimized further but I'm not sure it's necessary.
Account.cs/AccountView.cs: The container for an individual account, but only the data needed to show a list of users and decrypt each user's data when becoming the active account.
AvatarImageSource.cs: The icon used to represent the active account in the Toolbar. The color is generated using a duplicate of our web algorithm. (Currently color is only observed on Android, see known issues in iOS above)
AuthenticationStatus.cs: Enum used for each user's state in account list
StorageOptions.cs/StorageLocation.cs: Used for driving data management within StateService
OrganizationService.cs: A dedicated service for handling organization data which was previously handled by UserService
CustomNavigationRenderer.cs/ExtendedToolbarItem.cs: Used for avatar visual customizations in the iOS Toolbar
cog_environment/cog_settings: Replacement images for cog since it's used in the toolbar now and differs from the Settings cog (in iOS; The same image with a different color is used in Android)
Xamarin.Forms: Updated to 5.0.0.2291
Xamarin.CommunityToolkit: Added 1.3.2 for account switching list drop Shadow
SkiaSharp.Views.Forms: Added 2.80.3 for drawing Avatar image
Everything else: Since StateService is used for all storage requests now, nearly everything was touched in order to point to the new service and uses a nearly identical pattern (via dedicated methods modeled after this new interface instead of generic storage requests that we had before).

Screenshots

Testing requirements

Account management (adding, switching, deleting)
Vault Timeout (locking, logging out)
Autofill (making sure only data from the active account is used)
Full Regression (this feature required a system-wide change to the way we store data)

Before you submit

I have added unit tests where it makes sense to do so (encouraged but not required)
This change requires a documentation update (notify the documentation team)
This change has particular deployment requirements (notify the DevOps team)

…account

vvolkgang · 2022-01-21T16:38:27Z

+
+        public override Func<CancellationToken, Task<Stream>> Stream => GetStreamAsync;
+
+        private Task<Stream> GetStreamAsync(CancellationToken userToken = new CancellationToken())


AFAIK we shouldn't directly instantiate CancellationToken objects, it should instead be provided by an external CancellationTokenSource instead.

Good point, I'll add this to the list to fix (or better yet replace outright per your comment below)

vvolkgang · 2022-01-21T17:09:26Z

+        }
+        protected async Task<AvatarImageSource> GetAvatarImageSourceAsync(bool useCurrentActiveAccount = true)
+        {
+            return new AvatarImageSource(useCurrentActiveAccount ? await _stateService.GetEmailAsync() : null);


Performance wise, wondering if instead we should just have a round frame + label and just change the color / change initials.

Agreed - this was one of the first things I did and tbh I don't remember what pushed me in this direction. Adding it to the list.

vvolkgang · 2022-01-21T17:49:39Z

        private readonly HashSet<string> _migrateToPreferences = new HashSet<string>
        {
-            Constants.EnvironmentUrlsKey,
+            "environmentUrls",


Out of curiosity, what's the reason behind removing that key / what's the difference for the new PreAuthEnvironmentUrlsKey ?

The pre-auth URLs are what the app uses when there's no user/account context, otherwise the per-account URLs are used.

vvolkgang · 2022-01-21T18:37:58Z

 {
    public static class Constants
    {
+        public const int MaxAccounts = 5;


Not arguing with the limit, just curious, do we have technical reasons to block this?

Technical, no (the limit was a directive from early on). Reading/writing the State object could become an issue once a certain number of accounts are added, but I believe that number would have be pretty high.

vvolkgang · 2022-01-21T18:55:13Z

+
+        public ExtendedObservableCollection<AccountView> AccountViews { get; set; }
+
+        public StateService(IStorageService storageService, IStorageService secureStorageService)


Not relevant for this PR but just noticed this, @fedemkr for your IoC work you'll probably want to have a separate interface for secure storage or merging both classes into a single service with GetAsync + GetSecureAsync methods.

vvolkgang · 2022-01-21T19:11:20Z


 namespace Bit.Core.Services
 {
    public class StateService : IStateService


We already talked about naming improvements but now looking a bit better into it, it's mixing a couple of concerns, could be 2-3 services instead.

AppDataService: All of the data access helper methods, building on top of the storage service

AccountsStateService: everything else

AccountsViewManager: optional, separating service code from UI related updates.

* Account Switching (#1720) * Account switching * WIP * wip * wip * updates to send test logic * fixed Send tests * fixes for theme handling on account switching and re-adding existing account * switch fixes * fixes * fixes * cleanup * vault timeout fixes * account list status enhancements * logout fixes and token handling improvements * merge latest (#1727) * remove duplicate dependency * fix for initial login token storage paradox (#1730) * Fix avatar color update toolbar item issue on iOS for account switching (#1735) * Updated account switching menu UI (#1733) * updated account switching menu UI * additional changes * add key suffix to constant * GetFirstLetters method tweaks * Fix crash on account switching when logging out when having more than user at a time (#1740) * single account migration to multi-account on app update (#1741) * Account Switching Tap to dismiss (#1743) * Added tap to dismiss on the Account switching overlay and improved a bit the code * Fix account switching overlay background transparent on the proper place * Fixed transparent background and the shadow on the account switching overlay * Fix iOS top space on Account switching list overlay after modal (#1746) * Fix top space added to Account switching list overlay after closing modal * Fix top space added to Account switching list overlay after closing modal on lock, login and home views just in case we add modals in the future there as well * Usability: dismiss account list on certain events (#1748) * dismiss account list on certain events * use new FireAndForget method for back button logic * Create and use Account Switching overlay control (#1753) * Added Account switching overlay control and its own ViewModel and refactored accordingly * Fix account switching Accounts list binding update * Implemented dismiss account switching overlay when changing tabs and when selecting the same tab. Also updated the deprecated listener on CustomTabbedRenderer on Android (#1755) * Overriden Equals on AvatarImageSource so it doesn't get set multiple times when it's the same image thus producing blinking on tab chaged (#1756) * Usability improvements for logout on vault timeout (#1781) * accountswitching fixes (#1784) * Fix for invalid PIN lock state when switching accounts (#1792) * fix for pin lock flow * named tuple values and updated async * clear send service cache on account switch (#1796) * Global theme and account removal (#1793) * Global theme and account removal * remove redundant call to hide account list overlay * cleanup and additional tweaks * add try/catch to remove account dialog flow Co-authored-by: Federico Maccaroni <fedemkr@gmail.com>

mpbw2 added 18 commits January 16, 2022 22:43

Account switching

ebae258

Merge branch 'master' into feature-accountswitch

d1ece79

WIP

120fb70

wip

6573948

Merge branch 'master' into feature-accountswitch

e3a8d3b

wip

e20ae26

updates to send test logic

f6d4b0a

Merge branch 'master' into feature-accountswitch

6aa96b8

fixed Send tests

1c6a896

fixes for theme handling on account switching and re-adding existing …

ed2bb9b

…account

switch fixes

7e288c6

fixes

a3922f7

fixes

afecc1f

cleanup

59e7969

vault timeout fixes

796cf6d

Merge branch 'master' into feature-accountswitch

d52a1bf

account list status enhancements

e717992

logout fixes and token handling improvements

d41e1a0

mpbw2 changed the base branch from master to accountswitching January 20, 2022 14:59

mpbw2 marked this pull request as ready for review January 20, 2022 15:53

mpbw2 requested a review from a team January 20, 2022 15:53

mpbw2 mentioned this pull request Jan 21, 2022

Account Switching with latest from master #1726

Closed

vvolkgang approved these changes Jan 21, 2022

View reviewed changes

mpbw2 merged commit 59c8c7c into accountswitching Jan 21, 2022

mpbw2 deleted the feature-accountswitch branch January 21, 2022 21:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Account Switching#1720

Account Switching#1720
mpbw2 merged 18 commits intoaccountswitchingfrom
feature-accountswitch

mpbw2 commented Jan 18, 2022 •

edited

Loading

Uh oh!

vvolkgang Jan 21, 2022

Uh oh!

mpbw2 Jan 21, 2022

Uh oh!

vvolkgang Jan 21, 2022

Uh oh!

mpbw2 Jan 21, 2022

Uh oh!

vvolkgang Jan 21, 2022

Uh oh!

mpbw2 Jan 21, 2022

Uh oh!

vvolkgang Jan 21, 2022

Uh oh!

mpbw2 Jan 21, 2022

Uh oh!

vvolkgang Jan 21, 2022

Uh oh!

vvolkgang Jan 21, 2022 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		public override Func<CancellationToken, Task<Stream>> Stream => GetStreamAsync;

		private Task<Stream> GetStreamAsync(CancellationToken userToken = new CancellationToken())


		public ExtendedObservableCollection<AccountView> AccountViews { get; set; }

		public StateService(IStorageService storageService, IStorageService secureStorageService)

Conversation

mpbw2 commented Jan 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Objective

Code changes

Screenshots

Testing requirements

Before you submit

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vvolkgang Jan 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mpbw2 commented Jan 18, 2022 •

edited

Loading

vvolkgang Jan 21, 2022 •

edited

Loading