[ISSUE-16503] Fix createUser to use EntityResource (#16549)

* Fix createUser to use EntityResource

* fix broken tests

* Fix Tests - 3
This commit is contained in:
Mohit Yadav 2024-06-07 21:03:50 +05:30 committed by GitHub
parent 38e2793705
commit aeb020ae3b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 103 additions and 37 deletions

View File

@ -182,6 +182,12 @@ public final class CatalogExceptionMessage {
return String.format("Principal: CatalogPrincipal{name='%s'} is not admin", name);
}
public static String operationNotAllowed(String name, MetadataOperation operation) {
return String.format(
"Principal: CatalogPrincipal{name='%s'} operations [%s] not allowed",
name, operation.value());
}
public static String notReviewer(String name) {
return String.format("User '%s' is not a reviewer", name);
}

View File

@ -548,18 +548,13 @@ public class UserResource extends EntityResource<User, UserRepository> {
@Context ContainerRequestContext containerRequestContext,
@Valid CreateUser create) {
User user = getUser(securityContext.getUserPrincipal().getName(), create);
if (Boolean.TRUE.equals(create.getIsAdmin())) {
authorizer.authorizeAdmin(securityContext);
}
if (Boolean.TRUE.equals(create.getIsBot())) {
addAuthMechanismToBot(user, create, uriInfo);
}
if (isBasicAuth()) {
//
try {
// basic auth doesn't allow duplicate emails, since username part of the email is used as
// login name
validateEmailAlreadyExists(create.getEmail());
validateAndAddUserAuthForBasic(user, create);
} catch (RuntimeException ex) {
return Response.status(CONFLICT)
.type(MediaType.APPLICATION_JSON_TYPE)
@ -568,6 +563,26 @@ public class UserResource extends EntityResource<User, UserRepository> {
CONFLICT.getStatusCode(), CatalogExceptionMessage.ENTITY_ALREADY_EXISTS))
.build();
}
// Add the roles on user creation
updateUserRolesIfRequired(user, containerRequestContext);
// TODO do we need to authenticate user is creating himself?
Response createdUser = create(uriInfo, securityContext, user);
// Send Invite mail to user
sendInviteMailToUserForBasicAuth(uriInfo, user, create);
// Update response to remove auth fields
decryptOrNullify(securityContext, (User) createdUser.getEntity());
return createdUser;
}
private void validateAndAddUserAuthForBasic(User user, CreateUser create) {
if (isBasicAuth()) {
// basic auth doesn't allow duplicate emails, since username part of the email is used as
// login name
validateEmailAlreadyExists(create.getEmail());
user.setName(user.getEmail().split("@")[0]);
if (Boolean.FALSE.equals(create.getIsBot())
&& create.getCreatePasswordType() == ADMIN_CREATE) {
@ -575,17 +590,18 @@ public class UserResource extends EntityResource<User, UserRepository> {
}
// else the user will get a mail if configured smtp
}
}
// Add the roles on user creation
private void updateUserRolesIfRequired(
User user, ContainerRequestContext containerRequestContext) {
if (Boolean.TRUE.equals(authorizerConfiguration.getUseRolesFromProvider())
&& Boolean.FALSE.equals(user.getIsBot() != null && user.getIsBot())) {
user.setRoles(
validateAndGetRolesRef(getRolesFromAuthorizationToken(containerRequestContext)));
}
}
// TODO do we need to authenticate user is creating himself?
addHref(uriInfo, repository.create(uriInfo, user));
private void sendInviteMailToUserForBasicAuth(UriInfo uriInfo, User user, CreateUser create) {
if (isBasicAuth() && isEmailServiceEnabled) {
try {
authHandler.sendInviteMailToUser(
@ -598,9 +614,6 @@ public class UserResource extends EntityResource<User, UserRepository> {
LOG.error("Error in sending invite to User" + ex.getMessage());
}
}
Response response = Response.created(user.getHref()).entity(user).build();
decryptOrNullify(securityContext, (User) response.getEntity());
return response;
}
private boolean isBasicAuth() {

View File

@ -258,6 +258,7 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
public static Domain DOMAIN1;
// Users
public static User USER_WITH_CREATE_ACCESS;
public static User USER1;
public static EntityReference USER1_REF;
public static User USER2;
@ -272,11 +273,9 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
public static Team
TEAM2; // Team 2 has team only policy and does not allow access to users not in team hierarchy
public static Team TEAM21; // Team under Team2
public static User DATA_STEWARD;
public static Persona DATA_ENGINEER;
public static Persona DATA_SCIENTIST;
public static Document ACTIVITY_FEED_KNOWLEDGE_PANEL;
public static Document MY_DATA_KNOWLEDGE_PANEL;
public static User USER_WITH_DATA_STEWARD_ROLE;
@ -286,9 +285,10 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
public static EntityReference DATA_CONSUMER_REF;
public static Role DATA_CONSUMER_ROLE;
public static EntityReference DATA_CONSUMER_ROLE_REF;
public static Role CREATE_ACCESS_ROLE;
public static Role ROLE1;
public static EntityReference ROLE1_REF;
public static Policy CREATE_ACCESS_PERMISSION_POLICY;
public static Policy POLICY1;
public static Policy POLICY2;
public static Policy TEAM_ONLY_POLICY;
@ -964,7 +964,8 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
if (supportsFollowers) {
UserResourceTest userResourceTest = new UserResourceTest();
User user1 =
userResourceTest.createEntity(userResourceTest.createRequest(test, 1), TEST_AUTH_HEADERS);
userResourceTest.createEntity(
userResourceTest.createRequest(test, 1), USER_WITH_CREATE_HEADERS);
addFollower(entity.getId(), user1.getId(), OK, TEST_AUTH_HEADERS);
}
entity = validateGetWithDifferentFields(entity, false);
@ -1618,7 +1619,8 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
// Add follower to the entity
UserResourceTest userResourceTest = new UserResourceTest();
User user1 =
userResourceTest.createEntity(userResourceTest.createRequest(test, 1), TEST_AUTH_HEADERS);
userResourceTest.createEntity(
userResourceTest.createRequest(test, 1), USER_WITH_CREATE_HEADERS);
addAndCheckFollower(entityId, user1.getId(), OK, 1, TEST_AUTH_HEADERS);
// Add the same user as follower and make sure no errors are thrown
@ -1627,7 +1629,8 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
// Add a new follower to the entity
User user2 =
userResourceTest.createEntity(userResourceTest.createRequest(test, 2), TEST_AUTH_HEADERS);
userResourceTest.createEntity(
userResourceTest.createRequest(test, 2), USER_WITH_CREATE_HEADERS);
addAndCheckFollower(entityId, user2.getId(), OK, 2, TEST_AUTH_HEADERS);
// Delete followers and make sure they are deleted
@ -1648,7 +1651,8 @@ public abstract class EntityResourceTest<T extends EntityInterface, K extends Cr
// Add follower to the entity
UserResourceTest userResourceTest = new UserResourceTest();
User user1 =
userResourceTest.createEntity(userResourceTest.createRequest(test, 1), TEST_AUTH_HEADERS);
userResourceTest.createEntity(
userResourceTest.createRequest(test, 1), USER_WITH_CREATE_HEADERS);
addAndCheckFollower(entityId, user1.getId(), OK, 1, TEST_AUTH_HEADERS);
deleteEntity(entityId, ADMIN_AUTH_HEADERS);

View File

@ -104,6 +104,8 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
}
public void setupPolicies() throws IOException {
CREATE_ACCESS_PERMISSION_POLICY =
createEntity(createAccessControlPolicyWithCreateRule(), ADMIN_AUTH_HEADERS);
POLICY1 = createEntity(createRequest("policy1").withOwner(null), ADMIN_AUTH_HEADERS);
POLICY2 = createEntity(createRequest("policy2").withOwner(null), ADMIN_AUTH_HEADERS);
TEAM_ONLY_POLICY = getEntityByName("TeamOnlyPolicy", "", ADMIN_AUTH_HEADERS);
@ -769,6 +771,19 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
.withOwner(USER1_REF);
}
private CreatePolicy createAccessControlPolicyWithCreateRule() {
return new CreatePolicy()
.withName("CreatePermissionPolicy")
.withDescription("Create User Permission")
.withRules(
List.of(
new Rule()
.withName("CreatePermission")
.withResources(List.of(ALL_RESOURCES))
.withOperations(List.of(MetadataOperation.CREATE))
.withEffect(ALLOW)));
}
private void validateCondition(String expression) throws HttpResponseException {
WebTarget target = getResource(collectionName + "/validation/condition/" + expression);
TestUtils.get(target, ADMIN_AUTH_HEADERS);

View File

@ -85,9 +85,11 @@ public class PersonaResourceTest extends EntityResourceTest<Persona, CreatePerso
// Add team to user relationships while creating a team
UserResourceTest userResourceTest = new UserResourceTest();
User user1 =
userResourceTest.createEntity(userResourceTest.createRequest(test, 1), TEST_AUTH_HEADERS);
userResourceTest.createEntity(
userResourceTest.createRequest(test, 1), USER_WITH_CREATE_HEADERS);
User user2 =
userResourceTest.createEntity(userResourceTest.createRequest(test, 2), TEST_AUTH_HEADERS);
userResourceTest.createEntity(
userResourceTest.createRequest(test, 2), USER_WITH_CREATE_HEADERS);
List<UUID> users = Arrays.asList(user1.getId(), user2.getId());
CreatePersona create =

View File

@ -74,6 +74,13 @@ public class RoleResourceTest extends EntityResourceTest<Role, CreateRole> {
ROLE1 = createEntity(createRequest(test), ADMIN_AUTH_HEADERS);
ROLE1_REF = ROLE1.getEntityReference();
CREATE_ACCESS_ROLE =
createEntity(
new CreateRole()
.withName("CreateAccessRole")
.withPolicies(List.of(CREATE_ACCESS_PERMISSION_POLICY.getFullyQualifiedName())),
ADMIN_AUTH_HEADERS);
}
/** Creates the given number of roles */

View File

@ -50,6 +50,7 @@ import static org.openmetadata.service.util.EntityUtil.fieldUpdated;
import static org.openmetadata.service.util.TestUtils.ADMIN_AUTH_HEADERS;
import static org.openmetadata.service.util.TestUtils.TEST_AUTH_HEADERS;
import static org.openmetadata.service.util.TestUtils.TEST_USER_NAME;
import static org.openmetadata.service.util.TestUtils.USER_WITH_CREATE_HEADERS;
import static org.openmetadata.service.util.TestUtils.UpdateType.CHANGE_CONSOLIDATED;
import static org.openmetadata.service.util.TestUtils.UpdateType.MINOR_UPDATE;
import static org.openmetadata.service.util.TestUtils.UpdateType.NO_CHANGE;
@ -195,9 +196,11 @@ public class TeamResourceTest extends EntityResourceTest<Team, CreateTeam> {
// Add team to user relationships while creating a team
UserResourceTest userResourceTest = new UserResourceTest();
User user1 =
userResourceTest.createEntity(userResourceTest.createRequest(test, 1), TEST_AUTH_HEADERS);
userResourceTest.createEntity(
userResourceTest.createRequest(test, 1), USER_WITH_CREATE_HEADERS);
User user2 =
userResourceTest.createEntity(userResourceTest.createRequest(test, 2), TEST_AUTH_HEADERS);
userResourceTest.createEntity(
userResourceTest.createRequest(test, 2), USER_WITH_CREATE_HEADERS);
List<UUID> users = Arrays.asList(user1.getId(), user2.getId());
RoleResourceTest roleResourceTest = new RoleResourceTest();

View File

@ -41,6 +41,7 @@ import static org.openmetadata.service.Entity.USER;
import static org.openmetadata.service.exception.CatalogExceptionMessage.PASSWORD_INVALID_FORMAT;
import static org.openmetadata.service.exception.CatalogExceptionMessage.entityNotFound;
import static org.openmetadata.service.exception.CatalogExceptionMessage.notAdmin;
import static org.openmetadata.service.exception.CatalogExceptionMessage.operationNotAllowed;
import static org.openmetadata.service.exception.CatalogExceptionMessage.permissionNotAllowed;
import static org.openmetadata.service.resources.teams.UserResource.USER_PROTECTED_FIELDS;
import static org.openmetadata.service.security.SecurityUtil.authHeaders;
@ -51,6 +52,8 @@ import static org.openmetadata.service.util.TestUtils.ADMIN_AUTH_HEADERS;
import static org.openmetadata.service.util.TestUtils.INGESTION_BOT;
import static org.openmetadata.service.util.TestUtils.TEST_AUTH_HEADERS;
import static org.openmetadata.service.util.TestUtils.TEST_USER_NAME;
import static org.openmetadata.service.util.TestUtils.USER_WITH_CREATE_HEADERS;
import static org.openmetadata.service.util.TestUtils.USER_WITH_CREATE_PERMISSION_NAME;
import static org.openmetadata.service.util.TestUtils.UpdateType.CHANGE_CONSOLIDATED;
import static org.openmetadata.service.util.TestUtils.UpdateType.MINOR_UPDATE;
import static org.openmetadata.service.util.TestUtils.UpdateType.REVERT;
@ -153,6 +156,14 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
}
public void setupUsers(TestInfo test) throws HttpResponseException {
CreateUser createUserWithAccess =
new CreateUser()
.withName(USER_WITH_CREATE_PERMISSION_NAME)
.withEmail(USER_WITH_CREATE_PERMISSION_NAME + "@open-metadata.org")
.withProfile(PROFILE)
.withRoles(List.of(CREATE_ACCESS_ROLE.getId()))
.withIsBot(false);
USER_WITH_CREATE_ACCESS = createEntity(createUserWithAccess, ADMIN_AUTH_HEADERS);
CreateUser create = createRequest(test).withRoles(List.of(DATA_CONSUMER_ROLE.getId()));
USER1 = createEntity(create, ADMIN_AUTH_HEADERS);
USER1_REF = USER1.getEntityReference();
@ -317,7 +328,9 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
.withIsAdmin(true);
assertResponse(
() -> createAndCheckEntity(create, TEST_AUTH_HEADERS), FORBIDDEN, notAdmin(TEST_USER_NAME));
() -> createAndCheckEntity(create, TEST_AUTH_HEADERS),
FORBIDDEN,
operationNotAllowed(TEST_USER_NAME, MetadataOperation.CREATE));
}
@Test
@ -613,7 +626,7 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
User user =
createEntity(
createRequest(test, 6).withName("test2").withEmail("test2@email.com"),
authHeaders("test2@email.com"));
USER_WITH_CREATE_HEADERS);
String userJson = JsonUtils.pojoToJson(user);
user.setIsAdmin(Boolean.TRUE);
assertResponse(
@ -871,7 +884,7 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
.withEmail("ingestion-bot-jwt@email.com")
.withRoles(List.of(ROLE1_REF.getId()))
.withAuthenticationMechanism(authMechanism);
User user = createEntity(create, authHeaders("ingestion-bot-jwt@email.com"));
User user = createEntity(create, USER_WITH_CREATE_HEADERS);
user = getEntity(user.getId(), "*", ADMIN_AUTH_HEADERS);
assertEquals(1, user.getRoles().size());
TestUtils.put(
@ -922,7 +935,7 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
.withCreatePasswordType(CreateUser.CreatePasswordType.ADMIN_CREATE)
.withPassword("Test@1234")
.withConfirmPassword("Test@1234"),
authHeaders("testBasicAuth@email.com"));
USER_WITH_CREATE_HEADERS);
// jwtAuth Response should be null always
user = getEntity(user.getId(), ADMIN_AUTH_HEADERS);

View File

@ -104,6 +104,9 @@ public final class TestUtils {
public static final String TEST_USER_NAME = "test";
public static final Map<String, String> TEST_AUTH_HEADERS =
authHeaders(TEST_USER_NAME + "@open-metadata.org");
public static final String USER_WITH_CREATE_PERMISSION_NAME = "testWithCreateUserPermission";
public static final Map<String, String> USER_WITH_CREATE_HEADERS =
authHeaders(USER_WITH_CREATE_PERMISSION_NAME + "@open-metadata.org");
public static final UUID NON_EXISTENT_ENTITY = UUID.randomUUID();