Skip to content

Commit

Permalink
PM-6798 Fix account switch on iOS Autofill extension and also changed…
Browse files Browse the repository at this point in the history 
… to Try... actions for TaskCompletionSource to avoid exceptions on some occasions. (#3121)

Co-authored-by: Álison Fernandes <vvolkgang@users.noreply.github.com>
  • Loading branch information
@fedemkr @vvolkgang
fedemkr and vvolkgang committed Mar 27, 2024
1 parent 310d8b3 commit 8fd9e02
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 66 deletions.
115 changes: 72 additions & 43 deletions src/Core/Services/Fido2AuthenticatorService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,28 +36,42 @@ public async Task<Fido2AuthenticatorMakeCredentialResult> MakeCredentialAsync(Fi
throw new NotSupportedError();
}

await userInterface.EnsureUnlockedVaultAsync();
await _syncService.FullSyncAsync(false);
string cipherId = null;
var userVerified = false;
var accountSwitched = false;

var existingCipherIds = await FindExcludedCredentialsAsync(
makeCredentialParams.ExcludeCredentialDescriptorList
);
if (existingCipherIds.Length > 0)
do
{
await userInterface.InformExcludedCredentialAsync(existingCipherIds);
throw new NotAllowedError();
}
try
{
accountSwitched = false;

var response = await userInterface.ConfirmNewCredentialAsync(new Fido2ConfirmNewCredentialParams
{
CredentialName = makeCredentialParams.RpEntity.Name,
UserName = makeCredentialParams.UserEntity.Name,
UserVerificationPreference = makeCredentialParams.UserVerificationPreference,
RpId = makeCredentialParams.RpEntity.Id
});
await userInterface.EnsureUnlockedVaultAsync();
await _syncService.FullSyncAsync(false);

var existingCipherIds = await FindExcludedCredentialsAsync(
makeCredentialParams.ExcludeCredentialDescriptorList
);
if (existingCipherIds.Length > 0)
{
await userInterface.InformExcludedCredentialAsync(existingCipherIds);
throw new NotAllowedError();
}

(cipherId, userVerified) = await userInterface.ConfirmNewCredentialAsync(new Fido2ConfirmNewCredentialParams
{
CredentialName = makeCredentialParams.RpEntity.Name,
UserName = makeCredentialParams.UserEntity.Name,
UserVerificationPreference = makeCredentialParams.UserVerificationPreference,
RpId = makeCredentialParams.RpEntity.Id
});
}
catch (AccountSwitchedException)
{
accountSwitched = true;
}
} while (accountSwitched);

var cipherId = response.CipherId;
var userVerified = response.UserVerified;
string credentialId;
if (cipherId == null)
{
Expand Down Expand Up @@ -118,37 +132,52 @@ public async Task<Fido2AuthenticatorMakeCredentialResult> MakeCredentialAsync(Fi

public async Task<Fido2AuthenticatorGetAssertionResult> GetAssertionAsync(Fido2AuthenticatorGetAssertionParams assertionParams, IFido2GetAssertionUserInterface userInterface)
{
List<CipherView> cipherOptions;
List<CipherView> cipherOptions = new List<CipherView>();

await userInterface.EnsureUnlockedVaultAsync();
await _syncService.FullSyncAsync(false);
string selectedCipherId = null;
var userVerified = false;
var accountSwitched = false;

if (assertionParams.AllowCredentialDescriptorList?.Length > 0)
{
cipherOptions = await FindCredentialsByIdAsync(
assertionParams.AllowCredentialDescriptorList,
assertionParams.RpId
);
}
else
do
{
cipherOptions = await FindCredentialsByRpAsync(assertionParams.RpId);
}
try
{
accountSwitched = false;

if (cipherOptions.Count == 0)
{
throw new NotAllowedError();
}
await userInterface.EnsureUnlockedVaultAsync();
await _syncService.FullSyncAsync(false);

if (assertionParams.AllowCredentialDescriptorList?.Length > 0)
{
cipherOptions = await FindCredentialsByIdAsync(
assertionParams.AllowCredentialDescriptorList,
assertionParams.RpId
);
}
else
{
cipherOptions = await FindCredentialsByRpAsync(assertionParams.RpId);
}

if (cipherOptions.Count == 0)
{
throw new NotAllowedError();
}

var response = await userInterface.PickCredentialAsync(
cipherOptions.Select((cipher) => new Fido2GetAssertionUserInterfaceCredential
(selectedCipherId, userVerified) = await userInterface.PickCredentialAsync(
cipherOptions.Select((cipher) => new Fido2GetAssertionUserInterfaceCredential
{
CipherId = cipher.Id,
UserVerificationPreference = Fido2UserVerificationPreferenceExtensions.GetUserVerificationPreferenceFrom(assertionParams.UserVerificationPreference, cipher.Reprompt)
}).ToArray()
);

}
catch (AccountSwitchedException)
{
CipherId = cipher.Id,
UserVerificationPreference = Fido2UserVerificationPreferenceExtensions.GetUserVerificationPreferenceFrom(assertionParams.UserVerificationPreference, cipher.Reprompt)
}).ToArray()
);
var selectedCipherId = response.CipherId;
var userVerified = response.UserVerified;
accountSwitched = true;
}
} while (accountSwitched);

var selectedCipher = cipherOptions.FirstOrDefault((c) => c.Id == selectedCipherId);
if (selectedCipher == null)
Expand Down
2 changes: 1 addition & 1 deletion src/Core/Services/Logging/ClipLogger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
// {
// _currentBreadcrumbs.AppendLine($"{DateTime.Now.ToShortTimeString()}: {breadcrumb}");
//#if IOS
// UIPasteboard.General.String = _currentBreadcrumbs.ToString();
// MainThread.BeginInvokeOnMainThread(() => UIPasteboard.General.String = _currentBreadcrumbs.ToString());
//#endif
// }

Expand Down
7 changes: 7 additions & 0 deletions src/Core/Utilities/Fido2/Fido2AuthenticatorException.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,11 @@ public UnknownError() : base("UnknownError")
{
}
}

