diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/handlers/CAConfigurationChangedHandler.java b/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/handlers/CAConfigurationChangedHandler.java index 9cb01a8dc..5d99d7f86 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/handlers/CAConfigurationChangedHandler.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/handlers/CAConfigurationChangedHandler.java @@ -49,7 +49,7 @@ public void listen() { * @param event Certificate authority configuration change event */ @Override - public void accept(CAConfigurationChanged event) { + public synchronized void accept(CAConfigurationChanged event) { CAConfiguration configuration = event.getConfiguration(); try { diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/usecases/ConfigureCustomCertificateAuthority.java b/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/usecases/ConfigureCustomCertificateAuthority.java index 6ce1cc4c3..aebd807d7 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/usecases/ConfigureCustomCertificateAuthority.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/usecases/ConfigureCustomCertificateAuthority.java @@ -45,10 +45,6 @@ public ConfigureCustomCertificateAuthority(CertificateManager certificateManager @Override public Void apply(CAConfiguration configuration) throws UseCaseException { - // TODO: We need to synchronize the changes that configuration has on the state of the service. There is - // a possibility that 2 threads run different use cases and change the certificate authority concurrently - // causing potential race conditions - try { logger.info("Configuring custom certificate authority."); // NOTE: We will pull the configureCustomCA out of the certificate Manager to here diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/usecases/ConfigureManagedCertificateAuthority.java b/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/usecases/ConfigureManagedCertificateAuthority.java index fc01b9eb3..cefe9b8a8 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/usecases/ConfigureManagedCertificateAuthority.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/certificate/usecases/ConfigureManagedCertificateAuthority.java @@ -46,10 +46,6 @@ public Void apply(CAConfiguration configuration) throws UseCaseException { // TODO: We should not be passing the entire configuration just what changed. We are just doing it for // its convenience but eventually syncing the runtime config can be its own use case triggered by events. - // TODO: We need to synchronize the changes that configuration has on the state of the service. There is - // a possibility that 2 threads run different use cases and change the certificate authority concurrently - // causing potential race conditions - logger.info("Configuring Greengrass managed certificate authority."); try { 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 9edafcec3..20593442f 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 @@ -51,6 +51,10 @@ public CreateIoTThingSession(ThingRegistry thingRegistry, CertificateRegistry ce */ @Override public Session apply(CreateSessionDTO dto) throws AuthenticationException { + if (dto.getThingName() != null && dto.getThingName().length() > 65_535) { + throw new AuthenticationException("Thing name is too long"); + } + Certificate certificate = getActiveCertificateFromRegistry(dto); String thingName = dto.getThingName(); Thing thing = thingRegistry.getOrCreateThing(thingName); diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/session/MqttSessionFactory.java b/src/main/java/com/aws/greengrass/clientdevices/auth/session/MqttSessionFactory.java index 4469c8fb1..7144b3798 100644 --- a/src/main/java/com/aws/greengrass/clientdevices/auth/session/MqttSessionFactory.java +++ b/src/main/java/com/aws/greengrass/clientdevices/auth/session/MqttSessionFactory.java @@ -45,7 +45,7 @@ public Session createSession(Map credentialMap) throws Authentic } private Session createIotThingSession(MqttCredential mqttCredential) throws AuthenticationException { - // NOTE: We should remove calling this useCase from here, but for now it serves its purpose. We will + // NOTE: We should remove calling this useCase from here, but for now it serves its purpose. We will // refactor this later CreateIoTThingSession useCase = useCases.get(CreateIoTThingSession.class); CreateSessionDTO command = new CreateSessionDTO(mqttCredential.clientId, mqttCredential.certificatePem); 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 467e37240..a517dffe1 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 @@ -36,6 +36,7 @@ import java.util.Optional; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.mockito.ArgumentMatchers.any; @@ -100,6 +101,15 @@ void GIVEN_credentialsWithInvalidCertificate_WHEN_createSession_THEN_throwsAuthe Assertions.assertThrows(AuthenticationException.class, () -> mqttSessionFactory.createSession(credentialMap)); } + @Test + void GIVEN_credentialsWithLongClientId_WHEN_createSession_THEN_throwsAuthenticationException() { + AuthenticationException ex = Assertions.assertThrows(AuthenticationException.class, + () -> mqttSessionFactory.createSession( + ImmutableMap.of("certificatePem", "PEM", "clientId", new String(new byte[65536]), "username", + "", "password", ""))); + assertThat(ex.getMessage(), containsString("too long")); + } + @Test void GIVEN_credentialsWithCertificate_WHEN_createSession_AND_cloudError_THEN_throwsAuthenticationException() throws InvalidCertificateException, CloudServiceInteractionException {