From d2fbf5bdeab5fcbad032dc9c813921f8337f48d7 Mon Sep 17 00:00:00 2001 From: Federico Maccaroni Date: Thu, 14 Jul 2022 19:17:04 -0300 Subject: [PATCH] EC-312 Fix crash on entering invalid credentials five times on Autofill (#1988) --- src/App/Abstractions/IAccountsManager.cs | 1 + src/App/App.xaml.cs | 2 +- .../AccountManagement/AccountsManager.cs | 12 +- src/Core/Services/AuthService.cs | 3 +- .../CredentialProviderViewController.cs | 347 +++++++++++------- .../BaseLockPasswordViewController.cs | 41 +-- 6 files changed, 236 insertions(+), 170 deletions(-) diff --git a/src/App/Abstractions/IAccountsManager.cs b/src/App/Abstractions/IAccountsManager.cs index 684230d01..37acbc1db 100644 --- a/src/App/Abstractions/IAccountsManager.cs +++ b/src/App/Abstractions/IAccountsManager.cs @@ -8,5 +8,6 @@ namespace Bit.App.Abstractions { void Init(Func getOptionsFunc, IAccountsManagerHost accountsManagerHost); Task NavigateOnAccountChangeAsync(bool? isAuthed = null); + Task LogOutAsync(string userId, bool userInitiated, bool expired); } } diff --git a/src/App/App.xaml.cs b/src/App/App.xaml.cs index 3415fd7e9..8d2b74538 100644 --- a/src/App/App.xaml.cs +++ b/src/App/App.xaml.cs @@ -301,7 +301,7 @@ namespace Bit.App UpdateThemeAsync(); }; Current.MainPage = new NavigationPage(new HomePage(Options)); - var mainPageTask = _accountsManager.NavigateOnAccountChangeAsync(); + _accountsManager.NavigateOnAccountChangeAsync().FireAndForget(); ServiceContainer.Resolve("platformUtilsService").Init(); } diff --git a/src/App/Utilities/AccountManagement/AccountsManager.cs b/src/App/Utilities/AccountManagement/AccountsManager.cs index dd5b639f1..88701f2b7 100644 --- a/src/App/Utilities/AccountManagement/AccountsManager.cs +++ b/src/App/Utilities/AccountManagement/AccountsManager.cs @@ -123,7 +123,11 @@ namespace Bit.App.Utilities.AccountManagement await _vaultTimeoutService.LockAsync(true); break; case AccountsManagerMessageCommands.LOGOUT: - await Device.InvokeOnMainThreadAsync(() => LogOutAsync(message.Data as Tuple)); + var extras = message.Data as Tuple; + var userId = extras?.Item1; + var userInitiated = extras?.Item2 ?? true; + var expired = extras?.Item3 ?? false; + await Device.InvokeOnMainThreadAsync(() => LogOutAsync(userId, userInitiated, expired)); break; case AccountsManagerMessageCommands.LOGGED_OUT: // Clean up old migrated key if they ever log out. @@ -181,12 +185,8 @@ namespace Bit.App.Utilities.AccountManagement }); } - private async Task LogOutAsync(Tuple extras) + public async Task LogOutAsync(string userId, bool userInitiated, bool expired) { - var userId = extras?.Item1; - var userInitiated = extras?.Item2 ?? true; - var expired = extras?.Item3 ?? false; - await AppHelpers.LogOutAsync(userId, userInitiated); await NavigateOnAccountChangeAsync(); _authService.LogOut(() => diff --git a/src/Core/Services/AuthService.cs b/src/Core/Services/AuthService.cs index c1fb38fa8..7316a758e 100644 --- a/src/Core/Services/AuthService.cs +++ b/src/Core/Services/AuthService.cs @@ -6,6 +6,7 @@ using Bit.Core.Enums; using Bit.Core.Exceptions; using Bit.Core.Models.Domain; using Bit.Core.Models.Request; +using Bit.Core.Utilities; namespace Bit.Core.Services { @@ -173,7 +174,7 @@ namespace Bit.Core.Services public void LogOut(Action callback) { callback.Invoke(); - _messagingService.Send("loggedOut"); + _messagingService.Send(AccountsManagerMessageCommands.LOGGED_OUT); } public List GetSupportedTwoFactorProviders() diff --git a/src/iOS.Autofill/CredentialProviderViewController.cs b/src/iOS.Autofill/CredentialProviderViewController.cs index 298a6e1cb..9bd087f17 100644 --- a/src/iOS.Autofill/CredentialProviderViewController.cs +++ b/src/iOS.Autofill/CredentialProviderViewController.cs @@ -8,10 +8,12 @@ using Bit.App.Utilities; using Bit.App.Utilities.AccountManagement; using Bit.Core.Abstractions; using Bit.Core.Enums; +using Bit.Core.Services; using Bit.Core.Utilities; using Bit.iOS.Autofill.Models; using Bit.iOS.Core.Utilities; using Bit.iOS.Core.Views; +using CoreFoundation; using CoreNFC; using Foundation; using UIKit; @@ -36,88 +38,128 @@ namespace Bit.iOS.Autofill public override void ViewDidLoad() { - InitApp(); - base.ViewDidLoad(); - Logo.Image = new UIImage(ThemeHelpers.LightTheme ? "logo.png" : "logo_white.png"); - View.BackgroundColor = ThemeHelpers.SplashBackgroundColor; - _context = new Context + try { - ExtContext = ExtensionContext - }; + InitApp(); + base.ViewDidLoad(); + Logo.Image = new UIImage(ThemeHelpers.LightTheme ? "logo.png" : "logo_white.png"); + View.BackgroundColor = ThemeHelpers.SplashBackgroundColor; + _context = new Context + { + ExtContext = ExtensionContext + }; + } + catch (Exception ex) + { + LoggerHelper.LogEvenIfCantBeResolved(ex); + throw; + } } public override async void PrepareCredentialList(ASCredentialServiceIdentifier[] serviceIdentifiers) { - InitAppIfNeeded(); - _context.ServiceIdentifiers = serviceIdentifiers; - if (serviceIdentifiers.Length > 0) + try { - var uri = serviceIdentifiers[0].Identifier; - if (serviceIdentifiers[0].Type == ASCredentialServiceIdentifierType.Domain) + InitAppIfNeeded(); + _context.ServiceIdentifiers = serviceIdentifiers; + if (serviceIdentifiers.Length > 0) { - uri = string.Concat("https://", uri); + var uri = serviceIdentifiers[0].Identifier; + if (serviceIdentifiers[0].Type == ASCredentialServiceIdentifierType.Domain) + { + uri = string.Concat("https://", uri); + } + _context.UrlString = uri; } - _context.UrlString = uri; - } - if (!await IsAuthed()) - { - await _accountsManager.NavigateOnAccountChangeAsync(false); - } - else if (await IsLocked()) - { - PerformSegue("lockPasswordSegue", this); - } - else - { - if (_context.ServiceIdentifiers == null || _context.ServiceIdentifiers.Length == 0) + if (!await IsAuthed()) { - PerformSegue("loginSearchSegue", this); + await _accountsManager.NavigateOnAccountChangeAsync(false); + } + else if (await IsLocked()) + { + PerformSegue("lockPasswordSegue", this); } else { - PerformSegue("loginListSegue", this); + if (_context.ServiceIdentifiers == null || _context.ServiceIdentifiers.Length == 0) + { + PerformSegue("loginSearchSegue", this); + } + else + { + PerformSegue("loginListSegue", this); + } } } + catch (Exception ex) + { + LoggerHelper.LogEvenIfCantBeResolved(ex); + throw; + } } public override async void ProvideCredentialWithoutUserInteraction(ASPasswordCredentialIdentity credentialIdentity) { - InitAppIfNeeded(); - await _stateService.Value.SetPasswordRepromptAutofillAsync(false); - await _stateService.Value.SetPasswordVerifiedAutofillAsync(false); - if (!await IsAuthed() || await IsLocked()) + try { - var err = new NSError(new NSString("ASExtensionErrorDomain"), - Convert.ToInt32(ASExtensionErrorCode.UserInteractionRequired), null); - ExtensionContext.CancelRequest(err); - return; + InitAppIfNeeded(); + await _stateService.Value.SetPasswordRepromptAutofillAsync(false); + await _stateService.Value.SetPasswordVerifiedAutofillAsync(false); + if (!await IsAuthed() || await IsLocked()) + { + var err = new NSError(new NSString("ASExtensionErrorDomain"), + Convert.ToInt32(ASExtensionErrorCode.UserInteractionRequired), null); + ExtensionContext.CancelRequest(err); + return; + } + _context.CredentialIdentity = credentialIdentity; + await ProvideCredentialAsync(false); + } + catch (Exception ex) + { + LoggerHelper.LogEvenIfCantBeResolved(ex); + throw; } - _context.CredentialIdentity = credentialIdentity; - await ProvideCredentialAsync(false); } public override async void PrepareInterfaceToProvideCredential(ASPasswordCredentialIdentity credentialIdentity) { - InitAppIfNeeded(); - if (!await IsAuthed()) + try { - await _accountsManager.NavigateOnAccountChangeAsync(false); - return; + InitAppIfNeeded(); + if (!await IsAuthed()) + { + await _accountsManager.NavigateOnAccountChangeAsync(false); + return; + } + _context.CredentialIdentity = credentialIdentity; + await CheckLockAsync(async () => await ProvideCredentialAsync()); + } + catch (Exception ex) + { + LoggerHelper.LogEvenIfCantBeResolved(ex); + throw; } - _context.CredentialIdentity = credentialIdentity; - CheckLock(async () => await ProvideCredentialAsync()); } public override async void PrepareInterfaceForExtensionConfiguration() { - InitAppIfNeeded(); - _context.Configuring = true; - if (!await IsAuthed()) + try { - await _accountsManager.NavigateOnAccountChangeAsync(false); - return; + InitAppIfNeeded(); + _context.Configuring = true; + if (!await IsAuthed()) + { + await _accountsManager.NavigateOnAccountChangeAsync(false); + return; + } + await CheckLockAsync(() => PerformSegue("setupSegue", this)); + } + catch (Exception ex) + { + LoggerHelper.LogEvenIfCantBeResolved(ex); + throw; } - CheckLock(() => PerformSegue("setupSegue", this)); } public void CompleteRequest(string id = null, string username = null, @@ -159,34 +201,43 @@ namespace Bit.iOS.Autofill public override void PrepareForSegue(UIStoryboardSegue segue, NSObject sender) { - if (segue.DestinationViewController is UINavigationController navController) + try { - if (navController.TopViewController is LoginListViewController listLoginController) + if (segue.DestinationViewController is UINavigationController navController) { - listLoginController.Context = _context; - listLoginController.CPViewController = this; - segue.DestinationViewController.PresentationController.Delegate = - new CustomPresentationControllerDelegate(listLoginController.DismissModalAction); - } - else if (navController.TopViewController is LoginSearchViewController listSearchController) - { - listSearchController.Context = _context; - listSearchController.CPViewController = this; - segue.DestinationViewController.PresentationController.Delegate = - new CustomPresentationControllerDelegate(listSearchController.DismissModalAction); - } - else if (navController.TopViewController is LockPasswordViewController passwordViewController) - { - passwordViewController.CPViewController = this; - segue.DestinationViewController.PresentationController.Delegate = - new CustomPresentationControllerDelegate(passwordViewController.DismissModalAction); - } - else if (navController.TopViewController is SetupViewController setupViewController) - { - setupViewController.CPViewController = this; - segue.DestinationViewController.PresentationController.Delegate = - new CustomPresentationControllerDelegate(setupViewController.DismissModalAction); + if (navController.TopViewController is LoginListViewController listLoginController) + { + listLoginController.Context = _context; + listLoginController.CPViewController = this; + segue.DestinationViewController.PresentationController.Delegate = + new CustomPresentationControllerDelegate(listLoginController.DismissModalAction); + } + else if (navController.TopViewController is LoginSearchViewController listSearchController) + { + listSearchController.Context = _context; + listSearchController.CPViewController = this; + segue.DestinationViewController.PresentationController.Delegate = + new CustomPresentationControllerDelegate(listSearchController.DismissModalAction); + } + else if (navController.TopViewController is LockPasswordViewController passwordViewController) + { + passwordViewController.CPViewController = this; + segue.DestinationViewController.PresentationController.Delegate = + new CustomPresentationControllerDelegate(passwordViewController.DismissModalAction); + } + else if (navController.TopViewController is SetupViewController setupViewController) + { + setupViewController.CPViewController = this; + segue.DestinationViewController.PresentationController.Delegate = + new CustomPresentationControllerDelegate(setupViewController.DismissModalAction); + } } + + } + catch (Exception ex) + { + LoggerHelper.LogEvenIfCantBeResolved(ex); + throw; } } @@ -194,93 +245,109 @@ namespace Bit.iOS.Autofill { DismissViewController(false, async () => { - if (_context.CredentialIdentity != null) + try { - await ProvideCredentialAsync(); - return; + if (_context.CredentialIdentity != null) + { + await ProvideCredentialAsync(); + return; + } + if (_context.Configuring) + { + PerformSegue("setupSegue", this); + return; + } + if (_context.ServiceIdentifiers == null || _context.ServiceIdentifiers.Length == 0) + { + PerformSegue("loginSearchSegue", this); + } + else + { + PerformSegue("loginListSegue", this); + } } - if (_context.Configuring) + catch (Exception ex) { - PerformSegue("setupSegue", this); - return; - } - if (_context.ServiceIdentifiers == null || _context.ServiceIdentifiers.Length == 0) - { - PerformSegue("loginSearchSegue", this); - } - else - { - PerformSegue("loginListSegue", this); + LoggerHelper.LogEvenIfCantBeResolved(ex); + throw; } }); } private async Task ProvideCredentialAsync(bool userInteraction = true) { - var cipherService = ServiceContainer.Resolve("cipherService", true); - Bit.Core.Models.Domain.Cipher cipher = null; - var cancel = cipherService == null || _context.CredentialIdentity?.RecordIdentifier == null; - if (!cancel) + try { - cipher = await cipherService.GetAsync(_context.CredentialIdentity.RecordIdentifier); - cancel = cipher == null || cipher.Type != Bit.Core.Enums.CipherType.Login || cipher.Login == null; - } - if (cancel) - { - var err = new NSError(new NSString("ASExtensionErrorDomain"), - Convert.ToInt32(ASExtensionErrorCode.CredentialIdentityNotFound), null); - ExtensionContext?.CancelRequest(err); - return; - } - - var decCipher = await cipher.DecryptAsync(); - if (decCipher.Reprompt != Bit.Core.Enums.CipherRepromptType.None) - { - // Prompt for password using either the lock screen or dialog unless - // already verified the password. - if (!userInteraction) + var cipherService = ServiceContainer.Resolve("cipherService", true); + Bit.Core.Models.Domain.Cipher cipher = null; + var cancel = cipherService == null || _context.CredentialIdentity?.RecordIdentifier == null; + if (!cancel) + { + cipher = await cipherService.GetAsync(_context.CredentialIdentity.RecordIdentifier); + cancel = cipher == null || cipher.Type != Bit.Core.Enums.CipherType.Login || cipher.Login == null; + } + if (cancel) { - await _stateService.Value.SetPasswordRepromptAutofillAsync(true); var err = new NSError(new NSString("ASExtensionErrorDomain"), - Convert.ToInt32(ASExtensionErrorCode.UserInteractionRequired), null); + Convert.ToInt32(ASExtensionErrorCode.CredentialIdentityNotFound), null); ExtensionContext?.CancelRequest(err); return; } - else if (!await _stateService.Value.GetPasswordVerifiedAutofillAsync()) + + var decCipher = await cipher.DecryptAsync(); + if (decCipher.Reprompt != Bit.Core.Enums.CipherRepromptType.None) { - // Add a timeout to resolve keyboard not always showing up. - await Task.Delay(250); - var passwordRepromptService = ServiceContainer.Resolve("passwordRepromptService"); - if (!await passwordRepromptService.ShowPasswordPromptAsync()) + // Prompt for password using either the lock screen or dialog unless + // already verified the password. + if (!userInteraction) { + await _stateService.Value.SetPasswordRepromptAutofillAsync(true); var err = new NSError(new NSString("ASExtensionErrorDomain"), - Convert.ToInt32(ASExtensionErrorCode.UserCanceled), null); + Convert.ToInt32(ASExtensionErrorCode.UserInteractionRequired), null); ExtensionContext?.CancelRequest(err); return; } + else if (!await _stateService.Value.GetPasswordVerifiedAutofillAsync()) + { + // Add a timeout to resolve keyboard not always showing up. + await Task.Delay(250); + var passwordRepromptService = ServiceContainer.Resolve("passwordRepromptService"); + if (!await passwordRepromptService.ShowPasswordPromptAsync()) + { + var err = new NSError(new NSString("ASExtensionErrorDomain"), + Convert.ToInt32(ASExtensionErrorCode.UserCanceled), null); + ExtensionContext?.CancelRequest(err); + return; + } + } } - } - string totpCode = null; - var disableTotpCopy = await _stateService.Value.GetDisableAutoTotpCopyAsync(); - if (!disableTotpCopy.GetValueOrDefault(false)) - { - var canAccessPremiumAsync = await _stateService.Value.CanAccessPremiumAsync(); - if (!string.IsNullOrWhiteSpace(decCipher.Login.Totp) && - (canAccessPremiumAsync || cipher.OrganizationUseTotp)) + string totpCode = null; + var disableTotpCopy = await _stateService.Value.GetDisableAutoTotpCopyAsync(); + if (!disableTotpCopy.GetValueOrDefault(false)) { - var totpService = ServiceContainer.Resolve("totpService"); - totpCode = await totpService.GetCodeAsync(decCipher.Login.Totp); + var canAccessPremiumAsync = await _stateService.Value.CanAccessPremiumAsync(); + if (!string.IsNullOrWhiteSpace(decCipher.Login.Totp) && + (canAccessPremiumAsync || cipher.OrganizationUseTotp)) + { + var totpService = ServiceContainer.Resolve("totpService"); + totpCode = await totpService.GetCodeAsync(decCipher.Login.Totp); + } } - } - CompleteRequest(decCipher.Id, decCipher.Login.Username, decCipher.Login.Password, totpCode); + CompleteRequest(decCipher.Id, decCipher.Login.Username, decCipher.Login.Password, totpCode); + } + catch (Exception ex) + { + LoggerHelper.LogEvenIfCantBeResolved(ex); + throw; + } } - private async void CheckLock(Action notLockedAction) + private async Task CheckLockAsync(Action notLockedAction) { if (await IsLocked() || await _stateService.Value.GetPasswordRepromptAutofillAsync()) { - PerformSegue("lockPasswordSegue", this); + DispatchQueue.MainQueue.DispatchAsync(() => PerformSegue("lockPasswordSegue", this)); } else { @@ -303,15 +370,21 @@ namespace Bit.iOS.Autofill { NSRunLoop.Main.BeginInvokeOnMainThread(async () => { - if (await IsAuthed()) + try { - await AppHelpers.LogOutAsync(await _stateService.Value.GetActiveUserIdAsync()); - var deviceActionService = ServiceContainer.Resolve("deviceActionService"); - if (deviceActionService.SystemMajorVersion() >= 12) + if (await IsAuthed()) { - await ASCredentialIdentityStore.SharedStore?.RemoveAllCredentialIdentitiesAsync(); + await AppHelpers.LogOutAsync(await _stateService.Value.GetActiveUserIdAsync()); + if (UIDevice.CurrentDevice.CheckSystemVersion(12, 0)) + { + await ASCredentialIdentityStore.SharedStore?.RemoveAllCredentialIdentitiesAsync(); + } } } + catch (Exception ex) + { + LoggerHelper.LogEvenIfCantBeResolved(ex); + } }); } diff --git a/src/iOS.Core/Controllers/BaseLockPasswordViewController.cs b/src/iOS.Core/Controllers/BaseLockPasswordViewController.cs index e4ad3248a..a0f16b6a7 100644 --- a/src/iOS.Core/Controllers/BaseLockPasswordViewController.cs +++ b/src/iOS.Core/Controllers/BaseLockPasswordViewController.cs @@ -28,6 +28,7 @@ namespace Bit.iOS.Core.Controllers private IPlatformUtilsService _platformUtilsService; private IBiometricService _biometricService; private IKeyConnectorService _keyConnectorService; + private IAccountsManager _accountManager; private bool _isPinProtected; private bool _isPinProtectedWithKey; private bool _pinLock; @@ -84,7 +85,7 @@ namespace Bit.iOS.Core.Controllers } public abstract UITableView TableView { get; } - + public override async void ViewDidLoad() { _vaultTimeoutService = ServiceContainer.Resolve("vaultTimeoutService"); @@ -95,6 +96,7 @@ namespace Bit.iOS.Core.Controllers _platformUtilsService = ServiceContainer.Resolve("platformUtilsService"); _biometricService = ServiceContainer.Resolve("biometricService"); _keyConnectorService = ServiceContainer.Resolve("keyConnectorService"); + _accountManager = ServiceContainer.Resolve("accountsManager"); // We re-use the lock screen for autofill extension to verify master password // when trying to access protected items. @@ -265,13 +267,7 @@ namespace Bit.iOS.Core.Controllers } if (failed) { - var invalidUnlockAttempts = await AppHelpers.IncrementInvalidUnlockAttemptsAsync(); - if (invalidUnlockAttempts >= 5) - { - await LogOutAsync(); - return; - } - InvalidValue(); + await HandleFailedCredentialsAsync(); } } else @@ -306,17 +302,22 @@ namespace Bit.iOS.Core.Controllers } else { - var invalidUnlockAttempts = await AppHelpers.IncrementInvalidUnlockAttemptsAsync(); - if (invalidUnlockAttempts >= 5) - { - await LogOutAsync(); - return; - } - InvalidValue(); + await HandleFailedCredentialsAsync(); } } } + private async Task HandleFailedCredentialsAsync() + { + var invalidUnlockAttempts = await AppHelpers.IncrementInvalidUnlockAttemptsAsync(); + if (invalidUnlockAttempts >= 5) + { + await _accountManager.LogOutAsync(await _stateService.GetActiveUserIdAsync(), false, false); + return; + } + InvalidValue(); + } + public async Task PromptBiometricAsync() { if (!_biometricLock || !_biometricIntegrityValid) @@ -395,16 +396,6 @@ namespace Bit.iOS.Core.Controllers PresentViewController(alert, true, null); } - private async Task LogOutAsync() - { - await AppHelpers.LogOutAsync(await _stateService.GetActiveUserIdAsync()); - var authService = ServiceContainer.Resolve("authService"); - authService.LogOut(() => - { - Cancel?.Invoke(); - }); - } - protected override void Dispose(bool disposing) { base.Dispose(disposing);