diff --git a/e2e/test/iothub/AuthenticationWithTokenRefreshDisposalTests.cs b/e2e/test/iothub/AuthenticationWithTokenRefreshDisposalTests.cs index fc00c3e82b..0f876ff835 100644 --- a/e2e/test/iothub/AuthenticationWithTokenRefreshDisposalTests.cs +++ b/e2e/test/iothub/AuthenticationWithTokenRefreshDisposalTests.cs @@ -55,19 +55,72 @@ public async Task DeviceSak_ReusableAuthenticationMethod_SingleDevicePerConnecti [LoggedTestMethod] public async Task DeviceSak_ReusableAuthenticationMethod_MuxedDevicesPerConnection_Amqp() { - await ReuseAuthenticationMethod_MuxedDevices(Client.TransportType.Amqp_Tcp_Only, 2); + await ReuseAuthenticationMethod_MuxedDevices(Client.TransportType.Amqp_Tcp_Only, 2).ConfigureAwait(false); ; } [LoggedTestMethod] public async Task DeviceSak_ReusableAuthenticationMethod_MuxedDevicesPerConnection_AmqpWs() { - await ReuseAuthenticationMethod_MuxedDevices(Client.TransportType.Amqp_WebSocket_Only, 2); + await ReuseAuthenticationMethod_MuxedDevices(Client.TransportType.Amqp_WebSocket_Only, 2).ConfigureAwait(false); ; + } + + [LoggedTestMethod] + public async Task DeviceClient_AuthenticationMethodDisposesTokenRefresher_Http() + { + await AuthenticationMethodDisposesTokenRefresher(Client.TransportType.Http1).ConfigureAwait(false); + } + + [LoggedTestMethod] + public async Task DeviceClient_AuthenticationMethodDisposesTokenRefresher_Amqp() + { + await AuthenticationMethodDisposesTokenRefresher(Client.TransportType.Amqp_Tcp_Only).ConfigureAwait(false); + } + + [LoggedTestMethod] + public async Task DeviceClient_AuthenticationMethodDisposesTokenRefresher_AmqpWs() + { + await AuthenticationMethodDisposesTokenRefresher(Client.TransportType.Amqp_WebSocket_Only).ConfigureAwait(false); + } + + [LoggedTestMethod] + public async Task DeviceClient_AuthenticationMethodDisposesTokenRefresher_Mqtt() + { + await AuthenticationMethodDisposesTokenRefresher(Client.TransportType.Mqtt_Tcp_Only).ConfigureAwait(false); + } + + [LoggedTestMethod] + public async Task DeviceClient_AuthenticationMethodDisposesTokenRefresher_MqttWs() + { + await AuthenticationMethodDisposesTokenRefresher(Client.TransportType.Mqtt_WebSocket_Only).ConfigureAwait(false); + } + + private async Task AuthenticationMethodDisposesTokenRefresher(Client.TransportType transport) + { + TestDevice testDevice = await TestDevice.GetTestDeviceAsync(Logger, _devicePrefix).ConfigureAwait(false); + var authenticationMethod = new DeviceAuthenticationSasToken(testDevice.ConnectionString, disposalBySdk: true); + + // Create an instance of the device client, send a test message and then close and dispose it. + DeviceClient deviceClient = DeviceClient.Create(testDevice.IoTHubHostName, authenticationMethod, transport); + using var message1 = new Client.Message(); + await deviceClient.SendEventAsync(message1).ConfigureAwait(false); + await deviceClient.CloseAsync(); + deviceClient.Dispose(); + Logger.Trace("Test with instance 1 completed"); + + // Perform the same steps again, reusing the previously created authentication method instance. + // Since the default behavior is to dispose AuthenticationWithTokenRefresh authentication method on DeviceClient disposal, + // this should now throw an ObjectDisposedException. + DeviceClient deviceClient2 = DeviceClient.Create(testDevice.IoTHubHostName, authenticationMethod, transport); + using var message2 = new Client.Message(); + + Func act = async () => await deviceClient2.SendEventAsync(message2).ConfigureAwait(false); + await act.Should().ThrowAsync(); } private async Task ReuseAuthenticationMethod_SingleDevice(Client.TransportType transport) { TestDevice testDevice = await TestDevice.GetTestDeviceAsync(Logger, _devicePrefix).ConfigureAwait(false); - var authenticationMethod = new DeviceAuthenticationSasToken(testDevice.ConnectionString); + var authenticationMethod = new DeviceAuthenticationSasToken(testDevice.ConnectionString, disposalBySdk: false); // Create an instance of the device client, send a test message and then close and dispose it. DeviceClient deviceClient = DeviceClient.Create(testDevice.IoTHubHostName, authenticationMethod, transport); @@ -110,7 +163,7 @@ private async Task ReuseAuthenticationMethod_MuxedDevices(Client.TransportType t { TestDevice testDevice = await TestDevice.GetTestDeviceAsync(Logger, _devicePrefix).ConfigureAwait(false); #pragma warning disable CA2000 // Dispose objects before losing scope - the authentication method is disposed at the end of the test. - var authenticationMethod = new DeviceAuthenticationSasToken(testDevice.ConnectionString); + var authenticationMethod = new DeviceAuthenticationSasToken(testDevice.ConnectionString, disposalBySdk: false); #pragma warning restore CA2000 // Dispose objects before losing scope testDevices.Add(testDevice); @@ -215,9 +268,13 @@ private class DeviceAuthenticationSasToken : DeviceAuthenticationWithTokenRefres private const string SasTokenTargetFormat = "{0}/devices/{1}"; private readonly IotHubConnectionStringBuilder _connectionStringBuilder; + private static readonly int s_suggestedSasTimeToLiveInSeconds = (int)TimeSpan.FromMinutes(30).TotalSeconds; + private static readonly int s_sasRenewalBufferPercentage = 50; + public DeviceAuthenticationSasToken( - string connectionString) - : base(GetDeviceIdFromConnectionString(connectionString)) + string connectionString, + bool disposalBySdk) + : base(GetDeviceIdFromConnectionString(connectionString), s_suggestedSasTimeToLiveInSeconds, s_sasRenewalBufferPercentage, disposalBySdk) { if (connectionString == null) { diff --git a/iothub/device/src/AuthenticationWithTokenRefresh.cs b/iothub/device/src/AuthenticationWithTokenRefresh.cs index 4bd6585646..864430d0b3 100644 --- a/iothub/device/src/AuthenticationWithTokenRefresh.cs +++ b/iothub/device/src/AuthenticationWithTokenRefresh.cs @@ -32,6 +32,22 @@ public abstract class AuthenticationWithTokenRefresh : IAuthenticationMethod, ID public AuthenticationWithTokenRefresh( int suggestedTimeToLiveSeconds, int timeBufferPercentage) + : this(suggestedTimeToLiveSeconds, timeBufferPercentage, true) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// Token time to live suggested value. The implementations of this abstract + /// may choose to ignore this value. + /// Time buffer before expiry when the token should be renewed expressed as + /// a percentage of the time to live. + /// True if the authentication method should be disposed of by sdk; false if you intend to reuse the authentication method. + public AuthenticationWithTokenRefresh( + int suggestedTimeToLiveSeconds, + int timeBufferPercentage, + bool disposalBySdk) { if (suggestedTimeToLiveSeconds < 0) { @@ -48,6 +64,8 @@ public AuthenticationWithTokenRefresh( ExpiresOn = DateTime.UtcNow.AddSeconds(-_suggestedTimeToLiveSeconds); Debug.Assert(IsExpiring); UpdateTimeBufferSeconds(_suggestedTimeToLiveSeconds); + + DisposalBySdk = disposalBySdk; } /// @@ -65,21 +83,25 @@ public AuthenticationWithTokenRefresh( /// public bool IsExpiring => (ExpiresOn - DateTime.UtcNow).TotalSeconds <= _bufferSeconds; - // This internal property is used by the sdk to determine if the instance was created by the sdk, - // and thus, if it should be disposed by the sdk. - internal bool InstanceCreatedBySdk { get; set; } + // This internal property is used by the sdk to determine if the disposal should be handled by the sdk. + internal bool DisposalBySdk { get; } /// /// Gets a snapshot of the security token associated with the device. This call is thread-safe. /// public async Task GetTokenAsync(string iotHub) { + if (_isDisposed) + { + throw new ObjectDisposedException("This authentication method instance has already been disposed. " + + "Please create a new instance and pass it to the DeviceClient/ ModuleClient during initialization."); + } + if (!IsExpiring) { return _token; } - Debug.Assert(_lock != null); await _lock.WaitAsync().ConfigureAwait(false); try diff --git a/iothub/device/src/DeviceAuthenticationWithSakRefresh.cs b/iothub/device/src/DeviceAuthenticationWithSakRefresh.cs index 95561eef50..aa5b89b40c 100644 --- a/iothub/device/src/DeviceAuthenticationWithSakRefresh.cs +++ b/iothub/device/src/DeviceAuthenticationWithSakRefresh.cs @@ -24,7 +24,9 @@ internal DeviceAuthenticationWithSakRefresh( string deviceId, IotHubConnectionString connectionString, TimeSpan sasTokenTimeToLive, - int sasTokenRenewalBuffer) : base(deviceId, (int)sasTokenTimeToLive.TotalSeconds, sasTokenRenewalBuffer) + int sasTokenRenewalBuffer, + bool disposalBySdk) + : base(deviceId, (int)sasTokenTimeToLive.TotalSeconds, sasTokenRenewalBuffer, disposalBySdk) { _connectionString = connectionString ?? throw new ArgumentNullException(nameof(connectionString)); } diff --git a/iothub/device/src/DeviceAuthenticationWithTokenRefresh.cs b/iothub/device/src/DeviceAuthenticationWithTokenRefresh.cs index 401d6f7a7d..7cb566493c 100644 --- a/iothub/device/src/DeviceAuthenticationWithTokenRefresh.cs +++ b/iothub/device/src/DeviceAuthenticationWithTokenRefresh.cs @@ -40,7 +40,29 @@ public DeviceAuthenticationWithTokenRefresh( string deviceId, int suggestedTimeToLiveSeconds, int timeBufferPercentage) - : base(SetSasTokenSuggestedTimeToLiveSeconds(suggestedTimeToLiveSeconds), SetSasTokenRenewalBufferPercentage(timeBufferPercentage)) + : this(deviceId, suggestedTimeToLiveSeconds, timeBufferPercentage, true) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// Device Identifier. + /// + /// The suggested time to live value for the generated SAS tokens. + /// The default value is 1 hour. + /// + /// + /// The time buffer before expiry when the token should be renewed, expressed as a percentage of the time to live. + /// The default behavior is that the token will be renewed when it has 15% or less of its lifespan left. + /// True if the authentication method should be disposed of by sdk; false if you intend to reuse the authentication method. + /// + public DeviceAuthenticationWithTokenRefresh( + string deviceId, + int suggestedTimeToLiveSeconds, + int timeBufferPercentage, + bool disposalBySdk) + : base(SetSasTokenSuggestedTimeToLiveSeconds(suggestedTimeToLiveSeconds), SetSasTokenRenewalBufferPercentage(timeBufferPercentage), disposalBySdk) { if (deviceId.IsNullOrWhiteSpace()) { diff --git a/iothub/device/src/DeviceAuthenticationWithTpm.cs b/iothub/device/src/DeviceAuthenticationWithTpm.cs index 5f94d5365b..198b568550 100644 --- a/iothub/device/src/DeviceAuthenticationWithTpm.cs +++ b/iothub/device/src/DeviceAuthenticationWithTpm.cs @@ -26,7 +26,8 @@ public sealed class DeviceAuthenticationWithTpm : DeviceAuthenticationWithTokenR /// Device Security Provider settings for TPM Hardware Security Modules. public DeviceAuthenticationWithTpm( string deviceId, - SecurityProviderTpm securityProvider) : base(deviceId) + SecurityProviderTpm securityProvider) + : base(deviceId) { _securityProvider = securityProvider ?? throw new ArgumentNullException(nameof(securityProvider)); } @@ -43,7 +44,27 @@ public DeviceAuthenticationWithTpm( string deviceId, SecurityProviderTpm securityProvider, int suggestedTimeToLiveSeconds, - int timeBufferPercentage) : base(deviceId, suggestedTimeToLiveSeconds, timeBufferPercentage) + int timeBufferPercentage) + : this(deviceId, securityProvider, suggestedTimeToLiveSeconds, timeBufferPercentage, true) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// Device Identifier. + /// Device Security Provider settings for TPM Hardware Security Modules. + /// Token time to live suggested value. + /// Time buffer before expiry when the token should be renewed expressed as percentage of + /// the time to live. EX: If you want a SAS token to live for 85% of life before proactive renewal, this value should be 15. + /// True if the authentication method should be disposed of by sdk; false if you intend to reuse the authentication method. + public DeviceAuthenticationWithTpm( + string deviceId, + SecurityProviderTpm securityProvider, + int suggestedTimeToLiveSeconds, + int timeBufferPercentage, + bool disposalBySdk) + : base(deviceId, suggestedTimeToLiveSeconds, timeBufferPercentage, disposalBySdk) { _securityProvider = securityProvider ?? throw new ArgumentNullException(nameof(securityProvider)); } diff --git a/iothub/device/src/Edge/EdgeModuleClientFactory.cs b/iothub/device/src/Edge/EdgeModuleClientFactory.cs index 07fa16be6c..08407d6636 100644 --- a/iothub/device/src/Edge/EdgeModuleClientFactory.cs +++ b/iothub/device/src/Edge/EdgeModuleClientFactory.cs @@ -98,11 +98,8 @@ private async Task CreateInternalClientFromEnvironmentAsync() int sasTokenRenewalBuffer = _options?.SasTokenRenewalBuffer ?? default; #pragma warning disable CA2000 // Dispose objects before losing scope - IDisposable ModuleAuthenticationWithHsm is disposed when the client is disposed. - var authMethod = new ModuleAuthenticationWithHsm(signatureProvider, deviceId, moduleId, generationId, sasTokenTimeToLive, sasTokenRenewalBuffer) - { - // Since the sdk creates the instance of disposable ModuleAuthenticationWithHsm, the sdk needs to dispose it once the client is diposed. - InstanceCreatedBySdk = true, - }; + // Since the sdk creates the instance of disposable ModuleAuthenticationWithHsm, the sdk needs to dispose it once the client is diposed. + var authMethod = new ModuleAuthenticationWithHsm(signatureProvider, deviceId, moduleId, generationId, sasTokenTimeToLive, sasTokenRenewalBuffer, disposalBySdk: true); #pragma warning restore CA2000 // Dispose objects before losing scope - IDisposable ModuleAuthenticationWithHsm is disposed when the client is disposed. Debug.WriteLine("EdgeModuleClientFactory setupTrustBundle from service"); diff --git a/iothub/device/src/HsmAuthentication/ModuleAuthenticationWithHsm.cs b/iothub/device/src/HsmAuthentication/ModuleAuthenticationWithHsm.cs index 5394922851..feba9a38ce 100644 --- a/iothub/device/src/HsmAuthentication/ModuleAuthenticationWithHsm.cs +++ b/iothub/device/src/HsmAuthentication/ModuleAuthenticationWithHsm.cs @@ -35,7 +35,9 @@ internal ModuleAuthenticationWithHsm( string moduleId, string generationId, TimeSpan sasTokenTimeToLive, - int sasTokenRenewalBuffer) : base(deviceId, moduleId, (int)sasTokenTimeToLive.TotalSeconds, sasTokenRenewalBuffer) + int sasTokenRenewalBuffer, + bool disposalBySdk) + : base(deviceId, moduleId, (int)sasTokenTimeToLive.TotalSeconds, sasTokenRenewalBuffer, disposalBySdk) { _signatureProvider = signatureProvider ?? throw new ArgumentNullException(nameof(signatureProvider)); _generationId = generationId ?? throw new ArgumentNullException(nameof(generationId)); diff --git a/iothub/device/src/IotHubConnectionString.cs b/iothub/device/src/IotHubConnectionString.cs index e52e01d243..650f9fdff9 100644 --- a/iothub/device/src/IotHubConnectionString.cs +++ b/iothub/device/src/IotHubConnectionString.cs @@ -52,11 +52,8 @@ public IotHubConnectionString(IotHubConnectionStringBuilder builder) { if (ModuleId.IsNullOrWhiteSpace()) { - TokenRefresher = new DeviceAuthenticationWithSakRefresh(DeviceId, this, builder.SasTokenTimeToLive, builder.SasTokenRenewalBuffer) - { - // Since the sdk creates the instance of disposable DeviceAuthenticationWithSakRefresh, the sdk needs to dispose it once the client is diposed. - InstanceCreatedBySdk = true, - }; + // Since the sdk creates the instance of disposable DeviceAuthenticationWithSakRefresh, the sdk needs to dispose it once the client is disposed. + TokenRefresher = new DeviceAuthenticationWithSakRefresh(DeviceId, this, builder.SasTokenTimeToLive, builder.SasTokenRenewalBuffer, disposalBySdk: true); if (Logging.IsEnabled) { @@ -65,11 +62,8 @@ public IotHubConnectionString(IotHubConnectionStringBuilder builder) } else { - TokenRefresher = new ModuleAuthenticationWithSakRefresh(DeviceId, ModuleId, this, builder.SasTokenTimeToLive, builder.SasTokenRenewalBuffer) - { - // Since the sdk creates the instance of disposable ModuleAuthenticationWithSakRefresh, the sdk needs to dispose it once the client is diposed. - InstanceCreatedBySdk = true, - }; + // Since the sdk creates the instance of disposable ModuleAuthenticationWithSakRefresh, the sdk needs to dispose it once the client is disposed. + TokenRefresher = new ModuleAuthenticationWithSakRefresh(DeviceId, ModuleId, this, builder.SasTokenTimeToLive, builder.SasTokenRenewalBuffer, disposalBySdk: true); if (Logging.IsEnabled) { diff --git a/iothub/device/src/ModuleAuthenticationWithSakRefresh.cs b/iothub/device/src/ModuleAuthenticationWithSakRefresh.cs index db79b2de67..2b71fbec2b 100644 --- a/iothub/device/src/ModuleAuthenticationWithSakRefresh.cs +++ b/iothub/device/src/ModuleAuthenticationWithSakRefresh.cs @@ -16,7 +16,8 @@ internal class ModuleAuthenticationWithSakRefresh : ModuleAuthenticationWithToke public ModuleAuthenticationWithSakRefresh( string deviceId, string moduleId, - IotHubConnectionString connectionString) : base(deviceId, moduleId) + IotHubConnectionString connectionString) + : base(deviceId, moduleId) { _connectionString = connectionString ?? throw new ArgumentNullException(nameof(connectionString)); } @@ -26,7 +27,9 @@ internal ModuleAuthenticationWithSakRefresh( string moduleId, IotHubConnectionString connectionString, TimeSpan sasTokenTimeToLive, - int sasTokenRenewalBuffer) : base(deviceId, moduleId, (int)sasTokenTimeToLive.TotalSeconds, sasTokenRenewalBuffer) + int sasTokenRenewalBuffer, + bool disposalBySdk = true) + : base(deviceId, moduleId, (int)sasTokenTimeToLive.TotalSeconds, sasTokenRenewalBuffer, disposalBySdk) { _connectionString = connectionString ?? throw new ArgumentNullException(nameof(connectionString)); } diff --git a/iothub/device/src/ModuleAuthenticationWithTokenRefresh.cs b/iothub/device/src/ModuleAuthenticationWithTokenRefresh.cs index fd67ae5ed6..d9856340d2 100644 --- a/iothub/device/src/ModuleAuthenticationWithTokenRefresh.cs +++ b/iothub/device/src/ModuleAuthenticationWithTokenRefresh.cs @@ -53,7 +53,31 @@ public ModuleAuthenticationWithTokenRefresh( string moduleId, int suggestedTimeToLiveSeconds, int timeBufferPercentage) - : base(SetSasTokenSuggestedTimeToLiveSeconds(suggestedTimeToLiveSeconds), SetSasTokenRenewalBufferPercentage(timeBufferPercentage)) + : this(deviceId, moduleId, suggestedTimeToLiveSeconds, timeBufferPercentage, true) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The device Id. + /// The module Id. + /// + /// The suggested time to live value for the generated SAS tokens. + /// The default value is 1 hour. + /// + /// + /// The time buffer before expiry when the token should be renewed, expressed as a percentage of the time to live. + /// The default behavior is that the token will be renewed when it has 15% or less of its lifespan left. + /// True if the authentication method should be disposed of by sdk; false if you intend to reuse the authentication method. + /// + public ModuleAuthenticationWithTokenRefresh( + string deviceId, + string moduleId, + int suggestedTimeToLiveSeconds, + int timeBufferPercentage, + bool disposalBySdk) + : base(SetSasTokenSuggestedTimeToLiveSeconds(suggestedTimeToLiveSeconds), SetSasTokenRenewalBufferPercentage(timeBufferPercentage), disposalBySdk) { if (moduleId.IsNullOrWhiteSpace()) { diff --git a/iothub/device/src/Transport/Amqp/AmqpConnectionPool.cs b/iothub/device/src/Transport/Amqp/AmqpConnectionPool.cs index 1916e4e4b0..c940452699 100644 --- a/iothub/device/src/Transport/Amqp/AmqpConnectionPool.cs +++ b/iothub/device/src/Transport/Amqp/AmqpConnectionPool.cs @@ -47,7 +47,7 @@ public AmqpUnit CreateAmqpUnit( if (deviceIdentity.AuthenticationModel == AuthenticationModel.SasGrouped && !ReferenceEquals(amqpConnectionHolder.GetDeviceIdentityOfAuthenticationProvider(), deviceIdentity) && deviceIdentity.IotHubConnectionString?.TokenRefresher != null - && deviceIdentity.IotHubConnectionString.TokenRefresher.InstanceCreatedBySdk) + && deviceIdentity.IotHubConnectionString.TokenRefresher.DisposalBySdk) { deviceIdentity.IotHubConnectionString.TokenRefresher.Dispose(); } diff --git a/iothub/device/src/Transport/AmqpIot/AmqpIotCbsTokenProvider.cs b/iothub/device/src/Transport/AmqpIot/AmqpIotCbsTokenProvider.cs index 047f2558b4..adc7c0936d 100644 --- a/iothub/device/src/Transport/AmqpIot/AmqpIotCbsTokenProvider.cs +++ b/iothub/device/src/Transport/AmqpIot/AmqpIotCbsTokenProvider.cs @@ -73,7 +73,7 @@ protected virtual void Dispose(bool disposing) if (disposing) { if (_connectionString?.TokenRefresher != null - && _connectionString.TokenRefresher.InstanceCreatedBySdk) + && _connectionString.TokenRefresher.DisposalBySdk) { _connectionString.TokenRefresher.Dispose(); } diff --git a/iothub/device/src/Transport/HttpClientHelper.cs b/iothub/device/src/Transport/HttpClientHelper.cs index b93f38a63d..88dd29aec7 100644 --- a/iothub/device/src/Transport/HttpClientHelper.cs +++ b/iothub/device/src/Transport/HttpClientHelper.cs @@ -550,7 +550,7 @@ public void Dispose() if (_isClientPrimaryTransportHandler && _authenticationHeaderProvider is IotHubConnectionString iotHubConnectionString && iotHubConnectionString.TokenRefresher != null - && iotHubConnectionString.TokenRefresher.InstanceCreatedBySdk) + && iotHubConnectionString.TokenRefresher.DisposalBySdk) { iotHubConnectionString.TokenRefresher.Dispose(); } diff --git a/iothub/device/src/Transport/Mqtt/MqttIotHubAdapter.cs b/iothub/device/src/Transport/Mqtt/MqttIotHubAdapter.cs index 93c322dca5..424ce37946 100644 --- a/iothub/device/src/Transport/Mqtt/MqttIotHubAdapter.cs +++ b/iothub/device/src/Transport/Mqtt/MqttIotHubAdapter.cs @@ -1019,7 +1019,7 @@ private async void ShutdownAsync(IChannelHandlerContext context) { if (_passwordProvider is IotHubConnectionString iotHubConnectionString && iotHubConnectionString.TokenRefresher != null - && iotHubConnectionString.TokenRefresher.InstanceCreatedBySdk) + && iotHubConnectionString.TokenRefresher.DisposalBySdk) { iotHubConnectionString.TokenRefresher.Dispose(); }