1
0
mirror of https://github.com/bitwarden/mobile synced 2025-12-05 23:53:33 +00:00

[PM-7385] Fix for allowing switching accounts while creating a passkey of Android (#3155)

* PM-7385 Fixed for allowing switching accounts while creating a passkey on Android.
This fixes also include scenarios where we need to unlock the vault after switching
Also fixed the issue where tapping on cipher won't do anything after switching.

* PM-7385 ensure the Options.Fido2CredentialAction and FromFido2Framework are reset when the Credential flow is started to avoid erratic behaviors when switching accounts, app is in background or other edge case scenarios.
These properties where replaced by calls to _fido2MakeCredentialConfirmationUserInterface.IsConfirmingNewCredential instead.

* Minor changes and added comments

* [PM-7385] Implemented several changes suggested in PR for better/cleaner code.

* PM-7385 Added several minor code improvemments.
This commit is contained in:
Dinis Vieira
2024-04-16 21:52:52 +01:00
committed by GitHub
parent f596f31ffa
commit 5a4a54f4af
11 changed files with 110 additions and 40 deletions

View File

@@ -15,8 +15,10 @@ namespace Bit.App.Platforms.Android.Autofill
private readonly IUserVerificationMediatorService _userVerificationMediatorService;
private readonly IDeviceActionService _deviceActionService;
private readonly IPlatformUtilsService _platformUtilsService;
private LazyResolve<IMessagingService> _messagingService = new LazyResolve<IMessagingService>();
private TaskCompletionSource<(string cipherId, bool? userVerified)> _confirmCredentialTcs;
private TaskCompletionSource<bool> _unlockVaultTcs;
private Fido2UserVerificationOptions? _currentDefaultUserVerificationOptions;
private Func<bool> _checkHasVaultBeenUnlockedInThisTransaction;
@@ -37,6 +39,9 @@ namespace Bit.App.Platforms.Android.Autofill
public bool HasVaultBeenUnlockedInThisTransaction => _checkHasVaultBeenUnlockedInThisTransaction?.Invoke() == true;
public bool IsConfirmingNewCredential => _confirmCredentialTcs?.Task != null && !_confirmCredentialTcs.Task.IsCompleted;
public bool IsWaitingUnlockVault => _unlockVaultTcs?.Task != null && !_unlockVaultTcs.Task.IsCompleted;
public async Task<(string CipherId, bool UserVerified)> ConfirmNewCredentialAsync(Fido2ConfirmNewCredentialParams confirmNewCredentialParams)
{
_confirmCredentialTcs?.TrySetCanceled();
@@ -44,9 +49,8 @@ namespace Bit.App.Platforms.Android.Autofill
_confirmCredentialTcs = new TaskCompletionSource<(string cipherId, bool? userVerified)>();
_currentDefaultUserVerificationOptions = new Fido2UserVerificationOptions(false, confirmNewCredentialParams.UserVerificationPreference, HasVaultBeenUnlockedInThisTransaction, confirmNewCredentialParams.RpId);
var messagingService = ServiceContainer.Resolve<IMessagingService>("messagingService");
messagingService?.Send("fidoNavigateToAutofillCipher", confirmNewCredentialParams);
_messagingService.Value.Send(Bit.Core.Constants.CredentialNavigateToAutofillCipherMessageCommand, confirmNewCredentialParams);
var (cipherId, isUserVerified) = await _confirmCredentialTcs.Task;
@@ -99,11 +103,32 @@ namespace Bit.App.Platforms.Android.Autofill
public async Task EnsureUnlockedVaultAsync()
{
if (!await _stateService.IsAuthenticatedAsync() || await _vaultTimeoutService.IsLockedAsync())
if (!await _stateService.IsAuthenticatedAsync()
||
await _vaultTimeoutService.IsLoggedOutByTimeoutAsync()
||
await _vaultTimeoutService.ShouldLogOutByTimeoutAsync())
{
// this should never happen but just in case.
throw new InvalidOperationException("Not authed or vault locked");
await NavigateAndWaitForUnlockAsync(Bit.Core.Enums.NavigationTarget.HomeLogin);
return;
}
if (!await _vaultTimeoutService.IsLockedAsync())
{
return;
}
await NavigateAndWaitForUnlockAsync(Bit.Core.Enums.NavigationTarget.Lock);
}
private async Task NavigateAndWaitForUnlockAsync(Bit.Core.Enums.NavigationTarget navTarget)
{
_unlockVaultTcs?.TrySetCanceled();
_unlockVaultTcs = new TaskCompletionSource<bool>();
_messagingService.Value.Send(Bit.Core.Constants.NavigateToMessageCommand, navTarget);
await _unlockVaultTcs.Task;
}
public Task InformExcludedCredentialAsync(string[] existingCipherIds)
@@ -118,6 +143,7 @@ namespace Bit.App.Platforms.Android.Autofill
}
public void Confirm(string cipherId, bool? userVerified) => _confirmCredentialTcs?.TrySetResult((cipherId, userVerified));
public void ConfirmVaultUnlocked() => _unlockVaultTcs?.TrySetResult(true);
public async Task ConfirmAsync(string cipherId, bool alreadyHasFido2Credential, bool? userVerified)
{

View File

@@ -583,7 +583,11 @@ namespace Bit.Droid.Services
await ExecuteFido2CreateCredentialAsync();
}
appOptions.Fido2CredentialAction = null; //Clear CredentialAction Value
// Clear CredentialAction and FromFido2Framework values to avoid erratic behaviors in subsequent navigation/flows
// For Fido2CredentialGet these are no longer needed as a new Activity will be initiated.
// For Fido2CredentialCreate the app will rely on IFido2MakeCredentialConfirmationUserInterface.IsConfirmingNewCredential
appOptions.Fido2CredentialAction = null;
appOptions.FromFido2Framework = false;
}
private async Task ExecuteFido2GetCredentialAsync(AppOptions appOptions)

