Update AuthenticationWithTokenRefresh disposal to be non-breaking

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<Task> act = async () => await deviceClient2.SendEventAsync(message2).ConfigureAwait(false);
+            await act.Should().ThrowAsync<ObjectDisposedException>();
         }
 
         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)
                 {

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)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="AuthenticationWithTokenRefresh"/> class.
+        /// </summary>
+        /// <param name="suggestedTimeToLiveSeconds">Token time to live suggested value. The implementations of this abstract
+        /// may choose to ignore this value.</param>
+        /// <param name="timeBufferPercentage">Time buffer before expiry when the token should be renewed expressed as
+        /// a percentage of the time to live.</param>
+        /// <param name="disposalBySdk">True if the authentication method should be disposed of by sdk; false if you intend to reuse the authentication method.</param>
+        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;
         }
 
         /// <summary>
@@ -65,21 +83,25 @@ public AuthenticationWithTokenRefresh(
         /// </summary>
         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; }
 
         /// <summary>
         /// Gets a snapshot of the security token associated with the device. This call is thread-safe.
         /// </summary>
         public async Task<string> 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

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));
         }

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)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="DeviceAuthenticationWithTokenRefresh"/> class.
+        /// </summary>
+        /// <param name="deviceId">Device Identifier.</param>
+        /// <param name="suggestedTimeToLiveSeconds">
+        /// The suggested time to live value for the generated SAS tokens.
+        /// The default value is 1 hour.
+        /// </param>
+        /// <param name="timeBufferPercentage">
+        /// 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.
+        /// <param name="disposalBySdk">True if the authentication method should be disposed of by sdk; false if you intend to reuse the authentication method.</param>
+        ///</param>
+        public DeviceAuthenticationWithTokenRefresh(
+            string deviceId,
+            int suggestedTimeToLiveSeconds,
+            int timeBufferPercentage,
+            bool disposalBySdk)
+            : base(SetSasTokenSuggestedTimeToLiveSeconds(suggestedTimeToLiveSeconds), SetSasTokenRenewalBufferPercentage(timeBufferPercentage), disposalBySdk)
         {
             if (deviceId.IsNullOrWhiteSpace())
             {

iothub/device/src/DeviceAuthenticationWithTpm.cs

@@ -26,7 +26,8 @@ public sealed class DeviceAuthenticationWithTpm : DeviceAuthenticationWithTokenR
         /// <param name="securityProvider">Device Security Provider settings for TPM Hardware Security Modules.</param>
         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)
+        {
+        }
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="DeviceAuthenticationWithTpm"/> class.
+        /// </summary>
+        /// <param name="deviceId">Device Identifier.</param>
+        /// <param name="securityProvider">Device Security Provider settings for TPM Hardware Security Modules.</param>
+        /// <param name="suggestedTimeToLiveSeconds">Token time to live suggested value.</param>
+        /// <param name="timeBufferPercentage">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.</param>
+        /// <param name="disposalBySdk">True if the authentication method should be disposed of by sdk; false if you intend to reuse the authentication method.</param>
+        public DeviceAuthenticationWithTpm(
+            string deviceId,
+            SecurityProviderTpm securityProvider,
+            int suggestedTimeToLiveSeconds,
+            int timeBufferPercentage,
+            bool disposalBySdk)
+            : base(deviceId, suggestedTimeToLiveSeconds, timeBufferPercentage, disposalBySdk)
         {
             _securityProvider = securityProvider ?? throw new ArgumentNullException(nameof(securityProvider));
         }

iothub/device/src/Edge/EdgeModuleClientFactory.cs

@@ -98,11 +98,8 @@ private async Task<ModuleClient> 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");

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));

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)
                     {

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));
         }