public class AccountSwitchedException : Fido2AuthenticatorException
{
public AccountSwitchedException() : base(nameof(AccountSwitchedException))
{
}
}
}
38 changes: 23 additions & 15 deletions src/iOS.Autofill/CredentialProviderViewController.Passkeys.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ private void OnConfirmingNewCredential()
{
try
{
PerformSegue(SegueConstants.LOGIN_LIST, this);
DismissViewController(false, () => PerformSegue(SegueConstants.LOGIN_LIST, this));
}
catch (Exception ex)
{
Expand All @@ -352,26 +352,22 @@ private async Task EnsureUnlockedVaultAsync()
{
if (_context.IsCreatingPasskey)
{
if (!await IsLocked())
if (!await IsAuthed()
||
await _vaultTimeoutService.Value.IsLoggedOutByTimeoutAsync()
||
await _vaultTimeoutService.Value.ShouldLogOutByTimeoutAsync())
{
await NavigateAndWaitForUnlockAsync(Bit.Core.Enums.NavigationTarget.HomeLogin);
return;
}

_context.UnlockVaultTcs?.SetCanceled();
_context.UnlockVaultTcs = new TaskCompletionSource<bool>();
MainThread.BeginInvokeOnMainThread(() =>
if (!await IsLocked())
{
try
{
PerformSegue(SegueConstants.LOCK, this);
}
catch (Exception ex)
{
LoggerHelper.LogEvenIfCantBeResolved(ex);
}
});
return;
}

await _context.UnlockVaultTcs.Task;
await NavigateAndWaitForUnlockAsync(Bit.Core.Enums.NavigationTarget.Lock);
return;
}

Expand All @@ -381,5 +377,17 @@ private async Task EnsureUnlockedVaultAsync()
throw new InvalidOperationNeedsUIException("Not authed or locked");
}
}

private async Task NavigateAndWaitForUnlockAsync(Bit.Core.Enums.NavigationTarget navTarget)
{
_context.UnlockVaultTcs?.TrySetCanceled();
_context.UnlockVaultTcs = new TaskCompletionSource<bool>();
await MainThread.InvokeOnMainThreadAsync(() =>
{
DoNavigate(navTarget);
});

await _context.UnlockVaultTcs.Task;
}
}
}
40 changes: 37 additions & 3 deletions src/iOS.Autofill/CredentialProviderViewController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Bit.App.Utilities.AccountManagement;
using Bit.Core.Abstractions;
using Bit.Core.Enums;
using Bit.Core.Models.Domain;
using Bit.Core.Services;
using Bit.Core.Utilities;
using Bit.Core.Utilities.Fido2;
Expand All @@ -34,6 +35,8 @@ public partial class CredentialProviderViewController : ASCredentialProviderView

private readonly LazyResolve<IStateService> _stateService = new LazyResolve<IStateService>();
private readonly LazyResolve<IConditionedAwaiterManager> _conditionedAwaiterManager = new LazyResolve<IConditionedAwaiterManager>();
private readonly LazyResolve<IBroadcasterService> _broadcasterService = new LazyResolve<IBroadcasterService>();
private readonly LazyResolve<IVaultTimeoutService> _vaultTimeoutService = new LazyResolve<IVaultTimeoutService>();

public CredentialProviderViewController(IntPtr handle)
: base(handle)
Expand Down Expand Up @@ -377,7 +380,7 @@ public async Task OnLockDismissedAsync()

if (_context.IsCreatingPasskey)
{
_context.UnlockVaultTcs.SetResult(true);
_context.UnlockVaultTcs.TrySetResult(true);
return;
}

