diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/api/authorization/Privilege.java b/src/main/java/ch/ethz/seb/sebserver/gbl/api/authorization/Privilege.java index e085ab48..b8d17628 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/api/authorization/Privilege.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/api/authorization/Privilege.java @@ -8,10 +8,14 @@ package ch.ethz.seb.sebserver.gbl.api.authorization; +import java.util.EnumSet; + import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import ch.ethz.seb.sebserver.gbl.api.EntityType; +import ch.ethz.seb.sebserver.gbl.model.user.UserAccount; +import ch.ethz.seb.sebserver.gbl.model.user.UserInfo; import ch.ethz.seb.sebserver.gbl.model.user.UserRole; /** Defines a Privilege by combining a PrivilegeType for base (overall) rights, @@ -165,4 +169,31 @@ public final class Privilege { } } + /** Checks if the current user has role based edit access to a specified user account. + * + * If user account has UserRole.SEB_SERVER_ADMIN this always gives true + * If user account has UserRole.INSTITUTIONAL_ADMIN this is true if the given user account has + * not the UserRole.SEB_SERVER_ADMIN (institutional administrators should not be able to edit SEB Server + * administrators) + * If the current user is the same as the given user account this is always true no matter if there are any + * user-account based privileges (every user shall see its own account) + * + * @param userAccount the user account the check role based edit access + * @return true if the current user has role based edit access to a specified user account */ + public static boolean hasRoleBasedUserAccountEditGrant(final UserAccount userAccount, final UserInfo currentUser) { + final EnumSet userRolesOfUserAccount = userAccount.getUserRoles(); + final EnumSet userRolesOfCurrentUser = currentUser.getUserRoles(); + if (userRolesOfCurrentUser.contains(UserRole.SEB_SERVER_ADMIN)) { + return true; + } + if (userRolesOfCurrentUser.contains(UserRole.INSTITUTIONAL_ADMIN)) { + return !userRolesOfUserAccount.contains(UserRole.SEB_SERVER_ADMIN); + } + if (currentUser.equals(userAccount)) { + return true; + } + + return false; + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/model/user/UserInfo.java b/src/main/java/ch/ethz/seb/sebserver/gbl/model/user/UserInfo.java index edfac3c9..f8d15393 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/model/user/UserInfo.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/model/user/UserInfo.java @@ -215,6 +215,31 @@ public final class UserInfo implements UserAccount, Activatable, Serializable { return UserInfo.of(this); } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((this.uuid == null) ? 0 : this.uuid.hashCode()); + return result; + } + + @Override + public boolean equals(final Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + final UserInfo other = (UserInfo) obj; + if (this.uuid == null) { + if (other.uuid != null) + return false; + } else if (!this.uuid.equals(other.uuid)) + return false; + return true; + } + @Override public String toString() { final StringBuilder builder = new StringBuilder(); diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/UserAccountForm.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/UserAccountForm.java index f115f01f..cd5a2d17 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/UserAccountForm.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/UserAccountForm.java @@ -19,6 +19,9 @@ import org.springframework.stereotype.Component; import ch.ethz.seb.sebserver.gbl.Constants; import ch.ethz.seb.sebserver.gbl.api.API; +import ch.ethz.seb.sebserver.gbl.api.EntityType; +import ch.ethz.seb.sebserver.gbl.api.authorization.Privilege; +import ch.ethz.seb.sebserver.gbl.api.authorization.PrivilegeType; import ch.ethz.seb.sebserver.gbl.model.Domain; import ch.ethz.seb.sebserver.gbl.model.Domain.USER_ROLE; import ch.ethz.seb.sebserver.gbl.model.EntityKey; @@ -123,8 +126,13 @@ public class UserAccountForm implements TemplateComposer { final boolean ownAccount = user.uuid.equals(userAccount.getModelId()); final EntityGrantCheck userGrantCheck = currentUser.entityGrantCheck(userAccount); - final boolean writeGrant = userGrantCheck.w(); - final boolean modifyGrant = userGrantCheck.m(); + final boolean roleBasedEditGrant = Privilege.hasRoleBasedUserAccountEditGrant(userAccount, currentUser.get()); + final boolean writeGrant = roleBasedEditGrant && userGrantCheck.w(); + final boolean modifyGrant = roleBasedEditGrant && userGrantCheck.m(); + final boolean institutionalWriteGrant = currentUser.hasInstitutionalPrivilege( + PrivilegeType.WRITE, + EntityType.USER); + final boolean institutionActive = restService.getBuilder(GetInstitution.class) .withURIVariable(API.PARAM_MODEL_ID, String.valueOf(userAccount.getInstitutionId())) .call() @@ -217,7 +225,7 @@ public class UserAccountForm implements TemplateComposer { this.pageService.pageActionBuilder(formContext.clearEntityKeys()) .newAction(ActionDefinition.USER_ACCOUNT_NEW) - .publishIf(() -> writeGrant && readonly && institutionActive) + .publishIf(() -> institutionalWriteGrant && readonly && institutionActive) .newAction(ActionDefinition.USER_ACCOUNT_MODIFY) .withEntityKey(entityKey) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/ResourceService.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/ResourceService.java index cf56aa6e..659f3755 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/ResourceService.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/ResourceService.java @@ -246,12 +246,13 @@ public class ResourceService { public List> examSupporterResources() { final UserInfo userInfo = this.currentUser.get(); - return this.restService.getBuilder(GetUserAccountNames.class) + final List selection = this.restService.getBuilder(GetUserAccountNames.class) .withQueryParam(Entity.FILTER_ATTR_INSTITUTION, String.valueOf(userInfo.institutionId)) .withQueryParam(Entity.FILTER_ATTR_ACTIVE, Constants.TRUE_STRING) .withQueryParam(UserInfo.FILTER_ATTR_ROLE, UserRole.EXAM_SUPPORTER.name()) .call() - .getOr(Collections.emptyList()) + .getOr(Collections.emptyList()); + return selection .stream() .map(entityName -> new Tuple<>(entityName.modelId, entityName.name)) .collect(Collectors.toList()); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/authorization/AuthorizationService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/authorization/AuthorizationService.java index c734f770..a97fc285 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/authorization/AuthorizationService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/authorization/AuthorizationService.java @@ -9,7 +9,6 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.authorization; import java.util.Collection; -import java.util.EnumSet; import java.util.Set; import ch.ethz.seb.sebserver.gbl.api.EntityType; @@ -230,31 +229,4 @@ public interface AuthorizationService { return check(PrivilegeType.WRITE, grantEntity); } - /** Checks if the current user has role based view access to a specified user account. - * - * If user account has UserRole.SEB_SERVER_ADMIN this always gives true - * If user account has UserRole.INSTITUTIONAL_ADMIN this is true if the given user account has - * not the UserRole.SEB_SERVER_ADMIN (institutional administrators should not see SEB Server administrators) - * If the current user is the same as the given user account this is always true no matter if there are any - * user-account based privileges (every user shall see its own account) - * - * @param userAccount the user account the check role based view access - * @return true if the current user has role based view access to a specified user account */ - default boolean hasRoleBasedUserAccountViewGrant(final UserInfo userAccount) { - final EnumSet userRolesOfUserAccount = userAccount.getUserRoles(); - final SEBServerUser currentUser = getUserService().getCurrentUser(); - final EnumSet userRolesOfCurrentUser = currentUser.getUserRoles(); - if (userRolesOfCurrentUser.contains(UserRole.SEB_SERVER_ADMIN)) { - return true; - } - if (userRolesOfCurrentUser.contains(UserRole.INSTITUTIONAL_ADMIN)) { - return !userRolesOfUserAccount.contains(UserRole.SEB_SERVER_ADMIN); - } - if (currentUser.uuid().equals(userAccount.uuid)) { - return true; - } - - return false; - } - } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/authorization/AuthorizationServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/authorization/AuthorizationServiceImpl.java index bc301cba..da9a9100 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/authorization/AuthorizationServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/authorization/AuthorizationServiceImpl.java @@ -72,6 +72,7 @@ public class AuthorizationServiceImpl implements AuthorizationService { .andForRole(UserRole.INSTITUTIONAL_ADMIN) .withInstitutionalPrivilege(PrivilegeType.WRITE) .andForRole(UserRole.EXAM_ADMIN) + .withInstitutionalPrivilege(PrivilegeType.READ) .withOwnerPrivilege(PrivilegeType.MODIFY) .andForRole(UserRole.EXAM_SUPPORTER) .withOwnerPrivilege(PrivilegeType.MODIFY) diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/UserDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/UserDAOImpl.java index 0704565a..30830942 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/UserDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/UserDAOImpl.java @@ -160,7 +160,7 @@ public class UserDAOImpl implements UserDAO { ? predicate.and(ui -> ui.roles.contains(userRole)) : predicate; - return this.userRecordMapper + final Collection userInfo = this.userRecordMapper .selectByExample() .where( UserRecordDynamicSqlSupport.active, @@ -187,6 +187,9 @@ public class UserDAOImpl implements UserDAO { .flatMap(DAOLoggingSupport::logAndSkipOnError) .filter(_predicate) .collect(Collectors.toList()); + ; + + return userInfo; }); } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/EntityController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/EntityController.java index 03ff29be..15885632 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/EntityController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/EntityController.java @@ -174,8 +174,10 @@ public abstract class EntityController { filterMap.putIfAbsent(API.PARAM_INSTITUTION_ID, String.valueOf(institutionId)); } - return getAll(filterMap) - .getOrThrow() + final Collection all = getAll(filterMap) + .getOrThrow(); + + return all .stream() .map(Entity::toName) .collect(Collectors.toList()); @@ -341,9 +343,10 @@ public abstract class EntityController { } protected Result> getAll(final FilterMap filterMap) { - return this.entityDAO.allMatching( + final Result> allMatching = this.entityDAO.allMatching( filterMap, this::hasReadAccess); + return allMatching; } protected Result notifyCreated(final T entity) { diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/UserAccountController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/UserAccountController.java index 1d4579c4..b4542db3 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/UserAccountController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/UserAccountController.java @@ -8,10 +8,8 @@ package ch.ethz.seb.sebserver.webservice.weblayer.api; -import java.util.Collection; import java.util.EnumSet; import java.util.List; -import java.util.stream.Collectors; import javax.validation.Valid; @@ -19,6 +17,7 @@ import org.mybatis.dynamic.sql.SqlTable; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.context.ApplicationEventPublisher; import org.springframework.http.MediaType; +import org.springframework.security.access.AccessDeniedException; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.validation.FieldError; import org.springframework.web.bind.annotation.RequestBody; @@ -32,6 +31,7 @@ import ch.ethz.seb.sebserver.gbl.api.APIMessage; import ch.ethz.seb.sebserver.gbl.api.APIMessage.APIMessageException; import ch.ethz.seb.sebserver.gbl.api.EntityType; import ch.ethz.seb.sebserver.gbl.api.POSTMapper; +import ch.ethz.seb.sebserver.gbl.api.authorization.Privilege; import ch.ethz.seb.sebserver.gbl.model.EntityKey; import ch.ethz.seb.sebserver.gbl.model.user.PasswordChange; import ch.ethz.seb.sebserver.gbl.model.user.UserAccount; @@ -46,7 +46,6 @@ import ch.ethz.seb.sebserver.webservice.servicelayer.PaginationService; import ch.ethz.seb.sebserver.webservice.servicelayer.authorization.AuthorizationService; import ch.ethz.seb.sebserver.webservice.servicelayer.authorization.SEBServerUser; import ch.ethz.seb.sebserver.webservice.servicelayer.bulkaction.BulkActionService; -import ch.ethz.seb.sebserver.webservice.servicelayer.dao.FilterMap; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.UserActivityLogDAO; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.UserDAO; import ch.ethz.seb.sebserver.webservice.servicelayer.validation.BeanValidationService; @@ -101,12 +100,18 @@ public class UserAccountController extends ActivatableEntityController> getAll(final FilterMap filterMap) { - return super.getAll(filterMap) - .map(result -> result - .stream() - .filter(this.authorization::hasRoleBasedUserAccountViewGrant) - .collect(Collectors.toList())); + protected Result checkModifyAccess(final UserInfo entity) { + return super.checkModifyAccess(entity) + .map(this::checkRoleBasedEditGrant); + } + + private UserInfo checkRoleBasedEditGrant(final UserInfo userInfo) { + final SEBServerUser currentUser = this.authorization.getUserService().getCurrentUser(); + if (Privilege.hasRoleBasedUserAccountEditGrant(userInfo, currentUser.getUserInfo())) { + return userInfo; + } else { + throw new AccessDeniedException("No edit right grant for user: " + currentUser.getUsername()); + } } @Override