From c39b86cfe882009dc6d722a3c353632badf8922e Mon Sep 17 00:00:00 2001 From: Ryan Holstien Date: Mon, 13 Nov 2023 15:42:54 -0600 Subject: [PATCH] fix(signup): prevent invalid email signup --- .../app/controllers/AuthenticationController.java | 4 +++- .../datahub/authentication/user/NativeUserService.java | 6 +++++- .../authentication/user/NativeUserServiceTest.java | 10 ++++++++++ .../auth/authentication/AuthServiceController.java | 8 ++++++++ 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/datahub-frontend/app/controllers/AuthenticationController.java b/datahub-frontend/app/controllers/AuthenticationController.java index 4f89f4f67e149..47e893ca70d5e 100644 --- a/datahub-frontend/app/controllers/AuthenticationController.java +++ b/datahub-frontend/app/controllers/AuthenticationController.java @@ -27,6 +27,7 @@ import org.pac4j.play.store.PlaySessionStore; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import play.data.validation.Constraints; import play.libs.Json; import play.mvc.Controller; import play.mvc.Http; @@ -199,7 +200,8 @@ public Result signUp(Http.Request request) { return Results.badRequest(invalidCredsJson); } - if (StringUtils.isBlank(email)) { + Constraints.EmailValidator emailValidator = new Constraints.EmailValidator(); + if (StringUtils.isBlank(email) || !emailValidator.isValid(email)) { JsonNode invalidCredsJson = Json.newObject().put("message", "Email must not be empty."); return Results.badRequest(invalidCredsJson); } diff --git a/metadata-service/auth-impl/src/main/java/com/datahub/authentication/user/NativeUserService.java b/metadata-service/auth-impl/src/main/java/com/datahub/authentication/user/NativeUserService.java index 7f0c16d28f121..2e253796fc967 100644 --- a/metadata-service/auth-impl/src/main/java/com/datahub/authentication/user/NativeUserService.java +++ b/metadata-service/auth-impl/src/main/java/com/datahub/authentication/user/NativeUserService.java @@ -45,7 +45,11 @@ public void createNativeUser(@Nonnull String userUrnString, @Nonnull String full Objects.requireNonNull(authentication, "authentication must not be null!"); final Urn userUrn = Urn.createFromString(userUrnString); - if (_entityService.exists(userUrn) || userUrn.toString().equals(SYSTEM_ACTOR)) { + if (_entityService.exists(userUrn) + // Should never fail these due to Controller level check, but just in case more usages get put in + || userUrn.toString().equals(SYSTEM_ACTOR) + || userUrn.toString().equals(DATAHUB_ACTOR) + || userUrn.toString().equals(UNKNOWN_ACTOR)) { throw new RuntimeException("This user already exists! Cannot create a new user."); } updateCorpUserInfo(userUrn, fullName, email, title, authentication); diff --git a/metadata-service/auth-impl/src/test/java/com/datahub/authentication/user/NativeUserServiceTest.java b/metadata-service/auth-impl/src/test/java/com/datahub/authentication/user/NativeUserServiceTest.java index b0b10af82155a..cb1246a105645 100644 --- a/metadata-service/auth-impl/src/test/java/com/datahub/authentication/user/NativeUserServiceTest.java +++ b/metadata-service/auth-impl/src/test/java/com/datahub/authentication/user/NativeUserServiceTest.java @@ -74,6 +74,16 @@ public void testCreateNativeUserUserAlreadyExists() throws Exception { _nativeUserService.createNativeUser(USER_URN_STRING, FULL_NAME, EMAIL, TITLE, PASSWORD, SYSTEM_AUTHENTICATION); } + @Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "This user already exists! Cannot create a new user.") + public void testCreateNativeUserUserDatahub() throws Exception { + _nativeUserService.createNativeUser(DATAHUB_ACTOR, FULL_NAME, EMAIL, TITLE, PASSWORD, SYSTEM_AUTHENTICATION); + } + + @Test(expectedExceptions = RuntimeException.class, expectedExceptionsMessageRegExp = "This user already exists! Cannot create a new user.") + public void testCreateNativeUserUserSystemUser() throws Exception { + _nativeUserService.createNativeUser(SYSTEM_ACTOR, FULL_NAME, EMAIL, TITLE, PASSWORD, SYSTEM_AUTHENTICATION); + } + @Test public void testCreateNativeUserPasses() throws Exception { when(_entityService.exists(any())).thenReturn(false); diff --git a/metadata-service/auth-servlet-impl/src/main/java/com/datahub/auth/authentication/AuthServiceController.java b/metadata-service/auth-servlet-impl/src/main/java/com/datahub/auth/authentication/AuthServiceController.java index b5ce99902108a..34354a47b7f04 100644 --- a/metadata-service/auth-servlet-impl/src/main/java/com/datahub/auth/authentication/AuthServiceController.java +++ b/metadata-service/auth-servlet-impl/src/main/java/com/datahub/auth/authentication/AuthServiceController.java @@ -12,6 +12,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.linkedin.common.urn.CorpuserUrn; import com.linkedin.common.urn.Urn; import com.linkedin.gms.factory.config.ConfigurationProvider; import java.util.concurrent.CompletableFuture; @@ -28,6 +29,8 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.web.client.HttpClientErrorException; +import static com.linkedin.metadata.Constants.*; + @Slf4j @RestController @@ -177,6 +180,11 @@ CompletableFuture> signUp(final HttpEntity httpEn } String userUrnString = userUrn.asText(); + String systemClientUser = new CorpuserUrn(_configProvider.getAuthentication().getSystemClientId()).toString(); + + if (userUrnString.equals(systemClientUser) || userUrnString.equals(DATAHUB_ACTOR) || userUrnString.equals(UNKNOWN_ACTOR)) { + return CompletableFuture.completedFuture(new ResponseEntity<>(HttpStatus.BAD_REQUEST)); + } String fullNameString = fullName.asText(); String emailString = email.asText(); String titleString = title.asText();