Expand Down Expand Up @@ -509,8 +512,7 @@ private async Task CheckLockAsync(Action notLockedAction)

private Task<bool> IsLocked()
{
var vaultTimeoutService = ServiceContainer.Resolve<IVaultTimeoutService>("vaultTimeoutService");
return vaultTimeoutService.IsLockedAsync();
return _vaultTimeoutService.Value.IsLockedAsync();
}

private Task<bool> IsAuthed()
Expand Down Expand Up @@ -544,6 +546,8 @@ private void InitApp()
{
iOSCoreHelpers.InitApp(this, Bit.Core.Constants.iOSAutoFillClearCiphersCacheKey,
_nfcSession, out _nfcDelegate, out _accountsManager);

_broadcasterService.Value.Subscribe(nameof(CredentialProviderViewController), OnMessageReceived);
}

private async Task InitAppIfNeededAsync()
Expand All @@ -556,6 +560,18 @@ private async Task InitAppIfNeededAsync()
await _stateService.Value.ReloadStateAsync();
}

private void OnMessageReceived(Message message)
{
if (message?.Command == AccountsManagerMessageCommands.ACCOUNT_SWITCH_COMPLETED
&&
_context != null)
{
_context.VaultUnlockedDuringThisSession = false;
_context.PickCredentialForFido2CreationTcs?.TrySetException(new AccountSwitchedException());
_context.PickCredentialForFido2GetAssertionFromListTcs?.TrySetException(new AccountSwitchedException());
}
}

private void LaunchHomePage()
{
var appOptions = new AppOptions { IosExtension = true };
Expand Down Expand Up @@ -751,6 +767,24 @@ private void LaunchDeviceApprovalOptionsFlow()
public Task UpdateThemeAsync() => Task.CompletedTask;

public void Navigate(NavigationTarget navTarget, INavigationParams navParams = null)
{
if (_context?.IsCreatingPasskey == true
&&
_context.PickCredentialForFido2CreationTcs != null
&&
!_context.PickCredentialForFido2CreationTcs.Task.IsCompleted)
{
// 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.
return;
}

DoNavigate(navTarget, navParams);
}

internal void DoNavigate(NavigationTarget navTarget, INavigationParams navParams = null)
{
switch (navTarget)
{
Expand Down
2 changes: 1 addition & 1 deletion src/iOS.Autofill/Fido2MakeCredentialUserInterface.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class Fido2MakeCredentialUserInterface : IFido2MakeCredentialUserInterfac

public async Task<(string CipherId, bool UserVerified)> ConfirmNewCredentialAsync(Fido2ConfirmNewCredentialParams confirmNewCredentialParams)
{
_context.PickCredentialForFido2CreationTcs?.SetCanceled();
_context.PickCredentialForFido2CreationTcs?.TrySetCanceled();
_context.PickCredentialForFido2CreationTcs = new TaskCompletionSource<(string, bool?)>();
_context.PasskeyCreationParams = confirmNewCredentialParams;

Expand Down
9 changes: 7 additions & 2 deletions src/iOS.Autofill/Utilities/BaseLoginListTableSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ public async override void RowSelected(UITableView tableView, NSIndexPath indexP

if (Context.IsPreparingListForPasskey && item.IsFido2ListItem)
{
Context.PickCredentialForFido2GetAssertionFromListTcs.SetResult(item.Id);
Context.PickCredentialForFido2GetAssertionFromListTcs.TrySetResult(item.Id);
return;
}

Expand Down Expand Up @@ -317,7 +317,12 @@ private async Task SelectRowForPasskeyCreationAsync(UITableView tableView, NSInd
return;
}

Context.PickCredentialForFido2CreationTcs.SetResult((item.Id, null));
if (!await _passwordRepromptService.PromptAndCheckPasswordIfNeededAsync(item.Reprompt))
{
return;
}

Context.PickCredentialForFido2CreationTcs.TrySetResult((item.Id, null));
}

private async Task<CipherViewModel> DeselectRowAndGetItemAsync(UITableView tableView, NSIndexPath indexPath)
Expand Down
2 changes: 1 addition & 1 deletion src/iOS.Autofill/Utilities/Fido2GetAssertionFromListUserInterface.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public async Task<(string CipherId, bool UserVerified)> PickCredentialAsync(Fido

_onAllowedFido2Credentials?.Invoke(credentials.Select(c => c.CipherId).ToList() ?? new List<string>());

_context.PickCredentialForFido2GetAssertionFromListTcs?.SetCanceled();
_context.PickCredentialForFido2GetAssertionFromListTcs?.TrySetCanceled();
_context.PickCredentialForFido2GetAssertionFromListTcs = new TaskCompletionSource<string>();

var cipherId = await _context.PickCredentialForFido2GetAssertionFromListTcs.Task;
Expand Down

0 comments on commit 8fd9e02

Please sign in to comment.