Fix #4356: Backend: User Patch API fails for roles and teams update (#4396)

This commit is contained in:
Vivek Ratnavel Subramanian 2022-04-23 08:01:04 -07:00 committed by GitHub
parent c51711d292
commit 6cd30ce945
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 130 additions and 11 deletions

View File

@ -61,6 +61,10 @@ public final class CatalogExceptionMessage {
return String.format("User %s is deactivated", id);
}
public static String userAlreadyPartOfTeam(String userName, String teamName) {
return String.format("User '%s' is already part of the team '%s'", userName, teamName);
}
public static String invalidColumnFQN(String fqn) {
return String.format("Invalid fully qualified column name %s", fqn);
}

View File

@ -87,8 +87,7 @@ public class DashboardRepository extends EntityRepository<Dashboard> {
.withId(original.getId())
.withFullyQualifiedName(original.getFullyQualifiedName())
.withName(original.getName())
.withService(original.getService())
.withId(original.getId());
.withService(original.getService());
}
private EntityReference getService(Dashboard dashboard) throws IOException {

View File

@ -21,6 +21,7 @@ import java.net.URI;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
@ -29,6 +30,7 @@ import org.jdbi.v3.sqlobject.transaction.Transaction;
import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.entity.teams.Team;
import org.openmetadata.catalog.entity.teams.User;
import org.openmetadata.catalog.exception.CatalogExceptionMessage;
import org.openmetadata.catalog.resources.teams.UserResource;
import org.openmetadata.catalog.type.ChangeDescription;
import org.openmetadata.catalog.type.EntityReference;
@ -83,6 +85,12 @@ public class UserRepository extends EntityRepository<User> {
user.setRoles(rolesRef);
}
@Override
public void restorePatchAttributes(User original, User updated) {
// Patch can't make changes to following fields. Ignore the changes
updated.withId(original.getId()).withName(original.getName());
}
private List<EntityReference> getTeamDefaultRoles(User user) throws IOException {
List<EntityReference> teamsRef = listOrEmpty(user.getTeams());
List<EntityReference> defaultRoles = new ArrayList<>();
@ -137,6 +145,22 @@ public class UserRepository extends EntityRepository<User> {
return user;
}
public boolean isTeamJoinable(String teamId) throws IOException {
Team team = daoCollection.teamDAO().findEntityById(UUID.fromString(teamId), Include.NON_DELETED);
return team.getIsJoinable();
}
/* Validate if the user is already part of the given team */
public void validateTeamAddition(String userId, String teamId) throws IOException {
User user = dao.findEntityById(UUID.fromString(userId));
List<EntityReference> teams = getTeams(user);
Optional<EntityReference> team = teams.stream().filter(t -> t.getId().equals(UUID.fromString(teamId))).findFirst();
if (team.isPresent()) {
throw new IllegalArgumentException(
CatalogExceptionMessage.userAlreadyPartOfTeam(user.getName(), team.get().getDisplayName()));
}
}
private List<EntityReference> getOwns(User user) throws IOException {
// Compile entities owned by the user
List<EntityReference> ownedEntities =
@ -287,6 +311,11 @@ public class UserRepository extends EntityRepository<User> {
entity.setName(name);
}
@Override
public EntityReference getOwner() {
return Entity.getEntityReference(entity);
}
@Override
public void setUpdateDetails(String updatedBy, long updatedAt) {
entity.setUpdatedBy(updatedBy);

View File

@ -372,6 +372,25 @@ public class UserResource extends EntityResource<User, UserRepository> {
if (path.equals("/isAdmin") || path.equals("/isBot")) {
SecurityUtil.authorizeAdmin(authorizer, securityContext, ADMIN | BOT);
}
// if path contains team, check if team is joinable by any user
if (patchOpObject.containsKey("op")
&& patchOpObject.getString("op").equals("add")
&& path.startsWith("/teams/")) {
JsonObject value = null;
try {
value = patchOpObject.getJsonObject("value");
} catch (Exception ex) {
// ignore exception if value is not an object
}
if (value != null) {
String teamId = value.getString("id");
dao.validateTeamAddition(id, teamId);
if (!dao.isTeamJoinable(teamId)) {
// Only admin can join closed teams
SecurityUtil.authorizeAdmin(authorizer, securityContext, ADMIN);
}
}
}
}
}
return patchInternal(uriInfo, securityContext, id, patch);

View File

@ -1015,7 +1015,10 @@ public abstract class EntityResourceTest<T, K> extends CatalogApplicationTest {
// Create entity without description, owner
T entity = createEntity(createRequest(getEntityName(test), "", null, null), ADMIN_AUTH_HEADERS);
EntityInterface<T> entityInterface = getEntityInterface(entity);
assertListNull(entityInterface.getOwner());
// user will always have the same user assigned as the owner
if (!entityInterface.getEntityType().equals(Entity.USER)) {
assertListNull(entityInterface.getOwner());
}
entity = getEntity(entityInterface.getId(), ADMIN_AUTH_HEADERS);
entityInterface = getEntityInterface(entity);

View File

@ -792,7 +792,10 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
@Override
public void validateCreatedEntity(User user, CreateUser createRequest, Map<String, String> authHeaders) {
validateCommonEntityFields(
getEntityInterface(user), createRequest.getDescription(), TestUtils.getPrincipal(authHeaders), null);
getEntityInterface(user),
createRequest.getDescription(),
TestUtils.getPrincipal(authHeaders),
Entity.getEntityReference(user));
assertEquals(createRequest.getName(), user.getName());
assertEquals(createRequest.getDisplayName(), user.getDisplayName());
@ -825,7 +828,10 @@ public class UserResourceTest extends EntityResourceTest<User, CreateUser> {
@Override
public void compareEntities(User expected, User updated, Map<String, String> authHeaders) {
validateCommonEntityFields(
getEntityInterface(expected), expected.getDescription(), TestUtils.getPrincipal(authHeaders), null);
getEntityInterface(expected),
expected.getDescription(),
TestUtils.getPrincipal(authHeaders),
Entity.getEntityReference(expected));
assertEquals(expected.getName(), expected.getName());
assertEquals(expected.getDisplayName(), expected.getDisplayName());

View File

@ -72,6 +72,7 @@ const TeamDetails = ({
handleTeamUsersSearchAction,
teamUserPaginHandler,
handleJoinTeamClick,
handleLeaveTeamClick,
handleAddUser,
removeUserFromTeam,
}: TeamDetailsProp) => {
@ -164,12 +165,6 @@ const TeamDetails = ({
}
};
const handleRemoveUser = () => {
removeUserFromTeam(deletingUser.user?.id as string).then(() => {
setDeletingUser(DELETE_USER_INITIAL_STATE);
});
};
const isAlreadyJoinedTeam = (teamId: string) => {
if (currentUser) {
return currentUser.teams?.find((team) => team.id === teamId);
@ -196,6 +191,7 @@ const TeamDetails = ({
newTeams.push({
id: currentTeam.id,
type: OwnerType.TEAM,
name: currentTeam.name,
});
const updatedData: User = {
@ -209,6 +205,36 @@ const TeamDetails = ({
}
};
const leaveTeam = (): Promise<void> => {
if (currentUser && currentTeam) {
let newTeams = cloneDeep(currentUser.teams ?? []);
newTeams = newTeams.filter((team) => team.id !== currentTeam.id);
const updatedData: User = {
...currentUser,
teams: newTeams,
};
const options = compare(currentUser, updatedData);
return handleLeaveTeamClick(currentUser.id, options);
}
return Promise.reject();
};
const handleRemoveUser = () => {
if (deletingUser.leave) {
leaveTeam().then(() => {
setDeletingUser(DELETE_USER_INITIAL_STATE);
});
} else {
removeUserFromTeam(deletingUser.user?.id as string).then(() => {
setDeletingUser(DELETE_USER_INITIAL_STATE);
});
}
};
const handleManageSave = (owner: TableDetail['owner']) => {
if (currentTeam) {
const updatedData: Team = {
@ -240,6 +266,7 @@ const TeamDetails = ({
/**
* Take user id as input to find out the user data and set it for delete
* @param id - user id
* @param leave - if "Leave Team" action is in progress
*/
const deleteUserHandler = (id: string, leave = false) => {
const user = [...(currentTeam?.users as Array<UserTeams>)].find(

View File

@ -38,6 +38,7 @@ const TeamsAndUsers = ({
handleUserSearchTerm,
handleDeleteUser,
handleJoinTeamClick,
handleLeaveTeamClick,
isRightPannelLoading,
hasAccess,
isTeamVisible,
@ -210,6 +211,7 @@ const TeamsAndUsers = ({
handleAddTeam={handleAddTeam}
handleAddUser={handleAddUser}
handleJoinTeamClick={handleJoinTeamClick}
handleLeaveTeamClick={handleLeaveTeamClick}
handleTeamUsersSearchAction={handleTeamUsersSearchAction}
hasAccess={hasAccess}
isAddingTeam={isAddingTeam}

View File

@ -66,6 +66,7 @@ export interface TeamsAndUsersProps {
descriptionHandler: (value: boolean) => void;
onDescriptionUpdate: (value: string) => void;
handleJoinTeamClick: (id: string, data: Operation[]) => void;
handleLeaveTeamClick: (id: string, data: Operation[]) => Promise<void>;
isAddingUsers: boolean;
getUniqueUserList: () => Array<UserTeams>;
addUsersToTeam: (data: Array<UserTeams>) => void;
@ -107,4 +108,5 @@ export interface TeamDetailsProp {
handleAddUser: (data: boolean) => void;
removeUserFromTeam: (id: string) => Promise<void>;
handleJoinTeamClick: (id: string, data: Operation[]) => void;
handleLeaveTeamClick: (id: string, data: Operation[]) => Promise<void>;
}

View File

@ -135,11 +135,13 @@ const jsonData = {
'feed-post-error': 'Error while posting the message!',
'join-team-error': 'Error while joining the team!',
'leave-team-error': 'Error while leaving the team!',
},
'api-success-messages': {
'create-conversation': 'Conversation created successfully!',
'join-team-success': 'Team joined successfully!',
'leave-team-success': 'Left the team successfully!',
'delete-test': 'Test deleted successfully!',
'delete-message': 'Message deleted successfully!',

View File

@ -403,6 +403,31 @@ const TeamsAndUsersPage = () => {
});
};
const handleLeaveTeamClick = (id: string, data: Operation[]) => {
return new Promise<void>((resolve, reject) => {
updateUserDetail(id, data)
.then((res: AxiosResponse) => {
if (res.data) {
AppState.updateUserDetails(res.data);
fetchCurrentTeam(teamAndUser);
showSuccessToast(
jsonData['api-success-messages']['leave-team-success']
);
resolve();
} else {
throw jsonData['api-error-messages']['leave-team-error'];
}
})
.catch((err: AxiosError) => {
showErrorToast(
err,
jsonData['api-error-messages']['leave-team-error']
);
reject();
});
});
};
/**
* Handle current team route
* @param name - team name
@ -620,6 +645,7 @@ const TeamsAndUsersPage = () => {
handleAddUser={handleAddUser}
handleDeleteUser={handleDeleteUser}
handleJoinTeamClick={handleJoinTeamClick}
handleLeaveTeamClick={handleLeaveTeamClick}
handleTeamUsersSearchAction={handleTeamUsersSearchAction}
handleUserSearchTerm={handleUserSearchTerm}
hasAccess={isAuthDisabled || isAdminUser}