From d8ff8f7fe86f9f6c31094e16968d55ac8e234bce Mon Sep 17 00:00:00 2001 From: SimoneFiorani Date: Wed, 6 Dec 2023 15:25:44 +0100 Subject: [PATCH 1/5] fix(rest.identity): adding specific error messages when user not found Signed-off-by: SimoneFiorani --- .../identity/provider/IdentityService.java | 18 ++++++++++++++---- .../provider/test/IdentityServiceTest.java | 4 +++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityService.java b/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityService.java index 0c1730757c2..947793f0333 100644 --- a/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityService.java +++ b/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityService.java @@ -23,12 +23,16 @@ import java.util.Optional; import java.util.Set; +import javax.ws.rs.WebApplicationException; +import javax.ws.rs.core.Response.Status; + import org.eclipse.kura.KuraErrorCode; import org.eclipse.kura.KuraException; import org.eclipse.kura.configuration.ComponentConfiguration; import org.eclipse.kura.configuration.ConfigurationService; import org.eclipse.kura.crypto.CryptoService; import org.eclipse.kura.internal.rest.identity.provider.dto.UserDTO; +import org.eclipse.kura.request.handler.jaxrs.DefaultExceptionHandler; import org.eclipse.kura.util.useradmin.UserAdminHelper; import org.eclipse.kura.util.useradmin.UserAdminHelper.FallibleConsumer; import org.eclipse.kura.util.validation.PasswordStrengthValidators; @@ -77,18 +81,24 @@ public void createUser(UserDTO user) throws KuraException { } } - public void deleteUser(String userName) { - this.userAdminHelper.deleteUser(userName); + public void deleteUser(String userName) throws WebApplicationException { + + if (this.userAdminHelper.getUser(userName).isPresent()) { + this.userAdminHelper.deleteUser(userName); + } else { + throw DefaultExceptionHandler.buildWebApplicationException(Status.NOT_FOUND, "Identity does not exist"); + } + } - public UserDTO getUser(String userName) throws KuraException { + public UserDTO getUser(String userName) throws WebApplicationException { Optional user = this.userAdminHelper.getUser(userName); if (user.isPresent()) { UserDTO userFound = initUserConfig(user.get()); fillPermissions(Collections.singletonMap(user.get().getName(), userFound)); return userFound; } else { - throw new KuraException(KuraErrorCode.NOT_FOUND, IDENTITY + userName + " not found"); + throw DefaultExceptionHandler.buildWebApplicationException(Status.NOT_FOUND, "Identity does not exist"); } } diff --git a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java index 62ead96de36..57bdbbe3275 100644 --- a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java +++ b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java @@ -36,6 +36,8 @@ import java.util.Objects; import java.util.Set; +import javax.ws.rs.WebApplicationException; + import org.eclipse.kura.KuraException; import org.eclipse.kura.configuration.ComponentConfiguration; import org.eclipse.kura.configuration.ConfigurationService; @@ -119,7 +121,7 @@ public void shoulGetUser() { private void whenGettingUser(String username) { try { this.user = this.identityService.getUser(username); - } catch (KuraException e) { + } catch (WebApplicationException e) { this.occurredException = e; } } From 1bfdc36d42dfd4e04c1e6b076509d393ecdeaa87 Mon Sep 17 00:00:00 2001 From: SimoneFiorani Date: Wed, 6 Dec 2023 17:36:35 +0100 Subject: [PATCH 2/5] Updated exception throwing strategy Signed-off-by: SimoneFiorani --- .../identity/provider/IdentityRestService.java | 15 +++++++++++++++ .../rest/identity/provider/IdentityService.java | 12 ++++-------- .../provider/test/IdentityServiceTest.java | 4 +--- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityRestService.java b/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityRestService.java index 3329fd2a93a..72d4ea5653f 100644 --- a/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityRestService.java +++ b/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityRestService.java @@ -22,7 +22,10 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import javax.ws.rs.core.Response.Status; +import org.eclipse.kura.KuraErrorCode; +import org.eclipse.kura.KuraException; import org.eclipse.kura.cloudconnection.request.RequestHandler; import org.eclipse.kura.cloudconnection.request.RequestHandlerRegistry; import org.eclipse.kura.configuration.ConfigurationService; @@ -138,6 +141,12 @@ public UserDTO getUser(final UserDTO userName) { try { logger.debug(DEBUG_MESSAGE, "getUser"); return this.identityService.getUser(userName.getUserName()); + } catch (KuraException e) { + if (e.getCode().equals(KuraErrorCode.NOT_FOUND)) { + throw DefaultExceptionHandler.buildWebApplicationException(Status.NOT_FOUND, "Identity does not exist"); + } else { + throw DefaultExceptionHandler.toWebApplicationException(e); + } } catch (Exception e) { throw DefaultExceptionHandler.toWebApplicationException(e); } @@ -152,6 +161,12 @@ public Response deleteUser(final UserDTO userName) { try { logger.debug(DEBUG_MESSAGE, "deleteUser"); this.identityService.deleteUser(userName.getUserName()); + } catch (KuraException e) { + if (e.getCode().equals(KuraErrorCode.NOT_FOUND)) { + throw DefaultExceptionHandler.buildWebApplicationException(Status.NOT_FOUND, "Identity does not exist"); + } else { + throw DefaultExceptionHandler.toWebApplicationException(e); + } } catch (Exception e) { throw DefaultExceptionHandler.toWebApplicationException(e); } diff --git a/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityService.java b/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityService.java index 947793f0333..f0543d1c60d 100644 --- a/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityService.java +++ b/kura/org.eclipse.kura.rest.identity.provider/src/main/java/org/eclipse/kura/internal/rest/identity/provider/IdentityService.java @@ -23,16 +23,12 @@ import java.util.Optional; import java.util.Set; -import javax.ws.rs.WebApplicationException; -import javax.ws.rs.core.Response.Status; - import org.eclipse.kura.KuraErrorCode; import org.eclipse.kura.KuraException; import org.eclipse.kura.configuration.ComponentConfiguration; import org.eclipse.kura.configuration.ConfigurationService; import org.eclipse.kura.crypto.CryptoService; import org.eclipse.kura.internal.rest.identity.provider.dto.UserDTO; -import org.eclipse.kura.request.handler.jaxrs.DefaultExceptionHandler; import org.eclipse.kura.util.useradmin.UserAdminHelper; import org.eclipse.kura.util.useradmin.UserAdminHelper.FallibleConsumer; import org.eclipse.kura.util.validation.PasswordStrengthValidators; @@ -81,24 +77,24 @@ public void createUser(UserDTO user) throws KuraException { } } - public void deleteUser(String userName) throws WebApplicationException { + public void deleteUser(String userName) throws KuraException { if (this.userAdminHelper.getUser(userName).isPresent()) { this.userAdminHelper.deleteUser(userName); } else { - throw DefaultExceptionHandler.buildWebApplicationException(Status.NOT_FOUND, "Identity does not exist"); + throw new KuraException(KuraErrorCode.NOT_FOUND, "Identity does not exist"); } } - public UserDTO getUser(String userName) throws WebApplicationException { + public UserDTO getUser(String userName) throws KuraException { Optional user = this.userAdminHelper.getUser(userName); if (user.isPresent()) { UserDTO userFound = initUserConfig(user.get()); fillPermissions(Collections.singletonMap(user.get().getName(), userFound)); return userFound; } else { - throw DefaultExceptionHandler.buildWebApplicationException(Status.NOT_FOUND, "Identity does not exist"); + throw new KuraException(KuraErrorCode.NOT_FOUND, "Identity does not exist"); } } diff --git a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java index 57bdbbe3275..62ead96de36 100644 --- a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java +++ b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java @@ -36,8 +36,6 @@ import java.util.Objects; import java.util.Set; -import javax.ws.rs.WebApplicationException; - import org.eclipse.kura.KuraException; import org.eclipse.kura.configuration.ComponentConfiguration; import org.eclipse.kura.configuration.ConfigurationService; @@ -121,7 +119,7 @@ public void shoulGetUser() { private void whenGettingUser(String username) { try { this.user = this.identityService.getUser(username); - } catch (WebApplicationException e) { + } catch (KuraException e) { this.occurredException = e; } } From 925a0cf454ede3dc23b6fdc74193244e131407b8 Mon Sep 17 00:00:00 2001 From: SimoneFiorani Date: Thu, 7 Dec 2023 11:21:36 +0100 Subject: [PATCH 3/5] Updated tests Signed-off-by: SimoneFiorani --- .../provider/test/IdentityServiceTest.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java index 62ead96de36..7855dc67f26 100644 --- a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java +++ b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java @@ -13,6 +13,7 @@ package org.eclipse.kura.internal.rest.identity.provider.test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -186,6 +187,24 @@ public void shouldNotValidateWrongPassword() { thenPasswordValidationIs(false); } + @Test + public void shouldThrowExceptionWhenDeletingNonExistingUser() { + givenIdentityService(); + + whenDeleting("NonExistingUser"); + + thenExceptionOccurred(KuraException.class); + } + + @Test + public void shouldThrowExceptionWhenGettingNonExistingUser() { + givenIdentityService(); + + whenGettingUser("NonExistingUser"); + + thenExceptionOccurred(KuraException.class); + } + private void whenValidatingPassword(String password) { try { this.identityService.validateUserPassword(password); @@ -330,6 +349,11 @@ private void thenNoExceptionOccurred() { assertNull(errorMessage, this.occurredException); } + private void thenExceptionOccurred(Class expectedExceptionClass) { + assertNotNull(this.occurredException); + assertEquals(expectedExceptionClass.getName(), this.occurredException.getClass().getName()); + } + public static class PermissionRole implements Group { private final String name; From 2971f7c8066f8e58c253e9eeb624cd97f1b3ba6b Mon Sep 17 00:00:00 2001 From: SimoneFiorani Date: Thu, 7 Dec 2023 16:29:14 +0100 Subject: [PATCH 4/5] Added tests for coverage Signed-off-by: SimoneFiorani --- .../provider/test/IdentityEndpointsTest.java | 26 +++++++++++++++++++ .../resources/getNonExistingUserResponse.json | 3 +++ .../provider/test/IdentityServiceTest.java | 24 ----------------- 3 files changed, 29 insertions(+), 24 deletions(-) create mode 100644 kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/resources/getNonExistingUserResponse.json diff --git a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityEndpointsTest.java b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityEndpointsTest.java index 7e9d1228331..abf559a6110 100644 --- a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityEndpointsTest.java +++ b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityEndpointsTest.java @@ -81,6 +81,10 @@ public class IdentityEndpointsTest extends AbstractRequestHandlerTest { IdentityEndpointsTest.class.getResourceAsStream("/getPasswordRequirementsResponse.json"), "UTF-8") .useDelimiter("\\A").next().replace(" ", ""); + private static final String EXPECTED_NON_EXISTING_USER_RESPONSE = new Scanner( + IdentityEndpointsTest.class.getResourceAsStream("/getNonExistingUserResponse.json"), "UTF-8") + .useDelimiter("\\A").next().replace(" ", ""); + private static Set userConfigs; private static ServiceRegistration identityServiceRegistration; @@ -178,6 +182,28 @@ public void shouldReturnPasswordRequirements() throws KuraException { thenResponseBodyEqualsJson(EXPECTED_GET_PASSWORD_REQUIREMENTS_RESPONSE); } + @Test + public void shouldReturnNonExistingUserDeleteResponse() throws KuraException { + givenIdentityService(); + + whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_DELETE, MQTT_METHOD_SPEC_DEL), "/identities", + "{\"userName\":\"nonExistingUser\"}"); + + thenResponseCodeIs(404); + thenResponseBodyEqualsJson(EXPECTED_NON_EXISTING_USER_RESPONSE); + } + + @Test + public void shouldReturnNonExistingUserPostResponse() throws KuraException { + givenIdentityService(); + + whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_POST), "/identities/byName", + "{\"userName\":\"nonExistingUser\"}"); + + thenResponseCodeIs(404); + thenResponseBodyEqualsJson(EXPECTED_NON_EXISTING_USER_RESPONSE); + } + private void givenUser(UserDTO userParam) { user = userParam; } diff --git a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/resources/getNonExistingUserResponse.json b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/resources/getNonExistingUserResponse.json new file mode 100644 index 00000000000..d50391c239e --- /dev/null +++ b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/resources/getNonExistingUserResponse.json @@ -0,0 +1,3 @@ +{ + "message": "Identity does not exist" +} \ No newline at end of file diff --git a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java index 7855dc67f26..62ead96de36 100644 --- a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java +++ b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/test/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityServiceTest.java @@ -13,7 +13,6 @@ package org.eclipse.kura.internal.rest.identity.provider.test; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -187,24 +186,6 @@ public void shouldNotValidateWrongPassword() { thenPasswordValidationIs(false); } - @Test - public void shouldThrowExceptionWhenDeletingNonExistingUser() { - givenIdentityService(); - - whenDeleting("NonExistingUser"); - - thenExceptionOccurred(KuraException.class); - } - - @Test - public void shouldThrowExceptionWhenGettingNonExistingUser() { - givenIdentityService(); - - whenGettingUser("NonExistingUser"); - - thenExceptionOccurred(KuraException.class); - } - private void whenValidatingPassword(String password) { try { this.identityService.validateUserPassword(password); @@ -349,11 +330,6 @@ private void thenNoExceptionOccurred() { assertNull(errorMessage, this.occurredException); } - private void thenExceptionOccurred(Class expectedExceptionClass) { - assertNotNull(this.occurredException); - assertEquals(expectedExceptionClass.getName(), this.occurredException.getClass().getName()); - } - public static class PermissionRole implements Group { private final String name; From bf89c81f7ed9f9534cfb0a60a5762710ec9eecb6 Mon Sep 17 00:00:00 2001 From: SimoneFiorani Date: Mon, 11 Dec 2023 11:48:30 +0100 Subject: [PATCH 5/5] Updated tests Signed-off-by: SimoneFiorani --- .../provider/test/IdentityEndpointsTest.java | 15 ++++++++++++--- .../resources/getNonExistingUserResponse.json | 4 +--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityEndpointsTest.java b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityEndpointsTest.java index abf559a6110..8703782a381 100644 --- a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityEndpointsTest.java +++ b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/java/org/eclipse/kura/internal/rest/identity/provider/test/IdentityEndpointsTest.java @@ -12,6 +12,7 @@ *******************************************************************************/ package org.eclipse.kura.internal.rest.identity.provider.test; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.when; @@ -27,6 +28,7 @@ import java.util.Set; import java.util.concurrent.TimeUnit; +import org.eclipse.kura.KuraErrorCode; import org.eclipse.kura.KuraException; import org.eclipse.kura.core.testutil.requesthandler.AbstractRequestHandlerTest; import org.eclipse.kura.core.testutil.requesthandler.MqttTransport; @@ -83,7 +85,7 @@ public class IdentityEndpointsTest extends AbstractRequestHandlerTest { private static final String EXPECTED_NON_EXISTING_USER_RESPONSE = new Scanner( IdentityEndpointsTest.class.getResourceAsStream("/getNonExistingUserResponse.json"), "UTF-8") - .useDelimiter("\\A").next().replace(" ", ""); + .useDelimiter("\\A").next(); private static Set userConfigs; @@ -187,7 +189,7 @@ public void shouldReturnNonExistingUserDeleteResponse() throws KuraException { givenIdentityService(); whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_DELETE, MQTT_METHOD_SPEC_DEL), "/identities", - "{\"userName\":\"nonExistingUser\"}"); + gson.toJson(new UserDTO("nonExistingUser", null, false, false))); thenResponseCodeIs(404); thenResponseBodyEqualsJson(EXPECTED_NON_EXISTING_USER_RESPONSE); @@ -198,7 +200,7 @@ public void shouldReturnNonExistingUserPostResponse() throws KuraException { givenIdentityService(); whenRequestIsPerformed(new MethodSpec(METHOD_SPEC_POST), "/identities/byName", - "{\"userName\":\"nonExistingUser\"}"); + gson.toJson(new UserDTO("nonExistingUser", null, false, false))); thenResponseCodeIs(404); thenResponseBodyEqualsJson(EXPECTED_NON_EXISTING_USER_RESPONSE); @@ -226,8 +228,15 @@ private static void givenIdentityService() throws KuraException { when(identityServiceMock.getUser("testuser")) .thenReturn(new UserDTO("testuser", Collections.emptySet(), true, false)); + } + when(identityServiceMock.getUser("nonExistingUser")) + .thenThrow(new KuraException(KuraErrorCode.NOT_FOUND, "Identity does not exist")); + + doThrow(new KuraException(KuraErrorCode.NOT_FOUND, "Identity does not exist")).when(identityServiceMock) + .deleteUser("nonExistingUser"); + when(identityServiceMock.getValidatorOptions()).thenReturn(new ValidatorOptions(8, false, false, false)); } diff --git a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/resources/getNonExistingUserResponse.json b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/resources/getNonExistingUserResponse.json index d50391c239e..32d48779ed7 100644 --- a/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/resources/getNonExistingUserResponse.json +++ b/kura/test/org.eclipse.kura.rest.identity.provider.test/src/main/resources/getNonExistingUserResponse.json @@ -1,3 +1 @@ -{ - "message": "Identity does not exist" -} \ No newline at end of file +{"message" : "Identity does not exist"} \ No newline at end of file