From dde84ddb1590381307177ef20d972a79db8d7162 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 15:31:59 -0700 Subject: [PATCH 01/12] rework --- .../CertificateSigningOperation.cs | 23 ++++---- .../CertificateSigningRequestErrorResponse.cs | 57 +++++++++++++++---- 2 files changed, 60 insertions(+), 20 deletions(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs index 410ed270d..c9c48a90a 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs @@ -14,21 +14,22 @@ namespace Microsoft.Azure.Iot.Device.Models.CertificateManagement /// /// /// If the operation fails at any point, both and will - /// throw a when awaited. The exception contains - /// structured error details such as , - /// , and for 409005 conflict errors, - /// . + /// throw a when awaited. The exception contains + /// structured error details such as , + /// , and the full + /// that IoT hub reported. A response from IoT hub that + /// cannot be read also fails the operation this way rather than leaving it pending. /// /// try /// { - /// CertificateAcceptedResponse accepted = await operation.Accepted; + /// CertificateSigningRequestAccepted accepted = await operation.Accepted; /// CertificateSigningResponse completed = await operation.Completed; /// } - /// catch (CertificateSigningRequestException ex) when (ex.ErrorCode == 409005) + /// catch (CertificateSigningRequestFailedException ex) when (ex.ErrorCode == 409005) /// { /// // Conflict: another CSR operation is active. Use Replace = "*" to override. /// } - /// catch (CertificateSigningRequestException ex) when (ex.RetryAfterSeconds.HasValue) + /// catch (CertificateSigningRequestFailedException ex) when (ex.RetryAfterSeconds.HasValue) /// { /// await Task.Delay(TimeSpan.FromSeconds(ex.RetryAfterSeconds.Value)); /// // Retry the operation. @@ -59,9 +60,11 @@ private readonly TaskCompletionSource _completed /// The result contains the certificate chain and correlation ID. /// /// - /// Thrown when the certificate issuance fails after acceptance. This can also be thrown if the initial - /// request was rejected, since a failure at any phase propagates to both and - /// tasks. + /// Thrown when the certificate issuance fails after acceptance, or when IoT hub's issued certificate response + /// could not be read. This can also be thrown if the initial request was rejected, since a failure at any phase + /// propagates to both and tasks. If the + /// callback throws, this task fails + /// with that exception instead. /// public Task Completed => _completed.Task; diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningRequestErrorResponse.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningRequestErrorResponse.cs index 8e0f4af4b..4f415b06b 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningRequestErrorResponse.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningRequestErrorResponse.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft. All rights reserved. Licensed under the MIT license. // See LICENSE file in the project root for full license information. +using System.Globalization; +using System.Text.Json; using System.Text.Json.Serialization; namespace Microsoft.Azure.Iot.Device.Models.CertificateManagement @@ -8,40 +10,75 @@ namespace Microsoft.Azure.Iot.Device.Models.CertificateManagement /// /// Represents an error response from IoT hub about a certificate signing request. /// + /// + /// IoT hub does not populate every field on every error, so every field here is optional. A field that IoT hub + /// omitted, or sent in a shape this client did not recognize, is left at its default value. + /// public class CertificateSigningRequestErrorResponse { + /// + /// The IoT hub error code (for example, 400040 for a certificate signing request that could not be decoded, or + /// 409005 for a conflicting active request). Zero if IoT hub did not send one. + /// [JsonPropertyName("errorCode")] - public required int ErrorCode { get; set; } + public int ErrorCode { get; set; } + /// + /// A human readable description of the error. + /// [JsonPropertyName("message")] public string? Message { get; set; } + /// + /// The IoT hub tracking ID for this error, for support purposes. + /// [JsonPropertyName("trackingId")] - public required string TrackingId { get; set; } + public string? TrackingId { get; set; } + /// + /// When IoT hub generated this error. + /// [JsonPropertyName("timestampUtc")] - public required DateTimeOffset TimestampUtc { get; set; } + public DateTimeOffset? TimestampUtc { get; set; } + /// + /// Certificate management specific details about the error. + /// [JsonPropertyName("info")] - public required CertificateSigningRequestErrorInfo Info { get; set; } + public CertificateSigningRequestErrorInfo Info { get; set; } = new(); + /// + /// How long IoT hub asked this device to wait before trying again, if it asked at all. + /// [JsonPropertyName("retryAfter")] public int? RetryAfterSeconds { get; set; } + /// + /// Certificate management specific details about a certificate signing request error. + /// public class CertificateSigningRequestErrorInfo { + /// + /// Correlation ID for diagnostic and support purposes. + /// [JsonPropertyName("correlationId")] - public required string CorrelationId { get; set; } + public string? CorrelationId { get; set; } + /// + /// The certificate management specific error (for example, "CertificateSigningRequestInvalid"). + /// [JsonPropertyName("credentialError")] - public required string CertificateSigningRequestError { get; set; } + public string? CredentialError { get; set; } + /// + /// A human readable description of . + /// [JsonPropertyName("credentialMessage")] - public string? CertificateSigningRequestMessage { get; set; } - - [JsonPropertyName("requestId")] - public required string RequestId { get; set; } + public string? CredentialMessage { get; set; } + /// + /// When the operation this error refers to expires, if IoT hub reported it. + /// [JsonPropertyName("operationExpires")] public DateTimeOffset? OperationExpires { get; set; } } From a008b722df3d43aafc4b47828301314707ac30ad Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 15:43:12 -0700 Subject: [PATCH 02/12] fixup --- .../CertificateSigningOperation.cs | 24 ++----------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs index c9c48a90a..22f1403a5 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs @@ -14,27 +14,7 @@ namespace Microsoft.Azure.Iot.Device.Models.CertificateManagement /// /// /// If the operation fails at any point, both and will - /// throw a when awaited. The exception contains - /// structured error details such as , - /// , and the full - /// that IoT hub reported. A response from IoT hub that - /// cannot be read also fails the operation this way rather than leaving it pending. - /// - /// try - /// { - /// CertificateSigningRequestAccepted accepted = await operation.Accepted; - /// CertificateSigningResponse completed = await operation.Completed; - /// } - /// catch (CertificateSigningRequestFailedException ex) when (ex.ErrorCode == 409005) - /// { - /// // Conflict: another CSR operation is active. Use Replace = "*" to override. - /// } - /// catch (CertificateSigningRequestFailedException ex) when (ex.RetryAfterSeconds.HasValue) - /// { - /// await Task.Delay(TimeSpan.FromSeconds(ex.RetryAfterSeconds.Value)); - /// // Retry the operation. - /// } - /// + /// throw a when awaited. /// public class CertificateSigningOperation { @@ -49,7 +29,7 @@ private readonly TaskCompletionSource _completed /// The result contains the correlation ID and operation expiration time. /// /// - /// Thrown when the CSR is rejected by IoT Hub. Inspect + /// Thrown when the CSR is rejected by IoT Hub. Inspect /// for the specific failure reason (e.g., 400040 for CSR decode failure, 409005 for an active conflicting operation, /// 429002/429003 for throttling). /// From 6997186748164dcfe485a4e6793de85b320b962b Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 15:45:08 -0700 Subject: [PATCH 03/12] more --- .../Unified/Connection/ConnectionClient.cs | 23 ++- .../Unified/ConnectionClientUnitTests.cs | 178 ++++++++++++++++++ 2 files changed, 200 insertions(+), 1 deletion(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs index 041e10f78..7c5d22f5c 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs @@ -145,6 +145,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv { if (args.Publish.Topic.StartsWith(CertificateSigningResponseTopic)) { + CertificateSigningOperation? pendingCertificateSigningOperation = null; try { string[] topicTokens = args.Publish.Topic.Split("/"); @@ -156,7 +157,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv string status = topicTokens[3]; string requestId = topicTokens[4].Split(RequestId)[1]; - if (!_pendingCertificateSigningOperations.TryGetValue(requestId, out var pendingCertificateSigningOperation)) + if (!_pendingCertificateSigningOperations.TryGetValue(requestId, out pendingCertificateSigningOperation)) { return; } @@ -180,16 +181,36 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv { Trace.TraceError("Certificate signing response could not update authentication provider because user never set \"HandleCertificateSigningCompleteAsync\" callback"); } + + // Certificate signing operation has ended, so stop tracking it locally + _pendingCertificateSigningOperations.TryRemove(requestId, out _); + pendingCertificateSigningOperation.SetCompleted(response); return; } else { + // Certificate signing operation has ended, so stop tracking it locally + _pendingCertificateSigningOperations.TryRemove(requestId, out _); + CertificateSigningRequestErrorResponse error = JsonSerializer.Deserialize(args.Publish.Payload)!; pendingCertificateSigningOperation.SetFailed(new CertificateSigningRequestFailedException() { Error = error }); return; } } + catch (JsonException ex) + { + // An unreadable response must fail the operation rather than leave it pending forever + pendingCertificateSigningOperation?.SetFailed(new CertificateSigningRequestFailedException() + { + Error = new CertificateSigningRequestErrorResponse() { Message = "Failed to read the certificate signing response from IoT hub: " + ex.Message }, + }); + } + catch (Exception ex) when (pendingCertificateSigningOperation != null) + { + // Includes the user's HandleCertificateSigningCompleteAsync callback throwing + pendingCertificateSigningOperation.SetFailed(ex); + } finally { await args.AcknowledgeAsync(CancellationToken.None); diff --git a/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs b/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs index af8d78a7b..5b9e33725 100644 --- a/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs +++ b/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs @@ -4,6 +4,7 @@ using Microsoft.Azure.Iot.Device.Unified.Connection; using Microsoft.Azure.Iot.Device.Unified.Telemetry; using Microsoft.Azure.Iot.Device.Models; +using Microsoft.Azure.Iot.Device.Models.CertificateManagement; using Microsoft.Azure.Iot.Device.Mqtt; using Xunit; @@ -24,6 +25,183 @@ private static ConnectionContext GetMockConnectionContext(bool isMQTTv5) #pragma warning restore SYSLIB0026 // Type or member is obsolete } + private const string AcceptedPayload = "{\"correlationId\":\"someCorrelationId\",\"operationExpires\":\"2030-01-01T00:00:00Z\"}"; + + private static readonly TimeSpan s_certSigningTimeout = TimeSpan.FromSeconds(10); + + private static async Task<(ConnectionClient Client, MockMqttClient Mqtt, CertificateSigningOperation Operation, string RequestId)> StartCertificateSigningAsync( + Func, Task>? completeCallback = null) + { + MockMqttClient mockMqttClient = new(false); + ConnectionClient connectionClient = new(new() { MqttClient = mockMqttClient }); + connectionClient.HandleCertificateSigningCompleteAsync = completeCallback; + await connectionClient.ConnectToHubAsync(GetMockConnectionContext(false), cancellationToken: TestContext.Current.CancellationToken); + + IotHubCertificateSigningRequest request = new("someDeviceId", "c29tZWNzcg=="); + CertificateSigningOperation operation = await connectionClient.SendCertificateSigningRequestAsync(request, TestContext.Current.CancellationToken); + return (connectionClient, mockMqttClient, operation, request.RequestId); + } + + // Delivers a response from IoT hub. Exceptions thrown by the handler are swallowed, as the MQTT layer would do, so tests can assert on the operation's tasks instead. + private static async Task DeliverCertificateSigningResponseAsync(MockMqttClient mqtt, string status, string requestId, string payload) + { + try + { + await mqtt.SimulateNewMessageAsync(new MqttPublish() + { + Topic = $"$iothub/credentials/res/{status}/?$rid={requestId}", + Payload = System.Text.Encoding.UTF8.GetBytes(payload), + }); + } + catch (Exception) + { + } + } + + private static async Task AssertFailsAsync(Task task) + { + return await Assert.ThrowsAsync( + () => task.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken)); + } + + [Theory] + [InlineData("400", 400040)] + [InlineData("409", 409005)] + [InlineData("429", 429002)] + [InlineData("500", 500001)] + public async Task CertificateSigningErrorBeforeAcceptancePropagatesToBothTasks(string status, int errorCode) + { + var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(); + using (client) + { + string payload = $"{{\"errorCode\":{errorCode},\"message\":\"some message\",\"trackingId\":\"tid\",\"retryAfter\":5}}"; + await DeliverCertificateSigningResponseAsync(mqtt, status, requestId, payload); + + var acceptedEx = await AssertFailsAsync(operation.Accepted); + var completedEx = await AssertFailsAsync(operation.Completed); + + Assert.Equal(errorCode, acceptedEx.Error.ErrorCode); + Assert.Equal("some message", acceptedEx.Error.Message); + Assert.Equal(5, acceptedEx.Error.RetryAfterSeconds); + Assert.Equal(errorCode, completedEx.Error.ErrorCode); + } + } + + [Fact] + public async Task CertificateSigningErrorAfterAcceptanceFailsOnlyCompletedTask() + { + var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(); + using (client) + { + await DeliverCertificateSigningResponseAsync(mqtt, "202", requestId, AcceptedPayload); + await operation.Accepted.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken); + + await DeliverCertificateSigningResponseAsync(mqtt, "500", requestId, "{\"errorCode\":500001,\"message\":\"issuance failed\"}"); + + var ex = await AssertFailsAsync(operation.Completed); + Assert.Equal(500001, ex.Error.ErrorCode); + + // Acceptance already succeeded, so a later failure must not rewrite it. + Assert.True(operation.Accepted.IsCompletedSuccessfully); + } + } + + [Fact] + public async Task CertificateSigningErrorWithMinimalPayloadStillFailsBothTasks() + { + var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(); + using (client) + { + await DeliverCertificateSigningResponseAsync(mqtt, "400", requestId, "{}"); + + var acceptedEx = await AssertFailsAsync(operation.Accepted); + var completedEx = await AssertFailsAsync(operation.Completed); + Assert.Equal(0, acceptedEx.Error.ErrorCode); + Assert.Same(acceptedEx, completedEx); + } + } + + [Theory] + [InlineData("400", "not json")] + [InlineData("400", "")] + [InlineData("400", "null")] + [InlineData("202", "not json")] + [InlineData("200", "not json")] + public async Task CertificateSigningUnreadableResponseFailsBothTasksInsteadOfHanging(string status, string payload) + { + var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(certs => Task.FromResult(GetMockConnectionContext(false).AuthenticationProvider)); + using (client) + { + await DeliverCertificateSigningResponseAsync(mqtt, status, requestId, payload); + + await AssertFailsAsync(operation.Accepted); + await AssertFailsAsync(operation.Completed); + } + } + + [Fact] + public async Task CertificateSigningCompleteCallbackThrowingFailsCompletedTask() + { + InvalidOperationException callbackException = new("callback failed"); + var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(certs => throw callbackException); + using (client) + { + await DeliverCertificateSigningResponseAsync(mqtt, "202", requestId, AcceptedPayload); + await operation.Accepted.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken); + + await DeliverCertificateSigningResponseAsync(mqtt, "200", requestId, "{\"certificates\":[\"cert1\"],\"correlationId\":\"someCorrelationId\"}"); + + var ex = await Assert.ThrowsAsync( + () => operation.Completed.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken)); + Assert.Same(callbackException, ex); + } + } + + [Fact] + public async Task CertificateSigningErrorForOneRequestDoesNotAffectAnother() + { + var (client, mqtt, failingOperation, failingId) = await StartCertificateSigningAsync(); + using (client) + { + IotHubCertificateSigningRequest otherRequest = new("someDeviceId", "c29tZWNzcg=="); + CertificateSigningOperation otherOperation = await client.SendCertificateSigningRequestAsync(otherRequest, TestContext.Current.CancellationToken); + + await DeliverCertificateSigningResponseAsync(mqtt, "400", failingId, "{\"errorCode\":400040}"); + + await AssertFailsAsync(failingOperation.Accepted); + await AssertFailsAsync(failingOperation.Completed); + Assert.False(otherOperation.Accepted.IsCompleted); + Assert.False(otherOperation.Completed.IsCompleted); + } + } + + [Fact] + public async Task CertificateSigningResponseForUnknownRequestIsIgnored() + { + var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(); + using (client) + { + await DeliverCertificateSigningResponseAsync(mqtt, "400", "unknown-request-id", "{\"errorCode\":400040}"); + + Assert.False(operation.Accepted.IsCompleted); + Assert.False(operation.Completed.IsCompleted); + } + } + + [Fact] + public async Task CertificateSigningRequestPublishFailureThrowsToCaller() + { + MockMqttClient mockMqttClient = new(false); + using ConnectionClient connectionClient = new(new() { MqttClient = mockMqttClient }); + await connectionClient.ConnectToHubAsync(GetMockConnectionContext(false), cancellationToken: TestContext.Current.CancellationToken); + + mockMqttClient.OnPublishAttempt += _ => Task.FromResult(new MqttPublishAck() { ReasonCode = MqttPublishAckReasonCode.NotAuthorized }); + + await Assert.ThrowsAnyAsync(() => connectionClient.SendCertificateSigningRequestAsync( + new IotHubCertificateSigningRequest("someDeviceId", "c29tZWNzcg=="), + TestContext.Current.CancellationToken)); + } + [Fact] public async Task ProvisionAndConnectUsesSeededConnectionContext() { From e6e474a7563e8a1858680d0bf37fd44770996317 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 15:47:33 -0700 Subject: [PATCH 04/12] fixup --- .../Unified/Connection/ConnectionClient.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs index 7c5d22f5c..f01a42fc9 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs @@ -182,7 +182,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv Trace.TraceError("Certificate signing response could not update authentication provider because user never set \"HandleCertificateSigningCompleteAsync\" callback"); } - // Certificate signing operation has ended, so stop tracking it locally + // Certificate signing operation has ended successfully, so stop tracking it locally _pendingCertificateSigningOperations.TryRemove(requestId, out _); pendingCertificateSigningOperation.SetCompleted(response); @@ -190,7 +190,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv } else { - // Certificate signing operation has ended, so stop tracking it locally + // Certificate signing operation has ended in an error, so stop tracking it locally _pendingCertificateSigningOperations.TryRemove(requestId, out _); CertificateSigningRequestErrorResponse error = JsonSerializer.Deserialize(args.Publish.Payload)!; From 5e41e92af24143a92d197d58a7fd28e8baf9b28b Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 15:49:17 -0700 Subject: [PATCH 05/12] fixup --- .../Unified/Connection/ConnectionClient.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs index f01a42fc9..7f9ac88b3 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs @@ -116,6 +116,7 @@ public override async Task HandleConnectedToHubAsync(MqttClientConnectedEventArg /// A set of tasks. One that completes when IoT hub accepts the request (and starts signing), one that completes when IoT hub completes the signing, and one that completes if any step in the process fails. public async Task SendCertificateSigningRequestAsync(IotHubCertificateSigningRequest request, CancellationToken cancellationToken = default) { + //TODO how does hub respond if device loses connection at any point during this process? ObjectDisposedException.ThrowIf(_isDisposed, this); if (CurrentConnectionContext == null) From 462b188d4a00c697a2bd4657c9d0f9716d9f0281 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 15:55:52 -0700 Subject: [PATCH 06/12] fixup --- .../Unified/Connection/ConnectionClient.cs | 15 +++--- .../Unified/ConnectionClientUnitTests.cs | 50 +++++++++++++++++++ 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs index 7f9ac88b3..5b495b75a 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs @@ -147,6 +147,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv if (args.Publish.Topic.StartsWith(CertificateSigningResponseTopic)) { CertificateSigningOperation? pendingCertificateSigningOperation = null; + string? requestId = null; try { string[] topicTokens = args.Publish.Topic.Split("/"); @@ -156,7 +157,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv } string status = topicTokens[3]; - string requestId = topicTokens[4].Split(RequestId)[1]; + requestId = topicTokens[4].Split(RequestId)[1]; if (!_pendingCertificateSigningOperations.TryGetValue(requestId, out pendingCertificateSigningOperation)) { @@ -183,17 +184,11 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv Trace.TraceError("Certificate signing response could not update authentication provider because user never set \"HandleCertificateSigningCompleteAsync\" callback"); } - // Certificate signing operation has ended successfully, so stop tracking it locally - _pendingCertificateSigningOperations.TryRemove(requestId, out _); - pendingCertificateSigningOperation.SetCompleted(response); return; } else { - // Certificate signing operation has ended in an error, so stop tracking it locally - _pendingCertificateSigningOperations.TryRemove(requestId, out _); - CertificateSigningRequestErrorResponse error = JsonSerializer.Deserialize(args.Publish.Payload)!; pendingCertificateSigningOperation.SetFailed(new CertificateSigningRequestFailedException() { Error = error }); return; @@ -214,6 +209,12 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv } finally { + // Any terminal outcome (success, hub error, unreadable response, or a throwing user callback) ends the operation, so stop tracking it locally + if (requestId != null && pendingCertificateSigningOperation != null && pendingCertificateSigningOperation.Completed.IsCompleted) + { + _pendingCertificateSigningOperations.TryRemove(requestId, out _); + } + await args.AcknowledgeAsync(CancellationToken.None); } } diff --git a/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs b/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs index 5b9e33725..15b60b442 100644 --- a/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs +++ b/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs @@ -154,9 +154,59 @@ public async Task CertificateSigningCompleteCallbackThrowingFailsCompletedTask() var ex = await Assert.ThrowsAsync( () => operation.Completed.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken)); Assert.Same(callbackException, ex); + Assert.Equal(0, GetPendingOperationCount(client)); } } + [Fact] + public async Task CertificateSigningCompleteCallbackThrowingBeforeAcceptanceFailsBothTasksAndClearsCache() + { + InvalidOperationException callbackException = new("callback failed"); + var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(certs => throw callbackException); + using (client) + { + Assert.Equal(1, GetPendingOperationCount(client)); + + await DeliverCertificateSigningResponseAsync(mqtt, "200", requestId, "{\"certificates\":[\"cert1\"],\"correlationId\":\"someCorrelationId\"}"); + + var completedEx = await Assert.ThrowsAsync( + () => operation.Completed.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken)); + var acceptedEx = await Assert.ThrowsAsync( + () => operation.Accepted.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken)); + Assert.Same(callbackException, completedEx); + Assert.Same(callbackException, acceptedEx); + Assert.Equal(0, GetPendingOperationCount(client)); + } + } + + [Fact] + public async Task CertificateSigningTerminalOutcomesClearPendingOperationCache() + { + var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(certs => Task.FromResult(GetMockConnectionContext(false).AuthenticationProvider)); + using (client) + { + await DeliverCertificateSigningResponseAsync(mqtt, "202", requestId, AcceptedPayload); + Assert.Equal(1, GetPendingOperationCount(client)); // Still in progress + + await DeliverCertificateSigningResponseAsync(mqtt, "200", requestId, "{\"certificates\":[\"cert1\"],\"correlationId\":\"someCorrelationId\"}"); + await operation.Completed.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken); + Assert.Equal(0, GetPendingOperationCount(client)); + + IotHubCertificateSigningRequest failing = new("someDeviceId", "c29tZWNzcg=="); + CertificateSigningOperation failingOperation = await client.SendCertificateSigningRequestAsync(failing, TestContext.Current.CancellationToken); + Assert.Equal(1, GetPendingOperationCount(client)); + await DeliverCertificateSigningResponseAsync(mqtt, "400", failing.RequestId, "{\"errorCode\":400040}"); + await AssertFailsAsync(failingOperation.Completed); + Assert.Equal(0, GetPendingOperationCount(client)); + } + } + + private static int GetPendingOperationCount(ConnectionClient client) + { + var field = typeof(ConnectionClient).GetField("_pendingCertificateSigningOperations", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!; + return ((System.Collections.IDictionary)field.GetValue(client)!).Count; + } + [Fact] public async Task CertificateSigningErrorForOneRequestDoesNotAffectAnother() { From 8a189ae9e8cd7f358d62259d154661cd51afaa93 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 16:02:25 -0700 Subject: [PATCH 07/12] requestId --- ...ertificateSigningRequestFailedException.cs | 2 + .../Unified/Connection/ConnectionClient.cs | 3 +- .../Unified/ConnectionClientUnitTests.cs | 38 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningRequestFailedException.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningRequestFailedException.cs index cb8fd295d..6dcc8f52f 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningRequestFailedException.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningRequestFailedException.cs @@ -10,5 +10,7 @@ namespace Microsoft.Azure.Iot.Device.Models.CertificateManagement public class CertificateSigningRequestFailedException : Exception { public required CertificateSigningRequestErrorResponse Error { get; set; } + + public string? RequestId { get; set; } } } diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs index 5b495b75a..f439a4fc1 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs @@ -190,7 +190,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv else { CertificateSigningRequestErrorResponse error = JsonSerializer.Deserialize(args.Publish.Payload)!; - pendingCertificateSigningOperation.SetFailed(new CertificateSigningRequestFailedException() { Error = error }); + pendingCertificateSigningOperation.SetFailed(new CertificateSigningRequestFailedException() { Error = error, RequestId = requestId }); return; } } @@ -200,6 +200,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv pendingCertificateSigningOperation?.SetFailed(new CertificateSigningRequestFailedException() { Error = new CertificateSigningRequestErrorResponse() { Message = "Failed to read the certificate signing response from IoT hub: " + ex.Message }, + RequestId = requestId, }); } catch (Exception ex) when (pendingCertificateSigningOperation != null) diff --git a/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs b/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs index 15b60b442..b33ead657 100644 --- a/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs +++ b/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs @@ -252,6 +252,44 @@ await Assert.ThrowsAnyAsync(() => connectionClient.SendCertificateSig TestContext.Current.CancellationToken)); } + [Theory] + [InlineData("400", "{\"errorCode\":400040}")] + [InlineData("409", "{\"errorCode\":409005,\"message\":\"conflict\"}")] + [InlineData("429", "{}")] + [InlineData("400", "not json")] + [InlineData("202", "not json")] + [InlineData("200", "not json")] + public async Task CertificateSigningFailureCarriesRequestIdFromTopic(string status, string payload) + { + var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(); + using (client) + { + await DeliverCertificateSigningResponseAsync(mqtt, status, requestId, payload); + + var acceptedEx = await AssertFailsAsync(operation.Accepted); + var completedEx = await AssertFailsAsync(operation.Completed); + Assert.Equal(requestId, acceptedEx.RequestId); + Assert.Equal(requestId, completedEx.RequestId); + } + } + + [Fact] + public async Task CertificateSigningFailureRequestIdMatchesTheFailingRequestWhenMultiplePending() + { + var (client, mqtt, firstOperation, firstId) = await StartCertificateSigningAsync(); + using (client) + { + IotHubCertificateSigningRequest secondRequest = new("someDeviceId", "c29tZWNzcg==", "second-request-id"); + CertificateSigningOperation secondOperation = await client.SendCertificateSigningRequestAsync(secondRequest, TestContext.Current.CancellationToken); + + await DeliverCertificateSigningResponseAsync(mqtt, "409", secondRequest.RequestId, "{\"errorCode\":409005}"); + await DeliverCertificateSigningResponseAsync(mqtt, "400", firstId, "{\"errorCode\":400040}"); + + Assert.Equal("second-request-id", (await AssertFailsAsync(secondOperation.Completed)).RequestId); + Assert.Equal(firstId, (await AssertFailsAsync(firstOperation.Completed)).RequestId); + } + } + [Fact] public async Task ProvisionAndConnectUsesSeededConnectionContext() { From 79e50644e0c4e95552f495e7b34a3aedf7cb3d73 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 16:59:32 -0700 Subject: [PATCH 08/12] Only remove from cache if request has finished --- .../Unified/Connection/ConnectionClient.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs index f439a4fc1..13a45ec56 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs @@ -148,6 +148,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv { CertificateSigningOperation? pendingCertificateSigningOperation = null; string? requestId = null; + bool hasRequestFinished = false; try { string[] topicTokens = args.Publish.Topic.Split("/"); @@ -172,6 +173,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv } else if (status.Equals("200")) { + hasRequestFinished = true; CertificateSigningResponse response = JsonSerializer.Deserialize(args.Publish.Payload)!; if (HandleCertificateSigningCompleteAsync != null) { @@ -189,6 +191,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv } else { + hasRequestFinished = true; CertificateSigningRequestErrorResponse error = JsonSerializer.Deserialize(args.Publish.Payload)!; pendingCertificateSigningOperation.SetFailed(new CertificateSigningRequestFailedException() { Error = error, RequestId = requestId }); return; @@ -211,7 +214,7 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv finally { // Any terminal outcome (success, hub error, unreadable response, or a throwing user callback) ends the operation, so stop tracking it locally - if (requestId != null && pendingCertificateSigningOperation != null && pendingCertificateSigningOperation.Completed.IsCompleted) + if (requestId != null && hasRequestFinished) { _pendingCertificateSigningOperations.TryRemove(requestId, out _); } From f9bf01342cf6db380f471a2db194e595ef6d0499 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 17:06:12 -0700 Subject: [PATCH 09/12] Not real scenarios --- .../Unified/ConnectionClientUnitTests.cs | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs b/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs index b33ead657..b5ebac3b6 100644 --- a/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs +++ b/dotnet/tests/Microsoft.Azure.Iot.Device.UnitTests/Unified/ConnectionClientUnitTests.cs @@ -126,7 +126,6 @@ public async Task CertificateSigningErrorWithMinimalPayloadStillFailsBothTasks() [InlineData("400", "")] [InlineData("400", "null")] [InlineData("202", "not json")] - [InlineData("200", "not json")] public async Task CertificateSigningUnreadableResponseFailsBothTasksInsteadOfHanging(string status, string payload) { var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(certs => Task.FromResult(GetMockConnectionContext(false).AuthenticationProvider)); @@ -158,27 +157,6 @@ public async Task CertificateSigningCompleteCallbackThrowingFailsCompletedTask() } } - [Fact] - public async Task CertificateSigningCompleteCallbackThrowingBeforeAcceptanceFailsBothTasksAndClearsCache() - { - InvalidOperationException callbackException = new("callback failed"); - var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(certs => throw callbackException); - using (client) - { - Assert.Equal(1, GetPendingOperationCount(client)); - - await DeliverCertificateSigningResponseAsync(mqtt, "200", requestId, "{\"certificates\":[\"cert1\"],\"correlationId\":\"someCorrelationId\"}"); - - var completedEx = await Assert.ThrowsAsync( - () => operation.Completed.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken)); - var acceptedEx = await Assert.ThrowsAsync( - () => operation.Accepted.WaitAsync(s_certSigningTimeout, TestContext.Current.CancellationToken)); - Assert.Same(callbackException, completedEx); - Assert.Same(callbackException, acceptedEx); - Assert.Equal(0, GetPendingOperationCount(client)); - } - } - [Fact] public async Task CertificateSigningTerminalOutcomesClearPendingOperationCache() { @@ -258,7 +236,6 @@ await Assert.ThrowsAnyAsync(() => connectionClient.SendCertificateSig [InlineData("429", "{}")] [InlineData("400", "not json")] [InlineData("202", "not json")] - [InlineData("200", "not json")] public async Task CertificateSigningFailureCarriesRequestIdFromTopic(string status, string payload) { var (client, mqtt, operation, requestId) = await StartCertificateSigningAsync(); From ff4ba30a028d13f087d9f20109e462ddb6a7ca72 Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 17:07:48 -0700 Subject: [PATCH 10/12] Add null check for CertificateSigningResponse Throw JsonException if response is null after deserialization. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../Unified/Connection/ConnectionClient.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs index 13a45ec56..0c6a293c7 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs @@ -174,7 +174,8 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv else if (status.Equals("200")) { hasRequestFinished = true; - CertificateSigningResponse response = JsonSerializer.Deserialize(args.Publish.Payload)!; +CertificateSigningResponse response = JsonSerializer.Deserialize(args.Publish.Payload) + ?? throw new JsonException("Certificate signing completion response was null."); if (HandleCertificateSigningCompleteAsync != null) { //TODO need a fault-injection like unit test that ensures that the client uses this new authentication provider upon reconnect since our API won't allow users to disconnect then reconnect to hub at will From 858614950bcd84bbcb5e2bd7ed18b2bea2e38a9c Mon Sep 17 00:00:00 2001 From: Tim Taylor Date: Fri, 9 Oct 2026 17:09:37 -0700 Subject: [PATCH 11/12] More null checks --- .../Unified/Connection/ConnectionClient.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs index 0c6a293c7..2e34f1820 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Unified/Connection/ConnectionClient.cs @@ -167,15 +167,16 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv if (status.Equals("202")) { - CertificateSigningRequestAccepted accepted = JsonSerializer.Deserialize(args.Publish.Payload)!; + CertificateSigningRequestAccepted accepted = JsonSerializer.Deserialize(args.Publish.Payload) + ?? throw new JsonException("Certificate signing completion response was null."); pendingCertificateSigningOperation.SetAccepted(accepted); return; } else if (status.Equals("200")) { hasRequestFinished = true; -CertificateSigningResponse response = JsonSerializer.Deserialize(args.Publish.Payload) - ?? throw new JsonException("Certificate signing completion response was null."); + CertificateSigningResponse response = JsonSerializer.Deserialize(args.Publish.Payload) + ?? throw new JsonException("Certificate signing completion response was null."); if (HandleCertificateSigningCompleteAsync != null) { //TODO need a fault-injection like unit test that ensures that the client uses this new authentication provider upon reconnect since our API won't allow users to disconnect then reconnect to hub at will @@ -193,7 +194,8 @@ private async Task HandleReceivedCertificateSigningPublish(MqttPublishReceivedEv else { hasRequestFinished = true; - CertificateSigningRequestErrorResponse error = JsonSerializer.Deserialize(args.Publish.Payload)!; + CertificateSigningRequestErrorResponse error = JsonSerializer.Deserialize(args.Publish.Payload) + ?? throw new JsonException("Certificate signing completion response was null."); pendingCertificateSigningOperation.SetFailed(new CertificateSigningRequestFailedException() { Error = error, RequestId = requestId }); return; } From 565fe313ee4dd2695c93629268eb29533ef61702 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 10 Oct 2026 00:15:50 +0000 Subject: [PATCH 12/12] Clarify certificate signing task failure docs Co-authored-by: timtay-microsoft <28789848+timtay-microsoft@users.noreply.github.com> --- .../CertificateSigningOperation.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs index 22f1403a5..0aaefb531 100644 --- a/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs +++ b/dotnet/src/Microsoft.Azure.Iot.Device/Models/CertificateManagement/CertificateSigningOperation.cs @@ -13,8 +13,10 @@ namespace Microsoft.Azure.Iot.Device.Models.CertificateManagement /// Phase 2 (Completed): IoT Hub delivers the issued certificate with a 200 response. /// /// - /// If the operation fails at any point, both and will - /// throw a when awaited. + /// If the operation fails before completes successfully, both tasks fault with the + /// operation's exception. If has already completed successfully, a later failure + /// affects only . If the completion callback throws, faults + /// with the callback's exception. Cancellation cancels each task that has not already completed. /// public class CertificateSigningOperation { @@ -41,10 +43,9 @@ private readonly TaskCompletionSource _completed /// /// /// Thrown when the certificate issuance fails after acceptance, or when IoT hub's issued certificate response - /// could not be read. This can also be thrown if the initial request was rejected, since a failure at any phase - /// propagates to both and tasks. If the - /// callback throws, this task fails - /// with that exception instead. + /// could not be read. It can also be thrown if the request is rejected before completes. + /// If the callback throws, this + /// task fails with that exception instead. /// public Task Completed => _completed.Task;