From 394226b7528a3dc803cc990ceb4cae6195d3db2d Mon Sep 17 00:00:00 2001 From: Joseph Cosentino Date: Tue, 31 Oct 2023 10:33:41 -0700 Subject: [PATCH 1/3] chore: cleanup exception messaging --- .../auth/CertificateManager.java | 14 +++-- .../CloudServiceInteractionException.java | 2 +- .../clientdevices/auth/iot/Certificate.java | 2 +- .../clientdevices/auth/iot/IotAuthClient.java | 14 +++-- .../iot/usecases/CreateIoTThingSession.java | 59 ++++++++++--------- .../VerifyThingAttachedToCertificate.java | 3 +- .../BackgroundCertificateRefreshTest.java | 4 +- .../auth/iot/IotAuthClientTest.java | 9 ++- .../auth/session/MqttSessionFactoryTest.java | 6 +- 9 files changed, 60 insertions(+), 53 deletions(-) diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/CertificateManager.java b/src/main/java/com/aws/greengrass/clientdevices/auth/CertificateManager.java index 7eaf7e5d6..5bbcdb256 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/CertificateManager.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/CertificateManager.java @@ -348,14 +348,16 @@ public void configureCustomCA(CAConfiguration configuration) throws InvalidConfi * * @param thingName Core device name * @param certificate the CA to upload to IoT core. Which will be provided by cloud discovery - * @throws CertificateEncodingException If unable to get certificate encoding - * @throws KeyStoreException If unable to retrieve the certificate - * @throws IOException If unable to read certificate - * @throws DeviceConfigurationException If unable to retrieve Greengrass V2 Data client + * @throws CertificateEncodingException If unable to get certificate encoding + * @throws KeyStoreException If unable to retrieve the certificate + * @throws IOException If unable to read certificate + * @throws DeviceConfigurationException If unable to retrieve Greengrass V2 Data client + * @throws CloudServiceInteractionException If cloud call fails */ @SuppressWarnings({"PMD.AvoidCatchingGenericException", "PMD.AvoidRethrowingException"}) public void uploadCoreDeviceCAs(String thingName, X509Certificate certificate) - throws CertificateEncodingException, KeyStoreException, IOException, DeviceConfigurationException { + throws CertificateEncodingException, KeyStoreException, IOException, + DeviceConfigurationException, CloudServiceInteractionException { String certificatePem = CertificateHelper.toPem(certificate); List retryAbleExceptions = @@ -386,4 +388,4 @@ public void uploadCoreDeviceCAs(String thingName, X509Certificate certificate) + "core device's IoT policy grants the greengrass:PutCertificateAuthorities permission.", e); } } -} \ No newline at end of file +} diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/exception/CloudServiceInteractionException.java b/src/main/java/com/aws/greengrass/clientdevices/auth/exception/CloudServiceInteractionException.java index 24ddc5ba5..fe9d7b20a 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/exception/CloudServiceInteractionException.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/exception/CloudServiceInteractionException.java @@ -5,7 +5,7 @@ package com.aws.greengrass.clientdevices.auth.exception; -public class CloudServiceInteractionException extends RuntimeException { +public class CloudServiceInteractionException extends Exception { private static final long serialVersionUID = -1L; diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/iot/Certificate.java b/src/main/java/com/aws/greengrass/clientdevices/auth/iot/Certificate.java index 397e70845..19795e258 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/iot/Certificate.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/iot/Certificate.java @@ -155,7 +155,7 @@ public static void updateMetadataTrustDurationMinutes(int newTrustDuration) { metadataTrustDurationMinutes.set(newTrustDuration); } - private boolean isStatusTrusted() { + public boolean isStatusTrusted() { Instant validTill = statusLastUpdated.plus(metadataTrustDurationMinutes.get(), ChronoUnit.MINUTES); return validTill.isAfter(Instant.now()); } diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/iot/IotAuthClient.java b/src/main/java/com/aws/greengrass/clientdevices/auth/iot/IotAuthClient.java index b282f43e0..a4c6ddf27 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/iot/IotAuthClient.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/iot/IotAuthClient.java @@ -41,13 +41,13 @@ import javax.inject.Inject; public interface IotAuthClient { - Optional getActiveCertificateId(String certificatePem); + Optional getActiveCertificateId(String certificatePem) throws CloudServiceInteractionException; Optional getIotCertificate(String certificatePem) throws InvalidCertificateException; - boolean isThingAttachedToCertificate(Thing thing, Certificate certificate); + boolean isThingAttachedToCertificate(Thing thing, Certificate certificate) throws CloudServiceInteractionException; - boolean isThingAttachedToCertificate(Thing thing, String certificateId); + boolean isThingAttachedToCertificate(Thing thing, String certificateId) throws CloudServiceInteractionException; Stream> getThingsAssociatedWithCoreDevice(); @@ -77,7 +77,7 @@ class Default implements IotAuthClient { @Override @SuppressWarnings("PMD.AvoidCatchingGenericException") - public Optional getActiveCertificateId(String certificatePem) { + public Optional getActiveCertificateId(String certificatePem) throws CloudServiceInteractionException { if (Utils.isEmpty(certificatePem)) { throw new IllegalArgumentException("Certificate PEM is empty"); } @@ -135,7 +135,8 @@ public Optional getIotCertificate(String certificatePem) throws Inv } @Override - public boolean isThingAttachedToCertificate(Thing thing, Certificate certificate) { + public boolean isThingAttachedToCertificate(Thing thing, Certificate certificate) + throws CloudServiceInteractionException { if (Objects.isNull(certificate)) { return false; } @@ -144,7 +145,8 @@ public boolean isThingAttachedToCertificate(Thing thing, Certificate certificate @Override @SuppressWarnings({"PMD.AvoidCatchingGenericException", "PMD.AvoidDuplicateLiterals"}) - public boolean isThingAttachedToCertificate(Thing thing, String certificateId) { + public boolean isThingAttachedToCertificate(Thing thing, String certificateId) + throws CloudServiceInteractionException { if (thing == null || Utils.isEmpty(thing.getThingName())) { throw new IllegalArgumentException("No thing name available to validate"); } diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/iot/usecases/CreateIoTThingSession.java b/src/main/java/com/aws/greengrass/clientdevices/auth/iot/usecases/CreateIoTThingSession.java index cb35b4ff9..9edafcec3 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/iot/usecases/CreateIoTThingSession.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/iot/usecases/CreateIoTThingSession.java @@ -7,7 +7,6 @@ import com.aws.greengrass.clientdevices.auth.api.UseCases; import com.aws.greengrass.clientdevices.auth.exception.AuthenticationException; -import com.aws.greengrass.clientdevices.auth.exception.CloudServiceInteractionException; import com.aws.greengrass.clientdevices.auth.iot.Certificate; import com.aws.greengrass.clientdevices.auth.iot.CertificateRegistry; import com.aws.greengrass.clientdevices.auth.iot.InvalidCertificateException; @@ -20,7 +19,6 @@ import com.aws.greengrass.logging.api.Logger; import com.aws.greengrass.logging.impl.LogManager; -import java.util.Optional; import javax.inject.Inject; public class CreateIoTThingSession implements UseCases.UseCase { @@ -53,37 +51,44 @@ public CreateIoTThingSession(ThingRegistry thingRegistry, CertificateRegistry ce */ @Override public Session apply(CreateSessionDTO dto) throws AuthenticationException { - String certificatePem = dto.getCertificatePem(); + Certificate certificate = getActiveCertificateFromRegistry(dto); String thingName = dto.getThingName(); - Optional certificate; + Thing thing = thingRegistry.getOrCreateThing(thingName); - try { - certificate = certificateRegistry.getCertificateFromPem(certificatePem); - - if (!certificate.isPresent() || !certificate.get().isActive()) { - throw new AuthenticationException("Certificate isn't active"); - } - - Thing thing = thingRegistry.getOrCreateThing(thingName); + VerifyThingAttachedToCertificate.Result result = + useCases.get(VerifyThingAttachedToCertificate.class) + .apply(new VerifyThingAttachedToCertificateDTO(thingName, certificate.getCertificateId())); - VerifyThingAttachedToCertificate verify = useCases.get(VerifyThingAttachedToCertificate.class); - VerifyThingAttachedToCertificate.Result result = verify.apply( - new VerifyThingAttachedToCertificateDTO(thingName, certificate.get().getCertificateId())); + logger.atDebug() + .kv("thingName", thingName) + .kv("thingHasValidAttachment", result.isThingHasValidAttachmentToCertificate()) + .kv("lastAttachedOn", result.getLastAttached()) + .kv("attachmentExpiration", result.getAttachmentExpiration()) + .kv("source", result.getVerificationSource()) + .log("Attachment verification result"); - logger.atDebug() - .kv("thingHasValidAttachment", result.isThingHasValidAttachmentToCertificate()) - .kv("lastAttachedOn", result.getLastAttached()) - .kv("attachmentExpiration", result.getAttachmentExpiration()) - .kv("source", result.getVerificationSource()) - .log("Attachment verification result"); - - if (result.isThingHasValidAttachmentToCertificate()) { - return new SessionImpl(certificate.get(), thing); - } - } catch (CloudServiceInteractionException | InvalidCertificateException e) { - throw new AuthenticationException("Failed to verify certificate with cloud", e); + if (result.isThingHasValidAttachmentToCertificate()) { + return new SessionImpl(certificate, thing); } throw new AuthenticationException("Failed to verify certificate attached to thing"); } + + private Certificate getActiveCertificateFromRegistry(CreateSessionDTO dto) throws AuthenticationException { + Certificate certificate; + try { + certificate = certificateRegistry.getCertificateFromPem(dto.getCertificatePem()) + .orElseThrow(() -> new AuthenticationException("Certificate not in local registry")); + } catch (InvalidCertificateException e) { + throw new AuthenticationException("Certificate is invalid", e); + } + if (certificate.isActive()) { + return certificate; + } + if (certificate.isStatusTrusted()) { + throw new AuthenticationException("Certificate is not active"); + } else { + throw new AuthenticationException("Certificate is not trusted"); + } + } } diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/iot/usecases/VerifyThingAttachedToCertificate.java b/src/main/java/com/aws/greengrass/clientdevices/auth/iot/usecases/VerifyThingAttachedToCertificate.java index 04ebe2792..c51262966 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/iot/usecases/VerifyThingAttachedToCertificate.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/iot/usecases/VerifyThingAttachedToCertificate.java @@ -61,7 +61,7 @@ private Result verifyLocally(Thing thing, String certificateId) { .build(); } - private Result verifyFromCloud(Thing thing, String certificateId) { + private Result verifyFromCloud(Thing thing, String certificateId) throws CloudServiceInteractionException { logger.atDebug().kv("thing", thing.getThingName()).kv("certificate", certificateId) .log("Network up, verifying thing attached to certificate from cloud"); @@ -111,7 +111,6 @@ public Result apply(VerifyThingAttachedToCertificateDTO dto) { if (isNetworkUp()) { return verifyFromCloud(thing, dto.getCertificateId()); } - return verifyLocally(thing, dto.getCertificateId()); } catch (CloudServiceInteractionException e) { return verifyLocally(thing, dto.getCertificateId()); diff --git a/src/test/java/com/aws/greengrass/clientdevices/auth/certificate/BackgroundCertificateRefreshTest.java b/src/test/java/com/aws/greengrass/clientdevices/auth/certificate/BackgroundCertificateRefreshTest.java index 4ef5988c0..d829ffd69 100644 --- a/src/test/java/com/aws/greengrass/clientdevices/auth/certificate/BackgroundCertificateRefreshTest.java +++ b/src/test/java/com/aws/greengrass/clientdevices/auth/certificate/BackgroundCertificateRefreshTest.java @@ -365,8 +365,8 @@ void GIVEN_cloudFailure_WHEN_verifyingAThingCertificateAttachment_THEN_refreshTa ArgumentCaptor doCaptor = ArgumentCaptor.forClass(VerifyThingAttachedToCertificateDTO.class); - when(verifyThingAttachedToCertificateMock.apply(doCaptor.capture())).thenThrow( - new CloudServiceInteractionException("Failed to verify association")).thenReturn( + when(verifyThingAttachedToCertificateMock.apply(doCaptor.capture())).thenReturn( + VerifyThingAttachedToCertificate.Result.builder().thingHasValidAttachmentToCertificate(false).build()).thenReturn( VerifyThingAttachedToCertificate.Result.builder().thingHasValidAttachmentToCertificate(true).build()); Instant twentyFourHoursLater = now.plus(Duration.ofHours(24)); mockInstant(twentyFourHoursLater.toEpochMilli()); diff --git a/src/test/java/com/aws/greengrass/clientdevices/auth/iot/IotAuthClientTest.java b/src/test/java/com/aws/greengrass/clientdevices/auth/iot/IotAuthClientTest.java index f182578cf..1eec32147 100644 --- a/src/test/java/com/aws/greengrass/clientdevices/auth/iot/IotAuthClientTest.java +++ b/src/test/java/com/aws/greengrass/clientdevices/auth/iot/IotAuthClientTest.java @@ -70,7 +70,7 @@ void beforeEach() throws DeviceConfigurationException { } @Test - void GIVEN_certificatePem_and_cloudProperResponse_WHEN_getActiveCertificateId_THEN_certificateIdReturned() { + void GIVEN_certificatePem_and_cloudProperResponse_WHEN_getActiveCertificateId_THEN_certificateIdReturned() throws CloudServiceInteractionException { VerifyClientDeviceIdentityResponse response = VerifyClientDeviceIdentityResponse.builder().clientDeviceCertificateId("certificateId").build(); when(client.verifyClientDeviceIdentity(any(VerifyClientDeviceIdentityRequest.class))).thenReturn(response); @@ -83,7 +83,7 @@ void GIVEN_certificatePem_and_cloudProperResponse_WHEN_getActiveCertificateId_TH } @Test - void GIVEN_cloudThrowValidationException_WHEN_getActiveCertificateId_THEN_returnNull(ExtensionContext context) { + void GIVEN_cloudThrowValidationException_WHEN_getActiveCertificateId_THEN_returnNull(ExtensionContext context) throws CloudServiceInteractionException { ignoreExceptionOfType(context, ValidationException.class); when(client.verifyClientDeviceIdentity(any(VerifyClientDeviceIdentityRequest.class))).thenThrow( ValidationException.class); @@ -130,7 +130,7 @@ void GIVEN_threadGotInterrupted_WHEN_getActiveCertificateId_THEN_throwCloudInter } @Test - void GIVEN_certificateAndThing_and_cloudVerificationSuccess_WHEN_isThingAttachedToCertificate_THEN_returnTrue() { + void GIVEN_certificateAndThing_and_cloudVerificationSuccess_WHEN_isThingAttachedToCertificate_THEN_returnTrue() throws CloudServiceInteractionException { when(thing.getThingName()).thenReturn("thingName"); when(certificate.getCertificateId()).thenReturn("certificateId"); when(client.verifyClientDeviceIoTCertificateAssociation( @@ -145,8 +145,7 @@ void GIVEN_certificateAndThing_and_cloudVerificationSuccess_WHEN_isThingAttached } @Test - void GIVEN_cloudThrowValidationException_WHEN_isThingAttachedToCertificate_THEN_returnFalse( - ExtensionContext context) { + void GIVEN_cloudThrowValidationException_WHEN_isThingAttachedToCertificate_THEN_returnFalse(ExtensionContext context) throws CloudServiceInteractionException{ ignoreExceptionOfType(context, ValidationException.class); when(thing.getThingName()).thenReturn("thingName"); when(certificate.getCertificateId()).thenReturn("certificateId"); diff --git a/src/test/java/com/aws/greengrass/clientdevices/auth/session/MqttSessionFactoryTest.java b/src/test/java/com/aws/greengrass/clientdevices/auth/session/MqttSessionFactoryTest.java index be9941166..467e37240 100644 --- a/src/test/java/com/aws/greengrass/clientdevices/auth/session/MqttSessionFactoryTest.java +++ b/src/test/java/com/aws/greengrass/clientdevices/auth/session/MqttSessionFactoryTest.java @@ -82,7 +82,7 @@ void afterEach() throws IOException { @Test void GIVEN_credentialsWithUnknownClientId_WHEN_createSession_THEN_throwsAuthenticationException() - throws InvalidCertificateException { + throws InvalidCertificateException, CloudServiceInteractionException { when(mockNetworkState.getConnectionState()).thenReturn(NetworkStateProvider.ConnectionState.NETWORK_UP); when(mockCertificateRegistry.getCertificateFromPem(any())).thenReturn( Optional.of(CertificateFake.activeCertificate())); @@ -102,7 +102,7 @@ void GIVEN_credentialsWithInvalidCertificate_WHEN_createSession_THEN_throwsAuthe @Test void GIVEN_credentialsWithCertificate_WHEN_createSession_AND_cloudError_THEN_throwsAuthenticationException() - throws InvalidCertificateException { + throws InvalidCertificateException, CloudServiceInteractionException { when(mockNetworkState.getConnectionState()).thenReturn(NetworkStateProvider.ConnectionState.NETWORK_UP); when(mockCertificateRegistry.getCertificateFromPem(any())).thenReturn( Optional.of(CertificateFake.activeCertificate())); @@ -115,7 +115,7 @@ void GIVEN_credentialsWithCertificate_WHEN_createSession_AND_cloudError_THEN_thr @Test void GIVEN_credentialsWithValidClientId_WHEN_createSession_THEN_returnsSession() - throws AuthenticationException, InvalidCertificateException { + throws AuthenticationException, InvalidCertificateException, CloudServiceInteractionException { when(mockNetworkState.getConnectionState()).thenReturn(NetworkStateProvider.ConnectionState.NETWORK_UP); when(mockThingRegistry.getOrCreateThing("clientId")).thenReturn(Thing.of("clientId")); when(mockThingRegistry.getThing(any())).thenReturn(Thing.of("clientId")); From a9d669f05ce087be6a26a8ca7ead1d504768ae40 Mon Sep 17 00:00:00 2001 From: Joseph Cosentino Date: Tue, 31 Oct 2023 10:52:01 -0700 Subject: [PATCH 2/3] fix: ipc test --- .../integrationtests/ipc/GetClientDeviceAuthTokenTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/integrationtests/java/com/aws/greengrass/integrationtests/ipc/GetClientDeviceAuthTokenTest.java b/src/integrationtests/java/com/aws/greengrass/integrationtests/ipc/GetClientDeviceAuthTokenTest.java index 043e59cd1..c2aa0bb4c 100644 --- a/src/integrationtests/java/com/aws/greengrass/integrationtests/ipc/GetClientDeviceAuthTokenTest.java +++ b/src/integrationtests/java/com/aws/greengrass/integrationtests/ipc/GetClientDeviceAuthTokenTest.java @@ -254,13 +254,13 @@ void GIVEN_brokerWithInvalidSession_WHEN_GetClientDeviceAuthToken_THEN_throwInva void GIVEN_brokerWithValidCredentials_WHEN_GetClientDeviceAuthTokenFails_THEN_throwServiceError( ExtensionContext context) throws Exception { kernel.getContext().put(SessionManager.class, sessionManager); - ignoreExceptionOfType(context, CloudServiceInteractionException.class); + ignoreExceptionOfType(context, AuthenticationException.class); startNucleusWithConfig("cda.yaml"); Map mqttCredentialMap = ImmutableMap.of("clientId", "some-client-id", "username", null, "password", null, "certificatePem", "-----BEGIN CERTIFICATE-----" + System.lineSeparator() + "PEM=" + System.lineSeparator() + "-----END CERTIFICATE-----" + System.lineSeparator()); - when(sessionManager.createSession("mqtt", mqttCredentialMap)).thenThrow(CloudServiceInteractionException.class); + when(sessionManager.createSession("mqtt", mqttCredentialMap)).thenThrow(AuthenticationException.class); try (EventStreamRPCConnection connection = IPCTestUtils.getEventStreamRpcConnection(kernel, "BrokerWithGetClientDeviceAuthTokenPermission")) { @@ -273,7 +273,7 @@ void GIVEN_brokerWithValidCredentials_WHEN_GetClientDeviceAuthTokenFails_THEN_th Exception err = assertThrows(Exception.class, () -> { clientDeviceAuthToken(ipcClient, request, null); }); - assertEquals(err.getCause().getClass(), ServiceError.class); + assertEquals(InvalidCredentialError.class, err.getCause().getClass()); } } From e6fe0e2a63ccd80b8c95729490f4af3ccf89ff4b Mon Sep 17 00:00:00 2001 From: Joseph Cosentino Date: Tue, 31 Oct 2023 10:52:21 -0700 Subject: [PATCH 3/3] chore: imports --- .../integrationtests/ipc/GetClientDeviceAuthTokenTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/integrationtests/java/com/aws/greengrass/integrationtests/ipc/GetClientDeviceAuthTokenTest.java b/src/integrationtests/java/com/aws/greengrass/integrationtests/ipc/GetClientDeviceAuthTokenTest.java index c2aa0bb4c..6e7b1e0dc 100644 --- a/src/integrationtests/java/com/aws/greengrass/integrationtests/ipc/GetClientDeviceAuthTokenTest.java +++ b/src/integrationtests/java/com/aws/greengrass/integrationtests/ipc/GetClientDeviceAuthTokenTest.java @@ -10,7 +10,6 @@ import com.aws.greengrass.dependency.State; import com.aws.greengrass.clientdevices.auth.ClientDevicesAuthService; import com.aws.greengrass.clientdevices.auth.exception.AuthenticationException; -import com.aws.greengrass.clientdevices.auth.exception.CloudServiceInteractionException; import com.aws.greengrass.clientdevices.auth.session.SessionManager; import com.aws.greengrass.lifecyclemanager.GlobalStateChangeListener; import com.aws.greengrass.lifecyclemanager.GreengrassService;