From a195f813a12f389a0421579c56c4372579473bd0 Mon Sep 17 00:00:00 2001 From: Max Thonagel <12283268+thoniTUB@users.noreply.github.com> Date: Mon, 7 Oct 2024 18:02:06 +0200 Subject: [PATCH] show warning in ui if user or role could not be resolved --- .../conquery/models/auth/entities/Group.java | 26 ++-- .../models/auth/entities/RoleOwner.java | 6 +- .../conquery/models/auth/entities/User.java | 112 +++++++------- .../oidc/IntrospectionDelegatingRealm.java | 136 ++++++++-------- .../resources/admin/rest/AdminProcessor.java | 18 ++- .../resources/admin/rest/GroupResource.java | 10 +- .../resources/admin/rest/RoleResource.java | 9 +- .../resources/admin/rest/UIProcessor.java | 146 +++++++++++------- .../resources/admin/rest/UserResource.java | 12 +- .../admin/ui/model/FrontendGroupContent.java | 4 +- .../model/FrontendPermissionOwnerContent.java | 5 +- .../admin/ui/model/FrontendRoleOwner.java | 2 +- .../admin/ui/model/FrontendUserContent.java | 2 +- .../com/bakdata/conquery/util/AuthUtil.java | 2 +- .../resources/admin/ui/group.html.ftl | 21 ++- .../conquery/resources/admin/ui/role.html.ftl | 8 +- .../admin/ui/templates/groupHandler.html.ftl | 4 +- .../admin/ui/templates/roleHandler.html.ftl | 21 ++- .../conquery/resources/admin/ui/user.html.ftl | 8 +- .../integration/tests/GroupHandlingTest.java | 2 +- .../tests/PermissionGroupHandlingTest.java | 2 +- .../tests/PermissionRoleHandlingTest.java | 2 +- .../integration/tests/RestartTest.java | 4 +- .../tests/RoleHandlingOnGroupTest.java | 2 +- .../integration/tests/RoleHandlingTest.java | 4 +- .../backend-admin-ui/test_3_smoketest.cy.js | 92 +++++++++-- cypress/integration-helpers/visitAdminUI.js | 8 +- 27 files changed, 385 insertions(+), 283 deletions(-) diff --git a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/Group.java b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/Group.java index 2d34ddadc8..7287547901 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/Group.java +++ b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/Group.java @@ -45,6 +45,13 @@ public Set getEffectivePermissions() { return permissions; } + public synchronized void addMember(User user) { + if (members.add(user.getId())) { + log.trace("Added user {} to group {}", user.getId(), getId()); + updateStorage(); + } + } + @Override public void updateStorage() { storage.updateGroup(this); @@ -55,16 +62,9 @@ public GroupId createId() { return new GroupId(name); } - public synchronized void addMember(User user) { - if (members.add(user.getId())) { - log.trace("Added user {} to group {}", user.getId(), getId()); - updateStorage(); - } - } - - public synchronized void removeMember(User user) { - if (members.remove(user.getId())) { - log.trace("Removed user {} from group {}", user.getId(), getId()); + public synchronized void removeMember(UserId user) { + if (members.remove(user)) { + log.trace("Removed user {} from group {}", user, getId()); updateStorage(); } } @@ -84,9 +84,9 @@ public synchronized void addRole(Role role) { } } - public synchronized void removeRole(Role role) { - if (roles.remove(role.getId())) { - log.trace("Removed role {} from group {}", role.getId(), getId()); + public synchronized void removeRole(RoleId role) { + if (roles.remove(role)) { + log.trace("Removed role {} from group {}", role, getId()); updateStorage(); } } diff --git a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/RoleOwner.java b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/RoleOwner.java index b2be368dd5..b352829e80 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/RoleOwner.java +++ b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/RoleOwner.java @@ -2,18 +2,14 @@ import java.util.Set; -import com.bakdata.conquery.io.storage.MetaStorage; -import com.bakdata.conquery.models.auth.permissions.ConqueryPermission; import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; import com.fasterxml.jackson.annotation.JsonIgnore; -import com.google.common.collect.Sets; -import lombok.extern.slf4j.Slf4j; public interface RoleOwner { void addRole(Role role); - void removeRole(Role role); + void removeRole(RoleId role); /** * Return a copy of the roles hold by the owner. diff --git a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/User.java b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/User.java index bc3b1fc8d1..142ea20e8a 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/auth/entities/User.java +++ b/backend/src/main/java/com/bakdata/conquery/models/auth/entities/User.java @@ -72,11 +72,6 @@ public Set getEffectivePermissions() { return permissions; } - @Override - public UserId createId() { - return new UserId(name); - } - public synchronized void addRole(Role role) { if (roles.add(role.getId())) { log.trace("Added role {} to user {}", role.getId(), getId()); @@ -85,9 +80,9 @@ public synchronized void addRole(Role role) { } @Override - public synchronized void removeRole(Role role) { - if (roles.remove(role.getId())) { - log.trace("Removed role {} from user {}", role.getId(), getId()); + public synchronized void removeRole(RoleId role) { + if (roles.remove(role)) { + log.trace("Removed role {} from user {}", role, getId()); updateStorage(); } } @@ -101,7 +96,59 @@ public void updateStorage() { storage.updateUser(this); } - public void authorize(@NonNull Authorized object, @NonNull Ability ability) { + @Override + public UserId createId() { + return new UserId(name); + } + + /** + * This class is non-static so it's a fixed part of the enclosing User object. + * It's protected for testing purposes only. + */ + public class ShiroUserAdapter extends FilteredUser { + + @Getter + private final ThreadLocal authenticationInfo = + ThreadLocal.withInitial(() -> new ConqueryAuthenticationInfo(User.this, null, null, false, null)); + + @Override + public Object getPrincipal() { + return getId(); + } @Override + public void checkPermission(Permission permission) throws AuthorizationException { + SecurityUtils.getSecurityManager().checkPermission(getPrincipals(), permission); + } + + @Override + public void checkPermissions(Collection permissions) throws AuthorizationException { + SecurityUtils.getSecurityManager().checkPermissions(getPrincipals(), permissions); + } + + @Override + public PrincipalCollection getPrincipals() { + return authenticationInfo.get().getPrincipals(); + } + + @Override + public boolean isPermitted(Permission permission) { + return SecurityUtils.getSecurityManager().isPermitted(getPrincipals(), permission); + } + + @Override + public boolean[] isPermitted(List permissions) { + return SecurityUtils.getSecurityManager().isPermitted(getPrincipals(), permissions); + } + + @Override + public boolean isPermittedAll(Collection permissions) { + return SecurityUtils.getSecurityManager().isPermittedAll(getPrincipals(), permissions); + } + + + + + + } public void authorize(@NonNull Authorized object, @NonNull Ability ability) { if (isOwner(object)) { return; } @@ -168,52 +215,5 @@ public User getUser() { } - /** - * This class is non-static so it's a fixed part of the enclosing User object. - * It's protected for testing purposes only. - */ - public class ShiroUserAdapter extends FilteredUser { - - @Getter - private final ThreadLocal authenticationInfo = - ThreadLocal.withInitial(() -> new ConqueryAuthenticationInfo(User.this, null, null, false, null)); - - @Override - public void checkPermission(Permission permission) throws AuthorizationException { - SecurityUtils.getSecurityManager().checkPermission(getPrincipals(), permission); - } - - @Override - public void checkPermissions(Collection permissions) throws AuthorizationException { - SecurityUtils.getSecurityManager().checkPermissions(getPrincipals(), permissions); - } - @Override - public PrincipalCollection getPrincipals() { - return authenticationInfo.get().getPrincipals(); - } - - @Override - public boolean isPermitted(Permission permission) { - return SecurityUtils.getSecurityManager().isPermitted(getPrincipals(), permission); - } - - @Override - public boolean[] isPermitted(List permissions) { - return SecurityUtils.getSecurityManager().isPermitted(getPrincipals(), permissions); - } - - @Override - public boolean isPermittedAll(Collection permissions) { - return SecurityUtils.getSecurityManager().isPermittedAll(getPrincipals(), permissions); - } - - - @Override - public Object getPrincipal() { - return getId(); - } - - - } } diff --git a/backend/src/main/java/com/bakdata/conquery/models/auth/oidc/IntrospectionDelegatingRealm.java b/backend/src/main/java/com/bakdata/conquery/models/auth/oidc/IntrospectionDelegatingRealm.java index 4913cf3c6a..f3f668a5b6 100644 --- a/backend/src/main/java/com/bakdata/conquery/models/auth/oidc/IntrospectionDelegatingRealm.java +++ b/backend/src/main/java/com/bakdata/conquery/models/auth/oidc/IntrospectionDelegatingRealm.java @@ -83,6 +83,20 @@ public IntrospectionDelegatingRealm(MetaStorage storage, IntrospectionDelegating this.keycloakApi = keycloakApi; } + private static String extractDisplayName(JWTClaimsSet claims) { + try { + final String name = claims.getStringClaim("name"); + + if (StringUtils.isBlank(name)) { + throw new UnsupportedTokenException("Claim 'name' was empty"); + } + return name; + } + catch (java.text.ParseException e) { + throw new IncorrectCredentialsException("Unable to extract username from token", e); + } + } + @Override protected void onInit() { super.onInit(); @@ -126,7 +140,6 @@ public ConqueryAuthenticationInfo doGetAuthenticationInfo(AuthenticationToken to return new ConqueryAuthenticationInfo(user, token, this, true, URI.create(logoutEndpoint)); } - /** * Is called on every request to ensure that the token is still valid. */ @@ -163,6 +176,12 @@ else if (!(response instanceof TokenIntrospectionSuccessResponse)) { } + private static UserId getUserId(JWTClaimsSet claims) { + final String subject = claims.getSubject(); + UserId userId = new UserId(subject); + log.trace("Extracted UserId {}", userId); + return userId; + } private void validateAudiences(JWTClaimsSet claims) { // Check if the token is intended for our client/resource @@ -182,28 +201,6 @@ private void validateAudiences(JWTClaimsSet claims) { } } - private static UserId getUserId(JWTClaimsSet claims) { - final String subject = claims.getSubject(); - UserId userId = new UserId(subject); - log.trace("Extracted UserId {}", userId); - return userId; - } - - private static String extractDisplayName(JWTClaimsSet claims) { - try { - final String name = claims.getStringClaim("name"); - - if (StringUtils.isBlank(name)) { - throw new UnsupportedTokenException("Claim 'name' was empty"); - } - return name; - } - catch (java.text.ParseException e) { - throw new IncorrectCredentialsException("Unable to extract username from token", e); - } - } - - /** * Validates token and synchronizes user and its group memberships with keycloak. */ @@ -232,6 +229,32 @@ public JWTClaimsSet load(String token) throws Exception { return claimsSet; } + private synchronized User getOrCreateUser(JWTClaimsSet claims) { + UserId userId = getUserId(claims); + final String displayName = extractDisplayName(claims); + + User user = storage.getUser(userId); + + if (user != null) { + log.trace("Found existing user: {}", user); + // Update display name if necessary + if (!user.getLabel().equals(displayName)) { + log.info("Updating display name of user [{}]: '{}' -> '{}'", user.getName(), user.getLabel(), displayName); + user.setLabel(displayName); + user.updateStorage(); + } + + return user; + } + + // Construct a new User if none could be found in the storage + user = new User(userId.getName(), displayName, storage); + storage.addUser(user); + log.info("Created new user: {}", user); + + return user; + } + @Nullable private Set getUserGroups(JWTClaimsSet claims, String groupIdAttribute) { final Set userGroups = keycloakApi.getUserGroups(claims.getSubject()); @@ -247,6 +270,25 @@ private Set getUserGroups(JWTClaimsSet claims, String groupIdAttribute) { .collect(Collectors.toSet()); } + private void syncGroupMappings(User user, Set mappedGroupsToDo) { + // TODO mark mappings as managed by keycloak + for (Group group : storage.getAllGroups()) { + if (group.containsMember(user)) { + if (mappedGroupsToDo.contains(group)) { + // Mapping is still valid, remove from todo-list + mappedGroupsToDo.remove(group); + } + else { + // Mapping is not valid any more remove user from group + group.removeMember(user.getId()); + } + } + } + + for (Group group : mappedGroupsToDo) { + group.addMember(user); + } + } private Optional tryGetGroup(KeycloakGroup keycloakGroup) { @@ -306,54 +348,6 @@ private synchronized Group createGroup(String name, String label) { return group; } - - private void syncGroupMappings(User user, Set mappedGroupsToDo) { - // TODO mark mappings as managed by keycloak - for (Group group : storage.getAllGroups()) { - if (group.containsMember(user)) { - if (mappedGroupsToDo.contains(group)) { - // Mapping is still valid, remove from todo-list - mappedGroupsToDo.remove(group); - } - else { - // Mapping is not valid any more remove user from group - group.removeMember(user); - } - } - } - - for (Group group : mappedGroupsToDo) { - group.addMember(user); - } - } - - - private synchronized User getOrCreateUser(JWTClaimsSet claims) { - UserId userId = getUserId(claims); - final String displayName = extractDisplayName(claims); - - User user = storage.getUser(userId); - - if (user != null) { - log.trace("Found existing user: {}", user); - // Update display name if necessary - if (!user.getLabel().equals(displayName)) { - log.info("Updating display name of user [{}]: '{}' -> '{}'", user.getName(), user.getLabel(), displayName); - user.setLabel(displayName); - user.updateStorage(); - } - - return user; - } - - // Construct a new User if none could be found in the storage - user = new User(userId.getName(), displayName, storage); - storage.addUser(user); - log.info("Created new user: {}", user); - - return user; - } - } } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/AdminProcessor.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/AdminProcessor.java index b02fa9bd33..aab83fa3a7 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/AdminProcessor.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/AdminProcessor.java @@ -11,6 +11,7 @@ import java.util.function.Predicate; import java.util.function.Supplier; import java.util.stream.Collectors; +import jakarta.validation.Validator; import com.bakdata.conquery.commands.ManagerNode; import com.bakdata.conquery.io.jackson.Jackson; @@ -25,6 +26,8 @@ import com.bakdata.conquery.models.config.ConqueryConfig; import com.bakdata.conquery.models.exceptions.JSONException; import com.bakdata.conquery.models.exceptions.ValidatorHelper; +import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; +import com.bakdata.conquery.models.identifiable.ids.specific.UserId; import com.bakdata.conquery.models.index.IndexKey; import com.bakdata.conquery.models.jobs.JobManager; import com.bakdata.conquery.models.jobs.JobManagerStatus; @@ -37,7 +40,6 @@ import com.google.common.collect.Multimap; import com.univocity.parsers.csv.CsvWriter; import groovy.lang.GroovyShell; -import jakarta.validation.Validator; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -87,7 +89,7 @@ public synchronized void addRole(Role role) throws JSONException { * * @param role the role to delete */ - public void deleteRole(Role role) { + public void deleteRole(RoleId role) { log.info("Deleting {}", role); for (User user : storage.getAllUsers()) { @@ -98,7 +100,7 @@ public void deleteRole(Role role) { group.removeRole(role); } - storage.removeRole(role.getId()); + storage.removeRole(role); } public SortedSet getAllRoles() { @@ -132,12 +134,12 @@ public TreeSet getAllUsers() { return new TreeSet<>(storage.getAllUsers()); } - public synchronized void deleteUser(User user) { + public synchronized void deleteUser(UserId user) { for (Group group : storage.getAllGroups()) { group.removeMember(user); } - storage.removeUser(user.getId()); - log.trace("Removed user {} from the storage.", user.getId()); + storage.removeUser(user); + log.trace("Removed user {} from the storage.", user); } public void addUsers(List users) { @@ -185,7 +187,7 @@ public void addUserToGroup(Group group, User user) { log.trace("Added user {} to group {}", user, group); } - public void deleteUserFromGroup(Group group, User user) { + public void deleteUserFromGroup(Group group, UserId user) { group.removeMember(user); log.trace("Removed user {} from group {}", user, group); } @@ -195,7 +197,7 @@ public void deleteGroup(Group group) { log.trace("Removed group {}", group); } - public void deleteRoleFrom(RoleOwner owner, Role role) { + public void deleteRoleFrom(RoleOwner owner, RoleId role) { owner.removeRole(role); log.trace("Removed role {} from {}", role, owner); } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/GroupResource.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/GroupResource.java index d9aca561ac..cb7e529e10 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/GroupResource.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/GroupResource.java @@ -4,7 +4,6 @@ import java.util.Collection; import java.util.List; - import jakarta.inject.Inject; import jakarta.validation.constraints.NotEmpty; import jakarta.ws.rs.Consumes; @@ -20,6 +19,8 @@ import com.bakdata.conquery.models.auth.entities.Group; import com.bakdata.conquery.models.auth.entities.Role; import com.bakdata.conquery.models.auth.entities.User; +import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; +import com.bakdata.conquery.models.identifiable.ids.specific.UserId; import lombok.RequiredArgsConstructor; @Produces(MediaType.APPLICATION_JSON) @@ -50,9 +51,8 @@ public Response deleteGroup(@PathParam(GROUP_ID) Group group) { } @POST - public Response postGroups(@NotEmpty List groups) { + public void postGroups(@NotEmpty List groups) { processor.addGroups(groups); - return Response.ok().build(); } @Path("{" + GROUP_ID + "}/" + USERS_PATH_ELEMENT + "/{" + USER_ID + "}") @@ -64,14 +64,14 @@ public Response addUserToGroup(@PathParam(GROUP_ID) Group group, @PathParam(USER @Path("{" + GROUP_ID + "}/" + USERS_PATH_ELEMENT + "/{" + USER_ID + "}") @DELETE - public Response deleteUserFromGroup(@PathParam(GROUP_ID) Group group, @PathParam(USER_ID) User user) { + public Response deleteUserFromGroup(@PathParam(GROUP_ID) Group group, @PathParam(USER_ID) UserId user) { processor.deleteUserFromGroup(group, user); return Response.ok().build(); } @Path("{" + GROUP_ID + "}/" + ROLES_PATH_ELEMENT + "/{" + ROLE_ID + "}") @DELETE - public Response deleteRoleFromUser(@PathParam(GROUP_ID) Group group, @PathParam(ROLE_ID) Role role) { + public Response deleteRoleFromUser(@PathParam(GROUP_ID) Group group, @PathParam(ROLE_ID) RoleId role) { processor.deleteRoleFrom(group, role); return Response.ok().build(); } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/RoleResource.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/RoleResource.java index b4cfdf3ca8..5997d07245 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/RoleResource.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/RoleResource.java @@ -4,9 +4,6 @@ import static com.bakdata.conquery.resources.ResourceConstants.ROLE_ID; import java.util.Collection; - -import com.bakdata.conquery.models.auth.entities.Role; -import com.bakdata.conquery.models.exceptions.JSONException; import jakarta.inject.Inject; import jakarta.ws.rs.Consumes; import jakarta.ws.rs.DELETE; @@ -17,6 +14,10 @@ import jakarta.ws.rs.Produces; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; + +import com.bakdata.conquery.models.auth.entities.Role; +import com.bakdata.conquery.models.exceptions.JSONException; +import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; import lombok.RequiredArgsConstructor; @Consumes(MediaType.APPLICATION_JSON) @@ -46,7 +47,7 @@ public Response getRole(@PathParam(ROLE_ID) Role role) throws JSONException { @Path("{" + ROLE_ID + "}") @DELETE - public Response deleteRole(@PathParam(ROLE_ID) Role role) throws JSONException { + public Response deleteRole(@PathParam(ROLE_ID) RoleId role) throws JSONException { processor.deleteRole(role); return Response.ok().build(); } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/UIProcessor.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/UIProcessor.java index 17c89ccdbf..edac579d1f 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/UIProcessor.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/rest/UIProcessor.java @@ -12,6 +12,7 @@ import java.util.TreeSet; import java.util.function.Predicate; import java.util.stream.Collectors; +import jakarta.inject.Inject; import com.bakdata.conquery.io.cps.CPSTypeIdResolver; import com.bakdata.conquery.io.storage.MetaStorage; @@ -30,6 +31,7 @@ import com.bakdata.conquery.models.datasets.concepts.tree.ConceptTreeNode; import com.bakdata.conquery.models.datasets.concepts.tree.TreeConcept; import com.bakdata.conquery.models.events.CBlock; +import com.bakdata.conquery.models.identifiable.ids.specific.RoleId; import com.bakdata.conquery.models.identifiable.ids.specific.UserId; import com.bakdata.conquery.models.index.IndexKey; import com.bakdata.conquery.models.worker.DatasetRegistry; @@ -43,7 +45,6 @@ import com.bakdata.conquery.resources.admin.ui.model.TableStatistics; import com.bakdata.conquery.resources.admin.ui.model.UIContext; import com.google.common.cache.CacheStats; -import jakarta.inject.Inject; import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -60,14 +61,6 @@ public class UIProcessor { @Getter private final AdminProcessor adminProcessor; - public DatasetRegistry getDatasetRegistry() { - return adminProcessor.getDatasetRegistry(); - } - - public MetaStorage getStorage() { - return adminProcessor.getStorage(); - } - public UIContext getUIContext(String csrfToken) { return new UIContext(adminProcessor.getNodeProvider(), csrfToken); } @@ -99,17 +92,91 @@ public FrontendAuthOverview getAuthOverview() { return FrontendAuthOverview.builder().overview(overview).build(); } + public MetaStorage getStorage() { + return adminProcessor.getStorage(); + } + + public FrontendGroupContent getGroupContent(Group group) { + + Set membersIds = group.getMembers(); + ArrayList availableMembers = new ArrayList<>(getStorage().getAllUsers()); + availableMembers.removeIf(u -> membersIds.contains(u.getId())); + + List members = membersIds.stream() + .map(id -> { + User user = getStorage().getUser(id); + if (user != null) { + return getUserContent(user); + } + return FrontendUserContent.builder().id(id).build(); + }) + .toList(); + + List roles = group.getRoles().stream() + .map(this::getFrontendRoleContent) + .toList(); + + return FrontendGroupContent + .builder() + .resolvable(true) + .label(group.getLabel()) + .id(group.getId()) + .members(members) + .availableMembers(availableMembers) + .roles(roles) + .availableRoles(getStorage().getAllRoles()) + .permissions(wrapInFEPermission(group.getPermissions())) + .permissionTemplateMap(preparePermissionTemplate()) + .build(); + } + + private FrontendRoleContent getFrontendRoleContent(RoleId id) { + Role role = getStorage().getRole(id); + if (role != null) { + return getRoleContent(role); + } + return FrontendRoleContent.builder().id(id).build(); + } + + public FrontendUserContent getUserContent(User user) { + final Collection availableGroups = new ArrayList<>(getStorage().getAllGroups()); + availableGroups.removeIf(g -> g.containsMember(user)); + + return FrontendUserContent + .builder() + .resolvable(true) + .label(user.getLabel()) + .id(user.getId()) + .groups(AuthorizationHelper.getGroupsOf(user, getStorage())) + .availableGroups(availableGroups) + .roles(user.getRoles().stream().map(this::getFrontendRoleContent).collect(Collectors.toList())) + .availableRoles(getStorage().getAllRoles()) + .permissions(wrapInFEPermission(user.getPermissions())) + .permissionTemplateMap(preparePermissionTemplate()) + .build(); + } public FrontendRoleContent getRoleContent(Role role) { return FrontendRoleContent.builder() + .resolvable(true) .permissions(wrapInFEPermission(role.getPermissions())) .permissionTemplateMap(preparePermissionTemplate()) .users(getUsers(role)) .groups(getGroups(role)) - .owner(role) + .id(role.getId()) + .label(role.getLabel()) .build(); } + private SortedSet wrapInFEPermission(Collection permissions) { + TreeSet fePermissions = new TreeSet<>(); + + for (ConqueryPermission permission : permissions) { + fePermissions.add(FrontendPermission.from(permission)); + } + return fePermissions; + } + private Map, List>> preparePermissionTemplate() { Map, List>> permissionTemplateMap = new HashMap<>(); @@ -144,49 +211,6 @@ private List getGroups(Role role) { .collect(Collectors.toList()); } - private SortedSet wrapInFEPermission(Collection permissions) { - TreeSet fePermissions = new TreeSet<>(); - - for (ConqueryPermission permission : permissions) { - fePermissions.add(FrontendPermission.from(permission)); - } - return fePermissions; - } - - public FrontendUserContent getUserContent(User user) { - final Collection availableGroups = new ArrayList<>(getStorage().getAllGroups()); - availableGroups.removeIf(g -> g.containsMember(user)); - - return FrontendUserContent - .builder() - .owner(user) - .groups(AuthorizationHelper.getGroupsOf(user, getStorage())) - .availableGroups(availableGroups) - .roles(user.getRoles().stream().map(getStorage()::getRole).collect(Collectors.toList())) - .availableRoles(getStorage().getAllRoles()) - .permissions(wrapInFEPermission(user.getPermissions())) - .permissionTemplateMap(preparePermissionTemplate()) - .build(); - } - - - public FrontendGroupContent getGroupContent(Group group) { - - Set membersIds = group.getMembers(); - ArrayList availableMembers = new ArrayList<>(getStorage().getAllUsers()); - availableMembers.removeIf(u -> membersIds.contains(u.getId())); - return FrontendGroupContent - .builder() - .owner(group) - .members(membersIds.stream().map(getStorage()::getUser).collect(Collectors.toList())) - .availableMembers(availableMembers) - .roles(group.getRoles().stream().map(getStorage()::getRole).collect(Collectors.toList())) - .availableRoles(getStorage().getAllRoles()) - .permissions(wrapInFEPermission(group.getPermissions())) - .permissionTemplateMap(preparePermissionTemplate()) - .build(); - } - public TableStatistics getTableStatistics(Table table) { final NamespaceStorage storage = getDatasetRegistry().get(table.getDataset().getId()).getStorage(); List imports = table.findImports(storage).collect(Collectors.toList()); @@ -214,12 +238,8 @@ public TableStatistics getTableStatistics(Table table) { ); } - public ImportStatistics getImportStatistics(Import imp) { - final NamespaceStorage storage = getDatasetRegistry().get(imp.getDataset().getId()).getStorage(); - - final long cBlockSize = calculateCBlocksSizeBytes(imp, storage.getAllConcepts()); - - return new ImportStatistics(imp, cBlockSize); + public DatasetRegistry getDatasetRegistry() { + return adminProcessor.getDatasetRegistry(); } public static long calculateCBlocksSizeBytes(Import imp, Collection> concepts) { @@ -242,4 +262,12 @@ public static long calculateCBlocksSizeBytes(Import imp, Collection implements FrontendRoleOwner { - public final Collection members; + public final Collection members; public final Collection availableMembers; - public final Collection roles; + public final Collection roles; public final Collection availableRoles; } \ No newline at end of file diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendPermissionOwnerContent.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendPermissionOwnerContent.java index 17caeb3242..04365b840e 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendPermissionOwnerContent.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendPermissionOwnerContent.java @@ -6,6 +6,7 @@ import com.bakdata.conquery.models.auth.entities.PermissionOwner; import com.bakdata.conquery.models.auth.permissions.Ability; +import com.bakdata.conquery.models.identifiable.ids.Id; import lombok.Getter; import lombok.experimental.SuperBuilder; import org.apache.commons.lang3.tuple.Pair; @@ -16,7 +17,9 @@ @Getter @SuperBuilder public class FrontendPermissionOwnerContent> { - private OWNER owner; + private String label; + private Id id; + private boolean resolvable; /** * Holds the owned permission objects and its JSON representation. diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendRoleOwner.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendRoleOwner.java index fc63e72483..4fa91f9308 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendRoleOwner.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendRoleOwner.java @@ -10,7 +10,7 @@ */ public interface FrontendRoleOwner { - Collection getRoles(); + Collection getRoles(); Collection getAvailableRoles(); } diff --git a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendUserContent.java b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendUserContent.java index 6a3626412d..739550e6f8 100644 --- a/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendUserContent.java +++ b/backend/src/main/java/com/bakdata/conquery/resources/admin/ui/model/FrontendUserContent.java @@ -15,7 +15,7 @@ @SuperBuilder public class FrontendUserContent extends FrontendPermissionOwnerContent implements FrontendRoleOwner { - public final Collection roles; + public final Collection roles; public final Collection availableRoles; public final Collection groups; public final Collection availableGroups; diff --git a/backend/src/main/java/com/bakdata/conquery/util/AuthUtil.java b/backend/src/main/java/com/bakdata/conquery/util/AuthUtil.java index a76e0040c2..3a84ffb4f7 100644 --- a/backend/src/main/java/com/bakdata/conquery/util/AuthUtil.java +++ b/backend/src/main/java/com/bakdata/conquery/util/AuthUtil.java @@ -45,7 +45,7 @@ public synchronized void cleanUpUserAndBelongings(User user, MetaStorage storage for (Group group : storage.getAllGroups()) { if (group.containsMember(user)) { - group.removeMember(user); + group.removeMember(user.getId()); group.updateStorage(); log.debug("Removed user '{}' from group '{}'", user.getId(), group.getId()); } diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/group.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/group.html.ftl index 611ea50a5c..ac58161b2c 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/group.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/group.html.ftl @@ -6,8 +6,8 @@
-

Group ${c.owner.label}

- ${c.owner.id} +

Group ${c.label}

+ ${c.id}
@@ -29,10 +29,10 @@
- <@permissionTable.permissionTable ownerId=c.owner.getId() permissions=c.permissions /> + <@permissionTable.permissionTable ownerId=c.id permissions=c.permissions />
- <@permissionCreator.permissionCreator ownerId=c.owner.getId() permissionTemplateMap=c.permissionTemplateMap /> + <@permissionCreator.permissionCreator ownerId=c.id permissionTemplateMap=c.permissionTemplateMap />
@@ -65,9 +65,14 @@ <#list c.members as member> - ${member.label} - ${member.id} - Remove from ${c.owner.label} + <#if member.resolvable> + ${member.label} + ${member.id} + <#else> + Unresolvable Member: ${member.id} + + + Remove from ${c.label} @@ -85,7 +90,7 @@ function addMember() { event.preventDefault(); rest( - '/admin/${ctx.staticUriElem.GROUPS_PATH_ELEMENT}/${c.owner.id}/${ctx.staticUriElem.USERS_PATH_ELEMENT}/'+document.getElementById('member_id').value, + '/admin/${ctx.staticUriElem.GROUPS_PATH_ELEMENT}/${c.id}/${ctx.staticUriElem.USERS_PATH_ELEMENT}/'+document.getElementById('member_id').value, { method: 'post', }) diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/role.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/role.html.ftl index b4cd405414..518ca50ed8 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/role.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/role.html.ftl @@ -6,8 +6,8 @@
-

Role ${c.owner.label}

- ${c.owner.id} +

Role ${c.label}

+ ${c.id}
@@ -32,11 +32,11 @@
- <@permissionTable.permissionTable ownerId=c.owner.getId() permissions=c.permissions /> + <@permissionTable.permissionTable ownerId=c.id permissions=c.permissions />
- <@permissionCreator.permissionCreator ownerId=c.owner.getId() + <@permissionCreator.permissionCreator ownerId=c.id permissionTemplateMap=c.permissionTemplateMap />
diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/groupHandler.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/groupHandler.html.ftl index dbef9a7327..4250fd8b74 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/groupHandler.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/groupHandler.html.ftl @@ -30,7 +30,7 @@ ${group.label} ${group.id} Remove + onclick="removeGroup('${adminPathBase}/${group.id}/${ctx.staticUriElem.USERS_PATH_ELEMENT}/${c.id}')">Remove from Entity @@ -41,7 +41,7 @@ function addGroup() { event.preventDefault(); rest( - '${adminPathBase}/' + document.getElementById('group_id').value + '/${ctx.staticUriElem.USERS_PATH_ELEMENT}/${c.owner.id}', + '${adminPathBase}/' + document.getElementById('group_id').value + '/${ctx.staticUriElem.USERS_PATH_ELEMENT}/${c.id}', { method: 'post', }).then(function () { location.reload() }); diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/roleHandler.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/roleHandler.html.ftl index 640fc0e0c6..6cda1644c3 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/roleHandler.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/templates/roleHandler.html.ftl @@ -26,13 +26,18 @@ <#list c.roles as role> - - ${role.label} - ${role.id} - Remove - from Entity - + + <#if role.resolvable > + ${role.label} + ${role.id} + <#else> + Unresolvable Role: ${role.id} + + + Remove + from Entity + @@ -41,7 +46,7 @@ function addRole() { event.preventDefault(); rest( - '${adminPathBase}/${c.owner.id}/${ctx.staticUriElem.ROLES_PATH_ELEMENT}/' + document.getElementById('role_id').value, + '${adminPathBase}/${c.id}/${ctx.staticUriElem.ROLES_PATH_ELEMENT}/' + document.getElementById('role_id').value, { method: 'post', }).then(function () { location.reload() }); diff --git a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/user.html.ftl b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/user.html.ftl index 081c45cfcc..867996e0c7 100644 --- a/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/user.html.ftl +++ b/backend/src/main/resources/com/bakdata/conquery/resources/admin/ui/user.html.ftl @@ -7,8 +7,8 @@
-

User ${c.owner.label}

- ${c.owner.id} +

User ${c.label}

+ ${c.id}
@@ -30,10 +30,10 @@
- <@permissionTable.permissionTable ownerId=c.owner.getId() permissions=c.permissions /> + <@permissionTable.permissionTable ownerId=c.id permissions=c.permissions />
- <@permissionCreator.permissionCreator ownerId=c.owner.getId() permissionTemplateMap=c.permissionTemplateMap /> + <@permissionCreator.permissionCreator ownerId=c.id permissionTemplateMap=c.permissionTemplateMap />
<@groupHandler.groupHandler c=c adminPathBase="/admin/${ctx.staticUriElem.GROUPS_PATH_ELEMENT}" /> diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/GroupHandlingTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/GroupHandlingTest.java index 8c156caa29..5053400c29 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/GroupHandlingTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/GroupHandlingTest.java @@ -37,7 +37,7 @@ public void execute(StandaloneSupport conquery) throws Exception { group1.addMember(user2); assertThat(group1.getMembers()).containsExactlyInAnyOrder(user1.getId(), user2.getId()); - group1.removeMember(user2); + group1.removeMember(user2.getId()); assertThat(group1.getMembers()).containsExactlyInAnyOrder(user1.getId()); diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionGroupHandlingTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionGroupHandlingTest.java index a6d45ee831..3dda1b4fd6 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionGroupHandlingTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionGroupHandlingTest.java @@ -52,7 +52,7 @@ public void execute(StandaloneSupport conquery) throws Exception { assertThat(user1.isPermitted(ExecutionPermission.onInstance(Ability.SHARE, query1))).isTrue(); // remove user from group - group1.removeMember(user1); + group1.removeMember(user1.getId()); assertThat(user1.isPermitted(ExecutionPermission.onInstance(Ability.READ, query1))).isTrue(); assertThat(user1.isPermitted(ExecutionPermission.onInstance(Ability.DELETE, query1))).isTrue(); diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionRoleHandlingTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionRoleHandlingTest.java index 105abea3f7..ea9afb556b 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionRoleHandlingTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/PermissionRoleHandlingTest.java @@ -58,7 +58,7 @@ public void execute(StandaloneSupport conquery) throws Exception { // Add permission to mandator, remove mandator from user mandator1.addPermission(dataset.createPermission(Ability.DOWNLOAD.asSet())); - user1.removeRole(mandator1); + user1.removeRole(mandator1.getId()); assertThat(user1.isPermitted(dataset.createPermission(Ability.READ.asSet()))).isTrue(); assertThat(user1.isPermitted(dataset.createPermission(Ability.DOWNLOAD.asSet()))).isFalse(); diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/RestartTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/RestartTest.java index 9960d8cc94..ba3de818a8 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/RestartTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/RestartTest.java @@ -130,8 +130,8 @@ public void execute(String name, TestConquery testConquery) throws Exception { // Delete entities //TODO use API - adminProcessor.deleteUser(userToDelete); - adminProcessor.deleteRole(roleToDelete); + adminProcessor.deleteUser(userToDelete.getId()); + adminProcessor.deleteRole(roleToDelete.getId()); adminProcessor.deleteGroup(groupToDelete); } diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingOnGroupTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingOnGroupTest.java index 205e39d5de..787468bcaf 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingOnGroupTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingOnGroupTest.java @@ -48,7 +48,7 @@ public void execute(StandaloneSupport conquery) throws Exception { //// Remove role from group - group1.removeRole(role); + group1.removeRole(role.getId()); assertThat(group1.getRoles()).isEmpty(); assertThat(user1.isPermitted(new DatasetPermission().instancePermission(Ability.READ, new DatasetId("testDataset")))).isFalse(); diff --git a/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingTest.java b/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingTest.java index ba797e04be..40dd3470f6 100644 --- a/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingTest.java +++ b/backend/src/test/java/com/bakdata/conquery/integration/tests/RoleHandlingTest.java @@ -46,10 +46,10 @@ public void execute(StandaloneSupport conquery) throws Exception { //// REMOVING - user1.removeRole(mandator2); + user1.removeRole(mandator2.getId()); assertThat(user1.getRoles()).containsExactlyInAnyOrder(mandator1.getId()); - user1.removeRole(mandator1); + user1.removeRole(mandator1.getId()); assertThat(user1.getRoles()).isEmpty(); } diff --git a/cypress/e2e/backend-admin-ui/test_3_smoketest.cy.js b/cypress/e2e/backend-admin-ui/test_3_smoketest.cy.js index a44dd12331..1d548877e4 100644 --- a/cypress/e2e/backend-admin-ui/test_3_smoketest.cy.js +++ b/cypress/e2e/backend-admin-ui/test_3_smoketest.cy.js @@ -1,25 +1,85 @@ /// -import { visitAdminUI } from "../../integration-helpers/visitAdminUI"; +import { adminBaseUrl, checkFreemarkerError, visitAdminUI } from "../../integration-helpers/visitAdminUI"; -context("Simplest Smoke Tests", () => { +context("Simple UI Render Smoke Tests", () => { - describe("UI renders", () => { - - it("Query Overview", () => { - - visitAdminUI("queries"); + describe("Query Overview renders", () => { - // Disable updates so the test ends - cy.get('#updateCheckBox').click() - cy.root().should('not.contain.text', 'FreeMarker template error') - }); + it("Visit Query Overview", () => { + + visitAdminUI("queries"); + + // Disable updates so the test ends eventually + cy.get('#updateCheckBox').click() + checkFreemarkerError() + }); + }); + + describe("Query Jobs renders", () => { + + it("Visit Jobs Overview", () => { + + visitAdminUI("jobs"); - - it("Jobs Overview", () => { - - visitAdminUI("jobs"); + checkFreemarkerError() + }); + }); + + describe("Groups renders", () => { - cy.root().should('not.contain.text', 'FreeMarker template error') + it("Post faulty group (member id not resolvable)", () => { + + cy.request({ + method: 'POST', + url: adminBaseUrl + "/admin/groups/", + body: [{ + name: "faulty_group", + label: "Faulty Group", + members: [ + "user.unresolvable" + ], + roles: [ + "role.unresolvable" + ] + }] }); }); + + + it("Visit Groups", () => { + + visitAdminUI("groups"); + + checkFreemarkerError(); + }); + + it("Visit Faulty Group", () => { + visitAdminUI("groups/group.faulty_group"); + + checkFreemarkerError(); + + cy.get('#member > .table-responsive').contains('Unresolvable Member') + + cy.get('#roles > .table-responsive').contains('Unresolvable Role') + }) + + + + it("Delete faulty member and role", () => { + visitAdminUI("groups/group.faulty_group"); + + cy.get('#members-tab').click() + cy.get('#member > .table-responsive > .table > tbody > tr > :nth-child(2) > a').click() + + cy.get('#roles-tab').click() + cy.get('#roles > .table-responsive > .table > tbody > tr > :nth-child(2) > a').click() + + checkFreemarkerError(); + + cy.get('#member > .table-responsive').should('not.contain.text', 'Unresolvable Member') + + cy.get('#roles > .table-responsive').should('not.contain.text', 'Unresolvable Role') + }) + }) + }) \ No newline at end of file diff --git a/cypress/integration-helpers/visitAdminUI.js b/cypress/integration-helpers/visitAdminUI.js index bf9911cf9c..ba45b81af5 100644 --- a/cypress/integration-helpers/visitAdminUI.js +++ b/cypress/integration-helpers/visitAdminUI.js @@ -1,4 +1,10 @@ +export const adminBaseUrl = 'http://localhost:8081' + export const visitAdminUI = (page = '') => { - cy.visit('http://localhost:8081/admin-ui/' + page); + cy.visit(adminBaseUrl + '/admin-ui/' + page); }; + +export const checkFreemarkerError = () => { + cy.root().should('not.contain.text', 'FreeMarker template error') +} \ No newline at end of file