From 66b2eebf1d4232029c6c3c54b12d78bd48e42892 Mon Sep 17 00:00:00 2001 From: anhefti Date: Wed, 26 Jun 2024 14:14:24 +0200 Subject: [PATCH] fixed some list mismatches due to read access + code cleanup --- .../seb/sebserver/gbl/model/exam/Exam.java | 2 +- ...InstitutionalAuthenticationEntryPoint.java | 2 +- .../OAuth2AuthorizationContextHolder.java | 1 - .../servicelayer/PaginationServiceImpl.java | 1 - .../authorization/AuthorizationService.java | 28 +++++++++++++++---- .../servicelayer/dao/FilterMap.java | 4 ++- .../servicelayer/dao/impl/ExamRecordDAO.java | 7 +++++ .../impl/SEBClientEventAdminServiceImpl.java | 2 +- .../api/ExamAdministrationController.java | 13 +++++++-- .../api/ExamMonitoringController.java | 2 +- .../weblayer/api/QuizController.java | 1 + 11 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Exam.java b/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Exam.java index 25955d2f..77039deb 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Exam.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/Exam.java @@ -322,7 +322,7 @@ public final class Exam implements GrantEntity { return this.institutionId; } - public boolean isOwner(final String userId) { + public boolean isOwnerOrSupporter(final String userId) { if (StringUtils.isBlank(userId)) { return false; } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/InstitutionalAuthenticationEntryPoint.java b/src/main/java/ch/ethz/seb/sebserver/gui/InstitutionalAuthenticationEntryPoint.java index aafd2449..493cd21e 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/InstitutionalAuthenticationEntryPoint.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/InstitutionalAuthenticationEntryPoint.java @@ -131,7 +131,7 @@ public final class InstitutionalAuthenticationEntryPoint implements Authenticati SEBServerAuthorizationContext authorizationContext = authorizationContextHolder .getAuthorizationContext(request.getSession()); - // check first if we already have an active session if so, invalidate ir + // check first if we already have an active session if so, invalidate it first if (authorizationContext.isLoggedIn()) { authorizationContext.logout(); authorizationContext = authorizationContextHolder.getAuthorizationContext(request.getSession()); diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/OAuth2AuthorizationContextHolder.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/OAuth2AuthorizationContextHolder.java index 2314ab37..f7486f6e 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/OAuth2AuthorizationContextHolder.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/remote/webservice/auth/OAuth2AuthorizationContextHolder.java @@ -15,7 +15,6 @@ import java.nio.charset.StandardCharsets; import javax.servlet.http.HttpSession; import ch.ethz.seb.sebserver.gbl.api.API; -import ch.ethz.seb.sebserver.gbl.model.EntityKey; import ch.ethz.seb.sebserver.gbl.model.user.LoginForward; import ch.ethz.seb.sebserver.gbl.model.user.TokenLoginInfo; import org.apache.commons.lang3.BooleanUtils; diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/PaginationServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/PaginationServiceImpl.java index 0cf63a68..bfc29bd1 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/PaginationServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/PaginationServiceImpl.java @@ -135,7 +135,6 @@ public class PaginationServiceImpl implements PaginationService { final Supplier>> delegate) { return Result.tryCatch(() -> { - //final SqlTable table = SqlTable.of(tableName); final com.github.pagehelper.Page page = setPagination(pageNumber, pageSize, sort, tableName); 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 4470b3e2..97d1af8a 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 @@ -8,11 +8,7 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.authorization; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Set; +import java.util.*; import ch.ethz.seb.sebserver.gbl.api.EntityType; import ch.ethz.seb.sebserver.gbl.api.authorization.Privilege; @@ -302,4 +298,26 @@ public interface AuthorizationService { currentUser.getUserInfo()); } } + + /** This will throw access exception when current user has only teacher role */ + default void checkNotOnlyTeacher(final EntityType type) { + final UserInfo userInfo = this.getUserService().getCurrentUser().getUserInfo(); + final EnumSet userRoles = userInfo.getUserRoles(); + if (userRoles.contains(UserRole.TEACHER) && userRoles.size() == 1) { + throw new PermissionDeniedException(type, PrivilegeType.READ, userInfo); + } + } + + /** If current user has only Supporter or Teacher role, this will get the UUID of the user back + * or null otherwise (of user has other privileges too. + * @return current user UUID if it is Supporter or Teacher only */ + default String getSupporterOnlyUUID() { + final UserInfo userInfo = this.getUserService().getCurrentUser().getUserInfo(); + final EnumSet userRoles = userInfo.getUserRoles(); + userRoles.removeAll(Arrays.asList(UserRole.TEACHER, UserRole.EXAM_SUPPORTER)); + if (userRoles.isEmpty()) { + return userInfo.uuid; + } + return null; + } } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/FilterMap.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/FilterMap.java index b081f450..5acc0b74 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/FilterMap.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/FilterMap.java @@ -53,6 +53,8 @@ public class FilterMap extends POSTMapper { public static final String ATTR_ADD_INSITUTION_JOIN = "ADD_INSITUTION_JOIN"; public static final String ATTR_ADD_LMS_SETUP_JOIN = "ADD_LMS_SETUP_JOIN"; + public static final String ATTR_SUPPORTER_USER_ID = "SUPPORTER_USER_UUID"; + public FilterMap() { super(new LinkedMultiValueMap<>(), null); } @@ -123,7 +125,6 @@ public class FilterMap extends POSTMapper { public DateTime getSEBClientConfigFromTime() { return Utils.toDateTime(getString(SEBClientConfig.FILTER_ATTR_CREATION_DATE)); } - public Long getLmsSetupId() { return getLong(LmsSetup.FILTER_ATTR_LMS_SETUP); } @@ -334,6 +335,7 @@ public class FilterMap extends POSTMapper { return null; } + public static final class Builder { private final FilterMap filterMap = new FilterMap(); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamRecordDAO.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamRecordDAO.java index 9eb9c27c..c1c4b05b 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamRecordDAO.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/ExamRecordDAO.java @@ -189,6 +189,13 @@ public class ExamRecordDAO { ExamRecordDynamicSqlSupport.type, isEqualToWhenPresent(filterMap.getExamType())); + final String supporterUUID = filterMap.getSQLWildcard(FilterMap.ATTR_SUPPORTER_USER_ID); + if (StringUtils.isNotBlank(supporterUUID)) { + whereClause = whereClause.and( + supporter, + SqlBuilder.isLike(supporterUUID)); + } + // SEBSERV-298 if (filterMap.getBoolean(Exam.FILTER_ATTR_HIDE_MISSING)) { whereClause = whereClause.and( diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/SEBClientEventAdminServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/SEBClientEventAdminServiceImpl.java index 2bcca458..ddb068a6 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/SEBClientEventAdminServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/SEBClientEventAdminServiceImpl.java @@ -182,7 +182,7 @@ public class SEBClientEventAdminServiceImpl implements SEBClientEventAdminServic final Exam exam = getExam(rec.getClientConnectionId()); - if (!isSupporterOnly || exam.isOwner(currentUser.uuid())) { + if (!isSupporterOnly || exam.isOwnerOrSupporter(currentUser.uuid())) { this.exporter.streamData( this.output, rec, diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java index 0f367246..507f53af 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java @@ -14,8 +14,6 @@ import java.util.stream.Collectors; import javax.validation.Valid; -import ch.ethz.seb.sebserver.gbl.model.Activatable; -import ch.ethz.seb.sebserver.gbl.model.user.UserInfo; import ch.ethz.seb.sebserver.gbl.util.Cryptor; import ch.ethz.seb.sebserver.webservice.servicelayer.exam.ExamImportService; import ch.ethz.seb.sebserver.webservice.servicelayer.exam.ExamUtils; @@ -134,6 +132,17 @@ public class ExamAdministrationController extends EntityController { this.cryptor = cryptor; this.fullLmsIntegrationService = fullLmsIntegrationService; } + @Override + protected Result> getAll(final FilterMap filterMap) { + // If current user has only supporter role, put user UUID to filter to get correct page result from DB + final String supporterId = authorization.getSupporterOnlyUUID(); + if (StringUtils.isNotBlank(supporterId)) { + filterMap.putIfAbsent(FilterMap.ATTR_SUPPORTER_USER_ID, supporterId); + } + return this.entityDAO.allMatching( + filterMap, + this::hasReadAccess); + } @Override protected SqlTable getSQLTableOfEntity() { diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamMonitoringController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamMonitoringController.java index 10f856dd..a49deb1c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamMonitoringController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamMonitoringController.java @@ -539,7 +539,7 @@ public class ExamMonitoringController { final UserInfo userInfo = this.authorization.getUserService().getCurrentUser().getUserInfo(); final String userId = userInfo.uuid; return exam.institutionId.equals(institution) - && (exam.isOwner(userId) || userInfo.hasRole(UserRole.EXAM_ADMIN)); + && (exam.isOwnerOrSupporter(userId) || userInfo.hasRole(UserRole.EXAM_ADMIN)); } private Predicate noneActiveFilter(final EnumSet filterStates) { diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/QuizController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/QuizController.java index 50280bf6..b6793d53 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/QuizController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/QuizController.java @@ -85,6 +85,7 @@ public class QuizController { @RequestParam final MultiValueMap allRequestParams, final HttpServletRequest request) { + this.authorization.checkNotOnlyTeacher(EntityType.EXAM); this.authorization.check( PrivilegeType.READ, EntityType.EXAM,