- LowerCase the UserNames and Email in user_entity (#12942)

* - LowerCase the UserNames and Email in user_entity
-- Added Migrations for the same

* revert name

* Fix Test

* Fix Failing User tests

* Move LowerCase to DAO

* Remove Unnecessary toLowerCase

* Reverted changes

* Fix Failing Test

* remove dao level shouldLowerCaseFQN

* Remove unnecesarry changes
This commit is contained in:
Mohit Yadav 2023-08-23 13:03:48 +05:30 committed by GitHub
parent 9d2bc14e5d
commit 6988334b45
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 97 additions and 22 deletions

View File

@ -104,6 +104,7 @@ import org.openmetadata.schema.settings.SettingsType;
import org.openmetadata.schema.tests.TestCase;
import org.openmetadata.schema.tests.TestDefinition;
import org.openmetadata.schema.tests.TestSuite;
import org.openmetadata.schema.type.Include;
import org.openmetadata.schema.type.Relationship;
import org.openmetadata.schema.type.TagLabel;
import org.openmetadata.schema.type.TaskStatus;
@ -2797,12 +2798,16 @@ public interface CollectionDAO {
@Bind("after") String after,
@Bind("relation") int relation);
@ConnectionAwareSqlQuery(value = "SELECT count(*) FROM user_entity WHERE email = :email", connectionType = MYSQL)
@ConnectionAwareSqlQuery(value = "SELECT count(*) FROM user_entity WHERE email = :email", connectionType = POSTGRES)
@SqlQuery("SELECT COUNT(*) FROM user_entity WHERE LOWER(email) = LOWER(:email)")
int checkEmailExists(@Bind("email") String email);
@SqlQuery("SELECT json FROM user_entity WHERE email = :email")
@SqlQuery("SELECT json FROM user_entity WHERE LOWER(email) = LOWER(:email)")
String findUserByEmail(@Bind("email") String email);
@Override
default User findEntityByName(String fqn, Include include) {
return EntityDAO.super.findEntityByName(fqn.toLowerCase(), include);
}
}
interface ChangeEventDAO {

View File

@ -18,6 +18,7 @@ import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty;
import static org.openmetadata.csv.CsvUtil.addEntityReferences;
import static org.openmetadata.csv.CsvUtil.addField;
import static org.openmetadata.schema.type.Include.ALL;
import static org.openmetadata.schema.utils.EntityInterfaceUtil.quoteName;
import static org.openmetadata.service.Entity.FIELD_DOMAIN;
import static org.openmetadata.service.Entity.ROLE;
import static org.openmetadata.service.Entity.TEAM;
@ -78,6 +79,13 @@ public class UserRepository extends EntityRepository<User> {
this.quoteFqn = true;
}
// with the introduction of fqnhash we added case sensitivity to all of the entities
// however usernames , emails cannot be case sensitive
@Override
public void setFullyQualifiedName(User user) {
user.setFullyQualifiedName(quoteName(user.getName().toLowerCase()));
}
public final Fields getFieldsWithUserAuth(String fields) {
Set<String> tempFields = getAllowedFieldsCopy();
if (fields != null && fields.equals("*")) {

View File

@ -1,5 +1,6 @@
package org.openmetadata.service.migration.mysql.v112;
import static org.openmetadata.service.migration.postgres.v112.Migration.lowerCaseUserNameAndEmail;
import static org.openmetadata.service.migration.postgres.v112.Migration.unquoteTestSuiteMigration;
import lombok.SneakyThrows;
@ -28,5 +29,8 @@ public class Migration extends MigrationProcessImpl {
public void runDataMigration() {
// Run Data Migration to Remove the quoted Fqn`
unquoteTestSuiteMigration(collectionDAO);
// Run UserName Migration to make lowercase
lowerCaseUserNameAndEmail(collectionDAO);
}
}

View File

@ -5,6 +5,7 @@ import java.util.Set;
import lombok.SneakyThrows;
import lombok.extern.slf4j.Slf4j;
import org.jdbi.v3.core.Handle;
import org.openmetadata.schema.entity.teams.User;
import org.openmetadata.schema.tests.TestSuite;
import org.openmetadata.schema.type.Include;
import org.openmetadata.service.jdbi3.CollectionDAO;
@ -13,6 +14,7 @@ import org.openmetadata.service.jdbi3.TestSuiteRepository;
import org.openmetadata.service.migration.api.MigrationProcessImpl;
import org.openmetadata.service.migration.utils.MigrationFile;
import org.openmetadata.service.util.EntityUtil;
import org.openmetadata.service.util.JsonUtils;
@Slf4j
public class Migration extends MigrationProcessImpl {
@ -35,6 +37,9 @@ public class Migration extends MigrationProcessImpl {
public void runDataMigration() {
// Run Data Migration to Remove the quoted Fqn`
unquoteTestSuiteMigration(collectionDAO);
// Run UserName Migration to make lowercase
lowerCaseUserNameAndEmail(collectionDAO);
}
public static void unquoteTestSuiteMigration(CollectionDAO collectionDAO) {
@ -55,4 +60,21 @@ public class Migration extends MigrationProcessImpl {
}
}
}
public static void lowerCaseUserNameAndEmail(CollectionDAO daoCollection) {
LOG.debug("Starting Migration UserName and Email to Lowercase");
int total = daoCollection.userDAO().listTotalCount();
int offset = 0;
int limit = 200;
while (offset < total) {
List<String> userEntities = daoCollection.userDAO().listAfterWithOffset(limit, offset);
for (String json : userEntities) {
User userEntity = JsonUtils.readValue(json, User.class);
userEntity.setFullyQualifiedName(userEntity.getFullyQualifiedName().toLowerCase());
daoCollection.userDAO().update(userEntity);
}
offset = offset + limit;
}
LOG.debug("Completed Migrating UserName and Email to Lowercase");
}
}

View File

@ -948,8 +948,8 @@ public class UserResource extends EntityResource<User, UserRepository> {
@Path("/checkEmailInUse")
@Operation(
operationId = "checkEmailInUse",
summary = "Check if a mail is already in use",
description = "Check if a mail is already in use",
summary = "Check if a email is already in use",
description = "Check if a email is already in use",
responses = {
@ApiResponse(
responseCode = "200",

View File

@ -120,7 +120,7 @@ public class JwtFilter implements ContainerRequestFilter {
LOG.debug("Token from header:{}", tokenFromHeader);
// the case where OMD generated the Token for the Client
if (AuthProvider.BASIC.equals(providerType) || AuthProvider.SAML.toString().equals(providerType)) {
if (AuthProvider.BASIC.equals(providerType) || AuthProvider.SAML.equals(providerType)) {
validateTokenIsNotUsedAfterLogout(tokenFromHeader);
}

View File

@ -6,6 +6,7 @@ import com.google.common.cache.LoadingCache;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import lombok.NonNull;
import org.jetbrains.annotations.NotNull;
import org.openmetadata.schema.api.configuration.LoginConfiguration;
import org.openmetadata.service.OpenMetadataApplicationConfig;
@ -26,7 +27,7 @@ public class LoginAttemptCache {
.expireAfterWrite(accessBlockTime, TimeUnit.SECONDS)
.build(
new CacheLoader<>() {
public Integer load(@NonNull String key) {
public @NotNull Integer load(@NonNull String username) {
return 0;
}
});
@ -40,38 +41,38 @@ public class LoginAttemptCache {
.expireAfterWrite(blockTimeInSec, TimeUnit.SECONDS)
.build(
new CacheLoader<>() {
public Integer load(@NonNull String key) {
public @NotNull Integer load(@NonNull String username) {
return 0;
}
});
}
public void recordSuccessfulLogin(String key) {
attemptsCache.invalidate(key);
public void recordSuccessfulLogin(String username) {
attemptsCache.invalidate(username.toLowerCase());
}
public void recordFailedLogin(String key) {
public void recordFailedLogin(String username) {
int attempts;
try {
attempts = attemptsCache.get(key);
attempts = attemptsCache.get(username.toLowerCase());
} catch (ExecutionException e) {
attempts = 0;
}
attempts++;
attemptsCache.put(key, attempts);
attemptsCache.put(username, attempts);
}
public boolean isLoginBlocked(String key) {
public boolean isLoginBlocked(String username) {
try {
return attemptsCache.get(key) >= maxAttempt;
return attemptsCache.get(username.toLowerCase()) >= maxAttempt;
} catch (ExecutionException e) {
return false;
}
}
public int getUserFailedLoginCount(String key) {
public int getUserFailedLoginCount(String username) {
try {
return attemptsCache.get(key);
return attemptsCache.get(username.toLowerCase());
} catch (ExecutionException e) {
return -1;
}

View File

@ -11,6 +11,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import org.apache.http.client.HttpResponseException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
@ -25,6 +26,7 @@ import org.openmetadata.service.resources.EntityResourceTest;
import org.openmetadata.service.resources.bots.BotResource.BotList;
import org.openmetadata.service.resources.teams.UserResourceTest;
import org.openmetadata.service.util.ResultList;
import org.openmetadata.service.util.TestUtils;
public class BotResourceTest extends EntityResourceTest<Bot, CreateBot> {
public static User botUser;
@ -113,7 +115,14 @@ public class BotResourceTest extends EntityResourceTest<Bot, CreateBot> {
@Override
public void validateCreatedEntity(Bot entity, CreateBot request, Map<String, String> authHeaders) {
assertReference(request.getBotUser(), entity.getBotUser());
if (request.getBotUser() != null) {
assertNotNull(entity.getBotUser());
TestUtils.validateEntityReference(entity.getBotUser());
Assertions.assertEquals(
request.getBotUser().toLowerCase(), entity.getBotUser().getFullyQualifiedName().toLowerCase());
} else {
Assertions.assertNull(entity.getBotUser());
}
}
@Override

View File

@ -818,10 +818,11 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
@Test
void post_createUser_BasicAuth_AdminCreate_login_200_ok(TestInfo test) throws HttpResponseException {
// Create a user with Auth and Try Logging in
String name = "testBasicAuth";
User user =
createEntity(
createRequest(test)
.withName("testBasicAuth")
.withName(name)
.withDisplayName("Test")
.withEmail("testBasicAuth@email.com")
.withIsBot(false)
@ -833,6 +834,8 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
// jwtAuth Response should be null always
user = getEntity(user.getId(), ADMIN_AUTH_HEADERS);
assertNull(user.getAuthenticationMechanism());
assertEquals(name, user.getName());
assertEquals(name.toLowerCase(), user.getFullyQualifiedName());
// Login With Correct Password
LoginRequest loginRequest =
@ -875,11 +878,12 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
@Test
void post_createUser_BasicAuth_SignUp_200_ok() throws HttpResponseException {
// Create a user with Auth and Try Logging in
String name = "testBasicAuth123";
RegistrationRequest newRegistrationRequest =
new RegistrationRequest()
.withFirstName("Test")
.withLastName("Test")
.withEmail("testBasicAuth123@email.com")
.withEmail(String.format("%s@email.com", name))
.withPassword("Test@1234");
TestUtils.post(getResource("users/signup"), newRegistrationRequest, String.class, ADMIN_AUTH_HEADERS);
@ -887,6 +891,8 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
// jwtAuth Response should be null always
User user = getEntityByName("testBasicAuth123", null, ADMIN_AUTH_HEADERS);
assertNull(user.getAuthenticationMechanism());
assertEquals(name, user.getName());
assertEquals(name.toLowerCase(), user.getFullyQualifiedName());
// Login With Correct Password
LoginRequest loginRequest =
@ -1043,6 +1049,25 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
assertEquals(false, jwt.getClaims().get("isBot").asBoolean());
}
@Test
void test_userNameIgnoreCase(TestInfo test) throws IOException {
// Create user with different optional fields
CreateUser create = createRequest(test, 1).withName("UserEmailTest").withEmail("UserEmailTest@domainx.com");
User created = createEntity(create, ADMIN_AUTH_HEADERS);
// Creating another user with different case should fail
create.withName("Useremailtest").withEmail("Useremailtest@Domainx.com");
assertResponse(() -> createEntity(create, ADMIN_AUTH_HEADERS), CONFLICT, "Entity already exists");
// get user with username in different case
User user = getEntityByName("UsERemailTEST", ADMIN_AUTH_HEADERS);
compareEntities(user, created, ADMIN_AUTH_HEADERS);
user.setName("UsERemailTEST");
user.setFullyQualifiedName("UsERemailTEST");
// delete user with different
deleteByNameAndCheckEntity(user, false, false, ADMIN_AUTH_HEADERS);
}
@Test
void testInheritedRole() throws HttpResponseException {
// USER1 inherits DATA_CONSUMER_ROLE from Organization
@ -1168,9 +1193,10 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
@Override
public CreateUser createRequest(String name) {
// user part of the email should be less than 64 in length
String emailUser = nullOrEmpty(name) ? UUID.randomUUID().toString() : name;
String entityName = name != null ? name.toLowerCase() : null;
String emailUser = nullOrEmpty(entityName) ? UUID.randomUUID().toString().toLowerCase() : entityName;
emailUser = emailUser.length() > 64 ? emailUser.substring(0, 64) : emailUser;
return new CreateUser().withName(name).withEmail(emailUser + "@open-metadata.org").withProfile(PROFILE);
return new CreateUser().withName(entityName).withEmail(emailUser + "@open-metadata.org").withProfile(PROFILE);
}
@Override