From d41e1a0bc3c9805f79ef743b87ef140c98ca14ff Mon Sep 17 00:00:00 2001 From: Matt Portune Date: Wed, 19 Jan 2022 16:15:09 -0500 Subject: [PATCH] logout fixes and token handling improvements --- src/App/App.xaml.cs | 16 +++- src/App/Pages/Accounts/HomePage.xaml.cs | 4 +- src/App/Pages/Accounts/LockPage.xaml.cs | 2 +- src/App/Pages/Accounts/LoginPage.xaml.cs | 2 +- src/App/Pages/BaseContentPage.cs | 7 +- .../Vault/GroupingsPage/GroupingsPage.xaml.cs | 2 +- .../GroupingsPage/GroupingsPageViewModel.cs | 13 +-- src/Core/Abstractions/IStateService.cs | 6 +- src/Core/Abstractions/ITokenService.cs | 2 +- src/Core/Models/Domain/StorageOptions.cs | 1 + src/Core/Services/StateService.cs | 32 ++++--- src/Core/Services/TokenService.cs | 83 ++++++------------- src/iOS/AppDelegate.cs | 12 ++- 13 files changed, 75 insertions(+), 107 deletions(-) diff --git a/src/App/App.xaml.cs b/src/App/App.xaml.cs index 61e03b630..9ed7f05ea 100644 --- a/src/App/App.xaml.cs +++ b/src/App/App.xaml.cs @@ -290,13 +290,23 @@ namespace Bit.App private async Task SetMainPageAsync() { - await _stateService.RefreshAccountViewsAsync(); var authed = await _stateService.IsAuthenticatedAsync(); if (authed) { - if (await _vaultTimeoutService.IsLockedAsync()) + var isLocked = await _vaultTimeoutService.IsLockedAsync(); + var shouldTimeout = await _vaultTimeoutService.ShouldTimeoutAsync(); + var vaultTimeoutAction = await _stateService.GetVaultTimeoutActionAsync(); + if (isLocked || shouldTimeout) { - Current.MainPage = new NavigationPage(new LockPage(Options)); + if (vaultTimeoutAction == "logOut") + { + var email = await _stateService.GetEmailAsync(); + Current.MainPage = new NavigationPage(new LoginPage(email, Options)); + } + else + { + Current.MainPage = new NavigationPage(new LockPage(Options)); + } } else if (Options.FromAutofillFramework && Options.SaveType.HasValue) { diff --git a/src/App/Pages/Accounts/HomePage.xaml.cs b/src/App/Pages/Accounts/HomePage.xaml.cs index 29ff8021a..ff0d83d6f 100644 --- a/src/App/Pages/Accounts/HomePage.xaml.cs +++ b/src/App/Pages/Accounts/HomePage.xaml.cs @@ -45,7 +45,7 @@ namespace Bit.App.Pages _mainContent.Content = _mainLayout; if (await ShowAccountSwitcherAsync()) { - _vm.AvatarImageSource = await GetAvatarImageSourceAsync(false); + _vm.AvatarImageSource = await GetAvatarImageSourceAsync(); } else { @@ -146,7 +146,7 @@ namespace Bit.App.Pages } else { - await RefreshAccountViewsAsync(_accountListView); + await RefreshAccountViewsAsync(_accountListView, false); await ShowAccountListAsync(true, _accountListContainer, _accountListOverlay); } } diff --git a/src/App/Pages/Accounts/LockPage.xaml.cs b/src/App/Pages/Accounts/LockPage.xaml.cs index 4cf99b537..854d5110b 100644 --- a/src/App/Pages/Accounts/LockPage.xaml.cs +++ b/src/App/Pages/Accounts/LockPage.xaml.cs @@ -163,7 +163,7 @@ namespace Bit.App.Pages } else { - await RefreshAccountViewsAsync(_accountListView); + await RefreshAccountViewsAsync(_accountListView, false); await ShowAccountListAsync(true, _accountListContainer, _accountListOverlay); } } diff --git a/src/App/Pages/Accounts/LoginPage.xaml.cs b/src/App/Pages/Accounts/LoginPage.xaml.cs index 1dc4affaa..ed65d869b 100644 --- a/src/App/Pages/Accounts/LoginPage.xaml.cs +++ b/src/App/Pages/Accounts/LoginPage.xaml.cs @@ -145,7 +145,7 @@ namespace Bit.App.Pages } else { - await RefreshAccountViewsAsync(_accountListView); + await RefreshAccountViewsAsync(_accountListView, false); await ShowAccountListAsync(true, _accountListContainer, _accountListOverlay); } } diff --git a/src/App/Pages/BaseContentPage.cs b/src/App/Pages/BaseContentPage.cs index 3a34150dc..45614a0b3 100644 --- a/src/App/Pages/BaseContentPage.cs +++ b/src/App/Pages/BaseContentPage.cs @@ -110,13 +110,12 @@ namespace Bit.App.Pages protected async Task ShowAccountSwitcherAsync() { - return await _stateService.HasMultipleAccountsAsync() - || await _stateService.IsAuthenticatedAsync(); + return await _stateService.HasMultipleAccountsAsync(); } - protected async Task RefreshAccountViewsAsync(Xamarin.Forms.ListView accountListView) + protected async Task RefreshAccountViewsAsync(Xamarin.Forms.ListView accountListView, bool allowAddAccountRow) { - await _stateService.RefreshAccountViewsAsync(); + await _stateService.RefreshAccountViewsAsync(allowAddAccountRow); // Property change trigger on account listview is yielding inconsistent results, using a hammer instead accountListView.ItemsSource = null; accountListView.ItemsSource = _stateService.AccountViews; diff --git a/src/App/Pages/Vault/GroupingsPage/GroupingsPage.xaml.cs b/src/App/Pages/Vault/GroupingsPage/GroupingsPage.xaml.cs index 2cde2e699..39de96ee8 100644 --- a/src/App/Pages/Vault/GroupingsPage/GroupingsPage.xaml.cs +++ b/src/App/Pages/Vault/GroupingsPage/GroupingsPage.xaml.cs @@ -302,7 +302,7 @@ namespace Bit.App.Pages } else { - await RefreshAccountViewsAsync(_accountListView); + await RefreshAccountViewsAsync(_accountListView, true); await ShowAccountListAsync(true, _accountListContainer, _accountListOverlay, _fab); } } diff --git a/src/App/Pages/Vault/GroupingsPage/GroupingsPageViewModel.cs b/src/App/Pages/Vault/GroupingsPage/GroupingsPageViewModel.cs index ad1e3d238..4dbbbbc5d 100644 --- a/src/App/Pages/Vault/GroupingsPage/GroupingsPageViewModel.cs +++ b/src/App/Pages/Vault/GroupingsPage/GroupingsPageViewModel.cs @@ -136,18 +136,7 @@ namespace Bit.App.Pages } public ExtendedObservableCollection AccountViews { - get - { - if (_stateService.AccountViews.Count < Constants.MaxAccounts) - { - // create a separate collection that includes the "add new" row - var accountViews = new ExtendedObservableCollection(); - accountViews.AddRange(_stateService.AccountViews); - accountViews.Add(new AccountView()); - return accountViews; - } - return _stateService.AccountViews; - } + get => _stateService.AccountViews; } public ExtendedObservableCollection GroupedItems { get; set; } diff --git a/src/Core/Abstractions/IStateService.cs b/src/Core/Abstractions/IStateService.cs index c3c74f650..184f4660e 100644 --- a/src/Core/Abstractions/IStateService.cs +++ b/src/Core/Abstractions/IStateService.cs @@ -18,7 +18,7 @@ namespace Bit.Core.Abstractions Task SetActiveUserAsync(string userId); Task IsAuthenticatedAsync(string userId = null); Task HasMultipleAccountsAsync(); - Task RefreshAccountViewsAsync(); + Task RefreshAccountViewsAsync(bool allowAddAccountRow); Task AddAccountAsync(Account account); Task ClearAsync(string userId); Task GetPreAuthEnvironmentUrlsAsync(); @@ -154,9 +154,9 @@ namespace Bit.Core.Abstractions Task> GetSettingsAsync(string userId = null); Task SetSettingsAsync(Dictionary value, string userId = null); Task GetAccessTokenAsync(string userId = null); - Task SetAccessTokenAsync(string value, string userId = null); + Task SetAccessTokenAsync(string value, bool skipTokenStorage, string userId = null); Task GetRefreshTokenAsync(string userId = null); - Task SetRefreshTokenAsync(string value, string userId = null); + Task SetRefreshTokenAsync(string value, bool skipTokenStorage, string userId = null); Task GetTwoFactorTokenAsync(string email = null); Task SetTwoFactorTokenAsync(string value, string email = null); } diff --git a/src/Core/Abstractions/ITokenService.cs b/src/Core/Abstractions/ITokenService.cs index 34febcc3a..91ba02200 100644 --- a/src/Core/Abstractions/ITokenService.cs +++ b/src/Core/Abstractions/ITokenService.cs @@ -23,7 +23,7 @@ namespace Bit.Core.Abstractions Task GetTwoFactorTokenAsync(string email); string GetUserId(); Task SetRefreshTokenAsync(string refreshToken); - Task SetTokenAsync(string token); + Task SetAccessTokenAsync(string token); Task SetTokensAsync(string accessToken, string refreshToken); Task SetTwoFactorTokenAsync(string token, string email); bool TokenNeedsRefresh(int minutes = 5); diff --git a/src/Core/Models/Domain/StorageOptions.cs b/src/Core/Models/Domain/StorageOptions.cs index 709f89b1b..0d9061876 100644 --- a/src/Core/Models/Domain/StorageOptions.cs +++ b/src/Core/Models/Domain/StorageOptions.cs @@ -8,5 +8,6 @@ namespace Bit.Core.Models.Domain public bool? UseSecureStorage { get; set; } public string UserId { get; set; } public string Email { get; set; } + public bool? SkipTokenStorage { get; set; } } } diff --git a/src/Core/Services/StateService.cs b/src/Core/Services/StateService.cs index 81e214f8d..f7783f0fc 100644 --- a/src/Core/Services/StateService.cs +++ b/src/Core/Services/StateService.cs @@ -72,10 +72,6 @@ namespace Bit.Core.Services public async Task IsAuthenticatedAsync(string userId = null) { - if (userId != null) - { - return await GetAccessTokenAsync(userId) != null && (_state?.Accounts?.ContainsKey(userId) ?? false); - } return await GetAccessTokenAsync(userId) != null; } @@ -85,7 +81,7 @@ namespace Bit.Core.Services return _state.Accounts?.Count > 1; } - public async Task RefreshAccountViewsAsync() + public async Task RefreshAccountViewsAsync(bool allowAddAccountRow) { await CheckStateAsync(); @@ -124,13 +120,17 @@ namespace Bit.Core.Services } AccountViews.Add(accountView); } + if (allowAddAccountRow && AccountViews.Count < Constants.MaxAccounts) + { + AccountViews.Add(new AccountView()); + } } public async Task AddAccountAsync(Account account) { await ScaffoldNewAccountAsync(account); await SetActiveUserAsync(account.Profile.UserId); - await RefreshAccountViewsAsync(); + await RefreshAccountViewsAsync(true); } public async Task ClearAsync(string userId) @@ -1224,9 +1224,10 @@ namespace Bit.Core.Services ))?.Tokens?.AccessToken; } - public async Task SetAccessTokenAsync(string value, string userId = null) + public async Task SetAccessTokenAsync(string value, bool skipTokenStorage, string userId = null) { - var reconciledOptions = ReconcileOptions(new StorageOptions { UserId = userId }, + var reconciledOptions = ReconcileOptions( + new StorageOptions { UserId = userId, SkipTokenStorage = skipTokenStorage }, await GetDefaultStorageOptionsAsync()); var account = await GetAccountAsync(reconciledOptions); account.Tokens.AccessToken = value; @@ -1240,9 +1241,10 @@ namespace Bit.Core.Services ))?.Tokens?.RefreshToken; } - public async Task SetRefreshTokenAsync(string value, string userId = null) + public async Task SetRefreshTokenAsync(string value, bool skipTokenStorage, string userId = null) { - var reconciledOptions = ReconcileOptions(new StorageOptions { UserId = userId }, + var reconciledOptions = ReconcileOptions( + new StorageOptions { UserId = userId, SkipTokenStorage = skipTokenStorage }, await GetDefaultStorageOptionsAsync()); var account = await GetAccountAsync(reconciledOptions); account.Tokens.RefreshToken = value; @@ -1356,7 +1358,7 @@ namespace Bit.Core.Services state.Accounts[account.Profile.UserId] = new Account(account); // If we have a vault timeout and the action is log out, don't store token - if (await SkipTokenStorageAsync()) + if (options?.SkipTokenStorage.GetValueOrDefault() ?? false) { state.Accounts[account.Profile.UserId].Tokens.AccessToken = null; state.Accounts[account.Profile.UserId].Tokens.RefreshToken = null; @@ -1366,13 +1368,6 @@ namespace Bit.Core.Services } } - private async Task SkipTokenStorageAsync() - { - var timeout = await GetVaultTimeoutAsync(); - var action = await GetVaultTimeoutActionAsync(); - return timeout.HasValue && action == "logOut"; - } - private async Task RemoveAccountAsync(string userId) { if (userId == null) @@ -1468,6 +1463,7 @@ namespace Bit.Core.Services requestedOptions.UseSecureStorage = requestedOptions.UseSecureStorage ?? defaultOptions.UseSecureStorage; requestedOptions.UserId = requestedOptions.UserId ?? defaultOptions.UserId; requestedOptions.Email = requestedOptions.Email ?? defaultOptions.Email; + requestedOptions.SkipTokenStorage = requestedOptions.SkipTokenStorage ?? defaultOptions.SkipTokenStorage; return requestedOptions; } diff --git a/src/Core/Services/TokenService.cs b/src/Core/Services/TokenService.cs index 897ea2bb5..daafab017 100644 --- a/src/Core/Services/TokenService.cs +++ b/src/Core/Services/TokenService.cs @@ -12,9 +12,8 @@ namespace Bit.Core.Services { private readonly IStateService _stateService; - private string _token; - private JObject _decodedToken; - private string _refreshToken; + private string _accessTokenForDecoding; + private JObject _decodedAccessToken; public TokenService(IStateService stateService) { @@ -24,70 +23,39 @@ namespace Bit.Core.Services public async Task SetTokensAsync(string accessToken, string refreshToken) { await Task.WhenAll( - SetTokenAsync(accessToken), + SetAccessTokenAsync(accessToken), SetRefreshTokenAsync(refreshToken)); } - public async Task SetTokenAsync(string token) + public async Task SetAccessTokenAsync(string accessToken) { - _token = token; - _decodedToken = null; - - if (await SkipTokenStorage()) - { - // If we have a vault timeout and the action is log out, don't store token - return; - } - - // await _stateService.SetAccessTokenAsync(token); + _accessTokenForDecoding = accessToken; + _decodedAccessToken = null; + await _stateService.SetAccessTokenAsync(accessToken, await SkipTokenStorage()); } public async Task GetTokenAsync() { - if (_token != null) - { - return _token; - } - _token = await _stateService.GetAccessTokenAsync(); - return _token; + _accessTokenForDecoding = await _stateService.GetAccessTokenAsync(); + return _accessTokenForDecoding; } public async Task SetRefreshTokenAsync(string refreshToken) { - _refreshToken = refreshToken; - - if (await SkipTokenStorage()) - { - // If we have a vault timeout and the action is log out, don't store token - return; - } - - // await _stateService.SetRefreshTokenAsync(refreshToken); + await _stateService.SetRefreshTokenAsync(refreshToken, await SkipTokenStorage()); } public async Task GetRefreshTokenAsync() { - if (_refreshToken != null) - { - return _refreshToken; - } - _refreshToken = await _stateService.GetRefreshTokenAsync(); - return _refreshToken; + return await _stateService.GetRefreshTokenAsync(); } public async Task ToggleTokensAsync() { + // load and re-save tokens to reflect latest value of SkipTokenStorage() var token = await GetTokenAsync(); var refreshToken = await GetRefreshTokenAsync(); - if (await SkipTokenStorage()) - { - await ClearTokenAsync(); - _token = token; - _refreshToken = refreshToken; - return; - } - - await SetTokenAsync(token); + await SetAccessTokenAsync(token); await SetRefreshTokenAsync(refreshToken); } @@ -110,28 +78,27 @@ namespace Bit.Core.Services { ClearCache(); await Task.WhenAll( - _stateService.SetAccessTokenAsync(null, userId), - _stateService.SetRefreshTokenAsync(null, userId)); + _stateService.SetAccessTokenAsync(null, false, userId), + _stateService.SetRefreshTokenAsync(null, false, userId)); } public void ClearCache() { - _token = null; - _decodedToken = null; - _refreshToken = null; + _accessTokenForDecoding = null; + _decodedAccessToken = null; } public JObject DecodeToken() { - if (_decodedToken != null) + if (_decodedAccessToken != null) { - return _decodedToken; + return _decodedAccessToken; } - if (_token == null) + if (_accessTokenForDecoding == null) { - throw new InvalidOperationException("Token not found."); + throw new InvalidOperationException("Access token not found."); } - var parts = _token.Split('.'); + var parts = _accessTokenForDecoding.Split('.'); if (parts.Length != 3) { throw new InvalidOperationException("JWT must have 3 parts."); @@ -141,8 +108,8 @@ namespace Bit.Core.Services { throw new InvalidOperationException("Cannot decode the token."); } - _decodedToken = JObject.Parse(Encoding.UTF8.GetString(decodedBytes)); - return _decodedToken; + _decodedAccessToken = JObject.Parse(Encoding.UTF8.GetString(decodedBytes)); + return _decodedAccessToken; } public DateTime? GetTokenExpirationDate() @@ -234,7 +201,7 @@ namespace Bit.Core.Services public async Task GetIsExternal() { - if (_token == null) + if (_accessTokenForDecoding == null) { await GetTokenAsync(); } diff --git a/src/iOS/AppDelegate.cs b/src/iOS/AppDelegate.cs index 433fe9c2f..c1857eb7f 100644 --- a/src/iOS/AppDelegate.cs +++ b/src/iOS/AppDelegate.cs @@ -141,11 +141,15 @@ namespace Bit.iOS await ASHelpers.ReplaceAllIdentities(); } } - else if (message.Command == "loggedOut") + else if (message.Command == "logout") { if (_deviceActionService.SystemMajorVersion() >= 12) { - // TODO make account-specific + var extras = message.Data as Tuple; + var userId = extras?.Item1; + var userInitiated = extras?.Item2; + var expired = extras?.Item3; + // TODO make specific to userId // await ASCredentialIdentityStore.SharedStore?.RemoveAllCredentialIdentitiesAsync(); } } @@ -159,7 +163,9 @@ namespace Bit.iOS var timeoutAction = await _stateService.GetVaultTimeoutActionAsync(); if (timeoutAction == "logOut") { - await ASCredentialIdentityStore.SharedStore?.RemoveAllCredentialIdentitiesAsync(); + var userId = await _stateService.GetActiveUserIdAsync(); + // TODO make specific to userId + // await ASCredentialIdentityStore.SharedStore?.RemoveAllCredentialIdentitiesAsync(); } else {