Skip to content

Commit

Permalink
chore: cleanup exception messaging (#409)
Browse files Browse the repository at this point in the history
  • Loading branch information
jcosentino11 authored Oct 31, 2023
1 parent 4d4aa0b commit 2fd5357
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -254,13 +253,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<String, String> 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")) {
Expand All @@ -273,7 +272,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());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Class> retryAbleExceptions =
Expand Down Expand Up @@ -386,4 +388,4 @@ public void uploadCoreDeviceCAs(String thingName, X509Certificate certificate)
+ "core device's IoT policy grants the greengrass:PutCertificateAuthorities permission.", e);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@
import javax.inject.Inject;

public interface IotAuthClient {
Optional<String> getActiveCertificateId(String certificatePem);
Optional<String> getActiveCertificateId(String certificatePem) throws CloudServiceInteractionException;

Optional<Certificate> 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<List<AssociatedClientDevice>> getThingsAssociatedWithCoreDevice();
Expand Down Expand Up @@ -77,7 +77,7 @@ class Default implements IotAuthClient {

@Override
@SuppressWarnings("PMD.AvoidCatchingGenericException")
public Optional<String> getActiveCertificateId(String certificatePem) {
public Optional<String> getActiveCertificateId(String certificatePem) throws CloudServiceInteractionException {
if (Utils.isEmpty(certificatePem)) {
throw new IllegalArgumentException("Certificate PEM is empty");
}
Expand Down Expand Up @@ -135,7 +135,8 @@ public Optional<Certificate> 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;
}
Expand All @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Session, CreateSessionDTO> {
Expand Down Expand Up @@ -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> 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");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,8 @@ void GIVEN_cloudFailure_WHEN_verifyingAThingCertificateAttachment_THEN_refreshTa
ArgumentCaptor<VerifyThingAttachedToCertificateDTO> 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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(
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand All @@ -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()));
Expand All @@ -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"));
Expand Down

0 comments on commit 2fd5357

Please sign in to comment.