diff --git a/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/auth/IotHubSasTokenAuthenticationProvider.java b/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/auth/IotHubSasTokenAuthenticationProvider.java index 14062f3b31..f049437b24 100644 --- a/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/auth/IotHubSasTokenAuthenticationProvider.java +++ b/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/auth/IotHubSasTokenAuthenticationProvider.java @@ -88,6 +88,12 @@ public boolean isAuthenticationProviderRenewalNecessary() return (this.sasToken != null && this.sasToken.isExpired()); } + public boolean isSasTokenExpired() + { + //similar to isAuthenticationProviderRenewalNecessary, but this won't be overriden by most classes that inherit from this class + return (this.sasToken != null && this.sasToken.isExpired()); + } + /** * Returns true if the saved token should be refreshed * diff --git a/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/auth/IotHubSasTokenProvidedAuthenticationProvider.java b/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/auth/IotHubSasTokenProvidedAuthenticationProvider.java index fe5d58f41f..5e9877d82e 100644 --- a/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/auth/IotHubSasTokenProvidedAuthenticationProvider.java +++ b/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/auth/IotHubSasTokenProvidedAuthenticationProvider.java @@ -74,4 +74,9 @@ public int getMillisecondsBeforeProactiveRenewal() double timeBufferMultiplier = this.timeBufferPercentage / 100.0; //Convert 85 to .85, for example. Percentage multipliers are in decimal return (int) (tokenValidSeconds * 1000 * timeBufferMultiplier); } + + public boolean isSasTokenExpired() + { + return (lastSasToken != null && IotHubSasToken.isExpired(new String(lastSasToken))); + } } diff --git a/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransport.java b/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransport.java index 3624fbd32f..df1cbcc147 100644 --- a/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransport.java +++ b/device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransport.java @@ -422,7 +422,7 @@ public void open(boolean withRetry) throws TransportException // registering any devices to it. No need to check for SAS token expiry if no devices are registered yet. if (this.getDefaultConfig() != null) { - if (this.isSasTokenExpired()) + if (this.isAuthenticationProviderExpired()) { throw new SecurityException("Your sas token has expired"); } @@ -1066,12 +1066,7 @@ private IotHubConnectionStatusChangeReason exceptionToStatusChangeReason(Throwab if (e instanceof TransportException) { TransportException transportException = (TransportException) e; - if (transportException.isRetryable()) - { - log.debug("Mapping throwable to NO_NETWORK because it was a retryable exception", e); - return IotHubConnectionStatusChangeReason.NO_NETWORK; - } - else if (isSasTokenExpired()) + if (isSasTokenExpired()) { log.debug("Mapping throwable to EXPIRED_SAS_TOKEN because it was a non-retryable exception and the saved sas token has expired", e); return IotHubConnectionStatusChangeReason.EXPIRED_SAS_TOKEN; @@ -1081,6 +1076,11 @@ else if (e instanceof UnauthorizedException || e instanceof MqttUnauthorizedExce log.debug("Mapping throwable to BAD_CREDENTIAL because it was a non-retryable exception authorization exception but the saved sas token has not expired yet", e); return IotHubConnectionStatusChangeReason.BAD_CREDENTIAL; } + else if (transportException.isRetryable()) + { + log.debug("Mapping throwable to NO_NETWORK because it was a retryable exception", e); + return IotHubConnectionStatusChangeReason.NO_NETWORK; + } } log.debug("Mapping exception throwable to COMMUNICATION_ERROR because the sdk was unable to classify the thrown exception to anything other category", e); @@ -1520,7 +1520,7 @@ private boolean isMessageValid(IotHubTransportPacket packet) return false; } - if (isSasTokenExpired()) + if (isAuthenticationProviderExpired()) { log.debug("Creating a callback for the message with expired sas token with UNAUTHORIZED status"); packet.setStatus(IotHubStatusCode.UNAUTHORIZED); @@ -1657,7 +1657,7 @@ else if (this.connectionStatusChangeCallbacks.containsKey(deviceId)) // check SAS token expiry when using SAS based auth, and there is always a SAS token authentication provider // when using SAS based auth. @SuppressWarnings("ConstantConditions") - private boolean isSasTokenExpired() + private boolean isAuthenticationProviderExpired() { if (this.getDefaultConfig() == null) { @@ -1668,6 +1668,21 @@ private boolean isSasTokenExpired() && this.getDefaultConfig().getSasTokenAuthentication().isAuthenticationProviderRenewalNecessary(); } + // warning is about how getSasTokenAuthentication() may return null. In this case, it never will since we only + // check SAS token expiry when using SAS based auth, and there is always a SAS token authentication provider + // when using SAS based auth. + @SuppressWarnings("ConstantConditions") + private boolean isSasTokenExpired() + { + if (this.getDefaultConfig() == null) + { + return false; + } + + return this.getDefaultConfig().getAuthenticationType() == DeviceClientConfig.AuthType.SAS_TOKEN + && this.getDefaultConfig().getSasTokenAuthentication().isSasTokenExpired(); + } + /** * Returns if the provided packet has lasted longer than the device operation timeout * @@ -1832,7 +1847,7 @@ private static void sleepUninterruptibly(long sleepFor, TimeUnit unit) */ private void checkForUnauthorizedException(TransportException transportException) { - if (!this.isSasTokenExpired() && (transportException instanceof MqttUnauthorizedException + if (!this.isAuthenticationProviderExpired() && (transportException instanceof MqttUnauthorizedException || transportException instanceof UnauthorizedException || transportException instanceof AmqpUnauthorizedAccessException)) { diff --git a/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportTest.java b/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportTest.java index 20cab4bbec..3dde413467 100644 --- a/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportTest.java +++ b/device/iot-device-client/src/test/java/com/microsoft/azure/sdk/iot/device/transport/IotHubTransportTest.java @@ -523,7 +523,7 @@ public void openThrowsIfSasTokenExpired() throws DeviceClientException //arrange new MockUp() { - @Mock boolean isSasTokenExpired() + @Mock boolean isAuthenticationProviderExpired() { return true; } @@ -781,14 +781,6 @@ public void exceptionToStatusChangeReasonSasTokenExpired() }; final IotHubTransport transport = new IotHubTransport(mockedConfig, mockedIotHubConnectionStatusChangeCallback, false); - new Expectations() - { - { - mockedTransportException.isRetryable(); - result = false; - } - }; - //act IotHubConnectionStatusChangeReason reason = Deencapsulation.invoke(transport, "exceptionToStatusChangeReason", new Class[] {Throwable.class}, mockedTransportException); @@ -2453,7 +2445,7 @@ public void isMessageValidWithMessageNotExpiredSasTokenExpired() methodsCalled.append("addToCallbackQueue"); } - @Mock boolean isSasTokenExpired() + @Mock boolean isAuthenticationProviderExpired() { return true; } @@ -2698,7 +2690,7 @@ public void isSasTokenExpiredAuthenticationTypeIsSasTokenAndExpired() { mockedConfig.getAuthenticationType(); result = DeviceClientConfig.AuthType.SAS_TOKEN; - mockedConfig.getSasTokenAuthentication().isAuthenticationProviderRenewalNecessary(); + mockedConfig.getSasTokenAuthentication().isSasTokenExpired(); result = true; } }; @@ -2952,7 +2944,7 @@ public void checkForUnauthorizedExceptionWithExpiredSASToken() //arrange new MockUp() { - @Mock boolean isSasTokenExpired() + @Mock boolean isAuthenticationProviderExpired() { return true; } diff --git a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/TokenRenewalTests.java b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/TokenRenewalTests.java index 3aac1b19d5..4b14d8fa7b 100644 --- a/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/TokenRenewalTests.java +++ b/iot-e2e-tests/common/src/test/java/tests/integration/com/microsoft/azure/sdk/iot/iothub/TokenRenewalTests.java @@ -150,17 +150,27 @@ public void tokenRenewalWorks() throws Exception Success[] amqpDisconnectDidNotHappenSuccesses = new Success[clients.size()]; Success[] mqttDisconnectDidHappenSuccesses = new Success[clients.size()]; Success[] shutdownWasGracefulSuccesses = new Success[clients.size()]; + Success[] mqttDisconnectHadTokenExpiredReasonSuccesses = new Success[clients.size()]; for (int clientIndex = 0; clientIndex < clients.size(); clientIndex++) { amqpDisconnectDidNotHappenSuccesses[clientIndex] = new Success(); mqttDisconnectDidHappenSuccesses[clientIndex] = new Success(); shutdownWasGracefulSuccesses[clientIndex] = new Success(); - + mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex] = new Success(); + amqpDisconnectDidNotHappenSuccesses[clientIndex].setResult(true); //assume success until unexpected DISCONNECTED_RETRYING mqttDisconnectDidHappenSuccesses[clientIndex].setResult(false); //assume failure until DISCONNECTED_RETRYING is triggered by token expiring shutdownWasGracefulSuccesses[clientIndex].setResult(true); //assume success until DISCONNECTED callback without CLIENT_CLOSE - - clients.get(clientIndex).registerConnectionStatusChangeCallback(new IotHubConnectionStatusChangeTokenRenewalCallbackVerifier(clients.get(clientIndex).getConfig().getProtocol(), amqpDisconnectDidNotHappenSuccesses[clientIndex], mqttDisconnectDidHappenSuccesses[clientIndex], shutdownWasGracefulSuccesses[clientIndex]), clients.get(clientIndex)); + mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex].setResult(false); //assume failure until first disconnected_retrying executes with reason EXPIRED_SAS_TOKEN + + clients.get(clientIndex).registerConnectionStatusChangeCallback( + new IotHubConnectionStatusChangeTokenRenewalCallbackVerifier( + clients.get(clientIndex).getConfig().getProtocol(), + amqpDisconnectDidNotHappenSuccesses[clientIndex], + mqttDisconnectDidHappenSuccesses[clientIndex], + shutdownWasGracefulSuccesses[clientIndex], + mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex]), + clients.get(clientIndex)); } openEachClient(clients); @@ -182,7 +192,7 @@ public void tokenRenewalWorks() throws Exception testIdentities.clear(); - verifyClientsConnectivityBehavedCorrectly(clients, amqpDisconnectDidNotHappenSuccesses, mqttDisconnectDidHappenSuccesses, shutdownWasGracefulSuccesses); + verifyClientsConnectivityBehavedCorrectly(clients, amqpDisconnectDidNotHappenSuccesses, mqttDisconnectDidHappenSuccesses, shutdownWasGracefulSuccesses, mqttDisconnectHadTokenExpiredReasonSuccesses); } private void closeClients(List clients) throws IOException @@ -201,7 +211,12 @@ private void closeClients(List clients) throws IOException } } - private void verifyClientsConnectivityBehavedCorrectly(List clients, Success[] amqpDisconnectDidNotHappenSuccesses, Success[] mqttDisconnectDidHappenSuccesses, Success[] shutdownWasGracefulSuccesses) + private void verifyClientsConnectivityBehavedCorrectly( + List clients, + Success[] amqpDisconnectDidNotHappenSuccesses, + Success[] mqttDisconnectDidHappenSuccesses, + Success[] shutdownWasGracefulSuccesses, + Success[] mqttDisconnectHadTokenExpiredReasonSuccesses) { for (int clientIndex = 0; clientIndex < clients.size(); clientIndex++) { @@ -212,6 +227,7 @@ private void verifyClientsConnectivityBehavedCorrectly(List clie { assertTrue(CorrelationDetailsLoggingAssert.buildExceptionMessage("MQTT connection was never lost, token renewal was not tested", client), mqttDisconnectDidHappenSuccesses[clientIndex].result); assertTrue(CorrelationDetailsLoggingAssert.buildExceptionMessage("Connection was lost during test, or shutdown was not graceful", client), shutdownWasGracefulSuccesses[clientIndex].result); + assertTrue(CorrelationDetailsLoggingAssert.buildExceptionMessage("MQTT connection was lost, but for an unexpected reason", client), mqttDisconnectHadTokenExpiredReasonSuccesses[clientIndex].result); } else if (protocol == AMQPS || protocol == AMQPS_WS) { @@ -304,13 +320,15 @@ private static class IotHubConnectionStatusChangeTokenRenewalCallbackVerifier im Success amqpDisconnectDidNotHappen; Success mqttDisconnectDidHappen; Success shutdownWasGraceful; + Success mqttDisconnectHadTokenExpiredReason; - public IotHubConnectionStatusChangeTokenRenewalCallbackVerifier(IotHubClientProtocol protocol, Success amqpDisconnectDidNotHappen, Success mqttDisconnectDidHappen, Success shutdownWasGraceful) + public IotHubConnectionStatusChangeTokenRenewalCallbackVerifier(IotHubClientProtocol protocol, Success amqpDisconnectDidNotHappen, Success mqttDisconnectDidHappen, Success shutdownWasGraceful, Success mqttDisconnectHadTokenExpiredReason) { this.protocol = protocol; this.mqttDisconnectDidHappen = mqttDisconnectDidHappen; this.amqpDisconnectDidNotHappen = amqpDisconnectDidNotHappen; this.shutdownWasGraceful = shutdownWasGraceful; + this.mqttDisconnectHadTokenExpiredReason = mqttDisconnectHadTokenExpiredReason; } @Override @@ -345,6 +363,12 @@ else if (status == IotHubConnectionStatus.DISCONNECTED_RETRYING) { amqpDisconnectDidNotHappen.setResult(false); } + + if (protocol == MQTT || protocol == MQTT_WS) + { + boolean reasonIsSasTokenExpired = statusChangeReason == IotHubConnectionStatusChangeReason.EXPIRED_SAS_TOKEN; + mqttDisconnectHadTokenExpiredReason.setResult(reasonIsSasTokenExpired); + } } } }