View File

@@ -43,6 +43,22 @@ namespace Bit.Core.Abstractions
/// </summary>
void OnConfirmationException(Exception ex);
/// <summary>
/// True if we are already confirming a new credential.
/// </summary>
bool IsConfirmingNewCredential { get; }
/// <summary>
/// Call this after the vault was unlocked so that Fido2 credential creation can proceed.
/// </summary>
void ConfirmVaultUnlocked();
/// <summary>
/// True if we are waiting for the vault to be unlocked.
/// </summary>
bool IsWaitingUnlockVault { get; }
Fido2UserVerificationOptions? GetCurrentUserVerificationOptions();
void SetCheckHasVaultBeenUnlockedInThisTransaction(Func<bool> checkHasVaultBeenUnlockedInThisTransaction);

View File

@@ -38,6 +38,7 @@ namespace Bit.App
private readonly IPushNotificationService _pushNotificationService;
private readonly IConfigService _configService;
private readonly ILogger _logger;
private LazyResolve<IFido2MakeCredentialConfirmationUserInterface> _userVerificationMediatorService = new LazyResolve<IFido2MakeCredentialConfirmationUserInterface>();
private static bool _isResumed;
// these variables are static because the app is launching new activities on notification click, creating new instances of App.
@@ -280,7 +281,7 @@ namespace Bit.App
}
}
}
else if (message.Command == "fidoNavigateToAutofillCipher" && message.Data is Fido2ConfirmNewCredentialParams createParams)
else if (message.Command == Constants.CredentialNavigateToAutofillCipherMessageCommand && message.Data is Fido2ConfirmNewCredentialParams createParams)
{
ArgumentNullException.ThrowIfNull(MainPage);
ArgumentNullException.ThrowIfNull(Options);
@@ -331,18 +332,24 @@ namespace Bit.App
|| message.Command == "unlocked"
|| message.Command == AccountsManagerMessageCommands.ACCOUNT_SWITCH_COMPLETED)
{
if (message.Command == AccountsManagerMessageCommands.ACCOUNT_SWITCH_COMPLETED)
if (message.Command == AccountsManagerMessageCommands.ACCOUNT_SWITCH_COMPLETED && _userVerificationMediatorService.Value.IsConfirmingNewCredential)
{
var userVerificationMediatorService = ServiceContainer.Resolve<IFido2MakeCredentialConfirmationUserInterface>();
userVerificationMediatorService?.OnConfirmationException(new AccountSwitchedException());
_userVerificationMediatorService.Value.OnConfirmationException(new AccountSwitchedException());
}
lock (_processingLoginRequestLock)
{
// lock doesn't allow for async execution
CheckPasswordlessLoginRequestsAsync().Wait();
}
}
else if (message.Command == Constants.NavigateToMessageCommand && message.Data is NavigationTarget navigationTarget)
{
await MainThread.InvokeOnMainThreadAsync(() =>
{
Navigate(navigationTarget, null);
});
}
}
catch (Exception ex)
{
@@ -713,6 +720,15 @@ namespace Bit.App
// If we are in background we add the Navigation Actions to a queue to execute when the app resumes.
// Links: https://github.com/dotnet/maui/issues/11501 and https://bitwarden.atlassian.net/wiki/spaces/NMME/pages/664862722/MainPage+Assignments+not+working+on+Android+on+Background+or+App+resume
#if ANDROID
if (_userVerificationMediatorService != null && _userVerificationMediatorService.Value.IsConfirmingNewCredential)
{
// if it's creating passkey
// and we have an active pending TaskCompletionSource
// then we let the Fido2 Authenticator flow manage the navigation to avoid issues
// like duplicated navigation to lock page.
return;
}
if (!_isResumed)
{
_onResumeActions.Enqueue(() => NavigateImpl(navTarget, navParams));

View File

@@ -48,6 +48,8 @@ namespace Bit.Core
public const string DisplayEuEnvironmentFlag = "display-eu-environment";
public const string RegionEnvironment = "regionEnvironment";
public const string DuoCallback = "bitwarden://duo-callback";
public const string NavigateToMessageCommand = "navigateTo";
public const string CredentialNavigateToAutofillCipherMessageCommand = "credentialNavigateToAutofillCipher";
/// <summary>
/// This key is used to store the value of "ShouldConnectToWatch" of the last user that had logged in

View File

@@ -7,16 +7,13 @@ using Bit.Core.Exceptions;
using Bit.Core.Models.View;
using Bit.Core.Resources.Localization;
using Bit.Core.Utilities;
using Bit.Core.Utilities.Fido2;
namespace Bit.App.Pages
{
public class AutofillCiphersPageViewModel : CipherSelectionPageViewModel
{
private CipherType? _fillType;
private bool _isAndroidFido2CredentialCreation;
private AppOptions _appOptions;
private readonly LazyResolve<IFido2MakeCredentialConfirmationUserInterface> _fido2MakeCredentialConfirmationUserInterface = new LazyResolve<IFido2MakeCredentialConfirmationUserInterface>();
public string Uri { get; set; }
@@ -25,7 +22,6 @@ namespace Bit.App.Pages
{
Uri = appOptions?.Uri;
_fillType = appOptions.FillType;
_isAndroidFido2CredentialCreation = appOptions.FromFido2Framework;
_appOptions = appOptions;
string name = null;
@@ -44,7 +40,7 @@ namespace Bit.App.Pages
Name = name;
PageTitle = string.Format(AppResources.ItemsForUri, Name ?? "--");
NoDataText = string.Format(AppResources.NoItemsForUri, Name ?? "--");
AddNewItemText = appOptions.FromFido2Framework ? AppResources.SavePasskeyAsNewLogin : AppResources.AddAnItem;
AddNewItemText = _fido2MakeCredentialConfirmationUserInterface.Value.IsConfirmingNewCredential ? AppResources.SavePasskeyAsNewLogin : AppResources.AddAnItem;
}
protected override async Task<List<GroupingsPageListGroup>> LoadGroupedItemsAsync()
@@ -54,7 +50,7 @@ namespace Bit.App.Pages
var matching = ciphers.Item1?.Select(c => new CipherItemViewModel(c, WebsiteIconsEnabled)
{
UsePasskeyIconAsPlaceholderFallback = _isAndroidFido2CredentialCreation
UsePasskeyIconAsPlaceholderFallback = _fido2MakeCredentialConfirmationUserInterface.Value.IsConfirmingNewCredential
}).ToList();
var hasMatching = matching?.Any() ?? false;
@@ -91,12 +87,9 @@ namespace Bit.App.Pages
return;
}
if (_appOptions.FromFido2Framework)
if (_fido2MakeCredentialConfirmationUserInterface.Value.IsConfirmingNewCredential)
{
if (_appOptions.Fido2CredentialAction == CredentialProviderConstants.Fido2CredentialCreate)
{
await _fido2MakeCredentialConfirmationUserInterface.Value.ConfirmAsync(cipher.Id, cipher.Login.HasFido2Credentials, null);
}
await _fido2MakeCredentialConfirmationUserInterface.Value.ConfirmAsync(cipher.Id, cipher.Login.HasFido2Credentials, null);
return;
}
@@ -155,7 +148,7 @@ namespace Bit.App.Pages
protected override async Task AddFabCipherAsync()
{
//Scenario for creating a new Fido2 credential on Android but showing the Cipher Page
if (_isAndroidFido2CredentialCreation)
if (_fido2MakeCredentialConfirmationUserInterface.Value.IsConfirmingNewCredential)
{
var pageForOther = new CipherAddEditPage(null, CipherType.Login, appOptions: _appOptions);
await Page.Navigation.PushModalAsync(new NavigationPage(pageForOther));
@@ -170,7 +163,7 @@ namespace Bit.App.Pages
protected override async Task AddCipherAsync()
{
//Scenario for creating a new Fido2 credential on Android
if (_isAndroidFido2CredentialCreation)
if (_fido2MakeCredentialConfirmationUserInterface.Value.IsConfirmingNewCredential)
{
_fido2MakeCredentialConfirmationUserInterface.Value.Confirm(null, null);
return;
@@ -190,9 +183,7 @@ namespace Bit.App.Pages
public void Cancel()
{
if (_appOptions?.FromFido2Framework == true
&&
_appOptions?.Fido2CredentialAction == CredentialProviderConstants.Fido2CredentialCreate)
if (_fido2MakeCredentialConfirmationUserInterface.Value.IsConfirmingNewCredential)
{
_fido2MakeCredentialConfirmationUserInterface.Value.Cancel();
}

View File

@@ -19,6 +19,7 @@ namespace Bit.App.Pages
private readonly IAutofillHandler _autofillHandler;
private readonly IVaultTimeoutService _vaultTimeoutService;
private readonly IUserVerificationService _userVerificationService;
private readonly LazyResolve<IFido2MakeCredentialConfirmationUserInterface> _fido2MakeCredentialConfirmationUserInterface = new LazyResolve<IFido2MakeCredentialConfirmationUserInterface>();
private CipherAddEditPageViewModel _vm;
private bool _fromAutofill;
@@ -45,7 +46,7 @@ namespace Bit.App.Pages
_appOptions = appOptions;
_fromAutofill = fromAutofill;
FromAutofillFramework = _appOptions?.FromAutofillFramework ?? false;
FromAndroidFido2Framework = _appOptions?.FromFido2Framework ?? false;
FromAndroidFido2Framework = _fido2MakeCredentialConfirmationUserInterface.Value.IsConfirmingNewCredential;
InitializeComponent();
_vm = BindingContext as CipherAddEditPageViewModel;
_vm.Page = this;

View File

@@ -332,7 +332,7 @@ namespace Bit.App.Pages
public async Task<bool> LoadAsync(AppOptions appOptions = null)
{
_fromOtp = appOptions?.OtpData != null;
IsFromFido2Framework = appOptions?.FromFido2Framework ?? false;
IsFromFido2Framework = _fido2MakeCredentialConfirmationUserInterface.IsConfirmingNewCredential;
var myEmail = await _stateService.GetEmailAsync();
OwnershipOptions.Add(new KeyValuePair<string, string>(myEmail, null));

View File

@@ -175,12 +175,9 @@ namespace Bit.App.Pages
public async Task SelectCipherAsync(CipherView cipher)
{
if (_appOptions.FromFido2Framework)
if (_fido2MakeCredentialConfirmationUserInterface.Value.IsConfirmingNewCredential)
{
if (_appOptions.Fido2CredentialAction == CredentialProviderConstants.Fido2CredentialCreate)
{
await _fido2MakeCredentialConfirmationUserInterface.Value.ConfirmAsync(cipher.Id, cipher.Login.HasFido2Credentials, null);
}
await _fido2MakeCredentialConfirmationUserInterface.Value.ConfirmAsync(cipher.Id, cipher.Login.HasFido2Credentials, null);
return;
}

View File

@@ -20,6 +20,7 @@ namespace Bit.App.Utilities.AccountManagement
private readonly IMessagingService _messagingService;
private readonly IWatchDeviceService _watchDeviceService;
private readonly IConditionedAwaiterManager _conditionedAwaiterManager;
private LazyResolve<IFido2MakeCredentialConfirmationUserInterface> _userVerificationMediatorService = new LazyResolve<IFido2MakeCredentialConfirmationUserInterface>();
Func<AppOptions> _getOptionsFunc;
private IAccountsManagerHost _accountsManagerHost;
@@ -82,7 +83,7 @@ namespace Bit.App.Utilities.AccountManagement
if (authed)
{
if (await _vaultTimeoutService.IsLoggedOutByTimeoutAsync() ||
await _vaultTimeoutService.ShouldLogOutByTimeoutAsync())
await _vaultTimeoutService.ShouldLogOutByTimeoutAsync())
{
// TODO implement orgIdentifier flow to SSO Login page, same as email flow below
// var orgIdentifier = await _stateService.GetOrgIdentifierAsync();
@@ -100,6 +101,12 @@ namespace Bit.App.Utilities.AccountManagement
{
_accountsManagerHost.Navigate(NavigationTarget.AddEditCipher);
}
else if (_userVerificationMediatorService.Value.IsConfirmingNewCredential)
{
// If we are already confirming a credential we don't need to navigate again.
// This could happen when switching accounts for example.
return;
}
else if (Options.FromFido2Framework)
{
var deviceActionService = Bit.Core.Utilities.ServiceContainer.Resolve<IDeviceActionService>();

View File

@@ -431,21 +431,31 @@ namespace Bit.App.Utilities
{
// this is called after login in or unlocking so we can assume the vault has been unlocked in this transaction here.
appOptions.HasUnlockedInThisTransaction = true;
ServiceContainer.Resolve<IFido2MakeCredentialConfirmationUserInterface>()
.SetCheckHasVaultBeenUnlockedInThisTransaction(() => appOptions?.HasUnlockedInThisTransaction == true);
var userVerificationMediatorService = ServiceContainer.Resolve<IFido2MakeCredentialConfirmationUserInterface>();
userVerificationMediatorService.SetCheckHasVaultBeenUnlockedInThisTransaction(() => appOptions?.HasUnlockedInThisTransaction == true);
if (appOptions.FromAutofillFramework && appOptions.SaveType.HasValue)
{
App.MainPage = new NavigationPage(new CipherAddEditPage(appOptions: appOptions));
return true;
}
// If we are waiting for an unlock vault we don't want to trigger 'ExecuteFido2CredentialActionAsync' again,
// as it's already running. We just need to 'ConfirmUnlockVault' on the 'userVerificationMediatorService'.
if (userVerificationMediatorService.IsWaitingUnlockVault)
{
userVerificationMediatorService.ConfirmVaultUnlocked();
return true;
}
if (appOptions.FromFido2Framework && !string.IsNullOrWhiteSpace(appOptions.Fido2CredentialAction))
{
var deviceActionService = Bit.Core.Utilities.ServiceContainer.Resolve<IDeviceActionService>();
deviceActionService.ExecuteFido2CredentialActionAsync(appOptions).FireAndForget();
return true;
}
if (appOptions.Uri != null
||
appOptions.OtpData != null)