diff --git a/CHANGELOG.md b/CHANGELOG.md index cd1c9ee..f5fea08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Breaking + +- **`ICredentialManager` members now take a `CancellationToken`** (#74). Every member gains a + trailing `CancellationToken cancellationToken = default`. Existing *callers* keep compiling — + the parameter is optional — but any external **implementation** of the interface must update + its signatures. The three built-in backends are updated. `ICredentialCollector` and + `ICredentialSummaryProvider` are untouched, so the provider packages are unaffected. + + The libsecret backend now binds the token to a `GCancellable` for every call, so + cancellation reaches libsecret rather than stopping at the managed boundary. The Keychain + backend honours the token at entry; Security.framework's `SecItem*` API offers no + mid-call cancellation, and that is documented rather than faked. + + **What this does not fix, verified rather than assumed:** it does not rescue the KWallet + wedge that prompted the issue. Against `ksecretd` the cancellation callback does fire, but + the call still never returns, because ksecretd emits the prompt's `Completed` signal with an + `ao` payload where libsecret expects `o` — libsecret's `secret_service_real_prompt_async` + GTask is then *"finalized without ever returning"* and its nested main loop never exits. + That is an upstream libsecret/KWallet incompatibility, and it is recorded in the backend's + remarks instead of being papered over. + + This makes the next release **2.0.0**; the version bump itself is a separate release PR. + ### Changed - **The libsecret backend is now documented as GNOME Keyring only** (#19). It was described diff --git a/src/NextIteration.SpectreConsole.Auth/Commands/AddCredentialCommand.cs b/src/NextIteration.SpectreConsole.Auth/Commands/AddCredentialCommand.cs index 0470345..d8cc304 100644 --- a/src/NextIteration.SpectreConsole.Auth/Commands/AddCredentialCommand.cs +++ b/src/NextIteration.SpectreConsole.Auth/Commands/AddCredentialCommand.cs @@ -83,14 +83,14 @@ protected override async Task ExecuteAsync(CommandContext context, Settings collector.ProviderName, settings.AccountName, environment, - credentialData).ConfigureAwait(false); + credentialData, cancellationToken).ConfigureAwait(false); AnsiConsole.MarkupLine($"[green]Successfully added credential with ID: {accountId}[/]"); // Ask if user wants to select this credential as active if (await AnsiConsole.ConfirmAsync("Do you want to set this as the active credential for this environment?", cancellationToken: cancellationToken).ConfigureAwait(false)) { - _ = await _credentialManager.SelectCredentialAsync(accountId).ConfigureAwait(false); + _ = await _credentialManager.SelectCredentialAsync(accountId, cancellationToken).ConfigureAwait(false); AnsiConsole.MarkupLine("[green]Credential selected as active.[/]"); } diff --git a/src/NextIteration.SpectreConsole.Auth/Commands/DeleteCredentialCommand.cs b/src/NextIteration.SpectreConsole.Auth/Commands/DeleteCredentialCommand.cs index d4bec62..5a38da8 100644 --- a/src/NextIteration.SpectreConsole.Auth/Commands/DeleteCredentialCommand.cs +++ b/src/NextIteration.SpectreConsole.Auth/Commands/DeleteCredentialCommand.cs @@ -31,7 +31,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings else { // Interactive selection - var providers = await _credentialManager.GetProviderNamesAsync().ConfigureAwait(false); + var providers = await _credentialManager.GetProviderNamesAsync(cancellationToken).ConfigureAwait(false); if (!providers.Any()) { AnsiConsole.MarkupLine("[yellow]No credentials found.[/]"); @@ -41,7 +41,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings var allCredentials = new List(); foreach (var provider in providers) { - var credentials = await _credentialManager.ListCredentialsAsync(provider).ConfigureAwait(false); + var credentials = await _credentialManager.ListCredentialsAsync(provider, cancellationToken).ConfigureAwait(false); allCredentials.AddRange(credentials); } @@ -70,7 +70,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings return 0; } - var success = await _credentialManager.DeleteCredentialAsync(accountId).ConfigureAwait(false); + var success = await _credentialManager.DeleteCredentialAsync(accountId, cancellationToken).ConfigureAwait(false); if (success) { diff --git a/src/NextIteration.SpectreConsole.Auth/Commands/ExportCredentialsCommand.cs b/src/NextIteration.SpectreConsole.Auth/Commands/ExportCredentialsCommand.cs index 9b11655..38a75f7 100644 --- a/src/NextIteration.SpectreConsole.Auth/Commands/ExportCredentialsCommand.cs +++ b/src/NextIteration.SpectreConsole.Auth/Commands/ExportCredentialsCommand.cs @@ -38,7 +38,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings } var service = new CredentialPortabilityService(_credentialManager); - var result = await service.ExportAsync(passphrase).ConfigureAwait(false); + var result = await service.ExportAsync(passphrase, cancellationToken).ConfigureAwait(false); if (result.Count == 0) { diff --git a/src/NextIteration.SpectreConsole.Auth/Commands/ImportCredentialsCommand.cs b/src/NextIteration.SpectreConsole.Auth/Commands/ImportCredentialsCommand.cs index ff13f5d..5382ad7 100644 --- a/src/NextIteration.SpectreConsole.Auth/Commands/ImportCredentialsCommand.cs +++ b/src/NextIteration.SpectreConsole.Auth/Commands/ImportCredentialsCommand.cs @@ -47,7 +47,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings var resolver = BuildConflictResolver(settings.OnConflict, forcedResolution); var service = new CredentialPortabilityService(_credentialManager); - var result = await service.ImportAsync(bundle, passphrase, resolver).ConfigureAwait(false); + var result = await service.ImportAsync(bundle, passphrase, resolver, cancellationToken).ConfigureAwait(false); AnsiConsole.MarkupLine( $"[green]Import complete:[/] {result.Added} added, {result.Overwritten} overwritten, {result.Skipped} skipped."); diff --git a/src/NextIteration.SpectreConsole.Auth/Commands/ListCredentialsCommand.cs b/src/NextIteration.SpectreConsole.Auth/Commands/ListCredentialsCommand.cs index 009b770..f4ae925 100644 --- a/src/NextIteration.SpectreConsole.Auth/Commands/ListCredentialsCommand.cs +++ b/src/NextIteration.SpectreConsole.Auth/Commands/ListCredentialsCommand.cs @@ -40,7 +40,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings { if (string.IsNullOrWhiteSpace(settings.Provider)) { - var providers = await _credentialManager.GetProviderNamesAsync().ConfigureAwait(false); + var providers = await _credentialManager.GetProviderNamesAsync(cancellationToken).ConfigureAwait(false); if (!providers.Any()) { @@ -50,13 +50,13 @@ protected override async Task ExecuteAsync(CommandContext context, Settings foreach (var provider in providers) { - await DisplayCredentialsForProvider(provider).ConfigureAwait(false); + await DisplayCredentialsForProvider(provider, cancellationToken).ConfigureAwait(false); AnsiConsole.WriteLine(); } } else { - await DisplayCredentialsForProvider(settings.Provider).ConfigureAwait(false); + await DisplayCredentialsForProvider(settings.Provider, cancellationToken).ConfigureAwait(false); } return 0; @@ -69,9 +69,9 @@ protected override async Task ExecuteAsync(CommandContext context, Settings } [System.Diagnostics.CodeAnalysis.SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "")] - private async Task DisplayCredentialsForProvider(string provider) + private async Task DisplayCredentialsForProvider(string provider, CancellationToken cancellationToken) { - var credentials = (await _credentialManager.ListCredentialsAsync(provider).ConfigureAwait(false)).ToList(); + var credentials = (await _credentialManager.ListCredentialsAsync(provider, cancellationToken).ConfigureAwait(false)).ToList(); if (credentials.Count == 0) { diff --git a/src/NextIteration.SpectreConsole.Auth/Commands/SelectCredentialCommand.cs b/src/NextIteration.SpectreConsole.Auth/Commands/SelectCredentialCommand.cs index f6fa7cb..3c3d44f 100644 --- a/src/NextIteration.SpectreConsole.Auth/Commands/SelectCredentialCommand.cs +++ b/src/NextIteration.SpectreConsole.Auth/Commands/SelectCredentialCommand.cs @@ -32,7 +32,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings else { // Interactive selection - var providers = await _credentialManager.GetProviderNamesAsync().ConfigureAwait(false); + var providers = await _credentialManager.GetProviderNamesAsync(cancellationToken).ConfigureAwait(false); if (!providers.Any()) { AnsiConsole.MarkupLine("[yellow]No credentials found.[/]"); @@ -42,7 +42,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings var allCredentials = new List(); foreach (var provider in providers) { - var credentials = await _credentialManager.ListCredentialsAsync(provider).ConfigureAwait(false); + var credentials = await _credentialManager.ListCredentialsAsync(provider, cancellationToken).ConfigureAwait(false); allCredentials.AddRange(credentials); } @@ -63,7 +63,7 @@ protected override async Task ExecuteAsync(CommandContext context, Settings accountId = allCredentials[selectedIndex].AccountId; } - var success = await _credentialManager.SelectCredentialAsync(accountId).ConfigureAwait(false); + var success = await _credentialManager.SelectCredentialAsync(accountId, cancellationToken).ConfigureAwait(false); if (success) { diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/AtomicFile.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/AtomicFile.cs index f30c215..5143015 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/AtomicFile.cs +++ b/src/NextIteration.SpectreConsole.Auth/Persistence/AtomicFile.cs @@ -28,12 +28,12 @@ namespace NextIteration.SpectreConsole.Auth.Persistence /// internal static class AtomicFile { - internal static async Task WriteAllTextAsync(string path, string contents, UnixFileMode? unixMode = null) + internal static async Task WriteAllTextAsync(string path, string contents, UnixFileMode? unixMode = null, CancellationToken cancellationToken = default) { var tempPath = BuildTempPath(path); try { - await WriteTempAsync(tempPath, System.Text.Encoding.UTF8.GetBytes(contents), unixMode).ConfigureAwait(false); + await WriteTempAsync(tempPath, System.Text.Encoding.UTF8.GetBytes(contents), unixMode, cancellationToken).ConfigureAwait(false); await ReplaceAtomicallyAsync(tempPath, path).ConfigureAwait(false); } catch @@ -54,7 +54,7 @@ internal static async Task WriteAllTextAsync(string path, string contents, UnixF /// accounts export writes to a path the user chooses — exporting to /tmp /// or any shared directory exposed every secret in the store for that window (#54). /// - private static async Task WriteTempAsync(string tempPath, byte[] bytes, UnixFileMode? unixMode) + private static async Task WriteTempAsync(string tempPath, byte[] bytes, UnixFileMode? unixMode, CancellationToken cancellationToken) { var options = new FileStreamOptions { @@ -71,7 +71,7 @@ private static async Task WriteTempAsync(string tempPath, byte[] bytes, UnixFile var stream = new FileStream(tempPath, options); await using (stream.ConfigureAwait(false)) { - await stream.WriteAsync(bytes).ConfigureAwait(false); + await stream.WriteAsync(bytes, cancellationToken).ConfigureAwait(false); } } @@ -92,7 +92,7 @@ private static async Task WriteTempAsync(string tempPath, byte[] bytes, UnixFile /// the winner. Used for the keystore, where an overwrite silently destroys every /// credential already encrypted under the replaced key. /// - internal static async Task TryWriteNewAsync(string path, byte[] bytes, UnixFileMode? unixMode = null) + internal static async Task TryWriteNewAsync(string path, byte[] bytes, UnixFileMode? unixMode = null, CancellationToken cancellationToken = default) { if (File.Exists(path)) { @@ -102,7 +102,7 @@ internal static async Task TryWriteNewAsync(string path, byte[] bytes, Uni var tempPath = BuildTempPath(path); try { - await WriteTempAsync(tempPath, bytes, unixMode).ConfigureAwait(false); + await WriteTempAsync(tempPath, bytes, unixMode, cancellationToken).ConfigureAwait(false); // Deliberately not overwrite:true. A racing creator that already landed // its keystore must win; we then fall back to reading theirs. @@ -122,12 +122,12 @@ internal static async Task TryWriteNewAsync(string path, byte[] bytes, Uni } } - internal static async Task WriteAllBytesAsync(string path, byte[] bytes, UnixFileMode? unixMode = null) + internal static async Task WriteAllBytesAsync(string path, byte[] bytes, UnixFileMode? unixMode = null, CancellationToken cancellationToken = default) { var tempPath = BuildTempPath(path); try { - await WriteTempAsync(tempPath, bytes, unixMode).ConfigureAwait(false); + await WriteTempAsync(tempPath, bytes, unixMode, cancellationToken).ConfigureAwait(false); await ReplaceAtomicallyAsync(tempPath, path).ConfigureAwait(false); } catch diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/FileCredentialManager.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/FileCredentialManager.cs index c78729f..52e03cb 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/FileCredentialManager.cs +++ b/src/NextIteration.SpectreConsole.Auth/Persistence/FileCredentialManager.cs @@ -50,7 +50,7 @@ public FileCredentialManager( } /// - public async Task> ListCredentialsAsync(string providerName) + public async Task> ListCredentialsAsync(string providerName, CancellationToken cancellationToken = default) { ValidateProviderName(providerName); @@ -61,7 +61,7 @@ public async Task> ListCredentialsAsync(string pr var credentialFiles = Directory.GetFiles(_credentialsDirectory, $"{providerPrefix}_*.json"); var credentials = new List(); - var selections = await LoadSelectionsAsync().ConfigureAwait(false); + var selections = await LoadSelectionsAsync(cancellationToken).ConfigureAwait(false); _ = _summaryProviders.TryGetValue(providerName, out var summaryProvider); foreach (var file in credentialFiles) @@ -69,7 +69,7 @@ public async Task> ListCredentialsAsync(string pr StoredCredential? credential; try { - var content = await File.ReadAllTextAsync(file).ConfigureAwait(false); + var content = await File.ReadAllTextAsync(file, cancellationToken).ConfigureAwait(false); credential = JsonSerializer.Deserialize(content, _jsonOptions); } catch (JsonException) @@ -138,7 +138,7 @@ public async Task> ListCredentialsAsync(string pr } /// - public async Task AddCredentialAsync(string providerName, string accountName, string environment, string credentialData) + public async Task AddCredentialAsync(string providerName, string accountName, string environment, string credentialData, CancellationToken cancellationToken = default) { ValidateProviderName(providerName); @@ -168,20 +168,21 @@ public async Task AddCredentialAsync(string providerName, string account await AtomicFile.WriteAllTextAsync( filePath, json, - OperatingSystem.IsWindows() ? null : UnixFileMode.UserRead | UnixFileMode.UserWrite).ConfigureAwait(false); + OperatingSystem.IsWindows() ? null : UnixFileMode.UserRead | UnixFileMode.UserWrite, + cancellationToken).ConfigureAwait(false); return accountId; } /// - public async Task DeleteCredentialAsync(string accountId) + public async Task DeleteCredentialAsync(string accountId, CancellationToken cancellationToken = default) { if (!IsValidAccountId(accountId)) { return false; } - var found = await FindCredentialByAccountIdAsync(accountId).ConfigureAwait(false); + var found = await FindCredentialByAccountIdAsync(accountId, cancellationToken).ConfigureAwait(false); if (found is null) { return false; @@ -192,46 +193,46 @@ public async Task DeleteCredentialAsync(string accountId) // Hold the selections lock across load+modify+save so a concurrent // select/delete on a different provider can't lose its update. - using var selectionsLock = await SelectionsLock.AcquireAsync(_selectionLockFile).ConfigureAwait(false); - var selections = await LoadSelectionsAsync().ConfigureAwait(false); + using var selectionsLock = await SelectionsLock.AcquireAsync(_selectionLockFile, cancellationToken).ConfigureAwait(false); + var selections = await LoadSelectionsAsync(cancellationToken).ConfigureAwait(false); if (selections.TryGetValue(credential.ProviderName, out var selectedId) && selectedId.Equals(accountId, StringComparison.OrdinalIgnoreCase)) { _ = selections.Remove(credential.ProviderName); - await SaveSelectionsAsync(selections).ConfigureAwait(false); + await SaveSelectionsAsync(selections, cancellationToken).ConfigureAwait(false); } return true; } /// - public async Task SelectCredentialAsync(string accountId) + public async Task SelectCredentialAsync(string accountId, CancellationToken cancellationToken = default) { if (!IsValidAccountId(accountId)) { return false; } - var found = await FindCredentialByAccountIdAsync(accountId).ConfigureAwait(false); + var found = await FindCredentialByAccountIdAsync(accountId, cancellationToken).ConfigureAwait(false); if (found is null) { return false; } var credential = found.Value.Credential; - using var selectionsLock = await SelectionsLock.AcquireAsync(_selectionLockFile).ConfigureAwait(false); - var selections = await LoadSelectionsAsync().ConfigureAwait(false); + using var selectionsLock = await SelectionsLock.AcquireAsync(_selectionLockFile, cancellationToken).ConfigureAwait(false); + var selections = await LoadSelectionsAsync(cancellationToken).ConfigureAwait(false); selections[credential.ProviderName] = accountId; - await SaveSelectionsAsync(selections).ConfigureAwait(false); + await SaveSelectionsAsync(selections, cancellationToken).ConfigureAwait(false); return true; } /// - public async Task GetSelectedCredentialAsync(string providerName) + public async Task GetSelectedCredentialAsync(string providerName, CancellationToken cancellationToken = default) { ValidateProviderName(providerName); - var selections = await LoadSelectionsAsync().ConfigureAwait(false); + var selections = await LoadSelectionsAsync(cancellationToken).ConfigureAwait(false); if (!selections.TryGetValue(providerName, out var selectedId) || !IsValidAccountId(selectedId)) { // A non-GUID id can only land here via tampered selections.json; @@ -240,16 +241,16 @@ public async Task SelectCredentialAsync(string accountId) return null; } - return await ReadAndDecryptByIdAsync(providerName, selectedId).ConfigureAwait(false); + return await ReadAndDecryptByIdAsync(providerName, selectedId, cancellationToken).ConfigureAwait(false); } /// - public async Task GetCredentialByIdAsync(string providerName, string accountId) + public async Task GetCredentialByIdAsync(string providerName, string accountId, CancellationToken cancellationToken = default) { ValidateProviderName(providerName); ValidateAccountId(accountId); - return await ReadAndDecryptByIdAsync(providerName, accountId).ConfigureAwait(false); + return await ReadAndDecryptByIdAsync(providerName, accountId, cancellationToken).ConfigureAwait(false); } /// @@ -261,7 +262,7 @@ public async Task SelectCredentialAsync(string accountId) /// the account id from selections.json) and /// (which takes it directly). /// - private async Task ReadAndDecryptByIdAsync(string providerName, string accountId) + private async Task ReadAndDecryptByIdAsync(string providerName, string accountId, CancellationToken cancellationToken) { var fileName = $"{providerName.ToLowerInvariant()}_{accountId}.json"; var filePath = Path.Join(_credentialsDirectory, fileName); @@ -273,7 +274,7 @@ public async Task SelectCredentialAsync(string accountId) StoredCredential? credential; try { - var content = await File.ReadAllTextAsync(filePath).ConfigureAwait(false); + var content = await File.ReadAllTextAsync(filePath, cancellationToken).ConfigureAwait(false); credential = JsonSerializer.Deserialize(content, _jsonOptions); } catch (JsonException) @@ -289,14 +290,14 @@ public async Task SelectCredentialAsync(string accountId) /// credentials directory for *_{accountId}.json. AccountIds are /// GUIDs so the pattern is expected to match at most one file. /// - private async Task<(string FilePath, StoredCredential Credential)?> FindCredentialByAccountIdAsync(string accountId) + private async Task<(string FilePath, StoredCredential Credential)?> FindCredentialByAccountIdAsync(string accountId, CancellationToken cancellationToken) { var matches = Directory.GetFiles(_credentialsDirectory, $"*_{accountId}.json"); foreach (var file in matches) { try { - var content = await File.ReadAllTextAsync(file).ConfigureAwait(false); + var content = await File.ReadAllTextAsync(file, cancellationToken).ConfigureAwait(false); var credential = JsonSerializer.Deserialize(content, _jsonOptions); if (credential is not null && credential.AccountId.Equals(accountId, StringComparison.OrdinalIgnoreCase)) @@ -314,7 +315,7 @@ public async Task SelectCredentialAsync(string accountId) } /// - public async Task> GetProviderNamesAsync() + public async Task> GetProviderNamesAsync(CancellationToken cancellationToken = default) { var credentialFiles = Directory.GetFiles(_credentialsDirectory, "*.json") .Where(f => !f.Equals(_selectionFile, StringComparison.OrdinalIgnoreCase)); @@ -325,7 +326,7 @@ public async Task> GetProviderNamesAsync() { try { - var content = await File.ReadAllTextAsync(file).ConfigureAwait(false); + var content = await File.ReadAllTextAsync(file, cancellationToken).ConfigureAwait(false); var credential = JsonSerializer.Deserialize(content, _jsonOptions); if (!string.IsNullOrWhiteSpace(credential?.ProviderName)) @@ -343,14 +344,14 @@ public async Task> GetProviderNamesAsync() } /// - public async Task> ExportCredentialsAsync() + public async Task> ExportCredentialsAsync(CancellationToken cancellationToken = default) { // Every *.json in the directory except the selection record is a // stored credential; mirrors the glob GetProviderNamesAsync uses. var credentialFiles = Directory.GetFiles(_credentialsDirectory, "*.json") .Where(f => !f.Equals(_selectionFile, StringComparison.OrdinalIgnoreCase)); - var selections = await LoadSelectionsAsync().ConfigureAwait(false); + var selections = await LoadSelectionsAsync(cancellationToken).ConfigureAwait(false); var exports = new List(); foreach (var file in credentialFiles) @@ -358,7 +359,7 @@ public async Task> ExportCredentialsAsync() StoredCredential? credential; try { - var content = await File.ReadAllTextAsync(file).ConfigureAwait(false); + var content = await File.ReadAllTextAsync(file, cancellationToken).ConfigureAwait(false); credential = JsonSerializer.Deserialize(content, _jsonOptions); } catch (JsonException) @@ -409,7 +410,7 @@ public async Task> ExportCredentialsAsync() } /// - public async Task RestoreCredentialAsync(CredentialExport credential) + public async Task RestoreCredentialAsync(CredentialExport credential, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(credential); @@ -441,14 +442,15 @@ public async Task RestoreCredentialAsync(CredentialExport credential) await AtomicFile.WriteAllTextAsync( filePath, json, - OperatingSystem.IsWindows() ? null : UnixFileMode.UserRead | UnixFileMode.UserWrite).ConfigureAwait(false); + OperatingSystem.IsWindows() ? null : UnixFileMode.UserRead | UnixFileMode.UserWrite, + cancellationToken).ConfigureAwait(false); if (credential.IsSelected) { - using var selectionsLock = await SelectionsLock.AcquireAsync(_selectionLockFile).ConfigureAwait(false); - var selections = await LoadSelectionsAsync().ConfigureAwait(false); + using var selectionsLock = await SelectionsLock.AcquireAsync(_selectionLockFile, cancellationToken).ConfigureAwait(false); + var selections = await LoadSelectionsAsync(cancellationToken).ConfigureAwait(false); selections[credential.ProviderName] = credential.AccountId; - await SaveSelectionsAsync(selections).ConfigureAwait(false); + await SaveSelectionsAsync(selections, cancellationToken).ConfigureAwait(false); } } @@ -509,7 +511,7 @@ private static void ValidateAccountId(string accountId) /// selected. A green tick in `accounts list` alongside "no credential selected" /// from the consumer's authenticate call (#48). /// - private async Task> LoadSelectionsAsync() + private async Task> LoadSelectionsAsync(CancellationToken cancellationToken) { if (!File.Exists(_selectionFile)) { @@ -519,7 +521,7 @@ private async Task> LoadSelectionsAsync() Dictionary? loaded; try { - var content = await File.ReadAllTextAsync(_selectionFile).ConfigureAwait(false); + var content = await File.ReadAllTextAsync(_selectionFile, cancellationToken).ConfigureAwait(false); loaded = JsonSerializer.Deserialize>(content, _jsonOptions); } catch (JsonException) @@ -544,7 +546,7 @@ private async Task> LoadSelectionsAsync() return selections; } - private async Task SaveSelectionsAsync(Dictionary selections) + private async Task SaveSelectionsAsync(Dictionary selections, CancellationToken cancellationToken) { var json = JsonSerializer.Serialize(selections, _jsonOptions); @@ -555,7 +557,8 @@ private async Task SaveSelectionsAsync(Dictionary selections) await AtomicFile.WriteAllTextAsync( _selectionFile, json, - OperatingSystem.IsWindows() ? null : UnixFileMode.UserRead | UnixFileMode.UserWrite).ConfigureAwait(false); + OperatingSystem.IsWindows() ? null : UnixFileMode.UserRead | UnixFileMode.UserWrite, + cancellationToken).ConfigureAwait(false); } private sealed class StoredCredential diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/ICredentialManager.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/ICredentialManager.cs index 12e486c..e53e6d7 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/ICredentialManager.cs +++ b/src/NextIteration.SpectreConsole.Auth/Persistence/ICredentialManager.cs @@ -9,6 +9,19 @@ namespace NextIteration.SpectreConsole.Auth.Persistence /// /// /// + /// Every member accepts a . Implementations backed + /// by an OS secret store must honour it: the Secret Service contract permits an + /// implementation to prompt (to unlock a collection, say) before serving a request, and a + /// synchronous call that cannot be cancelled then blocks with no way out (#74). + /// + /// + /// Honouring the token is not a guarantee that every block is escapable. The libsecret + /// backend wires the token to a GCancellable, which libsecret observes for its own + /// D-Bus work — but a Secret Service implementation that breaks libsecret's prompt + /// machinery can still wedge it beyond the reach of cancellation. See the remarks on + /// LibsecretCredentialManager for a case where exactly that happens. + /// + /// /// Provider names are matched case-insensitively and stored as supplied. /// A credential added as Adobe is found by a lookup for adobe, and /// the spelling the caller used is what @@ -32,7 +45,8 @@ public interface ICredentialManager /// Lists all stored credentials for a specific provider. /// /// The provider (e.g. Adobe). - Task> ListCredentialsAsync(string providerName); + /// Cancels the operation; see the interface remarks. + Task> ListCredentialsAsync(string providerName, CancellationToken cancellationToken = default); /// /// Stores a new credential and returns its generated account ID. @@ -42,28 +56,29 @@ public interface ICredentialManager /// Environment the credential targets (e.g. Production). /// Plaintext JSON payload to encrypt and persist. /// The account ID assigned to the new credential (a GUID). - Task AddCredentialAsync(string providerName, string accountName, string environment, string credentialData); + /// Cancels the operation; see the interface remarks. + Task AddCredentialAsync(string providerName, string accountName, string environment, string credentialData, CancellationToken cancellationToken = default); /// /// Deletes a credential by its account ID, and clears any selection /// that pointed to it. /// /// if the credential existed and was deleted. - Task DeleteCredentialAsync(string accountId); + Task DeleteCredentialAsync(string accountId, CancellationToken cancellationToken = default); /// /// Marks the credential as the active one for its provider. Exactly /// one credential per provider may be selected at a time. /// /// if the credential was found and selected. - Task SelectCredentialAsync(string accountId); + Task SelectCredentialAsync(string accountId, CancellationToken cancellationToken = default); /// /// Returns the decrypted JSON payload of the currently selected /// credential for , or /// if no credential is selected. /// - Task GetSelectedCredentialAsync(string providerName); + Task GetSelectedCredentialAsync(string providerName, CancellationToken cancellationToken = default); /// /// Returns the decrypted JSON payload of a specific credential by @@ -79,13 +94,14 @@ public interface ICredentialManager /// The decrypted JSON payload, or if no /// credential with that account id exists for the given provider. /// - Task GetCredentialByIdAsync(string providerName, string accountId); + /// Cancels the operation; see the interface remarks. + Task GetCredentialByIdAsync(string providerName, string accountId, CancellationToken cancellationToken = default); /// /// Returns the set of provider names that currently have at least /// one stored credential. /// - Task> GetProviderNamesAsync(); + Task> GetProviderNamesAsync(CancellationToken cancellationToken = default); /// /// Enumerates the stored credentials across all providers whose payload can @@ -117,7 +133,7 @@ public interface ICredentialManager /// , which reads metadata only. /// /// - Task> ExportCredentialsAsync(); + Task> ExportCredentialsAsync(CancellationToken cancellationToken = default); /// /// Recreates a credential from an export record, preserving its @@ -139,7 +155,8 @@ public interface ICredentialManager /// restored item's is not /// preserved there. /// - Task RestoreCredentialAsync(CredentialExport credential); + /// Cancels the operation; see the interface remarks. + Task RestoreCredentialAsync(CredentialExport credential, CancellationToken cancellationToken = default); } /// diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/Keychain/KeychainCredentialManager.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/Keychain/KeychainCredentialManager.cs index 20d1981..17a856a 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/Keychain/KeychainCredentialManager.cs +++ b/src/NextIteration.SpectreConsole.Auth/Persistence/Keychain/KeychainCredentialManager.cs @@ -59,8 +59,9 @@ public KeychainCredentialManager( } /// - public Task AddCredentialAsync(string providerName, string accountName, string environment, string credentialData) + public Task AddCredentialAsync(string providerName, string accountName, string environment, string credentialData, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); ValidateProviderName(providerName); var accountId = Guid.NewGuid().ToString(); @@ -110,8 +111,9 @@ private void ConfirmItemVisible(string service, string account) } /// - public Task> ListCredentialsAsync(string providerName) + public Task> ListCredentialsAsync(string providerName, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); ValidateProviderName(providerName); providerName = ResolveStoredProviderName(providerName); var service = ServiceFor(providerName); @@ -146,8 +148,9 @@ public Task> ListCredentialsAsync(string provider } /// - public Task DeleteCredentialAsync(string accountId) + public Task DeleteCredentialAsync(string accountId, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); if (!IsValidAccountId(accountId)) { return Task.FromResult(false); @@ -176,8 +179,9 @@ public Task DeleteCredentialAsync(string accountId) } /// - public Task SelectCredentialAsync(string accountId) + public Task SelectCredentialAsync(string accountId, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); if (!IsValidAccountId(accountId)) { return Task.FromResult(false); @@ -200,8 +204,9 @@ public Task SelectCredentialAsync(string accountId) } /// - public Task GetSelectedCredentialAsync(string providerName) + public Task GetSelectedCredentialAsync(string providerName, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); ValidateProviderName(providerName); providerName = ResolveStoredProviderName(providerName); var selectedId = ReadSelection(providerName); @@ -214,8 +219,9 @@ public Task SelectCredentialAsync(string accountId) } /// - public Task GetCredentialByIdAsync(string providerName, string accountId) + public Task GetCredentialByIdAsync(string providerName, string accountId, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); ValidateProviderName(providerName); providerName = ResolveStoredProviderName(providerName); ValidateAccountId(accountId); @@ -244,8 +250,9 @@ public Task SelectCredentialAsync(string accountId) } /// - public Task> GetProviderNamesAsync() + public Task> GetProviderNamesAsync(CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); // Query every generic-password item owned by this app and distinct // the provider portion out of the service string. var items = QueryAllItemsForApp(includeData: false); @@ -262,8 +269,9 @@ public Task> GetProviderNamesAsync() } /// - public Task> ExportCredentialsAsync() + public Task> ExportCredentialsAsync(CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); var items = QueryAllItemsForApp(includeData: true); var selectionCache = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -320,8 +328,9 @@ public Task> ExportCredentialsAsync() } /// - public Task RestoreCredentialAsync(CredentialExport credential) + public Task RestoreCredentialAsync(CredentialExport credential, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); ArgumentNullException.ThrowIfNull(credential); ValidateProviderName(credential.ProviderName); ValidateAccountId(credential.AccountId); diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs index 9a6d8e8..3f5944f 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs +++ b/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs @@ -21,10 +21,20 @@ namespace NextIteration.SpectreConsole.Auth.Persistence.Libsecret /// tested and fails unattended in two ways (#19): it does not provide the /// session collection GNOME Keyring offers, so selecting it fails with /// No such object path '/org/freedesktop/secrets/aliases/session'; and against - /// the default collection it raises a wallet-unlock prompt, which with no GUI blocks - /// the calling thread forever because the synchronous libsecret entry points here are - /// invoked with a NULL GCancellable. Other Secret Service implementations are - /// untested. + /// the default collection it raises a wallet-unlock prompt which, with no GUI, never + /// completes. + /// + /// + /// That second failure is not reachable by cancellation, and is worth recording + /// precisely. ksecretd emits the prompt's Completed signal with an ao + /// payload where libsecret expects o, so libsecret logs + /// received unexpected result type ao from Completed signal and its + /// secret_service_real_prompt_async GTask is then + /// finalized without ever returning. The nested main loop the synchronous wrapper + /// runs therefore never exits. Cancelling the GCancellable does fire — verified — + /// but cannot rescue a task libsecret has already abandoned. It is an upstream + /// incompatibility between libsecret and KWallet's shim, not something this backend can + /// work around. /// /// /// Requires a running Secret Service daemon. Headless containers and @@ -81,7 +91,7 @@ public LibsecretCredentialManager( } /// - public Task AddCredentialAsync(string providerName, string accountName, string environment, string credentialData) + public Task AddCredentialAsync(string providerName, string accountName, string environment, string credentialData, CancellationToken cancellationToken = default) { ValidateProviderName(providerName); var accountId = Guid.NewGuid().ToString(); @@ -98,13 +108,13 @@ public Task AddCredentialAsync(string providerName, string accountName, }; var label = $"{_appIdentifier}: {providerName}/{accountName}"; - StoreItem(attrs, label, credentialData); + StoreItem(cancellationToken, attrs, label, credentialData); // A just-stored Secret Service item isn't always immediately visible // to a follow-up search under concurrent access, so a caller doing // add-then-select/delete can race the item's appearance. Confirm it's // queryable before returning so that can't happen. - ConfirmItemVisible(providerName, accountId); + ConfirmItemVisible(providerName, accountId, cancellationToken); return Task.FromResult(accountId); } @@ -115,12 +125,12 @@ public Task AddCredentialAsync(string providerName, string accountName, /// returns after a bounded wait even if the item never appears, leaving /// any genuine failure to the caller's own lookup. /// - private void ConfirmItemVisible(string providerName, string accountId) + private void ConfirmItemVisible(string providerName, string accountId, CancellationToken cancellationToken) { const int maxAttempts = 20; for (var attempt = 1; attempt <= maxAttempts; attempt++) { - if (LookupCredentialByAccountId(providerName, accountId) is not null) + if (LookupCredentialByAccountId(providerName, accountId, cancellationToken) is not null) { return; } @@ -133,13 +143,13 @@ private void ConfirmItemVisible(string providerName, string accountId) } /// - public Task> ListCredentialsAsync(string providerName) + public Task> ListCredentialsAsync(string providerName, CancellationToken cancellationToken = default) { ValidateProviderName(providerName); - providerName = ResolveStoredProviderName(providerName); + providerName = ResolveStoredProviderName(providerName, cancellationToken); _summaryProviders.TryGetValue(providerName, out var summaryProvider); - var selectedId = ReadSelection(providerName); + var selectedId = ReadSelection(providerName, cancellationToken); var items = SearchItems( new Dictionary(StringComparer.Ordinal) { @@ -147,7 +157,7 @@ public Task> ListCredentialsAsync(string provider [AttrKind] = KindCredential, [AttrProvider] = providerName, }, - loadSecrets: summaryProvider is not null); + loadSecrets: summaryProvider is not null, cancellationToken); var result = items .Select(i => new CredentialSummary @@ -176,7 +186,7 @@ public Task> ListCredentialsAsync(string provider } /// - public Task DeleteCredentialAsync(string accountId) + public Task DeleteCredentialAsync(string accountId, CancellationToken cancellationToken = default) { if (!IsValidAccountId(accountId)) { @@ -191,14 +201,14 @@ public Task DeleteCredentialAsync(string accountId) [AttrKind] = KindCredential, [AttrAccount] = accountId, }, - loadSecrets: false).FirstOrDefault(); + loadSecrets: false, cancellationToken).FirstOrDefault(); if (match is null) { return Task.FromResult(false); } - var removed = ClearItem(new Dictionary(StringComparer.Ordinal) + var removed = ClearItem(cancellationToken, new Dictionary(StringComparer.Ordinal) { [AttrApp] = _appIdentifier, [AttrKind] = KindCredential, @@ -219,10 +229,10 @@ public Task DeleteCredentialAsync(string accountId) var providerName = match.Attributes.GetValueOrDefault(AttrProvider); if (providerName is not null) { - var selected = ReadSelection(providerName); + var selected = ReadSelection(providerName, cancellationToken); if (string.Equals(selected, accountId, StringComparison.OrdinalIgnoreCase)) { - ClearSelection(providerName); + ClearSelection(providerName, cancellationToken); } } @@ -230,7 +240,7 @@ public Task DeleteCredentialAsync(string accountId) } /// - public Task SelectCredentialAsync(string accountId) + public Task SelectCredentialAsync(string accountId, CancellationToken cancellationToken = default) { if (!IsValidAccountId(accountId)) { @@ -244,7 +254,7 @@ public Task SelectCredentialAsync(string accountId) [AttrKind] = KindCredential, [AttrAccount] = accountId, }, - loadSecrets: false).FirstOrDefault(); + loadSecrets: false, cancellationToken).FirstOrDefault(); if (match is null) { @@ -257,32 +267,32 @@ public Task SelectCredentialAsync(string accountId) return Task.FromResult(false); } - WriteSelection(providerName, accountId); + WriteSelection(providerName, accountId, cancellationToken); return Task.FromResult(true); } /// - public Task GetSelectedCredentialAsync(string providerName) + public Task GetSelectedCredentialAsync(string providerName, CancellationToken cancellationToken = default) { ValidateProviderName(providerName); - providerName = ResolveStoredProviderName(providerName); - var selectedId = ReadSelection(providerName); + providerName = ResolveStoredProviderName(providerName, cancellationToken); + var selectedId = ReadSelection(providerName, cancellationToken); if (selectedId is null) { return Task.FromResult(null); } - return Task.FromResult(LookupCredentialByAccountId(providerName, selectedId)); + return Task.FromResult(LookupCredentialByAccountId(providerName, selectedId, cancellationToken)); } /// - public Task GetCredentialByIdAsync(string providerName, string accountId) + public Task GetCredentialByIdAsync(string providerName, string accountId, CancellationToken cancellationToken = default) { ValidateProviderName(providerName); - providerName = ResolveStoredProviderName(providerName); + providerName = ResolveStoredProviderName(providerName, cancellationToken); ValidateAccountId(accountId); - return Task.FromResult(LookupCredentialByAccountId(providerName, accountId)); + return Task.FromResult(LookupCredentialByAccountId(providerName, accountId, cancellationToken)); } /// @@ -291,9 +301,9 @@ public Task SelectCredentialAsync(string accountId) /// and — neither modifies the /// selection record. /// - private string? LookupCredentialByAccountId(string providerName, string accountId) + private string? LookupCredentialByAccountId(string providerName, string accountId, CancellationToken cancellationToken) { - return LookupPassword(new Dictionary(StringComparer.Ordinal) + return LookupPassword(cancellationToken, new Dictionary(StringComparer.Ordinal) { [AttrApp] = _appIdentifier, [AttrKind] = KindCredential, @@ -303,7 +313,7 @@ public Task SelectCredentialAsync(string accountId) } /// - public Task> GetProviderNamesAsync() + public Task> GetProviderNamesAsync(CancellationToken cancellationToken = default) { var items = SearchItems( new Dictionary(StringComparer.Ordinal) @@ -311,7 +321,7 @@ public Task> GetProviderNamesAsync() [AttrApp] = _appIdentifier, [AttrKind] = KindCredential, }, - loadSecrets: false); + loadSecrets: false, cancellationToken); var names = items .Select(i => i.Attributes.GetValueOrDefault(AttrProvider)) @@ -324,7 +334,7 @@ public Task> GetProviderNamesAsync() } /// - public Task> ExportCredentialsAsync() + public Task> ExportCredentialsAsync(CancellationToken cancellationToken = default) { var items = SearchItems( new Dictionary(StringComparer.Ordinal) @@ -332,7 +342,7 @@ public Task> ExportCredentialsAsync() [AttrApp] = _appIdentifier, [AttrKind] = KindCredential, }, - loadSecrets: true); + loadSecrets: true, cancellationToken); var selectionCache = new Dictionary(StringComparer.OrdinalIgnoreCase); @@ -366,7 +376,7 @@ public Task> ExportCredentialsAsync() if (!selectionCache.TryGetValue(providerName, out var selectedId)) { - selectedId = ReadSelection(providerName); + selectedId = ReadSelection(providerName, cancellationToken); selectionCache[providerName] = selectedId; } @@ -388,7 +398,7 @@ public Task> ExportCredentialsAsync() } /// - public Task RestoreCredentialAsync(CredentialExport credential) + public Task RestoreCredentialAsync(CredentialExport credential, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(credential); ValidateProviderName(credential.ProviderName); @@ -410,11 +420,11 @@ public Task RestoreCredentialAsync(CredentialExport credential) }; var label = $"{_appIdentifier}: {credential.ProviderName}/{credential.AccountName}"; - StoreItem(attrs, label, credential.CredentialData); + StoreItem(cancellationToken, attrs, label, credential.CredentialData); if (credential.IsSelected) { - WriteSelection(credential.ProviderName, credential.AccountId); + WriteSelection(credential.ProviderName, credential.AccountId, cancellationToken); } return Task.CompletedTask; @@ -424,9 +434,9 @@ public Task RestoreCredentialAsync(CredentialExport credential) // Internal helpers // ========================= - private string? ReadSelection(string providerName) + private string? ReadSelection(string providerName, CancellationToken cancellationToken) { - return LookupPassword(new Dictionary(StringComparer.Ordinal) + return LookupPassword(cancellationToken, new Dictionary(StringComparer.Ordinal) { [AttrApp] = _appIdentifier, [AttrKind] = KindSelection, @@ -434,11 +444,12 @@ public Task RestoreCredentialAsync(CredentialExport credential) }); } - private void WriteSelection(string providerName, string accountId) + private void WriteSelection(string providerName, string accountId, CancellationToken cancellationToken) { // store overwrites an existing item with matching attributes, so // we don't need a separate add-or-update dance. StoreItem( + cancellationToken, new Dictionary(StringComparer.Ordinal) { [AttrApp] = _appIdentifier, @@ -449,12 +460,12 @@ private void WriteSelection(string providerName, string accountId) password: accountId); } - private void ClearSelection(string providerName) + private void ClearSelection(string providerName, CancellationToken cancellationToken) { // Discard deliberately: clearing a selection that was never recorded is a // no-op, not a failure. Only DeleteCredentialAsync needs the result, because // there "nothing removed" means the credential is still stored. - _ = ClearItem(new Dictionary(StringComparer.Ordinal) + _ = ClearItem(cancellationToken, new Dictionary(StringComparer.Ordinal) { [AttrApp] = _appIdentifier, [AttrKind] = KindSelection, @@ -490,7 +501,7 @@ private static DateTime ParseCreatedAt(string? value) /// the stored spelling is used for the query. Falls back to the caller's spelling /// when nothing matches, so a genuine miss still behaves as a miss (#49). /// - private string ResolveStoredProviderName(string providerName) + private string ResolveStoredProviderName(string providerName, CancellationToken cancellationToken) { var items = SearchItems( new Dictionary(StringComparer.Ordinal) @@ -498,7 +509,7 @@ private string ResolveStoredProviderName(string providerName) [AttrApp] = _appIdentifier, [AttrKind] = KindCredential, }, - loadSecrets: false); + loadSecrets: false, cancellationToken); foreach (var item in items) { @@ -513,6 +524,69 @@ private string ResolveStoredProviderName(string providerName) return providerName; } + /// + /// Binds a to a GCancellable for the lifetime of one + /// libsecret call, so a blocked synchronous call can actually be made to return. + /// + /// + /// The Secret Service contract lets an implementation prompt — to unlock a collection, + /// say — before serving a request, and the synchronous entry points here block until + /// that prompt is answered. With no GUI nothing answers it. Passing NULL for the + /// cancellable, as this backend used to, made that unrecoverable: KWallet's shim wedged + /// a process so completely that SIGINT would not end it, only SIGKILL (#74). + /// + /// g_cancellable_cancel is thread-safe, so the token callback can fire it from + /// another thread while this one is blocked inside libsecret, and the call returns with + /// a cancelled GError. + /// + /// + private sealed class CancelScope : IDisposable + { + private readonly CancellationTokenRegistration _registration; + + internal CancelScope(CancellationToken cancellationToken) + { + Handle = g_cancellable_new(); + if (Handle == IntPtr.Zero) + { + throw new InvalidOperationException("g_cancellable_new failed."); + } + + // Registering an already-cancelled token invokes the callback immediately, + // so a pre-cancelled call is cancelled before libsecret is even entered. + _registration = cancellationToken.Register( + static state => g_cancellable_cancel((IntPtr)state!), Handle); + } + + internal IntPtr Handle { get; } + + public void Dispose() + { + _registration.Dispose(); + g_object_unref(Handle); + } + } + + /// + /// Frees and throws — + /// when the caller cancelled, otherwise the usual wrapped GError. + /// + /// + /// Cancellation is checked first on purpose. libsecret reports a cancelled call as a + /// GError, and reporting that as a generic "operation failed" would hide the fact that + /// the caller asked for it. + /// + private static void ThrowIfCancelledOrGError(IntPtr error, string operation, CancellationToken cancellationToken) + { + if (error != IntPtr.Zero && cancellationToken.IsCancellationRequested) + { + g_error_free(error); + cancellationToken.ThrowIfCancellationRequested(); + } + + ThrowIfGError(error, operation); + } + private static void ValidateProviderName(string providerName) { ArgumentException.ThrowIfNullOrWhiteSpace(providerName); @@ -549,20 +623,21 @@ private sealed class StoredItem public string? Secret { get; init; } } - private void StoreItem(Dictionary attributes, string label, string password) + private void StoreItem(CancellationToken cancellationToken, Dictionary attributes, string label, string password) { var attrs = NewAttributes(attributes); try { + using var cancel = new CancelScope(cancellationToken); var status = secret_password_storev_sync( IntPtr.Zero, attrs, _collection, label, password, - IntPtr.Zero, + cancel.Handle, out var error); - ThrowIfGError(error, "secret_password_storev_sync"); + ThrowIfCancelledOrGError(error, "secret_password_storev_sync", cancellationToken); if (status == 0) { throw new InvalidOperationException("secret_password_storev_sync returned FALSE without a GError — the Secret Service may not be available."); @@ -574,13 +649,14 @@ private void StoreItem(Dictionary attributes, string label, stri } } - private static string? LookupPassword(Dictionary attributes) + private static string? LookupPassword(CancellationToken cancellationToken, Dictionary attributes) { var attrs = NewAttributes(attributes); try { - var result = secret_password_lookupv_sync(IntPtr.Zero, attrs, IntPtr.Zero, out var error); - ThrowIfGError(error, "secret_password_lookupv_sync"); + using var cancel = new CancelScope(cancellationToken); + var result = secret_password_lookupv_sync(IntPtr.Zero, attrs, cancel.Handle, out var error); + ThrowIfCancelledOrGError(error, "secret_password_lookupv_sync", cancellationToken); if (result == IntPtr.Zero) { return null; @@ -617,13 +693,14 @@ private void StoreItem(Dictionary attributes, string label, stri /// still in the keyring — a user revoking a leaked secret was told it was gone /// (#45). /// - private static bool ClearItem(Dictionary attributes) + private static bool ClearItem(CancellationToken cancellationToken, Dictionary attributes) { var attrs = NewAttributes(attributes); try { - var removed = secret_password_clearv_sync(IntPtr.Zero, attrs, IntPtr.Zero, out var error); - ThrowIfGError(error, "secret_password_clearv_sync"); + using var cancel = new CancelScope(cancellationToken); + var removed = secret_password_clearv_sync(IntPtr.Zero, attrs, cancel.Handle, out var error); + ThrowIfCancelledOrGError(error, "secret_password_clearv_sync", cancellationToken); return removed != 0; } finally @@ -632,14 +709,15 @@ private static bool ClearItem(Dictionary attributes) } } - private static List SearchItems(Dictionary attributes, bool loadSecrets) + private static List SearchItems(Dictionary attributes, bool loadSecrets, CancellationToken cancellationToken) { var attrs = NewAttributes(attributes); var flags = SecretSearchAll | (loadSecrets ? SecretSearchLoadSecrets | SecretSearchUnlock : 0); try { - var listPtr = secret_password_searchv_sync(IntPtr.Zero, attrs, flags, IntPtr.Zero, out var error); - ThrowIfGError(error, "secret_password_searchv_sync"); + using var cancel = new CancelScope(cancellationToken); + var listPtr = secret_password_searchv_sync(IntPtr.Zero, attrs, flags, cancel.Handle, out var error); + ThrowIfCancelledOrGError(error, "secret_password_searchv_sync", cancellationToken); var results = new List(); if (listPtr == IntPtr.Zero) @@ -668,8 +746,8 @@ private static List SearchItems(Dictionary attribute string? secret = null; if (loadSecrets) { - var secretValue = secret_retrievable_retrieve_secret_sync(retrievable, IntPtr.Zero, out var secretError); - ThrowIfGError(secretError, "secret_retrievable_retrieve_secret_sync"); + var secretValue = secret_retrievable_retrieve_secret_sync(retrievable, cancel.Handle, out var secretError); + ThrowIfCancelledOrGError(secretError, "secret_retrievable_retrieve_secret_sync", cancellationToken); try { secret = ReadSecretValueAsString(secretValue); diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretInterop.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretInterop.cs index 3cb9ea3..b58ed34 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretInterop.cs +++ b/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretInterop.cs @@ -66,6 +66,20 @@ internal static partial IntPtr g_hash_table_new_full( [LibraryImport(Libglib)] internal static partial void g_list_free(IntPtr list); + private const string Libgio = "libgio-2.0.so.0"; + + /// Creates a GCancellable. Release with . + [LibraryImport(Libgio)] + internal static partial IntPtr g_cancellable_new(); + + /// + /// Cancels a GCancellable. Thread-safe by contract, which is the whole point: it is + /// called from the cancellation-token callback while another thread sits blocked + /// inside a synchronous libsecret call, and it is what makes that call return. + /// + [LibraryImport(Libgio)] + internal static partial void g_cancellable_cancel(IntPtr cancellable); + /// /// Frees a GList and each node's data, using . /// diff --git a/src/NextIteration.SpectreConsole.Auth/Portability/CredentialPortabilityService.cs b/src/NextIteration.SpectreConsole.Auth/Portability/CredentialPortabilityService.cs index eb4decc..f3ae5ba 100644 --- a/src/NextIteration.SpectreConsole.Auth/Portability/CredentialPortabilityService.cs +++ b/src/NextIteration.SpectreConsole.Auth/Portability/CredentialPortabilityService.cs @@ -52,16 +52,16 @@ internal CredentialPortabilityService(ICredentialManager manager) /// Reads every stored credential and returns it as a passphrase-encrypted /// archive. /// - internal async Task ExportAsync(string passphrase) + internal async Task ExportAsync(string passphrase, CancellationToken cancellationToken = default) { - var credentials = await _manager.ExportCredentialsAsync().ConfigureAwait(false); + var credentials = await _manager.ExportCredentialsAsync(cancellationToken).ConfigureAwait(false); var bundle = CredentialArchive.Serialize(credentials, passphrase); // The backend drops credentials it cannot decrypt, so the archive may // hold fewer than the store does. Derive the difference from the // metadata listing rather than widening the public // ICredentialManager.ExportCredentialsAsync signature to report it. - var stored = await CountStoredCredentialsAsync().ConfigureAwait(false); + var stored = await CountStoredCredentialsAsync(cancellationToken).ConfigureAwait(false); return new ExportResult(bundle, credentials.Count, Math.Max(0, stored - credentials.Count)); } @@ -78,15 +78,17 @@ internal async Task ExportAsync(string passphrase) /// resolve it. Invoked only when there is an actual conflict. The existing /// side is metadata only — see . /// + /// Cancels the import. internal async Task ImportAsync( string bundle, string passphrase, - Func resolveConflict) + Func resolveConflict, + CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(resolveConflict); var incoming = CredentialArchive.Deserialize(bundle, passphrase); - var existingByKey = await BuildConflictIndexAsync().ConfigureAwait(false); + var existingByKey = await BuildConflictIndexAsync(cancellationToken).ConfigureAwait(false); var added = 0; var overwritten = 0; @@ -105,13 +107,13 @@ internal async Task ImportAsync( // Overwrite: drop the existing credential (which may carry a // different account id) before restoring the incoming one. - _ = await _manager.DeleteCredentialAsync(match.AccountId).ConfigureAwait(false); - await _manager.RestoreCredentialAsync(entry).ConfigureAwait(false); + _ = await _manager.DeleteCredentialAsync(match.AccountId, cancellationToken).ConfigureAwait(false); + await _manager.RestoreCredentialAsync(entry, cancellationToken).ConfigureAwait(false); overwritten++; } else { - await _manager.RestoreCredentialAsync(entry).ConfigureAwait(false); + await _manager.RestoreCredentialAsync(entry, cancellationToken).ConfigureAwait(false); added++; } } @@ -132,13 +134,13 @@ internal async Task ImportAsync( /// a single credential the local keystore cannot open aborted the entire /// import, which is exactly the store an import is meant to repair. /// - private async Task> BuildConflictIndexAsync() + private async Task> BuildConflictIndexAsync(CancellationToken cancellationToken) { var index = new Dictionary(StringComparer.Ordinal); - foreach (var provider in await _manager.GetProviderNamesAsync().ConfigureAwait(false)) + foreach (var provider in await _manager.GetProviderNamesAsync(cancellationToken).ConfigureAwait(false)) { - foreach (var summary in await _manager.ListCredentialsAsync(provider).ConfigureAwait(false)) + foreach (var summary in await _manager.ListCredentialsAsync(provider, cancellationToken).ConfigureAwait(false)) { // Duplicates on the same key are unusual but possible; keep the // first as the conflict target. @@ -155,12 +157,12 @@ private async Task> BuildConflictIndexAsyn /// Counts what the store holds, from metadata only, so an export can report /// how many credentials it had to leave out. /// - private async Task CountStoredCredentialsAsync() + private async Task CountStoredCredentialsAsync(CancellationToken cancellationToken) { var count = 0; - foreach (var provider in await _manager.GetProviderNamesAsync().ConfigureAwait(false)) + foreach (var provider in await _manager.GetProviderNamesAsync(cancellationToken).ConfigureAwait(false)) { - count += (await _manager.ListCredentialsAsync(provider).ConfigureAwait(false)).Count(); + count += (await _manager.ListCredentialsAsync(provider, cancellationToken).ConfigureAwait(false)).Count(); } return count; diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/NextIteration.SpectreConsole.Auth.Tests.csproj b/tests/NextIteration.SpectreConsole.Auth.Tests/NextIteration.SpectreConsole.Auth.Tests.csproj index 1fc1ef4..4803c92 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/NextIteration.SpectreConsole.Auth.Tests.csproj +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/NextIteration.SpectreConsole.Auth.Tests.csproj @@ -19,6 +19,14 @@ CA2007 (ConfigureAwait) doesn't apply in test contexts; there is no SynchronizationContext to recapture. + xUnit1051 (pass TestContext.Current.CancellationToken) fires at every call + into ICredentialManager now that its members accept a CancellationToken — + 530 sites. The analyzer's benefit is making a cancelled test run stop + promptly; these tests are sub-second and the platform tears them down + anyway, so threading a token through every call would add noise far out of + proportion to that. Suppressed deliberately, not by accident. New tests + that specifically exercise cancellation pass a real token explicitly. + IDE0005 (remove unnecessary usings) only runs at build when GenerateDocumentationFile is true — which a test project sets to false (above). With EnforceCodeStyleInBuild on (STANDARD.md 1.2.1), @@ -27,7 +35,7 @@ IDE0005 still gates the shipping project. STANDARD.md 2.7 needs the matching amendment estate-wide. --> - $(NoWarn);CA1707;CA1515;CA2007;IDE0005 + $(NoWarn);CA1707;CA1515;CA2007;IDE0005;xUnit1051 diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs index bb1e34d..74730bd 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs @@ -837,6 +837,28 @@ public async Task SelectCredentialAsync_ReplacingAnExistingSelection_LeavesExact Assert.False(listed.Single(c => c.AccountId == prod).IsSelected); } + [Fact] + public async Task Operations_HonourAnAlreadyCancelledToken() + { + using var temp = new TempDir(); + var manager = CreateManager(temp.Path); + var id = await manager.AddCredentialAsync("Adobe", "prod", "Production", "{}"); + + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + // Every member takes a token as of 2.0.0 (#74); a cancelled one must surface as + // cancellation rather than being silently ignored. + _ = await Assert.ThrowsAnyAsync( + () => manager.ListCredentialsAsync("Adobe", cts.Token)); + _ = await Assert.ThrowsAnyAsync( + () => manager.GetCredentialByIdAsync("Adobe", id, cts.Token)); + _ = await Assert.ThrowsAnyAsync( + () => manager.ExportCredentialsAsync(cts.Token)); + _ = await Assert.ThrowsAnyAsync( + () => manager.AddCredentialAsync("Adobe", "x", "Production", "{}", cts.Token)); + } + /// /// Minimal summary provider used only to verify that /// routes diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/LibsecretCredentialManagerTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/LibsecretCredentialManagerTests.cs index cc04425..d556be9 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/LibsecretCredentialManagerTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/LibsecretCredentialManagerTests.cs @@ -615,6 +615,24 @@ public async Task SelectCredentialAsync_ReplacingAnExistingSelection_LeavesExact Assert.False(listed.Single(c => c.AccountId == prod).IsSelected); } + [Fact] + [SupportedOSPlatform("linux")] + public async Task Operations_HonourAnAlreadyCancelledToken() + { + Assert.SkipWhen(_skip, SkipReason); + + var manager = NewManager(); + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + // The GCancellable is created from the token, so an already-cancelled token is + // cancelled before libsecret is entered (#74). + _ = await Assert.ThrowsAnyAsync( + () => manager.AddCredentialAsync("Adobe", "prod", "Production", "{}", cts.Token)); + _ = await Assert.ThrowsAnyAsync( + () => manager.ListCredentialsAsync("Adobe", cts.Token)); + } + private sealed class FakeAdobeSummaryProvider : ICredentialSummaryProvider { public string ProviderName => "Adobe";