From 319a96629f47b3e74b4ffc7d2d3d49b8891df2a9 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 11:16:29 -0600 Subject: [PATCH 1/9] fix(enrollment): keep the stored pairing until the new device is bound Enrollment saved the new pairing and session as soon as the server accepted the code, then bound the device. When the bind was refused or the connection dropped, the previous pairing had already been overwritten while the log said "any existing pairing is unchanged". The device is now bound and the identity checked first; the pairing is saved, the connection state reset and the session activated only after that. A failed bind costs one enrollment code and leaves both protected files untouched. The failure message says so only when it is true. Test: BindFailureAfterAcceptedEnrollmentKeepsThePreviousPairing (refused and connection-lost cases) fails on the old order at the pairing-file comparison and passes now; the full Windows suite passes (379). Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- .../AgentRuntime.Enrollment.cs | 28 +++++++--- .../AgentNewPairingTests.cs | 51 +++++++++++++++++++ .../SessionFixture.cs | 7 ++- 3 files changed, 77 insertions(+), 9 deletions(-) diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs index 120b457..5246627 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs @@ -15,10 +15,16 @@ namespace Qqq.PrintAgent.Windows; /// public sealed partial class AgentRuntime { - /// Exchanges the enrollment code for a device credential using the live user session on the selected server. Authentication changes are serialized by a gate, the identity is checked again after binding, and the old connection state is reset first so nothing from the previous pairing survives. + /// + /// Exchanges the enrollment code for a device credential using the live user session on the selected server, binds the + /// new device to that session and checks the identity again. Only then is the stored pairing replaced and the connection + /// state reset, so a refused or interrupted bind leaves the previous pairing and session exactly as they were. + /// Authentication changes are serialized by a gate. + /// private async Task EnrollNativeAsync(PipeRequest request, CancellationToken token) { await _authenticationGate.WaitAsync(token).ConfigureAwait(false); + bool pairingSaved = false; try { WriteLog("INFO", $"Enrollment requested on {request.ServerUrl ?? "(no server)"} with a {(request.EnrollmentCode ?? "").Length}-character enrollment code (never logged), computer name {Environment.MachineName}, agent {PrintApiClient.AgentVersion}."); @@ -33,16 +39,21 @@ public sealed partial class AgentRuntime using var api = PrintApiClient.ForSession(session); WriteLog("INFO", "Step 2 of 4: exchanging the enrollment code for a device credential."); EnrollmentResponse response = await api.EnrollAsync(new EnrollmentRequest(request.EnrollmentCode ?? "", Environment.MachineName, PrintApiClient.AgentVersion), token).ConfigureAwait(false); - WriteLog("INFO", $"Enrollment accepted: device {response.DeviceId}, poll interval {response.PollIntervalSeconds} s. Saving the pairing to protected storage."); - using AgentStore store = new(directory, recoverUnreadableConfiguration: true); - store.SaveConfiguration(new AgentConfiguration(request.ServerUrl, response.DeviceId, response.DeviceToken, response.PollIntervalSeconds)); + WriteLog("INFO", $"Enrollment accepted: device {response.DeviceId}, poll interval {response.PollIntervalSeconds} s."); QqqSessionBinding expected = identity with { DeviceId = response.DeviceId }; - store.SaveAuthentication(new AgentAuthentication(expected, session.ExportCredential())); - ResetConnectionState(); WriteLog("INFO", "Step 3 of 4: binding the new device to the session and confirming the identity."); QqqSessionBinding actual = await session.BindDeviceAsync(new QqqDeviceCredential(response.DeviceId, response.DeviceToken), cancellationToken: token).ConfigureAwait(false); RequireSameIdentity(expected, actual); - WriteLog("INFO", "Step 4 of 4: activating the session for printing."); + //////////////////////////////////////////////////////////////////////////// + // the stored pairing is replaced only after the server bound the device // + // and the identity matched: until here a failure costs the operator one // + // enrollment code, never the pairing that was already working // + //////////////////////////////////////////////////////////////////////////// + WriteLog("INFO", "Step 4 of 4: saving the pairing to protected storage and activating the session for printing."); + using AgentStore store = new(directory, recoverUnreadableConfiguration: true); + store.SaveConfiguration(new AgentConfiguration(request.ServerUrl, response.DeviceId, response.DeviceToken, response.PollIntervalSeconds)); + pairingSaved = true; + ResetConnectionState(); ActivateSession(session, actual, store); lock(_stateLock) { @@ -54,7 +65,8 @@ public sealed partial class AgentRuntime } catch(Exception exception) when(exception is not InvalidOperationException) { - WriteFailure("Enrollment failed; any existing pairing is unchanged", exception); + WriteFailure(pairingSaved ? "Enrollment failed after the new pairing was saved; sign in again to finish activating it" + : "Enrollment failed; any existing pairing is unchanged", exception); throw; } finally { _authenticationGate.Release(); } diff --git a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs index 81d0c2f..b34c107 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs @@ -161,6 +161,57 @@ public sealed class AgentNewPairingTests + /// + /// The server can accept the enrollment code and then fail the next step, which binds the new device to the session: either it + /// refuses, or the connection is lost. Neither may replace the stored pairing or session, because the operator is told the + /// existing pairing is unchanged. A later attempt with a fresh code still completes. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task BindFailureAfterAcceptedEnrollmentKeepsThePreviousPairing(bool connectionLost) + { + using SessionFixture fixture = new(); + fixture.Local.SeedSpooledJob(); + fixture.SaveSession(); + string path = Path.Combine(fixture.Local.DirectoryPath, "connection.dpapi"); + byte[] original = File.ReadAllBytes(path); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); + fixture.NextSession = UnboundSession(); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + using(AgentRuntime runtime = fixture.Runtime()) + { + _ = await runtime.SignInAsync(new PipeRequest("sign-in", SessionCredential: fixture.NextSession.ExportCredential(), StartNewPairing: true), timeout.Token); + byte[] signedIn = File.ReadAllBytes(authPath); + fixture.NextSession.FailBindingTransport = connectionLost; + fixture.NextSession.DenyBinding = !connectionLost; + + _ = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); + + Assert.Equal(new[] { "devices/enroll" }, fixture.NextSession.Paths, StringComparer.Ordinal); + Assert.Equal(original, File.ReadAllBytes(path)); + Assert.Equal(signedIn, File.ReadAllBytes(authPath)); + AgentSnapshot failed = runtime.GetSnapshot(); + Assert.False(failed.Authentication!.Authorized); + Assert.Equal("device", failed.DeviceId); + Assert.Contains(failed.Logs, entry => entry.Message.Contains("any existing pairing is unchanged", StringComparison.Ordinal)); + + fixture.NextSession.FailBindingTransport = false; + fixture.NextSession.DenyBinding = false; + _ = await runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-second-code"), timeout.Token); + Assert.True(runtime.GetSnapshot().Authentication!.Authorized); + } + using(AgentStore store = new(fixture.Local.DirectoryPath)) + { + Assert.Equal("replacement-device", store.LoadConfiguration()!.DeviceId); + } + File.WriteAllBytes(path, original); + using AgentStore originalStore = new(fixture.Local.DirectoryPath); + Assert.Equal("SPOOLED", originalStore.GetState("job")); + } + + + /// A fake session for a user with no device bound that treats the existing device id as revoked, which is what the server reports after a pairing is withdrawn. private static SessionFixture.FakeSession UnboundSession() { diff --git a/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs b/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs index 3530463..13189e4 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs @@ -65,6 +65,7 @@ internal sealed class SessionFixture : IDisposable internal QqqSessionException.FailureKind? ResumeFailure { get; set; } internal bool ClaimedLegacy; internal bool DenyBinding; + internal bool FailBindingTransport; internal string? RevokedDeviceId; internal HttpStatusCode Status { get; set; } = HttpStatusCode.OK; internal bool DelayUnauthorizedPoll { get; set; } @@ -145,9 +146,13 @@ internal sealed class SessionFixture : IDisposable - /// Refuses with permission-denied when binding is denied or the device was revoked; otherwise checks the device id matches and records whether a legacy device was claimed. + /// Fails as a lost connection when scripted to, refuses with permission-denied when binding is denied or the device was revoked; otherwise checks the device id matches and records whether a legacy device was claimed. public Task BindDeviceAsync(QqqDeviceCredential credential, bool claimLegacyDevice = false, CancellationToken cancellationToken = default) { + if(FailBindingTransport) + { + throw new IOException("Synthetic connection loss while binding the device."); + } if(DenyBinding || string.Equals(RevokedDeviceId, credential.DeviceId, StringComparison.Ordinal)) { throw new QqqSessionException(QqqSessionException.FailureKind.PermissionDenied, "Synthetic binding denied."); -- 2.54.0 From 86dced442c559021dce14ba61f1664ba44eb5152 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 11:34:17 -0600 Subject: [PATCH 2/9] fix(enrollment): close the failure windows the reorder opened Independent review of the reorder found no blockers and two new windows: - The pairing file and the session file were no longer written together: resetting the connection state sat between them, so a fault there left the pairing naming the new device and the stored session naming the old identity. The session is now saved straight after the pairing. - After the server bound the new device, a later failure left the live session bound to it while the stored files and the runtime still described the old pairing; nothing compared the two until the next renewal. Printing is now blocked until a sign-in in that case. The failure message follows how far the enrollment got, and is logged for any exception once the device is bound. Tests: the bind-failure case also runs with an unreadable old pairing and checks no recovery copy is made; a working pairing is shown to stay authorized after a failed bind; and three post-bind failures (another identity, pairing not writable, session not exportable) are shown to block printing and say what was saved. The seven changed or new cases fail on the previous commit; the Windows suite passes (385). Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- .../AgentRuntime.Enrollment.cs | 66 ++++++++++--- .../AgentNewPairingTests.cs | 99 +++++++++++++++---- .../SessionFixture.cs | 10 +- 3 files changed, 141 insertions(+), 34 deletions(-) diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs index 5246627..38cdd71 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs @@ -15,16 +15,34 @@ namespace Qqq.PrintAgent.Windows; /// public sealed partial class AgentRuntime { + /// How far an enrollment got, which decides what a failure has to report and whether the live session can still be trusted. + private enum EnrollmentProgress + { + /// Nothing on the server or on disk refers to a new device yet, or the server refused it. + NothingChanged, + + /// The server bound the new device to the live session; the stored pairing and session still describe the old one. + DeviceBound, + + /// The new pairing is stored; the session may not be stored or active yet. + PairingSaved, + + /// The new pairing and session are stored and the session is active. + Activated, + } + + + /// /// Exchanges the enrollment code for a device credential using the live user session on the selected server, binds the - /// new device to that session and checks the identity again. Only then is the stored pairing replaced and the connection - /// state reset, so a refused or interrupted bind leaves the previous pairing and session exactly as they were. - /// Authentication changes are serialized by a gate. + /// new device to that session and checks the identity again. Only then are the stored pairing and stored session replaced, + /// one straight after the other, and the connection state reset, so a refused or interrupted bind leaves both files as they + /// were. Authentication changes are serialized by a gate. /// private async Task EnrollNativeAsync(PipeRequest request, CancellationToken token) { await _authenticationGate.WaitAsync(token).ConfigureAwait(false); - bool pairingSaved = false; + EnrollmentProgress progress = EnrollmentProgress.NothingChanged; try { WriteLog("INFO", $"Enrollment requested on {request.ServerUrl ?? "(no server)"} with a {(request.EnrollmentCode ?? "").Length}-character enrollment code (never logged), computer name {Environment.MachineName}, agent {PrintApiClient.AgentVersion}."); @@ -43,18 +61,21 @@ public sealed partial class AgentRuntime QqqSessionBinding expected = identity with { DeviceId = response.DeviceId }; WriteLog("INFO", "Step 3 of 4: binding the new device to the session and confirming the identity."); QqqSessionBinding actual = await session.BindDeviceAsync(new QqqDeviceCredential(response.DeviceId, response.DeviceToken), cancellationToken: token).ConfigureAwait(false); + progress = EnrollmentProgress.DeviceBound; RequireSameIdentity(expected, actual); - //////////////////////////////////////////////////////////////////////////// - // the stored pairing is replaced only after the server bound the device // - // and the identity matched: until here a failure costs the operator one // - // enrollment code, never the pairing that was already working // - //////////////////////////////////////////////////////////////////////////// + /////////////////////////////////////////////////////////////////////// + // the stored pairing and session are replaced only after the server // + // bound the device and the identity matched; a failure before this // + // point costs one enrollment code and leaves both files untouched // + /////////////////////////////////////////////////////////////////////// WriteLog("INFO", "Step 4 of 4: saving the pairing to protected storage and activating the session for printing."); using AgentStore store = new(directory, recoverUnreadableConfiguration: true); store.SaveConfiguration(new AgentConfiguration(request.ServerUrl, response.DeviceId, response.DeviceToken, response.PollIntervalSeconds)); - pairingSaved = true; + progress = EnrollmentProgress.PairingSaved; + store.SaveAuthentication(new AgentAuthentication(actual, session.ExportCredential())); ResetConnectionState(); ActivateSession(session, actual, store); + progress = EnrollmentProgress.Activated; lock(_stateLock) { _queuedJobs = []; @@ -63,10 +84,9 @@ public sealed partial class AgentRuntime WakePoll(); return $"Enrolled device {response.DeviceId}"; } - catch(Exception exception) when(exception is not InvalidOperationException) + catch(Exception exception) when(progress != EnrollmentProgress.NothingChanged || exception is not InvalidOperationException) { - WriteFailure(pairingSaved ? "Enrollment failed after the new pairing was saved; sign in again to finish activating it" - : "Enrollment failed; any existing pairing is unchanged", exception); + ReportEnrollmentFailure(progress, exception); throw; } finally { _authenticationGate.Release(); } @@ -74,6 +94,26 @@ public sealed partial class AgentRuntime + /// + /// Says what a failed enrollment left behind, and stops the live session being used once the server has bound it to the new + /// device while this computer has not finished switching to it: that session no longer matches the stored pairing or the + /// state the runtime holds, so printing waits for a sign-in instead of running on a mismatch until the next renewal finds it. + /// + private void ReportEnrollmentFailure(EnrollmentProgress progress, Exception exception) + { + WriteFailure(progress switch + { + EnrollmentProgress.NothingChanged => "Enrollment failed; the stored pairing and session are unchanged", + EnrollmentProgress.DeviceBound => "Enrollment failed after the server bound the new device; the stored pairing and session are unchanged, and printing is blocked until you sign in again", + EnrollmentProgress.PairingSaved => "Enrollment failed after the new pairing was saved; printing is blocked until you sign in again, which finishes activating it", + _ => "Enrollment completed, but a later step failed", + }, exception); + if(progress is EnrollmentProgress.DeviceBound or EnrollmentProgress.PairingSaved) + { BlockAuthorization(); } + } + + + /// Logs why an enrollment request was refused before it reached the server, and returns the exception to throw. private InvalidOperationException Refused(string reason, InvalidOperationException exception) { diff --git a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs index b34c107..9c9048e 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs @@ -163,17 +163,23 @@ public sealed class AgentNewPairingTests /// /// The server can accept the enrollment code and then fail the next step, which binds the new device to the session: either it - /// refuses, or the connection is lost. Neither may replace the stored pairing or session, because the operator is told the - /// existing pairing is unchanged. A later attempt with a fresh code still completes. + /// refuses, or the connection is lost. Neither may rewrite the stored pairing or the stored session, whether or not the old + /// pairing could still be read, because the operator is told both are unchanged. The operator stays signed in, and a later + /// attempt with a fresh code completes on a new journal scope. /// [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task BindFailureAfterAcceptedEnrollmentKeepsThePreviousPairing(bool connectionLost) + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + public async Task BindFailureAfterAcceptedEnrollmentKeepsTheStoredPairingAndSession(bool connectionLost, bool unreadable) { using SessionFixture fixture = new(); fixture.Local.SeedSpooledJob(); - fixture.SaveSession(); + if(unreadable) + { ChangeOldConfiguration(fixture, unreadable: true); } + else + { fixture.SaveSession(); } string path = Path.Combine(fixture.Local.DirectoryPath, "connection.dpapi"); byte[] original = File.ReadAllBytes(path); string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); @@ -185,29 +191,88 @@ public sealed class AgentNewPairingTests byte[] signedIn = File.ReadAllBytes(authPath); fixture.NextSession.FailBindingTransport = connectionLost; fixture.NextSession.DenyBinding = !connectionLost; - _ = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); - Assert.Equal(new[] { "devices/enroll" }, fixture.NextSession.Paths, StringComparer.Ordinal); Assert.Equal(original, File.ReadAllBytes(path)); Assert.Equal(signedIn, File.ReadAllBytes(authPath)); + Assert.Empty(Directory.GetFiles(fixture.Local.DirectoryPath, "connection.recovery-*.dpapi")); AgentSnapshot failed = runtime.GetSnapshot(); - Assert.False(failed.Authentication!.Authorized); - Assert.Equal("device", failed.DeviceId); - Assert.Contains(failed.Logs, entry => entry.Message.Contains("any existing pairing is unchanged", StringComparison.Ordinal)); - + Assert.True(failed.Authentication!.SignedIn); + Assert.Equal(unreadable ? null : "device", failed.DeviceId); + Assert.Contains(failed.Logs, entry => entry.Message.Contains("the stored pairing and session are unchanged", StringComparison.Ordinal)); fixture.NextSession.FailBindingTransport = false; fixture.NextSession.DenyBinding = false; _ = await runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-second-code"), timeout.Token); Assert.True(runtime.GetSnapshot().Authentication!.Authorized); } - using(AgentStore store = new(fixture.Local.DirectoryPath)) + Assert.Equal(unreadable ? 1 : 0, Directory.GetFiles(fixture.Local.DirectoryPath, "connection.recovery-*.dpapi").Length); + using AgentStore store = new(fixture.Local.DirectoryPath); + Assert.Equal("replacement-device", store.LoadConfiguration()!.DeviceId); + Assert.Null(store.GetState("job")); + } + + + + /// + /// A pairing that is working keeps working when a second enrollment is accepted but the connection drops before the new + /// device is bound: neither file is rewritten and printing stays authorized. + /// + [Fact] + public async Task BindFailureLeavesAWorkingPairingAuthorized() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + string path = Path.Combine(fixture.Local.DirectoryPath, "connection.dpapi"); + byte[] original = File.ReadAllBytes(path); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); + using AgentRuntime runtime = fixture.Runtime(); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + _ = await runtime.PollControlsOnceAsync(timeout.Token); + Assert.True(runtime.GetSnapshot().Authentication!.Authorized); + byte[] session = File.ReadAllBytes(authPath); + fixture.Session.FailBindingTransport = true; + _ = await Assert.ThrowsAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); + Assert.Equal(original, File.ReadAllBytes(path)); + Assert.Equal(session, File.ReadAllBytes(authPath)); + Assert.True(runtime.GetSnapshot().Authentication!.Authorized); + } + + + + /// + /// Once the server has bound the new device, the live session no longer matches what this computer has stored or holds in + /// memory. If the bound identity then turns out to be someone else's, or the pairing or the session cannot be saved, printing + /// is blocked until a sign-in instead of running on the mismatch, and the log says what was and was not saved. + /// + [Theory] + [InlineData("identity")] + [InlineData("pairing")] + [InlineData("session")] + public async Task FailureAfterTheServerBoundTheDeviceBlocksPrinting(string failingStep) + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + string path = Path.Combine(fixture.Local.DirectoryPath, "connection.dpapi"); + byte[] original = File.ReadAllBytes(path); + using AgentRuntime runtime = fixture.Runtime(); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + _ = await runtime.PollControlsOnceAsync(timeout.Token); + Assert.True(runtime.GetSnapshot().Authentication!.Authorized); + fixture.Session.BindAsAnotherPrincipal = failingStep is "identity"; + fixture.Session.FailExport = failingStep is "session"; + if(failingStep is "pairing") { - Assert.Equal("replacement-device", store.LoadConfiguration()!.DeviceId); + // SaveConfiguration writes this temporary name first; a directory in its place makes that write fail. + _ = Directory.CreateDirectory(path + ".tmp"); } - File.WriteAllBytes(path, original); - using AgentStore originalStore = new(fixture.Local.DirectoryPath); - Assert.Equal("SPOOLED", originalStore.GetState("job")); + Exception thrown = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); + Assert.IsAssignableFrom(failingStep is "pairing" ? typeof(UnauthorizedAccessException) : typeof(IOException), thrown); + bool pairingSaved = failingStep is "session"; + Assert.Equal(pairingSaved, !original.AsSpan().SequenceEqual(File.ReadAllBytes(path))); + AgentSnapshot failed = runtime.GetSnapshot(); + Assert.False(failed.Authentication!.Authorized); + Assert.Contains("authorization is required", failed.Authentication.Message, StringComparison.Ordinal); + Assert.Contains(failed.Logs, entry => entry.Message.Contains(pairingSaved ? "after the new pairing was saved" : "after the server bound the new device", StringComparison.Ordinal)); } diff --git a/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs b/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs index 13189e4..1d89b78 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs @@ -66,6 +66,8 @@ internal sealed class SessionFixture : IDisposable internal bool ClaimedLegacy; internal bool DenyBinding; internal bool FailBindingTransport; + internal bool BindAsAnotherPrincipal; + internal bool FailExport; internal string? RevokedDeviceId; internal HttpStatusCode Status { get; set; } = HttpStatusCode.OK; internal bool DelayUnauthorizedPoll { get; set; } @@ -90,8 +92,8 @@ internal sealed class SessionFixture : IDisposable - /// Returns a fixed synthetic cookie credential for this origin. - public QqqSessionCredential ExportCredential() => new(Origin, "synthetic-qqq-cookie"); + /// Returns a fixed synthetic cookie credential for this origin, or fails as a lost connection when scripted to. + public QqqSessionCredential ExportCredential() => FailExport ? throw new IOException("Synthetic failure while exporting the session.") : new(Origin, "synthetic-qqq-cookie"); @@ -146,7 +148,7 @@ internal sealed class SessionFixture : IDisposable - /// Fails as a lost connection when scripted to, refuses with permission-denied when binding is denied or the device was revoked; otherwise checks the device id matches and records whether a legacy device was claimed. + /// Fails as a lost connection when scripted to, refuses with permission-denied when binding is denied or the device was revoked; otherwise checks the device id matches, records whether a legacy device was claimed and returns the binding (as another principal when scripted to). public Task BindDeviceAsync(QqqDeviceCredential credential, bool claimLegacyDevice = false, CancellationToken cancellationToken = default) { if(FailBindingTransport) @@ -159,7 +161,7 @@ internal sealed class SessionFixture : IDisposable } Assert.Equal(Binding.DeviceId, credential.DeviceId); ClaimedLegacy = claimLegacyDevice; - return Task.FromResult(Binding); + return Task.FromResult(BindAsAnotherPrincipal ? Binding with { PrincipalId = "someone-else" } : Binding); } -- 2.54.0 From 8dd722f426d8fd4750a88518dda50ee4d3dd3341 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 11:47:28 -0600 Subject: [PATCH 3/9] fix(enrollment): make each failure message name a recovery that works A second review found no blocker and two gaps in the previous commit. The message for "pairing saved, session not" told the operator to sign in again. When the stored session still carried the old device, that sign-in is refused, as is a restart. The session to save is now built before the pairing is written, so only one file write separates the two saves, and the progress has a state for each: after the pairing alone the log says to enroll again or start a new pairing; after both, that a sign-in or a restart finishes the switch. Printing is blocked before the failure is logged, so a fault while logging cannot leave the session usable. The tests now exercise every recovery the messages promise: an ordinary sign-in after the device was bound but nothing saved (three causes), enrolling again after the pairing alone was saved, and a restarted service after both were saved. The bind-failure cases assert that printing was not blocked, which the earlier substring could not tell apart, and the working pairing is shown to pass a further authorization pass. The fake session now takes on a device when it is bound, as the server does, instead of when the enrollment is accepted. Windows suite: 388 passed; build has zero warnings. Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- .../AgentRuntime.Enrollment.cs | 35 +++-- .../AgentNewPairingTests.cs | 120 +++++++++++++++--- .../SessionFixture.cs | 41 +++++- 3 files changed, 157 insertions(+), 39 deletions(-) diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs index 38cdd71..3dd179a 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs @@ -18,15 +18,18 @@ public sealed partial class AgentRuntime /// How far an enrollment got, which decides what a failure has to report and whether the live session can still be trusted. private enum EnrollmentProgress { - /// Nothing on the server or on disk refers to a new device yet, or the server refused it. + /// The server has not bound a new device to the session, and the stored pairing and session are untouched. NothingChanged, /// The server bound the new device to the live session; the stored pairing and session still describe the old one. DeviceBound, - /// The new pairing is stored; the session may not be stored or active yet. + /// The new pairing is stored; the stored session still describes the old identity. PairingSaved, + /// The new pairing and the matching session are both stored; the session is not active yet. + SessionSaved, + /// The new pairing and session are stored and the session is active. Activated, } @@ -63,16 +66,18 @@ public sealed partial class AgentRuntime QqqSessionBinding actual = await session.BindDeviceAsync(new QqqDeviceCredential(response.DeviceId, response.DeviceToken), cancellationToken: token).ConfigureAwait(false); progress = EnrollmentProgress.DeviceBound; RequireSameIdentity(expected, actual); - /////////////////////////////////////////////////////////////////////// - // the stored pairing and session are replaced only after the server // - // bound the device and the identity matched; a failure before this // - // point costs one enrollment code and leaves both files untouched // - /////////////////////////////////////////////////////////////////////// + //////////////////////////////////////////////////////////////////// + // the stored pairing and session are replaced only after the // + // server bound the device and the identity matched, so a failure // + // before this point leaves both files untouched // + //////////////////////////////////////////////////////////////////// WriteLog("INFO", "Step 4 of 4: saving the pairing to protected storage and activating the session for printing."); + AgentAuthentication authentication = new(actual, session.ExportCredential()); using AgentStore store = new(directory, recoverUnreadableConfiguration: true); store.SaveConfiguration(new AgentConfiguration(request.ServerUrl, response.DeviceId, response.DeviceToken, response.PollIntervalSeconds)); progress = EnrollmentProgress.PairingSaved; - store.SaveAuthentication(new AgentAuthentication(actual, session.ExportCredential())); + store.SaveAuthentication(authentication); + progress = EnrollmentProgress.SessionSaved; ResetConnectionState(); ActivateSession(session, actual, store); progress = EnrollmentProgress.Activated; @@ -95,21 +100,23 @@ public sealed partial class AgentRuntime /// - /// Says what a failed enrollment left behind, and stops the live session being used once the server has bound it to the new - /// device while this computer has not finished switching to it: that session no longer matches the stored pairing or the - /// state the runtime holds, so printing waits for a sign-in instead of running on a mismatch until the next renewal finds it. + /// Stops the live session being used once the server has bound it to the new device while this computer has not finished + /// switching to it, then says what the failed enrollment left behind and how to recover. Such a session no longer matches + /// the stored pairing or the state the runtime holds, so printing waits for the operator instead of running on a mismatch + /// until the next renewal finds it. Blocking comes first, so a failure while logging cannot leave the session usable. /// private void ReportEnrollmentFailure(EnrollmentProgress progress, Exception exception) { + if(progress is EnrollmentProgress.DeviceBound or EnrollmentProgress.PairingSaved or EnrollmentProgress.SessionSaved) + { BlockAuthorization(); } WriteFailure(progress switch { EnrollmentProgress.NothingChanged => "Enrollment failed; the stored pairing and session are unchanged", EnrollmentProgress.DeviceBound => "Enrollment failed after the server bound the new device; the stored pairing and session are unchanged, and printing is blocked until you sign in again", - EnrollmentProgress.PairingSaved => "Enrollment failed after the new pairing was saved; printing is blocked until you sign in again, which finishes activating it", + EnrollmentProgress.PairingSaved => "Enrollment failed after the new pairing was saved but before its session was; printing is blocked until you enroll again or start a new pairing", + EnrollmentProgress.SessionSaved => "Enrollment failed after the new pairing and its session were saved; printing is blocked until you sign in again or the service restarts, either of which finishes activating it", _ => "Enrollment completed, but a later step failed", }, exception); - if(progress is EnrollmentProgress.DeviceBound or EnrollmentProgress.PairingSaved) - { BlockAuthorization(); } } diff --git a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs index 9c9048e..ff2ebb4 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs @@ -164,8 +164,8 @@ public sealed class AgentNewPairingTests /// /// The server can accept the enrollment code and then fail the next step, which binds the new device to the session: either it /// refuses, or the connection is lost. Neither may rewrite the stored pairing or the stored session, whether or not the old - /// pairing could still be read, because the operator is told both are unchanged. The operator stays signed in, and a later - /// attempt with a fresh code completes on a new journal scope. + /// pairing could still be read, and neither blocks printing, because the operator is told both are unchanged. The operator + /// stays signed in, and a later attempt with a fresh code completes on a new journal scope. /// [Theory] [InlineData(false, false)] @@ -198,8 +198,9 @@ public sealed class AgentNewPairingTests Assert.Empty(Directory.GetFiles(fixture.Local.DirectoryPath, "connection.recovery-*.dpapi")); AgentSnapshot failed = runtime.GetSnapshot(); Assert.True(failed.Authentication!.SignedIn); + Assert.DoesNotContain("authorization is required", failed.Authentication.Message, StringComparison.Ordinal); Assert.Equal(unreadable ? null : "device", failed.DeviceId); - Assert.Contains(failed.Logs, entry => entry.Message.Contains("the stored pairing and session are unchanged", StringComparison.Ordinal)); + Assert.Contains(failed.Logs, entry => entry.Message.Contains("Enrollment failed; the stored pairing and session are unchanged", StringComparison.Ordinal)); fixture.NextSession.FailBindingTransport = false; fixture.NextSession.DenyBinding = false; _ = await runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-second-code"), timeout.Token); @@ -214,11 +215,13 @@ public sealed class AgentNewPairingTests /// - /// A pairing that is working keeps working when a second enrollment is accepted but the connection drops before the new - /// device is bound: neither file is rewritten and printing stays authorized. + /// A pairing that is working keeps working when a second enrollment is accepted but the new device is never bound, because the + /// server refused or the connection dropped: neither file is rewritten, and the next authorization pass still succeeds. /// - [Fact] - public async Task BindFailureLeavesAWorkingPairingAuthorized() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task BindFailureLeavesAWorkingPairingWorking(bool connectionLost) { using SessionFixture fixture = new(); fixture.SaveSession(); @@ -230,25 +233,33 @@ public sealed class AgentNewPairingTests _ = await runtime.PollControlsOnceAsync(timeout.Token); Assert.True(runtime.GetSnapshot().Authentication!.Authorized); byte[] session = File.ReadAllBytes(authPath); - fixture.Session.FailBindingTransport = true; - _ = await Assert.ThrowsAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); + fixture.Session.FailBindingTransport = connectionLost; + fixture.Session.DenyBinding = !connectionLost; + _ = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); Assert.Equal(original, File.ReadAllBytes(path)); Assert.Equal(session, File.ReadAllBytes(authPath)); Assert.True(runtime.GetSnapshot().Authentication!.Authorized); + fixture.Session.FailBindingTransport = false; + fixture.Session.DenyBinding = false; + _ = await runtime.PollControlsOnceAsync(timeout.Token); + AgentSnapshot afterNextPass = runtime.GetSnapshot(); + Assert.True(afterNextPass.Authentication!.Authorized); + Assert.Equal("device", afterNextPass.DeviceId); } /// /// Once the server has bound the new device, the live session no longer matches what this computer has stored or holds in - /// memory. If the bound identity then turns out to be someone else's, or the pairing or the session cannot be saved, printing - /// is blocked until a sign-in instead of running on the mismatch, and the log says what was and was not saved. + /// memory. If the bound identity then turns out to belong to another user, the session cannot be exported, or the pairing + /// cannot be written, nothing has been saved: printing is blocked instead of running on the mismatch, the log says so, and an + /// ordinary sign-in binds the old device again and restores the old pairing. /// [Theory] [InlineData("identity")] - [InlineData("pairing")] - [InlineData("session")] - public async Task FailureAfterTheServerBoundTheDeviceBlocksPrinting(string failingStep) + [InlineData("export")] + [InlineData("pairing-file")] + public async Task FailureAfterTheServerBoundTheDeviceBlocksPrintingUntilSignIn(string failingStep) { using SessionFixture fixture = new(); fixture.SaveSession(); @@ -259,20 +270,89 @@ public sealed class AgentNewPairingTests _ = await runtime.PollControlsOnceAsync(timeout.Token); Assert.True(runtime.GetSnapshot().Authentication!.Authorized); fixture.Session.BindAsAnotherPrincipal = failingStep is "identity"; - fixture.Session.FailExport = failingStep is "session"; - if(failingStep is "pairing") + fixture.Session.ExportsBeforeFailure = failingStep is "export" ? 0 : null; + if(failingStep is "pairing-file") { // SaveConfiguration writes this temporary name first; a directory in its place makes that write fail. _ = Directory.CreateDirectory(path + ".tmp"); } Exception thrown = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); - Assert.IsAssignableFrom(failingStep is "pairing" ? typeof(UnauthorizedAccessException) : typeof(IOException), thrown); - bool pairingSaved = failingStep is "session"; - Assert.Equal(pairingSaved, !original.AsSpan().SequenceEqual(File.ReadAllBytes(path))); + Assert.IsAssignableFrom(failingStep is "pairing-file" ? typeof(UnauthorizedAccessException) : typeof(IOException), thrown); + Assert.Equal(original, File.ReadAllBytes(path)); AgentSnapshot failed = runtime.GetSnapshot(); Assert.False(failed.Authentication!.Authorized); Assert.Contains("authorization is required", failed.Authentication.Message, StringComparison.Ordinal); - Assert.Contains(failed.Logs, entry => entry.Message.Contains(pairingSaved ? "after the new pairing was saved" : "after the server bound the new device", StringComparison.Ordinal)); + Assert.Contains(failed.Logs, entry => entry.Message.Contains("after the server bound the new device", StringComparison.Ordinal)); + fixture.Session.BindAsAnotherPrincipal = false; + fixture.Session.ExportsBeforeFailure = null; + _ = await runtime.SignInAsync(new PipeRequest("sign-in", SessionCredential: fixture.Session.ExportCredential()), timeout.Token); + AgentSnapshot recovered = runtime.GetSnapshot(); + Assert.True(recovered.Authentication!.Authorized); + Assert.Equal("device", recovered.DeviceId); + } + + + + /// + /// If the new pairing is saved and its session then cannot be written, the two files disagree and an ordinary sign-in would be + /// refused. Printing is blocked, the log names the recovery that works, and enrolling again with a fresh code completes. + /// + [Fact] + public async Task FailureBetweenSavingThePairingAndItsSessionBlocksPrintingUntilEnrolledAgain() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + string path = Path.Combine(fixture.Local.DirectoryPath, "connection.dpapi"); + byte[] original = File.ReadAllBytes(path); + string blocker = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi.tmp"); + using AgentRuntime runtime = fixture.Runtime(); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + _ = await runtime.PollControlsOnceAsync(timeout.Token); + Assert.True(runtime.GetSnapshot().Authentication!.Authorized); + // SaveAuthentication writes this temporary name first; a directory in its place makes that write fail. + _ = Directory.CreateDirectory(blocker); + _ = await Assert.ThrowsAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); + Assert.NotEqual(original, File.ReadAllBytes(path)); + AgentSnapshot failed = runtime.GetSnapshot(); + Assert.False(failed.Authentication!.Authorized); + Assert.Contains("authorization is required", failed.Authentication.Message, StringComparison.Ordinal); + Assert.Contains(failed.Logs, entry => entry.Message.Contains("after the new pairing was saved but before its session was", StringComparison.Ordinal)); + Directory.Delete(blocker); + _ = await runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-second-code"), timeout.Token); + AgentSnapshot recovered = runtime.GetSnapshot(); + Assert.True(recovered.Authentication!.Authorized); + Assert.Equal("replacement-device", recovered.DeviceId); + } + + + + /// + /// If activation fails after the new pairing and its session are both saved, the files agree with each other and with the + /// server. Printing is blocked in the running service, and a restarted service finishes the switch with no operator action. + /// + [Fact] + public async Task FailureAfterBothFilesAreSavedIsFinishedByARestart() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + using(AgentRuntime runtime = fixture.Runtime()) + { + _ = await runtime.PollControlsOnceAsync(timeout.Token); + Assert.True(runtime.GetSnapshot().Authentication!.Authorized); + // The first export builds the session to save; the second, during activation, is the one that fails. + fixture.Session.ExportsBeforeFailure = 1; + _ = await Assert.ThrowsAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); + AgentSnapshot failed = runtime.GetSnapshot(); + Assert.False(failed.Authentication!.Authorized); + Assert.Contains(failed.Logs, entry => entry.Message.Contains("after the new pairing and its session were saved", StringComparison.Ordinal)); + } + fixture.Session.ExportsBeforeFailure = null; + using AgentRuntime restarted = fixture.Runtime(); + _ = await restarted.PollControlsOnceAsync(timeout.Token); + AgentSnapshot recovered = restarted.GetSnapshot(); + Assert.True(recovered.Authentication!.Authorized); + Assert.Equal("replacement-device", recovered.DeviceId); } diff --git a/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs b/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs index 1d89b78..f6ad145 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs @@ -66,8 +66,13 @@ internal sealed class SessionFixture : IDisposable internal bool ClaimedLegacy; internal bool DenyBinding; internal bool FailBindingTransport; + /// When set, a successful bind reports a different principal, as if the device had been bound for another user. internal bool BindAsAnotherPrincipal; - internal bool FailExport; + + /// How many more credential exports succeed before one fails as a lost connection; null never fails. + internal int? ExportsBeforeFailure; + private string? _issuedDeviceId; + private string? _deviceBeforeIssue; internal string? RevokedDeviceId; internal HttpStatusCode Status { get; set; } = HttpStatusCode.OK; internal bool DelayUnauthorizedPoll { get; set; } @@ -92,8 +97,19 @@ internal sealed class SessionFixture : IDisposable - /// Returns a fixed synthetic cookie credential for this origin, or fails as a lost connection when scripted to. - public QqqSessionCredential ExportCredential() => FailExport ? throw new IOException("Synthetic failure while exporting the session.") : new(Origin, "synthetic-qqq-cookie"); + /// Returns a fixed synthetic cookie credential for this origin, or fails as a lost connection once the scripted number of exports is used up. + public QqqSessionCredential ExportCredential() + { + if(ExportsBeforeFailure is { } remaining) + { + if(remaining == 0) + { + throw new IOException("Synthetic failure while exporting the session."); + } + ExportsBeforeFailure = remaining - 1; + } + return new(Origin, "synthetic-qqq-cookie"); + } @@ -148,7 +164,13 @@ internal sealed class SessionFixture : IDisposable - /// Fails as a lost connection when scripted to, refuses with permission-denied when binding is denied or the device was revoked; otherwise checks the device id matches, records whether a legacy device was claimed and returns the binding (as another principal when scripted to). + /// + /// Fails as a lost connection when scripted to, and refuses with permission-denied when binding is denied or the device was + /// revoked. Otherwise it behaves as the server does: the session takes on a device only when that device is bound, so binding + /// the device an enrollment issued switches to it, and binding the device held before switches back. Any other device must + /// already be the one the session holds. It records whether a legacy device was claimed and returns the binding (as another + /// principal when scripted to). + /// public Task BindDeviceAsync(QqqDeviceCredential credential, bool claimLegacyDevice = false, CancellationToken cancellationToken = default) { if(FailBindingTransport) @@ -159,6 +181,15 @@ internal sealed class SessionFixture : IDisposable { throw new QqqSessionException(QqqSessionException.FailureKind.PermissionDenied, "Synthetic binding denied."); } + if(string.Equals(credential.DeviceId, _issuedDeviceId, StringComparison.Ordinal) && !string.Equals(Binding.DeviceId, _issuedDeviceId, StringComparison.Ordinal)) + { + _deviceBeforeIssue = Binding.DeviceId; + Binding = Binding with { DeviceId = _issuedDeviceId }; + } + else if(_deviceBeforeIssue is not null && string.Equals(credential.DeviceId, _deviceBeforeIssue, StringComparison.Ordinal)) + { + Binding = Binding with { DeviceId = _deviceBeforeIssue }; + } Assert.Equal(Binding.DeviceId, credential.DeviceId); ClaimedLegacy = claimLegacyDevice; return Task.FromResult(BindAsAnotherPrincipal ? Binding with { PrincipalId = "someone-else" } : Binding); @@ -180,7 +211,7 @@ internal sealed class SessionFixture : IDisposable { return new HttpResponseMessage(Status) { Content = new StringContent("{}", Encoding.UTF8, "application/json") }; } - Binding = Binding with { DeviceId = "replacement-device" }; + _issuedDeviceId = "replacement-device"; return new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(JsonSerializer.Serialize(new EnrollmentResponse(1, "replacement-device", "replacement-token", 5), PrintApiClient.JsonOptions), Encoding.UTF8, "application/json") }; } if(DelayUnauthorizedPoll) -- 2.54.0 From a9ad50ae1d8c1c97e2dd4acebaf8f96388ffd005 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 12:14:16 -0600 Subject: [PATCH 4/9] fix(enrollment): show the recovery that works when only the pairing was saved A confirmation review found one gap left. When the new pairing was saved and its session was not, the log named the right recovery, but blocking printing still set the status line and a warning to "Sign in again", and that sign-in is refused because the stored session names the old device. The block now takes the recovery to show; for this state it says to enroll again or sign in choosing a new pairing. The test asserts the status and log do not say "Sign in again", that an ordinary sign-in is refused, and that enrolling again then completes. The device-bound cases also pin the session file. Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- .../AgentRuntime.Authentication.cs | 9 +++++---- .../AgentRuntime.Enrollment.cs | 8 ++++++-- .../AgentNewPairingTests.cs | 16 ++++++++++++---- tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs | 2 ++ 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs index c84e1ab..1f7995b 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs @@ -220,9 +220,10 @@ public sealed partial class AgentRuntime /// /// Stops all new work after authorization is lost: clears readiness, cancels the realtime socket and tells the operator - /// to sign in again. Pairing, history and receipts are untouched; retrying a connection cannot restore a withdrawn permission. + /// how to recover. That is signing in again unless the caller knows a sign-in would be refused and names what works + /// instead. Pairing, history and receipts are untouched; retrying a connection cannot restore a withdrawn permission. /// - private void BlockAuthorization() + private void BlockAuthorization(string? recovery = null) { bool changed; lock(_stateLock) @@ -233,10 +234,10 @@ public sealed partial class AgentRuntime _controlReady = false; _realtimeReady = false; _realtimeLifetime?.Cancel(); - _authenticationMessage = $"{AppIdentity.ServerName} authorization is required. Sign in again; pairing and history are preserved."; + _authenticationMessage = $"{AppIdentity.ServerName} authorization is required. {recovery ?? "Sign in again; pairing and history are preserved."}"; } if(changed) - { WriteLog("WARN", $"Printing blocked: {AppIdentity.ServerName} authorization is required. Sign in again from the companion; pairing and history are preserved."); } + { WriteLog("WARN", $"Printing blocked: {AppIdentity.ServerName} authorization is required. {recovery ?? "Sign in again from the companion; pairing and history are preserved."}"); } SignalPipeline(); } diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs index 3dd179a..6d20b4b 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs @@ -103,12 +103,16 @@ public sealed partial class AgentRuntime /// Stops the live session being used once the server has bound it to the new device while this computer has not finished /// switching to it, then says what the failed enrollment left behind and how to recover. Such a session no longer matches /// the stored pairing or the state the runtime holds, so printing waits for the operator instead of running on a mismatch - /// until the next renewal finds it. Blocking comes first, so a failure while logging cannot leave the session usable. + /// until the next renewal finds it. Blocking comes first, so a failure while logging cannot leave the session usable. When + /// only the pairing was saved, the stored session still names the old device and an ordinary sign-in would be refused, so + /// the status shown to the operator names enrolling again instead. /// private void ReportEnrollmentFailure(EnrollmentProgress progress, Exception exception) { - if(progress is EnrollmentProgress.DeviceBound or EnrollmentProgress.PairingSaved or EnrollmentProgress.SessionSaved) + if(progress is EnrollmentProgress.DeviceBound or EnrollmentProgress.SessionSaved) { BlockAuthorization(); } + else if(progress is EnrollmentProgress.PairingSaved) + { BlockAuthorization("Enroll this computer again, or sign in choosing a new pairing; job history is preserved."); } WriteFailure(progress switch { EnrollmentProgress.NothingChanged => "Enrollment failed; the stored pairing and session are unchanged", diff --git a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs index ff2ebb4..6d79b45 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs @@ -265,10 +265,12 @@ public sealed class AgentNewPairingTests fixture.SaveSession(); string path = Path.Combine(fixture.Local.DirectoryPath, "connection.dpapi"); byte[] original = File.ReadAllBytes(path); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); using AgentRuntime runtime = fixture.Runtime(); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); _ = await runtime.PollControlsOnceAsync(timeout.Token); Assert.True(runtime.GetSnapshot().Authentication!.Authorized); + byte[] session = File.ReadAllBytes(authPath); fixture.Session.BindAsAnotherPrincipal = failingStep is "identity"; fixture.Session.ExportsBeforeFailure = failingStep is "export" ? 0 : null; if(failingStep is "pairing-file") @@ -279,9 +281,10 @@ public sealed class AgentNewPairingTests Exception thrown = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); Assert.IsAssignableFrom(failingStep is "pairing-file" ? typeof(UnauthorizedAccessException) : typeof(IOException), thrown); Assert.Equal(original, File.ReadAllBytes(path)); + Assert.Equal(session, File.ReadAllBytes(authPath)); AgentSnapshot failed = runtime.GetSnapshot(); Assert.False(failed.Authentication!.Authorized); - Assert.Contains("authorization is required", failed.Authentication.Message, StringComparison.Ordinal); + Assert.Contains("Sign in again", failed.Authentication.Message, StringComparison.Ordinal); Assert.Contains(failed.Logs, entry => entry.Message.Contains("after the server bound the new device", StringComparison.Ordinal)); fixture.Session.BindAsAnotherPrincipal = false; fixture.Session.ExportsBeforeFailure = null; @@ -294,8 +297,9 @@ public sealed class AgentNewPairingTests /// - /// If the new pairing is saved and its session then cannot be written, the two files disagree and an ordinary sign-in would be - /// refused. Printing is blocked, the log names the recovery that works, and enrolling again with a fresh code completes. + /// If the new pairing is saved and its session then cannot be written, the two files disagree and an ordinary sign-in is + /// refused. Printing is blocked, the status and the log name the recovery that works instead of a sign-in, and enrolling + /// again with a fresh code completes. /// [Fact] public async Task FailureBetweenSavingThePairingAndItsSessionBlocksPrintingUntilEnrolledAgain() @@ -315,9 +319,13 @@ public sealed class AgentNewPairingTests Assert.NotEqual(original, File.ReadAllBytes(path)); AgentSnapshot failed = runtime.GetSnapshot(); Assert.False(failed.Authentication!.Authorized); - Assert.Contains("authorization is required", failed.Authentication.Message, StringComparison.Ordinal); + Assert.Contains("Enroll this computer again", failed.Authentication.Message, StringComparison.Ordinal); + Assert.DoesNotContain("Sign in again", failed.Authentication.Message, StringComparison.Ordinal); + Assert.DoesNotContain(failed.Logs, entry => entry.Message.Contains("Sign in again", StringComparison.Ordinal)); Assert.Contains(failed.Logs, entry => entry.Message.Contains("after the new pairing was saved but before its session was", StringComparison.Ordinal)); Directory.Delete(blocker); + _ = await Assert.ThrowsAsync(() => runtime.SignInAsync(new PipeRequest("sign-in", SessionCredential: fixture.Session.ExportCredential()), timeout.Token)); + Assert.False(runtime.GetSnapshot().Authentication!.Authorized); _ = await runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-second-code"), timeout.Token); AgentSnapshot recovered = runtime.GetSnapshot(); Assert.True(recovered.Authentication!.Authorized); diff --git a/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs b/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs index f6ad145..e1a5493 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/SessionFixture.cs @@ -65,6 +65,8 @@ internal sealed class SessionFixture : IDisposable internal QqqSessionException.FailureKind? ResumeFailure { get; set; } internal bool ClaimedLegacy; internal bool DenyBinding; + + /// When set, binding a device fails as a lost connection before the server has bound anything. internal bool FailBindingTransport; /// When set, a successful bind reports a different principal, as if the device had been bound for another user. internal bool BindAsAnotherPrincipal; -- 2.54.0 From 498dd3ec7496eb8651ee3dde9b65af407b920757 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 12:25:31 -0600 Subject: [PATCH 5/9] fix(enrollment): save the pairing and its session as one staged step Review found that the earlier fix only held while the service kept running. The pairing and the session are two files, and a failure between the two writes left them disagreeing on disk. After a restart the service told the operator to sign in again, and that sign-in was refused. The store now stages the change: the session is written under a staged name, then the pairing is replaced, then the staged session is moved into place. Any reader of the session (enrollment itself, a sign-in, the next service start) finishes a staged change whose session names the stored pairing's device, and discards one that does not, which is what a crash before the pairing was replaced leaves behind. So the stored pairing and session always agree by the time anything acts on them, and "sign in again" is a true instruction in every blocked state. The separate "pairing only" state and its special status text are gone. Authorization is also re-checked after waiting for the gate, so a pass that was waiting while enrollment blocked printing does not carry on. Tests: an interrupted move into place is finished by an ordinary sign-in and by a restart; a staged session for another device, or an unreadable one, is discarded on read; a normal save removes a staged session; and the bound-but-not-saved cases assert nothing is left staged. Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- .../AgentRuntime.Authentication.cs | 14 +- .../AgentRuntime.Enrollment.cs | 32 ++--- .../AgentStore.Authentication.cs | 125 +++++++++++++++-- .../AgentNewPairingTests.cs | 129 ++++++++++++++---- 4 files changed, 238 insertions(+), 62 deletions(-) diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs index 1f7995b..aeb4874 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs @@ -82,6 +82,11 @@ public sealed partial class AgentRuntime await _authenticationGate.WaitAsync(token).ConfigureAwait(false); try { + if(_authorizationBlocked) + { + // Whoever held the gate (a sign-out, a failed enrollment) blocked authorization while this call was waiting. + return false; + } using AgentStore store = new(directory, recoverUnreadableConfiguration: true); AgentAuthentication? saved = store.LoadAuthentication(); AgentConfiguration? configuration = store.LoadConfiguration(); @@ -220,10 +225,9 @@ public sealed partial class AgentRuntime /// /// Stops all new work after authorization is lost: clears readiness, cancels the realtime socket and tells the operator - /// how to recover. That is signing in again unless the caller knows a sign-in would be refused and names what works - /// instead. Pairing, history and receipts are untouched; retrying a connection cannot restore a withdrawn permission. + /// to sign in again. Pairing, history and receipts are untouched; retrying a connection cannot restore a withdrawn permission. /// - private void BlockAuthorization(string? recovery = null) + private void BlockAuthorization() { bool changed; lock(_stateLock) @@ -234,10 +238,10 @@ public sealed partial class AgentRuntime _controlReady = false; _realtimeReady = false; _realtimeLifetime?.Cancel(); - _authenticationMessage = $"{AppIdentity.ServerName} authorization is required. {recovery ?? "Sign in again; pairing and history are preserved."}"; + _authenticationMessage = $"{AppIdentity.ServerName} authorization is required. Sign in again; pairing and history are preserved."; } if(changed) - { WriteLog("WARN", $"Printing blocked: {AppIdentity.ServerName} authorization is required. {recovery ?? "Sign in again from the companion; pairing and history are preserved."}"); } + { WriteLog("WARN", $"Printing blocked: {AppIdentity.ServerName} authorization is required. Sign in again from the companion; pairing and history are preserved."); } SignalPipeline(); } diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs index 6d20b4b..95edd93 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs @@ -24,11 +24,8 @@ public sealed partial class AgentRuntime /// The server bound the new device to the live session; the stored pairing and session still describe the old one. DeviceBound, - /// The new pairing is stored; the stored session still describes the old identity. - PairingSaved, - - /// The new pairing and the matching session are both stored; the session is not active yet. - SessionSaved, + /// The new pairing is stored with its session staged beside it, which any later reader moves into place; the session is not active yet. + Saved, /// The new pairing and session are stored and the session is active. Activated, @@ -68,16 +65,16 @@ public sealed partial class AgentRuntime RequireSameIdentity(expected, actual); //////////////////////////////////////////////////////////////////// // the stored pairing and session are replaced only after the // - // server bound the device and the identity matched, so a failure // - // before this point leaves both files untouched // + // server bound the device and the identity matched, and as one // + // staged step: a failure leaves either both files as they were, // + // or the new pairing with a session any later reader completes // //////////////////////////////////////////////////////////////////// WriteLog("INFO", "Step 4 of 4: saving the pairing to protected storage and activating the session for printing."); AgentAuthentication authentication = new(actual, session.ExportCredential()); using AgentStore store = new(directory, recoverUnreadableConfiguration: true); - store.SaveConfiguration(new AgentConfiguration(request.ServerUrl, response.DeviceId, response.DeviceToken, response.PollIntervalSeconds)); - progress = EnrollmentProgress.PairingSaved; - store.SaveAuthentication(authentication); - progress = EnrollmentProgress.SessionSaved; + store.StagePairing(new AgentConfiguration(request.ServerUrl, response.DeviceId, response.DeviceToken, response.PollIntervalSeconds), authentication); + progress = EnrollmentProgress.Saved; + store.CompleteStagedPairing(); ResetConnectionState(); ActivateSession(session, actual, store); progress = EnrollmentProgress.Activated; @@ -103,22 +100,19 @@ public sealed partial class AgentRuntime /// Stops the live session being used once the server has bound it to the new device while this computer has not finished /// switching to it, then says what the failed enrollment left behind and how to recover. Such a session no longer matches /// the stored pairing or the state the runtime holds, so printing waits for the operator instead of running on a mismatch - /// until the next renewal finds it. Blocking comes first, so a failure while logging cannot leave the session usable. When - /// only the pairing was saved, the stored session still names the old device and an ordinary sign-in would be refused, so - /// the status shown to the operator names enrolling again instead. + /// until the next renewal finds it. Blocking comes first, so a failure while logging cannot leave the session usable. In + /// both blocked states an ordinary sign-in recovers: before the save it binds the old device again, and after it the + /// store hands the sign-in the session that belongs to the new pairing. /// private void ReportEnrollmentFailure(EnrollmentProgress progress, Exception exception) { - if(progress is EnrollmentProgress.DeviceBound or EnrollmentProgress.SessionSaved) + if(progress is EnrollmentProgress.DeviceBound or EnrollmentProgress.Saved) { BlockAuthorization(); } - else if(progress is EnrollmentProgress.PairingSaved) - { BlockAuthorization("Enroll this computer again, or sign in choosing a new pairing; job history is preserved."); } WriteFailure(progress switch { EnrollmentProgress.NothingChanged => "Enrollment failed; the stored pairing and session are unchanged", EnrollmentProgress.DeviceBound => "Enrollment failed after the server bound the new device; the stored pairing and session are unchanged, and printing is blocked until you sign in again", - EnrollmentProgress.PairingSaved => "Enrollment failed after the new pairing was saved but before its session was; printing is blocked until you enroll again or start a new pairing", - EnrollmentProgress.SessionSaved => "Enrollment failed after the new pairing and its session were saved; printing is blocked until you sign in again or the service restarts, either of which finishes activating it", + EnrollmentProgress.Saved => "Enrollment failed after the new pairing and its session were saved; printing is blocked until you sign in again or the service restarts, either of which finishes activating it", _ => "Enrollment completed, but a later step failed", }, exception); } diff --git a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs index 329d784..9b0e9f0 100644 --- a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs @@ -16,6 +16,8 @@ namespace Qqq.PrintAgent.Windows; /// public sealed partial class AgentStore { + private const string SessionFile = "qqq-session.dpapi"; + private const string StagedSessionFile = "qqq-session.staged.dpapi"; private bool _authenticationRecoveryRequired; @@ -41,7 +43,7 @@ public sealed partial class AgentStore { if(_authenticationRecoveryRequired) { - File.Move(Path.Combine(_directory, "qqq-session.dpapi"), Path.Combine(_directory, "qqq-session.recovery-" + Guid.NewGuid().ToString("N") + ".dpapi")); + File.Move(Path.Combine(_directory, SessionFile), Path.Combine(_directory, "qqq-session.recovery-" + Guid.NewGuid().ToString("N") + ".dpapi")); _authenticationRecoveryRequired = false; return true; } @@ -51,12 +53,29 @@ public sealed partial class AgentStore /// - /// Reads and decrypts the saved session, or returns null when none exists. The file is size-bounded, decrypted for the - /// current service account, parsed and validated, and the decrypted bytes are zeroed afterwards. A damaged or invalid file throws. + /// Reads and decrypts the saved session, or returns null when none exists. A pairing change that was staged and not + /// finished is finished first, so the session returned always belongs to the stored pairing. The file is size-bounded, + /// decrypted for the current service account, parsed and validated, and the decrypted bytes are zeroed afterwards. A + /// damaged or invalid file throws. /// public AgentAuthentication? LoadAuthentication() { - string path = Path.Combine(_directory, "qqq-session.dpapi"); + try + { + CompleteStagedPairing(); + } + catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) + { + // The staged session could not be moved into place now; the next reader tries again, and until then the stored session is the earlier one. + } + return ReadAuthentication(Path.Combine(_directory, SessionFile)); + } + + + + /// Reads, decrypts, parses and validates one protected session file, or returns null when it does not exist. + private static AgentAuthentication? ReadAuthentication(string path) + { if(!File.Exists(path)) { return null; @@ -104,18 +123,104 @@ public sealed partial class AgentStore /// /// Saves the session protected for the service account, writing a temporary file and moving it into place so a crash cannot - /// leave a half-written secret. An unreadable earlier file is quarantined first, and the plaintext buffer is zeroed on every path. + /// leave a half-written secret. An unreadable earlier file is quarantined first. A staged session from an unfinished pairing + /// change is removed, because what is saved here is newer than it. /// public void SaveAuthentication(AgentAuthentication authentication) + { + string path = Path.Combine(_directory, SessionFile); + WriteProtected(path + ".tmp", authentication); + _ = QuarantineUnreadableAuthentication(); + File.Move(path + ".tmp", path, overwrite: true); + File.Delete(Path.Combine(_directory, StagedSessionFile)); + } + + + + /// + /// Starts replacing the pairing and its session as one step. The session is written first under a staged name and the + /// pairing is replaced second. From then on the change is decided: this caller, a sign-in or the next service start + /// finishes it with , because the staged session names the stored pairing's device. If + /// the pairing cannot be written the staged session is removed and nothing has changed; a staged session left by a crash + /// before the pairing was replaced names a different device and is discarded by the next reader. + /// + public void StagePairing(AgentConfiguration configuration, AgentAuthentication authentication) + { + string staged = Path.Combine(_directory, StagedSessionFile); + WriteProtected(staged + ".tmp", authentication); + File.Move(staged + ".tmp", staged, overwrite: true); + try + { + SaveConfiguration(configuration); + } + catch + { + File.Delete(staged); + throw; + } + } + + + + /// + /// Finishes a staged pairing change, or discards what is left of one that never replaced the pairing. A staged session + /// that names the stored pairing's server and device is moved into place as the session; any other (a different device, + /// an unreadable file, or no readable pairing) is deleted. An unreadable earlier session is quarantined, not overwritten. + /// + public void CompleteStagedPairing() + { + string staged = Path.Combine(_directory, StagedSessionFile); + if(!File.Exists(staged)) + { + return; + } + if(!StagedSessionBelongsToPairing(staged)) + { + File.Delete(staged); + return; + } + string path = Path.Combine(_directory, SessionFile); + try + { + _ = ReadAuthentication(path); + } + catch(Exception exception) when(exception is CryptographicException or JsonException or InvalidDataException or ArgumentException) + { + _authenticationRecoveryRequired = true; + } + _ = QuarantineUnreadableAuthentication(); + File.Move(staged, path, overwrite: true); + } + + + + /// True when the staged session can be read and names the same server and device as the stored pairing. + private bool StagedSessionBelongsToPairing(string staged) + { + try + { + AgentAuthentication? session = ReadAuthentication(staged); + AgentConfiguration? pairing = LoadConfiguration(); + return session is not null && pairing is not null + && string.Equals(session.Binding.DeviceId, pairing.DeviceId, StringComparison.Ordinal) + && Uri.TryCreate(pairing.ServerUrl, UriKind.Absolute, out Uri? server) + && string.Equals(server.GetLeftPart(UriPartial.Authority), session.Binding.Origin.GetLeftPart(UriPartial.Authority), StringComparison.OrdinalIgnoreCase); + } + catch(Exception exception) when(exception is CryptographicException or JsonException or InvalidDataException or ArgumentException or UriFormatException) + { + return false; + } + } + + + + /// Writes one session to a file protected for the service account, zeroing the plaintext buffer on every path. + private static void WriteProtected(string path, AgentAuthentication authentication) { byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(authentication, PrintApiClient.JsonOptions); try { - byte[] encrypted = ProtectedData.Protect(bytes, null, DataProtectionScope.CurrentUser); - string path = Path.Combine(_directory, "qqq-session.dpapi"); - File.WriteAllBytes(path + ".tmp", encrypted); - QuarantineUnreadableAuthentication(); - File.Move(path + ".tmp", path, overwrite: true); + File.WriteAllBytes(path, ProtectedData.Protect(bytes, null, DataProtectionScope.CurrentUser)); } finally { CryptographicOperations.ZeroMemory(bytes); } } diff --git a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs index 6d79b45..4d0de23 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs @@ -252,8 +252,8 @@ public sealed class AgentNewPairingTests /// /// Once the server has bound the new device, the live session no longer matches what this computer has stored or holds in /// memory. If the bound identity then turns out to belong to another user, the session cannot be exported, or the pairing - /// cannot be written, nothing has been saved: printing is blocked instead of running on the mismatch, the log says so, and an - /// ordinary sign-in binds the old device again and restores the old pairing. + /// cannot be written, nothing has been saved and nothing is left staged: printing is blocked instead of running on the + /// mismatch, the log says so, and an ordinary sign-in binds the old device again and restores the old pairing. /// [Theory] [InlineData("identity")] @@ -282,6 +282,7 @@ public sealed class AgentNewPairingTests Assert.IsAssignableFrom(failingStep is "pairing-file" ? typeof(UnauthorizedAccessException) : typeof(IOException), thrown); Assert.Equal(original, File.ReadAllBytes(path)); Assert.Equal(session, File.ReadAllBytes(authPath)); + Assert.False(File.Exists(StagedSession(fixture))); AgentSnapshot failed = runtime.GetSnapshot(); Assert.False(failed.Authentication!.Authorized); Assert.Contains("Sign in again", failed.Authentication.Message, StringComparison.Ordinal); @@ -297,39 +298,56 @@ public sealed class AgentNewPairingTests /// - /// If the new pairing is saved and its session then cannot be written, the two files disagree and an ordinary sign-in is - /// refused. Printing is blocked, the status and the log name the recovery that works instead of a sign-in, and enrolling - /// again with a fresh code completes. + /// The pairing and its session are two files. If the new pairing is written and the session then cannot be moved into + /// place, the session waits beside it under a staged name and printing is blocked. The recovery the operator is told about + /// works both ways: an ordinary sign-in, or a restarted service, moves the staged session into place and finishes the switch. /// - [Fact] - public async Task FailureBetweenSavingThePairingAndItsSessionBlocksPrintingUntilEnrolledAgain() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task InterruptedSaveIsFinishedBySignInOrRestart(bool restart) { using SessionFixture fixture = new(); fixture.SaveSession(); string path = Path.Combine(fixture.Local.DirectoryPath, "connection.dpapi"); byte[] original = File.ReadAllBytes(path); - string blocker = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi.tmp"); - using AgentRuntime runtime = fixture.Runtime(); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); - _ = await runtime.PollControlsOnceAsync(timeout.Token); - Assert.True(runtime.GetSnapshot().Authentication!.Authorized); - // SaveAuthentication writes this temporary name first; a directory in its place makes that write fail. - _ = Directory.CreateDirectory(blocker); - _ = await Assert.ThrowsAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); - Assert.NotEqual(original, File.ReadAllBytes(path)); - AgentSnapshot failed = runtime.GetSnapshot(); - Assert.False(failed.Authentication!.Authorized); - Assert.Contains("Enroll this computer again", failed.Authentication.Message, StringComparison.Ordinal); - Assert.DoesNotContain("Sign in again", failed.Authentication.Message, StringComparison.Ordinal); - Assert.DoesNotContain(failed.Logs, entry => entry.Message.Contains("Sign in again", StringComparison.Ordinal)); - Assert.Contains(failed.Logs, entry => entry.Message.Contains("after the new pairing was saved but before its session was", StringComparison.Ordinal)); - Directory.Delete(blocker); - _ = await Assert.ThrowsAsync(() => runtime.SignInAsync(new PipeRequest("sign-in", SessionCredential: fixture.Session.ExportCredential()), timeout.Token)); - Assert.False(runtime.GetSnapshot().Authentication!.Authorized); - _ = await runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-second-code"), timeout.Token); - AgentSnapshot recovered = runtime.GetSnapshot(); - Assert.True(recovered.Authentication!.Authorized); - Assert.Equal("replacement-device", recovered.DeviceId); + AgentRuntime runtime = fixture.Runtime(); + try + { + _ = await runtime.PollControlsOnceAsync(timeout.Token); + Assert.True(runtime.GetSnapshot().Authentication!.Authorized); + // An open handle that does not share deletion keeps the session file from being replaced, as a scanner or backup might. + using(FileStream held = new(authPath, FileMode.Open, FileAccess.Read, FileShare.Read)) + { + Exception thrown = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); + Assert.True(thrown is IOException or UnauthorizedAccessException, thrown.GetType().FullName); + } + Assert.NotEqual(original, File.ReadAllBytes(path)); + Assert.True(File.Exists(StagedSession(fixture))); + AgentSnapshot failed = runtime.GetSnapshot(); + Assert.False(failed.Authentication!.Authorized); + Assert.Contains(failed.Logs, entry => entry.Message.Contains("after the new pairing and its session were saved", StringComparison.Ordinal)); + if(restart) + { + runtime.Dispose(); + runtime = fixture.Runtime(); + _ = await runtime.PollControlsOnceAsync(timeout.Token); + } + else + { + _ = await runtime.SignInAsync(new PipeRequest("sign-in", SessionCredential: fixture.Session.ExportCredential()), timeout.Token); + } + AgentSnapshot recovered = runtime.GetSnapshot(); + Assert.True(recovered.Authentication!.Authorized); + Assert.Equal("replacement-device", recovered.DeviceId); + Assert.False(File.Exists(StagedSession(fixture))); + } + finally + { + runtime.Dispose(); + } } @@ -354,6 +372,7 @@ public sealed class AgentNewPairingTests AgentSnapshot failed = runtime.GetSnapshot(); Assert.False(failed.Authentication!.Authorized); Assert.Contains(failed.Logs, entry => entry.Message.Contains("after the new pairing and its session were saved", StringComparison.Ordinal)); + Assert.False(File.Exists(StagedSession(fixture))); } fixture.Session.ExportsBeforeFailure = null; using AgentRuntime restarted = fixture.Runtime(); @@ -365,6 +384,60 @@ public sealed class AgentNewPairingTests + /// + /// A staged session that does not belong to the stored pairing is what a crash leaves when the session was staged and the + /// pairing was never replaced. The next read discards it and returns the session that was already stored, unchanged. + /// + [Fact] + public void StagedSessionForAnotherDeviceIsDiscardedOnRead() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); + byte[] stored = File.ReadAllBytes(authPath); + QqqSessionBinding other = fixture.Session.Binding with { DeviceId = "interrupted-device" }; + using(AgentStore staging = new(fixture.Local.DirectoryPath)) + { + staging.StagePairing(new AgentConfiguration("https://synthetic.invalid", "interrupted-device", "synthetic-interrupted"), new AgentAuthentication(other, fixture.Session.ExportCredential())); + staging.SaveConfiguration(new AgentConfiguration("https://synthetic.invalid", "device", "synthetic-token")); + } + Assert.True(File.Exists(StagedSession(fixture))); + using AgentStore store = new(fixture.Local.DirectoryPath); + AgentAuthentication? loaded = store.LoadAuthentication(); + Assert.Equal("device", loaded!.Binding.DeviceId); + Assert.Equal(stored, File.ReadAllBytes(authPath)); + Assert.False(File.Exists(StagedSession(fixture))); + } + + + + /// + /// An unreadable staged session is discarded rather than trusted, and saving a session normally removes a staged one, + /// because what is saved is newer than what was staged. + /// + [Fact] + public void UnreadableOrSupersededStagedSessionIsRemoved() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + File.WriteAllBytes(StagedSession(fixture), [2, 4, 6]); + using(AgentStore store = new(fixture.Local.DirectoryPath)) + { + Assert.Equal("device", store.LoadAuthentication()!.Binding.DeviceId); + Assert.False(File.Exists(StagedSession(fixture))); + File.WriteAllBytes(StagedSession(fixture), [2, 4, 6]); + store.SaveAuthentication(new AgentAuthentication(fixture.Session.Binding, fixture.Session.ExportCredential())); + } + Assert.False(File.Exists(StagedSession(fixture))); + } + + + + /// Where the store keeps a session that belongs to a pairing change in progress. + private static string StagedSession(SessionFixture fixture) => Path.Combine(fixture.Local.DirectoryPath, "qqq-session.staged.dpapi"); + + + /// A fake session for a user with no device bound that treats the existing device id as revoked, which is what the server reports after a pairing is withdrawn. private static SessionFixture.FakeSession UnboundSession() { -- 2.54.0 From a6f5ac1e48c32f6fe3d79128bf543d6eebb2bca5 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 12:39:15 -0600 Subject: [PATCH 6/9] fix(enrollment): use, preserve and log a staged session in every case Review of the staged save found no blocker and these gaps, now closed: - When the staged session could not be moved into place, the reader fell back to the session of the previous pairing. A sign-out then saved that binding and removed the staged one, after which every ordinary sign-in was refused. A staged session that belongs to the stored pairing is now the one every reader gets, whether or not the move has happened yet. - An unreadable staged session was deleted. After the pairing has been replaced it may be the only copy, so it is moved aside under a recovery name instead. The staged file is flushed to disk before the pairing is replaced. - A staged session is a leftover, and is deleted, when it names another device or server, when no pairing is stored, or when the stored session already belongs to the pairing; so a stale staged credential cannot replace a newer stored one. - Moving, discarding or setting aside a session file was silent. The store reports what it did and the runtime logs it; sign-out's "revoke the previous session" warning also fires when the unreadable session was moved aside by this path. Tests cover each outcome at the store, an interrupted save followed by a sign-out and a sign-in, and promotion over an unreadable stored session. Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- docs/QQQ-SIGN-IN.md | 2 + .../AgentRuntime.Authentication.cs | 34 +++- .../AgentRuntime.Enrollment.cs | 7 +- .../AgentRuntime.SignIn.cs | 6 +- .../AgentStore.Authentication.cs | 172 ++++++++++++------ .../StagedPairingOutcome.cs | 32 ++++ .../AgentNewPairingTests.cs | 157 ++++++++++++++-- 7 files changed, 331 insertions(+), 79 deletions(-) create mode 100644 src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs diff --git a/docs/QQQ-SIGN-IN.md b/docs/QQQ-SIGN-IN.md index 366965c..0603b38 100644 --- a/docs/QQQ-SIGN-IN.md +++ b/docs/QQQ-SIGN-IN.md @@ -20,6 +20,8 @@ The server and its identity provider govern session lifetime, renewal and curren Explicit sign-in can replace unreadable saved QQQ session data only after fresh server authentication and, when device proof survives, authoritative device binding. Rejected binding leaves the unreadable file untouched. Successful recovery preserves its ciphertext in a uniquely named `qqq-session.recovery-*.dpapi` file in the protected data directory. +Enrollment replaces the pairing and its session as one staged step. The session for the new pairing is first written as `qqq-session.staged.dpapi`; once the pairing is replaced, the next read of the session (enrollment itself, a sign-in or a service start) moves it into place, preserving an unreadable earlier session as `qqq-session.recovery-*.dpapi`. A staged session that does not belong to the stored pairing is discarded, and one that cannot be read is preserved under a recovery name. The activity log records each of these. + If device proof itself is unreadable, sign-in does not enable printing or reconstruct that proof. Explicit enrollment is required; it preserves the old ciphertext as `connection.recovery-*.dpapi`, printer choices and the old connection's journal without attaching those receipts to the new device. Read-only status remains available. Explicit sign-out can quarantine unreadable session data locally, but cannot revoke an unavailable server session; revoke that previous session in QQQ. Automatic connection recovery does not perform this explicit credential replacement. ## Replace a revoked or expired pairing diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs index aeb4874..663c020 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs @@ -88,7 +88,7 @@ public sealed partial class AgentRuntime return false; } using AgentStore store = new(directory, recoverUnreadableConfiguration: true); - AgentAuthentication? saved = store.LoadAuthentication(); + AgentAuthentication? saved = LoadSavedSession(store, forRecovery: false); AgentConfiguration? configuration = store.LoadConfiguration(); if(saved?.Credential is null || (saved.Binding.HasApprovedDevice && (configuration is null || !SameOrigin(saved.Binding.Origin, configuration.ServerUrl)))) @@ -125,6 +125,38 @@ public sealed partial class AgentRuntime + /// + /// Reads the saved session and logs what that read did about a pairing change that was staged and not finished. The + /// store has no log, and moving, discarding or setting aside a credential file must leave a line behind. + /// + private AgentAuthentication? LoadSavedSession(AgentStore store, bool forRecovery) + { + AgentAuthentication? saved = forRecovery ? store.LoadAuthenticationForRecovery() : store.LoadAuthentication(); + switch(store.StagedPairing) + { + case StagedPairingOutcome.Completed: + WriteLog("INFO", "Finished a pairing change that was interrupted: the session saved with the new pairing is now in place."); + break; + case StagedPairingOutcome.CompletedAfterQuarantine: + WriteLog("WARN", "Finished a pairing change that was interrupted: the session saved with the new pairing is now in place, and the earlier session data, which could not be read, was moved aside for recovery."); + break; + case StagedPairingOutcome.Deferred: + WriteLog("WARN", "A pairing change is waiting to finish: the session saved with the new pairing is in use but could not be moved into place yet, and will be tried again."); + break; + case StagedPairingOutcome.Discarded: + WriteLog("INFO", "Discarded a session left over from a pairing change that did not replace the pairing."); + break; + case StagedPairingOutcome.SetAside: + WriteLog("WARN", "A session staged for a pairing change could not be read and was moved aside for recovery."); + break; + default: + break; + } + return saved; + } + + + /// Explains, for the log, why the service cannot talk to QQQ yet: no session, signed out, no pairing, or a session for a server other than the paired one. private static string DescribeMissingAuthorization(AgentAuthentication? saved, AgentConfiguration? configuration) => saved is null ? $"No saved {AppIdentity.ServerName} session: sign in from the companion to enable printing." diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs index 95edd93..1c1e4b8 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs @@ -36,8 +36,8 @@ public sealed partial class AgentRuntime /// /// Exchanges the enrollment code for a device credential using the live user session on the selected server, binds the /// new device to that session and checks the identity again. Only then are the stored pairing and stored session replaced, - /// one straight after the other, and the connection state reset, so a refused or interrupted bind leaves both files as they - /// were. Authentication changes are serialized by a gate. + /// as one staged step, and the connection state reset, so a refused or interrupted bind leaves both files as they were. + /// Authentication changes are serialized by a gate. /// private async Task EnrollNativeAsync(PipeRequest request, CancellationToken token) { @@ -74,7 +74,8 @@ public sealed partial class AgentRuntime using AgentStore store = new(directory, recoverUnreadableConfiguration: true); store.StagePairing(new AgentConfiguration(request.ServerUrl, response.DeviceId, response.DeviceToken, response.PollIntervalSeconds), authentication); progress = EnrollmentProgress.Saved; - store.CompleteStagedPairing(); + if(store.CompleteStagedPairing() is StagedPairingOutcome.Deferred) + { throw new IOException("The session saved with the new pairing could not be moved into place yet."); } ResetConnectionState(); ActivateSession(session, actual, store); progress = EnrollmentProgress.Activated; diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.SignIn.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.SignIn.cs index 12ae13c..49b25de 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.SignIn.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.SignIn.cs @@ -49,7 +49,7 @@ public sealed partial class AgentRuntime WriteLog("WARN", $"Sign-in refused: a new pairing needs a session without device proof, but the session already names device {binding.DeviceId}."); throw new QqqSessionException(QqqSessionException.FailureKind.PermissionDenied, $"New pairing requires a fresh {AppIdentity.ServerName} session without device proof."); } - AgentAuthentication? saved = store.LoadAuthenticationForRecovery(); + AgentAuthentication? saved = LoadSavedSession(store, forRecovery: true); if(!request.StartNewPairing && configuration is not null && SameOrigin(credential.Origin, configuration.ServerUrl)) { WriteLog("INFO", $"Step 3 of 4: the session is for the paired server; binding device {configuration.DeviceId}{(request.ClaimLegacyDevice ? " and claiming it as a legacy device" : "")}."); @@ -155,8 +155,8 @@ public sealed partial class AgentRuntime WriteLog("INFO", "Sign-out requested: blocking printing, removing the saved session credential and ending the server session. Pairing and history are kept."); BlockAuthorization(); using AgentStore store = new(directory, recoverUnreadableConfiguration: true); - AgentAuthentication? saved = store.LoadAuthenticationForRecovery(); - bool unreadableSession = store.QuarantineUnreadableAuthentication(); + AgentAuthentication? saved = LoadSavedSession(store, forRecovery: true); + bool unreadableSession = store.QuarantineUnreadableAuthentication() || store.StagedPairing is StagedPairingOutcome.CompletedAfterQuarantine; if(unreadableSession) { WriteLog("WARN", $"The saved session data could not be read and was moved aside for recovery; revoke the previous session in {AppIdentity.ServerName}."); } if(saved is not null) diff --git a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs index 9b0e9f0..e5b3dea 100644 --- a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs @@ -11,8 +11,10 @@ using Qqq.PrintAgent.Core.Printing; namespace Qqq.PrintAgent.Windows; /// -/// Saved QQQ session part of the store. The session identity and credential are kept in one DPAPI-protected file; an -/// unreadable file is quarantined (moved aside, never deleted) so a user can sign in again without losing evidence. +/// Saved QQQ session part of the store. The session identity and credential are kept in one DPAPI-protected file. While a +/// pairing change is in progress, the session that belongs to the new pairing waits beside it under a staged name until +/// it is moved into place. An unreadable session file, stored or staged, is moved aside under a recovery name and never +/// deleted, so a user can sign in again without losing evidence. /// public sealed partial class AgentStore { @@ -22,6 +24,11 @@ public sealed partial class AgentStore + /// What the last read of the session did about a staged pairing change, for the caller to log. + internal StagedPairingOutcome StagedPairing { get; private set; } + + + /// Loads the saved session for explicit recovery: a file that cannot be decrypted or parsed is flagged for quarantine and reported as absent, so setup can proceed instead of failing on a damaged file. internal AgentAuthentication? LoadAuthenticationForRecovery() { @@ -53,23 +60,12 @@ public sealed partial class AgentStore /// - /// Reads and decrypts the saved session, or returns null when none exists. A pairing change that was staged and not - /// finished is finished first, so the session returned always belongs to the stored pairing. The file is size-bounded, - /// decrypted for the current service account, parsed and validated, and the decrypted bytes are zeroed afterwards. A - /// damaged or invalid file throws. + /// Returns the saved session to act on, or null when there is none. A pairing change that was staged and not finished + /// is dealt with first: a staged session that belongs to the stored pairing is the one returned, whether or not it + /// could be moved into place yet, so a caller never acts on a session left over from the previous pairing. Files are + /// size-bounded, decrypted for the current service account, parsed and validated. A damaged stored file throws. /// - public AgentAuthentication? LoadAuthentication() - { - try - { - CompleteStagedPairing(); - } - catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) - { - // The staged session could not be moved into place now; the next reader tries again, and until then the stored session is the earlier one. - } - return ReadAuthentication(Path.Combine(_directory, SessionFile)); - } + public AgentAuthentication? LoadAuthentication() => ResolveStagedPairing() ?? ReadAuthentication(Path.Combine(_directory, SessionFile)); @@ -132,19 +128,19 @@ public sealed partial class AgentStore WriteProtected(path + ".tmp", authentication); _ = QuarantineUnreadableAuthentication(); File.Move(path + ".tmp", path, overwrite: true); - File.Delete(Path.Combine(_directory, StagedSessionFile)); + DeleteQuietly(Path.Combine(_directory, StagedSessionFile)); } /// - /// Starts replacing the pairing and its session as one step. The session is written first under a staged name and the - /// pairing is replaced second. From then on the change is decided: this caller, a sign-in or the next service start - /// finishes it with , because the staged session names the stored pairing's device. If - /// the pairing cannot be written the staged session is removed and nothing has changed; a staged session left by a crash - /// before the pairing was replaced names a different device and is discarded by the next reader. + /// Starts replacing the pairing and its session as one step. The session is written first, flushed to disk, under a + /// staged name, and the pairing is replaced second. From then on the change is decided: every reader of the session + /// uses the staged one and moves it into place, because it names the stored pairing's server and device. If the pairing + /// cannot be written the staged session is removed and nothing has changed; a staged session left by a crash before the + /// pairing was replaced names a different device and is discarded by the next reader. /// - public void StagePairing(AgentConfiguration configuration, AgentAuthentication authentication) + internal void StagePairing(AgentConfiguration configuration, AgentAuthentication authentication) { string staged = Path.Combine(_directory, StagedSessionFile); WriteProtected(staged + ".tmp", authentication); @@ -155,73 +151,145 @@ public sealed partial class AgentStore } catch { - File.Delete(staged); + DeleteQuietly(staged); throw; } } + /// Deals with a staged pairing change now, as every read of the session does, and reports what was done. + internal StagedPairingOutcome CompleteStagedPairing() + { + _ = ResolveStagedPairing(); + return StagedPairing; + } + + + /// - /// Finishes a staged pairing change, or discards what is left of one that never replaced the pairing. A staged session - /// that names the stored pairing's server and device is moved into place as the session; any other (a different device, - /// an unreadable file, or no readable pairing) is deleted. An unreadable earlier session is quarantined, not overwritten. + /// Decides what a staged session is and acts on it. One that belongs to the stored pairing, while the stored session + /// does not, is the session of a pairing change in progress: it is moved into place when that is possible, an unreadable + /// earlier session being moved aside first, and returned either way. One that names another server or device, or that + /// the stored session has already superseded, is a leftover and is deleted. One that cannot be read is moved aside. /// - public void CompleteStagedPairing() + private AgentAuthentication? ResolveStagedPairing() { string staged = Path.Combine(_directory, StagedSessionFile); - if(!File.Exists(staged)) + DeleteQuietly(staged + ".tmp"); + StagedPairing = StagedPairingOutcome.None; + AgentAuthentication? session = File.Exists(staged) ? ReadStagedOrSetAside(staged) : null; + if(session is null) { - return; - } - if(!StagedSessionBelongsToPairing(staged)) - { - File.Delete(staged); - return; + return null; } string path = Path.Combine(_directory, SessionFile); + AgentConfiguration? pairing = LoadPairingOrNull(); + AgentAuthentication? stored = ReadStoredOrFlag(path); + if(!BelongsToPairing(session, pairing) || (stored is not null && BelongsToPairing(stored, pairing))) + { + File.Delete(staged); + StagedPairing = StagedPairingOutcome.Discarded; + return null; + } try { - _ = ReadAuthentication(path); + bool quarantined = QuarantineUnreadableAuthentication(); + File.Move(staged, path, overwrite: true); + StagedPairing = quarantined ? StagedPairingOutcome.CompletedAfterQuarantine : StagedPairingOutcome.Completed; + } + catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) + { + // Something holds the stored session open. The staged one is still the right session to act on, and the next reader moves it. + StagedPairing = StagedPairingOutcome.Deferred; + } + return session; + } + + + + /// Reads the staged session; one that cannot be decrypted or parsed is moved aside under a recovery name, because after the pairing was replaced it may be the only copy. + private AgentAuthentication? ReadStagedOrSetAside(string staged) + { + try + { + return ReadAuthentication(staged); + } + catch(Exception exception) when(exception is CryptographicException or JsonException or InvalidDataException or ArgumentException) + { + File.Move(staged, Path.Combine(_directory, "qqq-session.recovery-" + Guid.NewGuid().ToString("N") + ".dpapi")); + StagedPairing = StagedPairingOutcome.SetAside; + return null; + } + } + + + + /// Reads the stored session for comparison; an unreadable one is flagged for quarantine and treated as absent. + private AgentAuthentication? ReadStoredOrFlag(string path) + { + try + { + return ReadAuthentication(path); } catch(Exception exception) when(exception is CryptographicException or JsonException or InvalidDataException or ArgumentException) { _authenticationRecoveryRequired = true; + return null; } - _ = QuarantineUnreadableAuthentication(); - File.Move(staged, path, overwrite: true); } - /// True when the staged session can be read and names the same server and device as the stored pairing. - private bool StagedSessionBelongsToPairing(string staged) + /// Reads the stored pairing, treating one that cannot be read as absent: a pairing change only ever leaves a readable pairing behind. + private AgentConfiguration? LoadPairingOrNull() { try { - AgentAuthentication? session = ReadAuthentication(staged); - AgentConfiguration? pairing = LoadConfiguration(); - return session is not null && pairing is not null - && string.Equals(session.Binding.DeviceId, pairing.DeviceId, StringComparison.Ordinal) - && Uri.TryCreate(pairing.ServerUrl, UriKind.Absolute, out Uri? server) - && string.Equals(server.GetLeftPart(UriPartial.Authority), session.Binding.Origin.GetLeftPart(UriPartial.Authority), StringComparison.OrdinalIgnoreCase); + return LoadConfiguration(); } - catch(Exception exception) when(exception is CryptographicException or JsonException or InvalidDataException or ArgumentException or UriFormatException) + catch(Exception exception) when(exception is CryptographicException or JsonException or ArgumentException or UriFormatException) { - return false; + return null; } } - /// Writes one session to a file protected for the service account, zeroing the plaintext buffer on every path. + /// True when a session names the same server and device as the pairing. + private static bool BelongsToPairing(AgentAuthentication session, AgentConfiguration? pairing) + => pairing is not null + && string.Equals(session.Binding.DeviceId, pairing.DeviceId, StringComparison.Ordinal) + && Uri.TryCreate(pairing.ServerUrl, UriKind.Absolute, out Uri? server) + && string.Equals(server.GetLeftPart(UriPartial.Authority), session.Binding.Origin.GetLeftPart(UriPartial.Authority), StringComparison.OrdinalIgnoreCase); + + + + /// Writes one session to a file protected for the service account and flushes it to disk, zeroing the plaintext buffer on every path. private static void WriteProtected(string path, AgentAuthentication authentication) { byte[] bytes = JsonSerializer.SerializeToUtf8Bytes(authentication, PrintApiClient.JsonOptions); try { - File.WriteAllBytes(path, ProtectedData.Protect(bytes, null, DataProtectionScope.CurrentUser)); + using FileStream stream = new(path, FileMode.Create, FileAccess.Write, FileShare.None); + stream.Write(ProtectedData.Protect(bytes, null, DataProtectionScope.CurrentUser)); + stream.Flush(flushToDisk: true); } finally { CryptographicOperations.ZeroMemory(bytes); } } + + + + /// Deletes a leftover file without failing the operation that noticed it. + private static void DeleteQuietly(string path) + { + try + { + File.Delete(path); + } + catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) + { + // A leftover is harmless: a staged session is examined again by the next reader, and a temporary file is overwritten by the next write. + } + } } diff --git a/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs b/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs new file mode 100644 index 0000000..3dfaedc --- /dev/null +++ b/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +namespace Qqq.PrintAgent.Windows; + +/// +/// What reading the saved session did about a pairing change that was staged and not finished. The store has no log of +/// its own, so it reports this and the runtime writes the line: moving, discarding or setting aside a credential file +/// should never happen silently. +/// +internal enum StagedPairingOutcome +{ + /// Nothing was staged. + None, + + /// The staged session belonged to the stored pairing and was moved into place. + Completed, + + /// As , after an unreadable earlier session was moved aside so it was not overwritten. + CompletedAfterQuarantine, + + /// The staged session belongs to the stored pairing and is being used, but could not be moved into place yet; the next reader tries again. + Deferred, + + /// The staged session did not belong to the stored pairing, or the stored session already did, so it was deleted. + Discarded, + + /// The staged session could not be read and was moved aside under a recovery name instead of being deleted. + SetAside, +} diff --git a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs index 4d0de23..88c5c0b 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs @@ -328,6 +328,7 @@ public sealed class AgentNewPairingTests Assert.True(File.Exists(StagedSession(fixture))); AgentSnapshot failed = runtime.GetSnapshot(); Assert.False(failed.Authentication!.Authorized); + Assert.Contains("Sign in again", failed.Authentication.Message, StringComparison.Ordinal); Assert.Contains(failed.Logs, entry => entry.Message.Contains("after the new pairing and its session were saved", StringComparison.Ordinal)); if(restart) { @@ -385,50 +386,166 @@ public sealed class AgentNewPairingTests /// - /// A staged session that does not belong to the stored pairing is what a crash leaves when the session was staged and the - /// pairing was never replaced. The next read discards it and returns the session that was already stored, unchanged. + /// "Sign in again" is read by some operators as sign out, then sign in. After an interrupted save that must work too: + /// the sign-out acts on the session that belongs to the new pairing, so the sign-in that follows is accepted. /// [Fact] - public void StagedSessionForAnotherDeviceIsDiscardedOnRead() + public async Task InterruptedSaveFollowedBySignOutStillRecoversBySignIn() { using SessionFixture fixture = new(); fixture.SaveSession(); string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); - byte[] stored = File.ReadAllBytes(authPath); - QqqSessionBinding other = fixture.Session.Binding with { DeviceId = "interrupted-device" }; - using(AgentStore staging = new(fixture.Local.DirectoryPath)) + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + using AgentRuntime runtime = fixture.Runtime(); + _ = await runtime.PollControlsOnceAsync(timeout.Token); + using(FileStream held = new(authPath, FileMode.Open, FileAccess.Read, FileShare.Read)) { - staging.StagePairing(new AgentConfiguration("https://synthetic.invalid", "interrupted-device", "synthetic-interrupted"), new AgentAuthentication(other, fixture.Session.ExportCredential())); - staging.SaveConfiguration(new AgentConfiguration("https://synthetic.invalid", "device", "synthetic-token")); + _ = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); } Assert.True(File.Exists(StagedSession(fixture))); + _ = await runtime.SignOutAsync(timeout.Token); + Assert.False(File.Exists(StagedSession(fixture))); + Assert.False(runtime.GetSnapshot().Authentication!.Authorized); + _ = await runtime.SignInAsync(new PipeRequest("sign-in", SessionCredential: fixture.Session.ExportCredential()), timeout.Token); + AgentSnapshot recovered = runtime.GetSnapshot(); + Assert.True(recovered.Authentication!.Authorized); + Assert.Equal("replacement-device", recovered.DeviceId); + Assert.Contains(recovered.Logs, entry => entry.Message.Contains("Finished a pairing change that was interrupted", StringComparison.Ordinal)); + } + + + + /// + /// While something holds the stored session open, a staged session that belongs to the stored pairing cannot be moved + /// into place. It is still the one a reader gets, so nothing acts on the session of the previous pairing, and the move + /// happens on the first read after the file is released. + /// + [Fact] + public void StagedSessionIsUsedBeforeItCanBeMovedIntoPlace() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); + Stage(fixture, "replacement-device", "https://synthetic.invalid/"); + using(FileStream held = new(authPath, FileMode.Open, FileAccess.Read, FileShare.Read)) + { + using AgentStore blocked = new(fixture.Local.DirectoryPath); + Assert.Equal("replacement-device", blocked.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(StagedPairingOutcome.Deferred, blocked.StagedPairing); + Assert.True(File.Exists(StagedSession(fixture))); + } using AgentStore store = new(fixture.Local.DirectoryPath); - AgentAuthentication? loaded = store.LoadAuthentication(); - Assert.Equal("device", loaded!.Binding.DeviceId); - Assert.Equal(stored, File.ReadAllBytes(authPath)); + Assert.Equal("replacement-device", store.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(StagedPairingOutcome.Completed, store.StagedPairing); + Assert.False(File.Exists(StagedSession(fixture))); + Assert.Equal("replacement-device", store.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(StagedPairingOutcome.None, store.StagedPairing); + } + + + + /// Moving a staged session into place over a stored session that cannot be read preserves the unreadable bytes under a recovery name. + [Fact] + public void StagedSessionReplacesAnUnreadableStoredSessionWithoutLosingIt() + { + using SessionFixture fixture = new(); + byte[] unreadable = [9, 8, 7]; + File.WriteAllBytes(Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"), unreadable); + Stage(fixture, "replacement-device", "https://synthetic.invalid/"); + using AgentStore store = new(fixture.Local.DirectoryPath); + Assert.Equal("replacement-device", store.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(StagedPairingOutcome.CompletedAfterQuarantine, store.StagedPairing); + Assert.Equal(unreadable, File.ReadAllBytes(Assert.Single(Directory.GetFiles(fixture.Local.DirectoryPath, "qqq-session.recovery-*.dpapi")))); Assert.False(File.Exists(StagedSession(fixture))); } /// - /// An unreadable staged session is discarded rather than trusted, and saving a session normally removes a staged one, - /// because what is saved is newer than what was staged. + /// A staged session is a leftover, and is deleted on the next read, when it names another device (a crash before the + /// pairing was replaced), another server, when no pairing is stored, or when the stored session already belongs to + /// the pairing. The stored session is returned as it was. /// - [Fact] - public void UnreadableOrSupersededStagedSessionIsRemoved() + [Theory] + [InlineData("another-device")] + [InlineData("another-server")] + [InlineData("no-pairing")] + [InlineData("already-stored")] + public void LeftoverStagedSessionIsDiscardedOnRead(string leftover) { using SessionFixture fixture = new(); fixture.SaveSession(); - File.WriteAllBytes(StagedSession(fixture), [2, 4, 6]); + string path = Path.Combine(fixture.Local.DirectoryPath, "connection.dpapi"); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); + Stage(fixture, leftover is "another-device" ? "interrupted-device" : "device", leftover is "another-server" ? "https://elsewhere.invalid/" : "https://synthetic.invalid/"); + using(AgentStore setup = new(fixture.Local.DirectoryPath)) + { + setup.SaveConfiguration(new AgentConfiguration("https://synthetic.invalid", "device", "synthetic-token")); + } + if(leftover is "no-pairing") + { File.Delete(path); } + if(leftover is "another-server") + { File.Delete(authPath); } + byte[]? stored = File.Exists(authPath) ? File.ReadAllBytes(authPath) : null; + using AgentStore store = new(fixture.Local.DirectoryPath); + AgentAuthentication? loaded = store.LoadAuthentication(); + Assert.Equal(StagedPairingOutcome.Discarded, store.StagedPairing); + Assert.False(File.Exists(StagedSession(fixture))); + Assert.Equal(stored is null ? null : "device", loaded?.Binding.DeviceId); + Assert.Equal(stored, File.Exists(authPath) ? File.ReadAllBytes(authPath) : null); + } + + + + /// A staged session that cannot be read is preserved under a recovery name, not deleted and not trusted; the stored session is returned. + [Fact] + public void UnreadableStagedSessionIsSetAside() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + byte[] unreadable = [2, 4, 6]; + File.WriteAllBytes(StagedSession(fixture), unreadable); + using AgentStore store = new(fixture.Local.DirectoryPath); + Assert.Equal("device", store.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(StagedPairingOutcome.SetAside, store.StagedPairing); + Assert.False(File.Exists(StagedSession(fixture))); + Assert.Equal(unreadable, File.ReadAllBytes(Assert.Single(Directory.GetFiles(fixture.Local.DirectoryPath, "qqq-session.recovery-*.dpapi")))); + } + + + + /// + /// Saving a session normally removes a readable staged session, because what is saved is newer; and staging again + /// replaces what was staged before, so the last pairing change is the one that is finished. + /// + [Fact] + public void SavingOrStagingAgainSupersedesAStagedSession() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + Stage(fixture, "first-device", "https://synthetic.invalid/"); using(AgentStore store = new(fixture.Local.DirectoryPath)) { - Assert.Equal("device", store.LoadAuthentication()!.Binding.DeviceId); - Assert.False(File.Exists(StagedSession(fixture))); - File.WriteAllBytes(StagedSession(fixture), [2, 4, 6]); - store.SaveAuthentication(new AgentAuthentication(fixture.Session.Binding, fixture.Session.ExportCredential())); + store.SaveAuthentication(new AgentAuthentication(fixture.Session.Binding with { DeviceId = "first-device" }, fixture.Session.ExportCredential())); } Assert.False(File.Exists(StagedSession(fixture))); + Stage(fixture, "second-device", "https://synthetic.invalid/"); + Stage(fixture, "third-device", "https://synthetic.invalid/"); + using AgentStore reader = new(fixture.Local.DirectoryPath); + Assert.Equal("third-device", reader.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(StagedPairingOutcome.Completed, reader.StagedPairing); + Assert.Equal("third-device", reader.LoadConfiguration()!.DeviceId); + } + + + + /// Stages a pairing for the given device together with a session that names the given device on the given server, without finishing the change. + private static void Stage(SessionFixture fixture, string deviceId, string sessionOrigin) + { + Uri origin = new(sessionOrigin); + using AgentStore store = new(fixture.Local.DirectoryPath); + store.StagePairing(new AgentConfiguration("https://synthetic.invalid", deviceId, "synthetic-" + deviceId), + new AgentAuthentication(new QqqSessionBinding(origin, "principal", "tenant", deviceId), new QqqSessionCredential(origin, "synthetic-qqq-cookie"))); } -- 2.54.0 From 3fb80deb4582f9c0ab2355593bf7993c1a73293a Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 12:52:10 -0600 Subject: [PATCH 7/9] fix(enrollment): never fail a session read over a leftover staged file A confirmation review found one regression in the previous commit and two test gaps. - Discarding a leftover staged session, and setting an unreadable one aside, could throw when the file was held open, although the stored session was already the right answer. Both are now best-effort: the read returns the stored session and the next reader deals with the leftover. - When the staged session cannot be moved into place, the cause is kept and logged, and enrollment's error carries it. - A session file that was set aside is reported at sign-out whether it was the stored one or the staged one. Tests: a sign-out while the staged file is held open, followed by a sign-in; recovery attempted while the stored file is still held, which must fail without changing the pairing or the staged session and succeed after release; the log lines for a deferred move, a discard and a file set aside; and the "another device" discard isolated from the "already stored" rule. Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- .../AgentRuntime.Authentication.cs | 22 ++++- .../AgentRuntime.Enrollment.cs | 24 +++++- .../AgentRuntime.SignIn.cs | 2 +- .../AgentStore.Authentication.cs | 23 ++++- .../AgentNewPairingTests.cs | 86 +++++++++++++++++-- 5 files changed, 139 insertions(+), 18 deletions(-) diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs index 663c020..e786ac2 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs @@ -131,7 +131,21 @@ public sealed partial class AgentRuntime /// private AgentAuthentication? LoadSavedSession(AgentStore store, bool forRecovery) { - AgentAuthentication? saved = forRecovery ? store.LoadAuthenticationForRecovery() : store.LoadAuthentication(); + try + { + return forRecovery ? store.LoadAuthenticationForRecovery() : store.LoadAuthentication(); + } + finally + { + NoteStagedPairing(store); + } + } + + + + /// Writes the log line for what the store last did about a staged pairing change, including when the read that did it then failed. + private void NoteStagedPairing(AgentStore store) + { switch(store.StagedPairing) { case StagedPairingOutcome.Completed: @@ -141,10 +155,11 @@ public sealed partial class AgentRuntime WriteLog("WARN", "Finished a pairing change that was interrupted: the session saved with the new pairing is now in place, and the earlier session data, which could not be read, was moved aside for recovery."); break; case StagedPairingOutcome.Deferred: - WriteLog("WARN", "A pairing change is waiting to finish: the session saved with the new pairing is in use but could not be moved into place yet, and will be tried again."); + WriteLog("WARN", "A pairing change is waiting to finish: the session saved with the new pairing is in use but could not be moved into place yet, and will be tried again. Reason: " + + (store.StagedPairingFailure is null ? "not recorded." : QqqSessionDiagnostics.Describe(store.StagedPairingFailure))); break; case StagedPairingOutcome.Discarded: - WriteLog("INFO", "Discarded a session left over from a pairing change that did not replace the pairing."); + WriteLog("INFO", "Discarded a session left over from a pairing change: it did not belong to the stored pairing, or the stored session already did."); break; case StagedPairingOutcome.SetAside: WriteLog("WARN", "A session staged for a pairing change could not be read and was moved aside for recovery."); @@ -152,7 +167,6 @@ public sealed partial class AgentRuntime default: break; } - return saved; } diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs index 1c1e4b8..fa72a9e 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Enrollment.cs @@ -74,8 +74,7 @@ public sealed partial class AgentRuntime using AgentStore store = new(directory, recoverUnreadableConfiguration: true); store.StagePairing(new AgentConfiguration(request.ServerUrl, response.DeviceId, response.DeviceToken, response.PollIntervalSeconds), authentication); progress = EnrollmentProgress.Saved; - if(store.CompleteStagedPairing() is StagedPairingOutcome.Deferred) - { throw new IOException("The session saved with the new pairing could not be moved into place yet."); } + FinishStagedPairing(store); ResetConnectionState(); ActivateSession(session, actual, store); progress = EnrollmentProgress.Activated; @@ -97,6 +96,27 @@ public sealed partial class AgentRuntime + /// + /// Moves the session staged with the new pairing into place. When that cannot happen yet the enrollment fails with the + /// cause, having already saved everything a sign-in or a restart needs to finish it. An earlier session that could not + /// be read is reported, because it was moved aside rather than replaced. + /// + private void FinishStagedPairing(AgentStore store) + { + switch(store.CompleteStagedPairing()) + { + case StagedPairingOutcome.Deferred: + throw new IOException("The session saved with the new pairing could not be moved into place yet.", store.StagedPairingFailure); + case StagedPairingOutcome.CompletedAfterQuarantine: + WriteLog("WARN", "The earlier session data could not be read and was moved aside for recovery."); + break; + default: + break; + } + } + + + /// /// Stops the live session being used once the server has bound it to the new device while this computer has not finished /// switching to it, then says what the failed enrollment left behind and how to recover. Such a session no longer matches diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.SignIn.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.SignIn.cs index 49b25de..8558ced 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.SignIn.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.SignIn.cs @@ -156,7 +156,7 @@ public sealed partial class AgentRuntime BlockAuthorization(); using AgentStore store = new(directory, recoverUnreadableConfiguration: true); AgentAuthentication? saved = LoadSavedSession(store, forRecovery: true); - bool unreadableSession = store.QuarantineUnreadableAuthentication() || store.StagedPairing is StagedPairingOutcome.CompletedAfterQuarantine; + bool unreadableSession = store.QuarantineUnreadableAuthentication() || store.StagedPairing is StagedPairingOutcome.CompletedAfterQuarantine or StagedPairingOutcome.SetAside; if(unreadableSession) { WriteLog("WARN", $"The saved session data could not be read and was moved aside for recovery; revoke the previous session in {AppIdentity.ServerName}."); } if(saved is not null) diff --git a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs index e5b3dea..85dca54 100644 --- a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs @@ -29,6 +29,11 @@ public sealed partial class AgentStore + /// Why the staged session could not be moved into place when the outcome is ; otherwise null. + internal Exception? StagedPairingFailure { get; private set; } + + + /// Loads the saved session for explicit recovery: a file that cannot be decrypted or parsed is flagged for quarantine and reported as absent, so setup can proceed instead of failing on a damaged file. internal AgentAuthentication? LoadAuthenticationForRecovery() { @@ -178,6 +183,7 @@ public sealed partial class AgentStore string staged = Path.Combine(_directory, StagedSessionFile); DeleteQuietly(staged + ".tmp"); StagedPairing = StagedPairingOutcome.None; + StagedPairingFailure = null; AgentAuthentication? session = File.Exists(staged) ? ReadStagedOrSetAside(staged) : null; if(session is null) { @@ -188,7 +194,8 @@ public sealed partial class AgentStore AgentAuthentication? stored = ReadStoredOrFlag(path); if(!BelongsToPairing(session, pairing) || (stored is not null && BelongsToPairing(stored, pairing))) { - File.Delete(staged); + // The stored session is already the right answer, so a leftover that cannot be deleted now must not fail the read. + DeleteQuietly(staged); StagedPairing = StagedPairingOutcome.Discarded; return null; } @@ -200,8 +207,9 @@ public sealed partial class AgentStore } catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) { - // Something holds the stored session open. The staged one is still the right session to act on, and the next reader moves it. + // Something holds one of the two files open. The staged session is still the right one to act on, and the next reader moves it. StagedPairing = StagedPairingOutcome.Deferred; + StagedPairingFailure = exception; } return session; } @@ -217,8 +225,15 @@ public sealed partial class AgentStore } catch(Exception exception) when(exception is CryptographicException or JsonException or InvalidDataException or ArgumentException) { - File.Move(staged, Path.Combine(_directory, "qqq-session.recovery-" + Guid.NewGuid().ToString("N") + ".dpapi")); - StagedPairing = StagedPairingOutcome.SetAside; + try + { + File.Move(staged, Path.Combine(_directory, "qqq-session.recovery-" + Guid.NewGuid().ToString("N") + ".dpapi")); + StagedPairing = StagedPairingOutcome.SetAside; + } + catch(Exception moveFailure) when(moveFailure is IOException or UnauthorizedAccessException) + { + // It stays where it is and the next reader sets it aside; the stored session is still the answer to this read. + } return null; } } diff --git a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs index 88c5c0b..af19d34 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs @@ -386,8 +386,9 @@ public sealed class AgentNewPairingTests /// - /// "Sign in again" is read by some operators as sign out, then sign in. After an interrupted save that must work too: - /// the sign-out acts on the session that belongs to the new pairing, so the sign-in that follows is accepted. + /// "Sign in again" is read by some operators as sign out, then sign in. That must work even when the staged session still + /// cannot be moved at sign-out, here because something holds the staged file open: the sign-out acts on the session that + /// belongs to the new pairing, the leftover staged file is discarded later, and the sign-in that follows is accepted. /// [Fact] public async Task InterruptedSaveFollowedBySignOutStillRecoversBySignIn() @@ -402,15 +403,83 @@ public sealed class AgentNewPairingTests { _ = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); } + using(FileStream held = new(StagedSession(fixture), FileMode.Open, FileAccess.Read, FileShare.Read)) + { + _ = await runtime.SignOutAsync(timeout.Token); + } Assert.True(File.Exists(StagedSession(fixture))); - _ = await runtime.SignOutAsync(timeout.Token); - Assert.False(File.Exists(StagedSession(fixture))); + Assert.Contains(runtime.GetSnapshot().Logs, entry => entry.Message.Contains("A pairing change is waiting to finish", StringComparison.Ordinal)); Assert.False(runtime.GetSnapshot().Authentication!.Authorized); _ = await runtime.SignInAsync(new PipeRequest("sign-in", SessionCredential: fixture.Session.ExportCredential()), timeout.Token); AgentSnapshot recovered = runtime.GetSnapshot(); Assert.True(recovered.Authentication!.Authorized); Assert.Equal("replacement-device", recovered.DeviceId); - Assert.Contains(recovered.Logs, entry => entry.Message.Contains("Finished a pairing change that was interrupted", StringComparison.Ordinal)); + Assert.False(File.Exists(StagedSession(fixture))); + Assert.Contains(recovered.Logs, entry => entry.Message.Contains("Discarded a session left over from a pairing change", StringComparison.Ordinal)); + } + + + + /// + /// While the stored session is still held open, recovery cannot finish, and it must not do damage either: a sign-in fails, + /// the new pairing and its staged session stay as they are, and the same sign-in succeeds once the file is released. + /// + [Fact] + public async Task RecoveryWaitsWhileTheStoredSessionIsHeldAndSucceedsAfter() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + string path = Path.Combine(fixture.Local.DirectoryPath, "connection.dpapi"); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + using AgentRuntime runtime = fixture.Runtime(); + _ = await runtime.PollControlsOnceAsync(timeout.Token); + using(FileStream held = new(authPath, FileMode.Open, FileAccess.Read, FileShare.Read)) + { + _ = await Assert.ThrowsAnyAsync(() => runtime.EnrollWithGatesAsync(new PipeRequest("enroll", "https://synthetic.invalid", "synthetic-accepted-code"), timeout.Token)); + byte[] pairing = File.ReadAllBytes(path); + Exception thrown = await Assert.ThrowsAnyAsync(() => runtime.SignInAsync(new PipeRequest("sign-in", SessionCredential: fixture.Session.ExportCredential()), timeout.Token)); + Assert.True(thrown is IOException or UnauthorizedAccessException, thrown.GetType().FullName); + Assert.Equal(pairing, File.ReadAllBytes(path)); + Assert.True(File.Exists(StagedSession(fixture))); + Assert.False(runtime.GetSnapshot().Authentication!.Authorized); + } + _ = await runtime.SignInAsync(new PipeRequest("sign-in", SessionCredential: fixture.Session.ExportCredential()), timeout.Token); + AgentSnapshot recovered = runtime.GetSnapshot(); + Assert.True(recovered.Authentication!.Authorized); + Assert.Equal("replacement-device", recovered.DeviceId); + Assert.False(File.Exists(StagedSession(fixture))); + } + + + + /// + /// What the store does about a staged session reaches the activity log, and a session file that was moved aside reaches + /// the operator at sign-out as a reminder to revoke it on the server. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SettingASessionFileAsideIsLoggedAndReportedAtSignOut(bool stagedIsUnreadable) + { + using SessionFixture fixture = new(); + if(stagedIsUnreadable) + { + fixture.SaveSession(); + File.WriteAllBytes(StagedSession(fixture), [2, 4, 6]); + } + else + { + File.WriteAllBytes(Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"), [9, 8, 7]); + Stage(fixture, "device", "https://synthetic.invalid/"); + } + using CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10)); + using AgentRuntime runtime = fixture.Runtime(); + string reply = await runtime.SignOutAsync(timeout.Token); + Assert.Contains("revoke the previous session", reply, StringComparison.Ordinal); + Assert.Contains(runtime.GetSnapshot().Logs, entry => entry.Message.Contains( + stagedIsUnreadable ? "staged for a pairing change could not be read and was moved aside" : "the earlier session data, which could not be read, was moved aside", StringComparison.Ordinal)); + _ = Assert.Single(Directory.GetFiles(fixture.Local.DirectoryPath, "qqq-session.recovery-*.dpapi")); } @@ -484,8 +553,11 @@ public sealed class AgentNewPairingTests } if(leftover is "no-pairing") { File.Delete(path); } - if(leftover is "another-server") - { File.Delete(authPath); } + if(leftover is "another-server" or "another-device") + { + // Without a stored session only the staged session's own server or device can be the reason it is discarded. + File.Delete(authPath); + } byte[]? stored = File.Exists(authPath) ? File.ReadAllBytes(authPath) : null; using AgentStore store = new(fixture.Local.DirectoryPath); AgentAuthentication? loaded = store.LoadAuthentication(); -- 2.54.0 From 2d5e2d26280223ae5ec73553dfe0d172b75ff1af Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 13:02:36 -0600 Subject: [PATCH 8/9] fix(enrollment): a newer saved session supersedes a staged one A confirmation review found the earlier items resolved and three left. - A staged session that could not be deleted when a newer session was saved (the file was held open) could later replace that newer session, if the newer one did not belong to the stored pairing, such as a sign-in for a new pairing. A stored session written after the staged one now supersedes it. - A leftover that could not be deleted was still reported as discarded, on every read. It is reported only once it is gone. - The guarded discard and set-aside paths had no test. Both are now run with the staged file held open, and again after it is released. Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- .../AgentRuntime.Authentication.cs | 2 +- .../AgentStore.Authentication.cs | 34 +++++----- .../StagedPairingOutcome.cs | 2 +- .../AgentNewPairingTests.cs | 62 +++++++++++++++++++ 4 files changed, 84 insertions(+), 16 deletions(-) diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs index e786ac2..d054c40 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs @@ -159,7 +159,7 @@ public sealed partial class AgentRuntime + (store.StagedPairingFailure is null ? "not recorded." : QqqSessionDiagnostics.Describe(store.StagedPairingFailure))); break; case StagedPairingOutcome.Discarded: - WriteLog("INFO", "Discarded a session left over from a pairing change: it did not belong to the stored pairing, or the stored session already did."); + WriteLog("INFO", "Discarded a session left over from a pairing change: it did not belong to the stored pairing, or the stored session already did or is newer."); break; case StagedPairingOutcome.SetAside: WriteLog("WARN", "A session staged for a pairing change could not be read and was moved aside for recovery."); diff --git a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs index 85dca54..f0f20d2 100644 --- a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs @@ -133,7 +133,7 @@ public sealed partial class AgentStore WriteProtected(path + ".tmp", authentication); _ = QuarantineUnreadableAuthentication(); File.Move(path + ".tmp", path, overwrite: true); - DeleteQuietly(Path.Combine(_directory, StagedSessionFile)); + _ = DeleteQuietly(Path.Combine(_directory, StagedSessionFile)); } @@ -156,7 +156,7 @@ public sealed partial class AgentStore } catch { - DeleteQuietly(staged); + _ = DeleteQuietly(staged); throw; } } @@ -174,16 +174,16 @@ public sealed partial class AgentStore /// /// Decides what a staged session is and acts on it. One that belongs to the stored pairing, while the stored session - /// does not, is the session of a pairing change in progress: it is moved into place when that is possible, an unreadable - /// earlier session being moved aside first, and returned either way. One that names another server or device, or that - /// the stored session has already superseded, is a leftover and is deleted. One that cannot be read is moved aside. + /// does not and is older, is the session of a pairing change in progress: it is moved into place when that is possible, + /// an unreadable earlier session being moved aside first, and returned either way. One that names another server or + /// device, or that the stored session has superseded, is a leftover and is deleted. One that cannot be read is moved aside. /// private AgentAuthentication? ResolveStagedPairing() { - string staged = Path.Combine(_directory, StagedSessionFile); - DeleteQuietly(staged + ".tmp"); StagedPairing = StagedPairingOutcome.None; StagedPairingFailure = null; + string staged = Path.Combine(_directory, StagedSessionFile); + _ = DeleteQuietly(staged + ".tmp"); AgentAuthentication? session = File.Exists(staged) ? ReadStagedOrSetAside(staged) : null; if(session is null) { @@ -192,11 +192,15 @@ public sealed partial class AgentStore string path = Path.Combine(_directory, SessionFile); AgentConfiguration? pairing = LoadPairingOrNull(); AgentAuthentication? stored = ReadStoredOrFlag(path); - if(!BelongsToPairing(session, pairing) || (stored is not null && BelongsToPairing(stored, pairing))) + if(!BelongsToPairing(session, pairing) || (stored is not null && (BelongsToPairing(stored, pairing) || File.GetLastWriteTimeUtc(path) > File.GetLastWriteTimeUtc(staged)))) { - // The stored session is already the right answer, so a leftover that cannot be deleted now must not fail the read. - DeleteQuietly(staged); - StagedPairing = StagedPairingOutcome.Discarded; + //////////////////////////////////////////////////////////////////////// + // a leftover: it is for another pairing, or the stored session is // + // already this pairing's, or was saved after it and so is newer. One // + // that cannot be deleted now must not fail the read, and is reported // + // as discarded only once it is gone // + //////////////////////////////////////////////////////////////////////// + StagedPairing = DeleteQuietly(staged) ? StagedPairingOutcome.Discarded : StagedPairingOutcome.None; return null; } try @@ -295,16 +299,18 @@ public sealed partial class AgentStore - /// Deletes a leftover file without failing the operation that noticed it. - private static void DeleteQuietly(string path) + /// Deletes a leftover file without failing the operation that noticed it, and says whether the file is gone. + private static bool DeleteQuietly(string path) { try { File.Delete(path); + return true; } catch(Exception exception) when(exception is IOException or UnauthorizedAccessException) { - // A leftover is harmless: a staged session is examined again by the next reader, and a temporary file is overwritten by the next write. + // A staged session is examined again by the next reader, which discards it by the same rules, and a temporary file is overwritten by the next write. + return false; } } } diff --git a/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs b/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs index 3dfaedc..fdadc2c 100644 --- a/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs +++ b/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs @@ -24,7 +24,7 @@ internal enum StagedPairingOutcome /// The staged session belongs to the stored pairing and is being used, but could not be moved into place yet; the next reader tries again. Deferred, - /// The staged session did not belong to the stored pairing, or the stored session already did, so it was deleted. + /// The staged session did not belong to the stored pairing, or the stored session already did or was saved after it, and it was deleted. Discarded, /// The staged session could not be read and was moved aside under a recovery name instead of being deleted. diff --git a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs index af19d34..b4deea2 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs @@ -449,6 +449,7 @@ public sealed class AgentNewPairingTests Assert.True(recovered.Authentication!.Authorized); Assert.Equal("replacement-device", recovered.DeviceId); Assert.False(File.Exists(StagedSession(fixture))); + Assert.Contains(recovered.Logs, entry => entry.Message.Contains("Finished a pairing change that was interrupted", StringComparison.Ordinal)); } @@ -611,6 +612,67 @@ public sealed class AgentNewPairingTests + /// + /// A leftover staged session that something holds open cannot be deleted or moved aside. The read must still return the + /// stored session, must not claim to have discarded or set aside a file that is still there, and the first read after + /// the file is released deals with it. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public void HeldLeftoverStagedSessionDoesNotFailTheReadAndIsDealtWithLater(bool unreadable) + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + if(unreadable) + { File.WriteAllBytes(StagedSession(fixture), [2, 4, 6]); } + else + { File.Copy(Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"), StagedSession(fixture)); } + using(FileStream held = new(StagedSession(fixture), FileMode.Open, FileAccess.Read, FileShare.Read)) + { + using AgentStore blocked = new(fixture.Local.DirectoryPath); + Assert.Equal("device", blocked.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(StagedPairingOutcome.None, blocked.StagedPairing); + Assert.True(File.Exists(StagedSession(fixture))); + Assert.Empty(Directory.GetFiles(fixture.Local.DirectoryPath, "qqq-session.recovery-*.dpapi")); + } + using AgentStore store = new(fixture.Local.DirectoryPath); + Assert.Equal("device", store.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(unreadable ? StagedPairingOutcome.SetAside : StagedPairingOutcome.Discarded, store.StagedPairing); + Assert.False(File.Exists(StagedSession(fixture))); + } + + + + /// + /// A session saved after a pairing change was staged is newer than the staged one, even when it does not belong to the + /// stored pairing (a sign-in for a new pairing, say) and the staged file could not be removed at the time. The staged + /// session must not replace it later. + /// + [Fact] + public void SessionSavedAfterStagingIsNotReplacedByTheStagedOne() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); + Stage(fixture, "replacement-device", "https://synthetic.invalid/"); + using(FileStream held = new(StagedSession(fixture), FileMode.Open, FileAccess.Read, FileShare.Read)) + { + using AgentStore saving = new(fixture.Local.DirectoryPath); + saving.SaveAuthentication(new AgentAuthentication(fixture.Session.Binding with { DeviceId = null }, fixture.Session.ExportCredential())); + } + Assert.True(File.Exists(StagedSession(fixture))); + File.SetLastWriteTimeUtc(authPath, File.GetLastWriteTimeUtc(StagedSession(fixture)).AddSeconds(1)); + byte[] saved = File.ReadAllBytes(authPath); + using AgentStore store = new(fixture.Local.DirectoryPath); + Assert.Null(store.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(StagedPairingOutcome.Discarded, store.StagedPairing); + Assert.Equal(saved, File.ReadAllBytes(authPath)); + Assert.False(File.Exists(StagedSession(fixture))); + } + + + /// Stages a pairing for the given device together with a session that names the given device on the given server, without finishing the change. private static void Stage(SessionFixture fixture, string deviceId, string sessionOrigin) { -- 2.54.0 From 710b600efd87d01cd4e050d6ae62641f682a53ea Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 13:12:14 -0600 Subject: [PATCH 9/9] fix(enrollment): order the stored and staged session without the clock A confirmation review found the previous items resolved and one left: the rule that a session saved after staging supersedes the staged one compared file times, so a clock that stepped back between the two writes could make the new pairing's session look like a leftover and delete it. The order is now a logical one. Staging never stamps the staged session earlier than the stored session it is about to replace, and a save that cannot remove a staged session stamps itself later than it. The tests set the other file's time into the future, as a clock step would leave it, instead of forging the time the rule depends on: one shows the save stamping itself later and the staged session being discarded, the other shows staging keeping its order and the staged session being moved into place. Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- docs/QQQ-SIGN-IN.md | 2 +- .../AgentRuntime.Authentication.cs | 2 +- .../AgentStore.Authentication.cs | 17 +++++++++-- .../StagedPairingOutcome.cs | 2 +- .../AgentNewPairingTests.cs | 30 +++++++++++++++++-- 5 files changed, 44 insertions(+), 9 deletions(-) diff --git a/docs/QQQ-SIGN-IN.md b/docs/QQQ-SIGN-IN.md index 0603b38..6727dc4 100644 --- a/docs/QQQ-SIGN-IN.md +++ b/docs/QQQ-SIGN-IN.md @@ -20,7 +20,7 @@ The server and its identity provider govern session lifetime, renewal and curren Explicit sign-in can replace unreadable saved QQQ session data only after fresh server authentication and, when device proof survives, authoritative device binding. Rejected binding leaves the unreadable file untouched. Successful recovery preserves its ciphertext in a uniquely named `qqq-session.recovery-*.dpapi` file in the protected data directory. -Enrollment replaces the pairing and its session as one staged step. The session for the new pairing is first written as `qqq-session.staged.dpapi`; once the pairing is replaced, the next read of the session (enrollment itself, a sign-in or a service start) moves it into place, preserving an unreadable earlier session as `qqq-session.recovery-*.dpapi`. A staged session that does not belong to the stored pairing is discarded, and one that cannot be read is preserved under a recovery name. The activity log records each of these. +Enrollment replaces the pairing and its session as one staged step. The session for the new pairing is first written as `qqq-session.staged.dpapi`; once the pairing is replaced, the next read of the session (enrollment itself, a sign-in or a service start) moves it into place, preserving an unreadable earlier session as `qqq-session.recovery-*.dpapi`. A staged session is discarded when it does not belong to the stored pairing, or when the stored session already does or was saved after it; one that cannot be read is preserved under a recovery name. Until a staged session can be moved into place it is still the one that is used. The activity log records each of these once it has happened; a leftover that cannot be removed yet is dealt with, and logged, by a later read. If device proof itself is unreadable, sign-in does not enable printing or reconstruct that proof. Explicit enrollment is required; it preserves the old ciphertext as `connection.recovery-*.dpapi`, printer choices and the old connection's journal without attaching those receipts to the new device. Read-only status remains available. Explicit sign-out can quarantine unreadable session data locally, but cannot revoke an unavailable server session; revoke that previous session in QQQ. Automatic connection recovery does not perform this explicit credential replacement. diff --git a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs index d054c40..8896902 100644 --- a/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs @@ -159,7 +159,7 @@ public sealed partial class AgentRuntime + (store.StagedPairingFailure is null ? "not recorded." : QqqSessionDiagnostics.Describe(store.StagedPairingFailure))); break; case StagedPairingOutcome.Discarded: - WriteLog("INFO", "Discarded a session left over from a pairing change: it did not belong to the stored pairing, or the stored session already did or is newer."); + WriteLog("INFO", "Discarded a session left over from a pairing change: it was for another pairing, or the stored session is newer or already belongs to this one."); break; case StagedPairingOutcome.SetAside: WriteLog("WARN", "A session staged for a pairing change could not be read and was moved aside for recovery."); diff --git a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs index f0f20d2..af44d83 100644 --- a/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs +++ b/src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs @@ -125,15 +125,20 @@ public sealed partial class AgentStore /// /// Saves the session protected for the service account, writing a temporary file and moving it into place so a crash cannot /// leave a half-written secret. An unreadable earlier file is quarantined first. A staged session from an unfinished pairing - /// change is removed, because what is saved here is newer than it. + /// change is removed, because what is saved here is newer than it. When it cannot be removed, the saved file is stamped + /// later than the staged one, so the order the two were written in does not depend on the clock having moved forward. /// public void SaveAuthentication(AgentAuthentication authentication) { string path = Path.Combine(_directory, SessionFile); + string staged = Path.Combine(_directory, StagedSessionFile); WriteProtected(path + ".tmp", authentication); _ = QuarantineUnreadableAuthentication(); File.Move(path + ".tmp", path, overwrite: true); - _ = DeleteQuietly(Path.Combine(_directory, StagedSessionFile)); + if(!DeleteQuietly(staged) && File.GetLastWriteTimeUtc(path) <= File.GetLastWriteTimeUtc(staged)) + { + File.SetLastWriteTimeUtc(path, File.GetLastWriteTimeUtc(staged).AddTicks(1)); + } } @@ -143,13 +148,19 @@ public sealed partial class AgentStore /// staged name, and the pairing is replaced second. From then on the change is decided: every reader of the session /// uses the staged one and moves it into place, because it names the stored pairing's server and device. If the pairing /// cannot be written the staged session is removed and nothing has changed; a staged session left by a crash before the - /// pairing was replaced names a different device and is discarded by the next reader. + /// pairing was replaced names a different device and is discarded by the next reader. The staged file is never stamped + /// earlier than the stored session it is about to replace, whatever the clock did in between. /// internal void StagePairing(AgentConfiguration configuration, AgentAuthentication authentication) { string staged = Path.Combine(_directory, StagedSessionFile); + string path = Path.Combine(_directory, SessionFile); WriteProtected(staged + ".tmp", authentication); File.Move(staged + ".tmp", staged, overwrite: true); + if(File.Exists(path) && File.GetLastWriteTimeUtc(path) > File.GetLastWriteTimeUtc(staged)) + { + File.SetLastWriteTimeUtc(staged, File.GetLastWriteTimeUtc(path)); + } try { SaveConfiguration(configuration); diff --git a/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs b/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs index fdadc2c..8d12cb9 100644 --- a/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs +++ b/src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs @@ -12,7 +12,7 @@ namespace Qqq.PrintAgent.Windows; /// internal enum StagedPairingOutcome { - /// Nothing was staged. + /// Nothing was staged, or a leftover could not be removed yet and waits for the next read. None, /// The staged session belonged to the stored pairing and was moved into place. diff --git a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs index b4deea2..40d769f 100644 --- a/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs +++ b/tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs @@ -646,8 +646,9 @@ public sealed class AgentNewPairingTests /// /// A session saved after a pairing change was staged is newer than the staged one, even when it does not belong to the - /// stored pairing (a sign-in for a new pairing, say) and the staged file could not be removed at the time. The staged - /// session must not replace it later. + /// stored pairing (a sign-in for a new pairing, say) and the staged file could not be removed at the time. The save + /// stamps itself later than the staged file, here although the staged file carries a time in the future as it would + /// after the clock stepped back, and the staged session does not replace it later. /// [Fact] public void SessionSavedAfterStagingIsNotReplacedByTheStagedOne() @@ -656,13 +657,14 @@ public sealed class AgentNewPairingTests fixture.SaveSession(); string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); Stage(fixture, "replacement-device", "https://synthetic.invalid/"); + File.SetLastWriteTimeUtc(StagedSession(fixture), DateTime.UtcNow.AddHours(1)); using(FileStream held = new(StagedSession(fixture), FileMode.Open, FileAccess.Read, FileShare.Read)) { using AgentStore saving = new(fixture.Local.DirectoryPath); saving.SaveAuthentication(new AgentAuthentication(fixture.Session.Binding with { DeviceId = null }, fixture.Session.ExportCredential())); } Assert.True(File.Exists(StagedSession(fixture))); - File.SetLastWriteTimeUtc(authPath, File.GetLastWriteTimeUtc(StagedSession(fixture)).AddSeconds(1)); + Assert.True(File.GetLastWriteTimeUtc(authPath) > File.GetLastWriteTimeUtc(StagedSession(fixture))); byte[] saved = File.ReadAllBytes(authPath); using AgentStore store = new(fixture.Local.DirectoryPath); Assert.Null(store.LoadAuthentication()!.Binding.DeviceId); @@ -673,6 +675,28 @@ public sealed class AgentNewPairingTests + /// + /// Staging never stamps the staged session earlier than the stored session it is about to replace, so a clock that + /// stepped back between the two writes cannot make the new pairing's session look like a leftover: it is still moved + /// into place. The stored session here carries a time in the future, as it would after such a step. + /// + [Fact] + public void StagedSessionIsNotOlderThanTheSessionItReplaces() + { + using SessionFixture fixture = new(); + fixture.SaveSession(); + string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi"); + File.SetLastWriteTimeUtc(authPath, DateTime.UtcNow.AddHours(1)); + Stage(fixture, "replacement-device", "https://synthetic.invalid/"); + Assert.Equal(File.GetLastWriteTimeUtc(authPath), File.GetLastWriteTimeUtc(StagedSession(fixture))); + using AgentStore store = new(fixture.Local.DirectoryPath); + Assert.Equal("replacement-device", store.LoadAuthentication()!.Binding.DeviceId); + Assert.Equal(StagedPairingOutcome.Completed, store.StagedPairing); + Assert.False(File.Exists(StagedSession(fixture))); + } + + + /// Stages a pairing for the given device together with a session that names the given device on the given server, without finishing the change. private static void Stage(SessionFixture fixture, string deviceId, string sessionOrigin) { -- 2.54.0