Skip to content

Commit

Permalink
fix(signup): prevent invalid email signup (#9234)
Browse files Browse the repository at this point in the history
  • Loading branch information
RyanHolstien authored Nov 14, 2023
1 parent ec13847 commit 486e394
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 7 deletions.
16 changes: 12 additions & 4 deletions datahub-frontend/app/auth/NativeAuthenticationConfigs.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,26 @@
public class NativeAuthenticationConfigs {

public static final String NATIVE_AUTHENTICATION_ENABLED_CONFIG_PATH = "auth.native.enabled";
public static final String NATIVE_AUTHENTICATION_ENFORCE_VALID_EMAIL_ENABLED_CONFIG_PATH = "auth.native.signUp.enforceValidEmail";

private Boolean _isEnabled = true;
private Boolean _isEnforceValidEmailEnabled = true;

public NativeAuthenticationConfigs(final com.typesafe.config.Config configs) {
if (configs.hasPath(NATIVE_AUTHENTICATION_ENABLED_CONFIG_PATH)
&& Boolean.FALSE.equals(
Boolean.parseBoolean(configs.getValue(NATIVE_AUTHENTICATION_ENABLED_CONFIG_PATH).toString()))) {
_isEnabled = false;
if (configs.hasPath(NATIVE_AUTHENTICATION_ENABLED_CONFIG_PATH)) {
_isEnabled = Boolean.parseBoolean(configs.getValue(NATIVE_AUTHENTICATION_ENABLED_CONFIG_PATH).toString());
}
if (configs.hasPath(NATIVE_AUTHENTICATION_ENFORCE_VALID_EMAIL_ENABLED_CONFIG_PATH)) {
_isEnforceValidEmailEnabled =
Boolean.parseBoolean(configs.getValue(NATIVE_AUTHENTICATION_ENFORCE_VALID_EMAIL_ENABLED_CONFIG_PATH).toString());
}
}

public boolean isNativeAuthenticationEnabled() {
return _isEnabled;
}

public boolean isEnforceValidEmailEnabled() {
return _isEnforceValidEmailEnabled;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -203,6 +204,13 @@ public Result signUp(Http.Request request) {
JsonNode invalidCredsJson = Json.newObject().put("message", "Email must not be empty.");
return Results.badRequest(invalidCredsJson);
}
if (_nativeAuthenticationConfigs.isEnforceValidEmailEnabled()) {
Constraints.EmailValidator emailValidator = new Constraints.EmailValidator();
if (!emailValidator.isValid(email)) {
JsonNode invalidCredsJson = Json.newObject().put("message", "Email must not be empty.");
return Results.badRequest(invalidCredsJson);
}
}

if (StringUtils.isBlank(password)) {
JsonNode invalidCredsJson = Json.newObject().put("message", "Password must not be empty.");
Expand Down
4 changes: 4 additions & 0 deletions datahub-frontend/conf/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,10 @@ auth.oidc.preferredJwsAlgorithm = ${?AUTH_OIDC_PREFERRED_JWS_ALGORITHM} # Which
#
auth.jaas.enabled = ${?AUTH_JAAS_ENABLED}
auth.native.enabled = ${?AUTH_NATIVE_ENABLED}

# Enforces the usage of a valid email for user sign up
auth.native.signUp.enforceValidEmail = true
auth.native.signUp.enforceValidEmail = ${?ENFORCE_VALID_EMAIL}
#
# To disable all authentication to the app, and proxy all users through a master "datahub" account, make sure that,
# jaas, native and oidc auth are disabled:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package com.datahub.authentication.user;

import com.datahub.authentication.Authentication;
import com.datahub.authentication.AuthenticationConfiguration;
import com.linkedin.common.AuditStamp;
import com.linkedin.common.urn.CorpuserUrn;
import com.linkedin.common.urn.Urn;
import com.linkedin.entity.client.EntityClient;
import com.linkedin.events.metadata.ChangeType;
Expand Down Expand Up @@ -34,6 +36,7 @@ public class NativeUserService {
private final EntityService _entityService;
private final EntityClient _entityClient;
private final SecretService _secretService;
private final AuthenticationConfiguration _authConfig;

public void createNativeUser(@Nonnull String userUrnString, @Nonnull String fullName, @Nonnull String email,
@Nonnull String title, @Nonnull String password, @Nonnull Authentication authentication) throws Exception {
Expand All @@ -45,7 +48,12 @@ 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(new CorpuserUrn(_authConfig.getSystemClientId()).toString())
|| 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.datahub.authentication.Actor;
import com.datahub.authentication.ActorType;
import com.datahub.authentication.Authentication;
import com.datahub.authentication.AuthenticationConfiguration;
import com.linkedin.common.urn.CorpuserUrn;
import com.linkedin.common.urn.Urn;
import com.linkedin.entity.client.EntityClient;
Expand Down Expand Up @@ -48,8 +49,10 @@ public void setupTest() throws Exception {
_entityService = mock(EntityService.class);
_entityClient = mock(EntityClient.class);
_secretService = mock(SecretService.class);
AuthenticationConfiguration authenticationConfiguration = new AuthenticationConfiguration();
authenticationConfiguration.setSystemClientId("someCustomId");

_nativeUserService = new NativeUserService(_entityService, _entityClient, _secretService);
_nativeUserService = new NativeUserService(_entityService, _entityClient, _secretService, authenticationConfiguration);
}

@Test
Expand All @@ -74,6 +77,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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -177,6 +180,11 @@ CompletableFuture<ResponseEntity<String>> signUp(final HttpEntity<String> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package com.linkedin.gms.factory.auth;

import com.datahub.authentication.user.NativeUserService;
import com.linkedin.gms.factory.config.ConfigurationProvider;
import com.linkedin.metadata.client.JavaEntityClient;
import com.linkedin.metadata.spring.YamlPropertySourceFactory;
import com.linkedin.metadata.entity.EntityService;
Expand Down Expand Up @@ -31,10 +32,14 @@ public class NativeUserServiceFactory {
@Qualifier("dataHubSecretService")
private SecretService _secretService;

@Autowired
private ConfigurationProvider _configurationProvider;

@Bean(name = "nativeUserService")
@Scope("singleton")
@Nonnull
protected NativeUserService getInstance() throws Exception {
return new NativeUserService(this._entityService, this._javaEntityClient, this._secretService);
return new NativeUserService(_entityService, _javaEntityClient, _secretService,
_configurationProvider.getAuthentication());
}
}

0 comments on commit 486e394

Please sign in to comment.