From 4ab317d76387802c5d7d23052e81d03518090ceb Mon Sep 17 00:00:00 2001 From: anhefti Date: Tue, 21 Jun 2022 15:33:46 +0200 Subject: [PATCH] fixed connection config activation and and deleting and improved OAuth2 error handling for exam API --- .../dao/ActivatableEntityDAO.java | 2 +- .../servicelayer/dao/ClientConnectionDAO.java | 1 + .../servicelayer/dao/SEBClientConfigDAO.java | 13 ------ .../dao/impl/SEBClientConfigDAOImpl.java | 39 +++++++++++++++- .../api/ActivatableEntityController.java | 44 ------------------- .../oauth/AuthorizationServerConfig.java | 19 +++++++- .../weblayer/oauth/RevokeTokenEndpoint.java | 25 +++++++++++ .../oauth/WebClientDetailsService.java | 4 +- .../config/application-dev.properties | 2 +- 9 files changed, 86 insertions(+), 63 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ActivatableEntityDAO.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ActivatableEntityDAO.java index 8e2cde4d..b03dadda 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ActivatableEntityDAO.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ActivatableEntityDAO.java @@ -42,7 +42,7 @@ public interface ActivatableEntityDAO Result> setActive(Set all, boolean active); default Result setActive(final T entity, final boolean active) { - return setActive(new HashSet<>(Arrays.asList(entity.getEntityKey())), true) + return setActive(new HashSet<>(Arrays.asList(entity.getEntityKey())), active) .flatMap(result -> byModelId(result.iterator().next().modelId)); } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientConnectionDAO.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientConnectionDAO.java index 7462bf8f..3e097324 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientConnectionDAO.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/ClientConnectionDAO.java @@ -140,6 +140,7 @@ public interface ClientConnectionDAO extends * @return Result refer to a collection of deleted entities or to an error if happened */ @Override @CacheEvict(cacheNames = CONNECTION_TOKENS_CACHE, allEntries = true) + // TODO this probably is nor working when called from BulkActionSupportDAO Result> delete(Set all); /** Get a ClientConnection by connection token. diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/SEBClientConfigDAO.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/SEBClientConfigDAO.java index 0d803429..3717f378 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/SEBClientConfigDAO.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/SEBClientConfigDAO.java @@ -8,17 +8,10 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.dao; -import java.util.Collection; -import java.util.Set; - -import org.springframework.cache.annotation.CacheEvict; - import ch.ethz.seb.sebserver.gbl.client.ClientCredentials; -import ch.ethz.seb.sebserver.gbl.model.EntityKey; import ch.ethz.seb.sebserver.gbl.model.sebconfig.SEBClientConfig; import ch.ethz.seb.sebserver.gbl.util.Result; import ch.ethz.seb.sebserver.webservice.servicelayer.bulkaction.BulkActionSupportDAO; -import ch.ethz.seb.sebserver.webservice.servicelayer.sebconfig.ClientConfigService; /** Concrete EntityDAO interface of SEBClientConfig entities */ public interface SEBClientConfigDAO extends @@ -54,10 +47,4 @@ public interface SEBClientConfigDAO extends * @return encrypted configuration password */ Result getConfigPasswordCipherByClientName(String clientName); - @Override - @CacheEvict( - cacheNames = ClientConfigService.EXAM_CLIENT_DETAILS_CACHE, - allEntries = true) - Result> delete(Set all); - } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/SEBClientConfigDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/SEBClientConfigDAOImpl.java index 415ae5ca..e11e4a27 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/SEBClientConfigDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/SEBClientConfigDAOImpl.java @@ -24,6 +24,8 @@ import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; +import org.springframework.cache.CacheManager; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Lazy; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; @@ -55,6 +57,8 @@ import ch.ethz.seb.sebserver.webservice.servicelayer.dao.FilterMap; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.ResourceNotFoundException; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.SEBClientConfigDAO; import ch.ethz.seb.sebserver.webservice.servicelayer.dao.TransactionHandler; +import ch.ethz.seb.sebserver.webservice.servicelayer.sebconfig.ClientConfigService; +import ch.ethz.seb.sebserver.webservice.weblayer.oauth.RevokeTokenEndpoint.RevokeExamTokenEvent; @Lazy @Component @@ -65,17 +69,23 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { private final ClientCredentialService clientCredentialService; private final AdditionalAttributesDAOImpl additionalAttributesDAO; private final DAOUserServcie daoUserServcie; + private final ApplicationEventPublisher applicationEventPublisher; + private final CacheManager cacheManager; protected SEBClientConfigDAOImpl( final SebClientConfigRecordMapper sebClientConfigRecordMapper, final ClientCredentialService clientCredentialService, final AdditionalAttributesDAOImpl additionalAttributesDAO, - final DAOUserServcie daoUserServcie) { + final DAOUserServcie daoUserServcie, + final ApplicationEventPublisher applicationEventPublisher, + final CacheManager cacheManager) { this.sebClientConfigRecordMapper = sebClientConfigRecordMapper; this.clientCredentialService = clientCredentialService; this.additionalAttributesDAO = additionalAttributesDAO; this.daoUserServcie = daoUserServcie; + this.applicationEventPublisher = applicationEventPublisher; + this.cacheManager = cacheManager; } @Override @@ -144,6 +154,7 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { } @Override + @Transactional(readOnly = true) public Result byClientName(final String clientName) { return Result.tryCatch(() -> this.sebClientConfigRecordMapper .selectByExample() @@ -217,6 +228,7 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { return ids.stream() .map(id -> new EntityKey(id, EntityType.SEB_CLIENT_CONFIGURATION)) + .map(this::revokeClientConnection) .collect(Collectors.toList()); }); } @@ -303,6 +315,7 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { return ids.stream() .map(id -> new EntityKey(id, EntityType.SEB_CLIENT_CONFIGURATION)) + .map(this::revokeClientConnection) .collect(Collectors.toList()); }); } @@ -657,4 +670,28 @@ public class SEBClientConfigDAOImpl implements SEBClientConfigDAO { } } + private EntityKey revokeClientConnection(final EntityKey key) { + + try { + final SebClientConfigRecord rec = recordById(Long.parseLong(key.modelId)) + .getOrThrow(); + + // revoke token + try { + this.applicationEventPublisher + .publishEvent(new RevokeExamTokenEvent(rec.getClientName())); + } catch (final Exception e) { + log.error("Failed to revoke token for SEB client connection. Connection Configuration: {}", key, e); + } + + // clear cache + this.cacheManager.getCache(ClientConfigService.EXAM_CLIENT_DETAILS_CACHE) + .evictIfPresent(rec.getClientName()); + + } catch (final Exception e) { + log.error("Failed to revoke SEB client connection. Connection Configuration: {}", key, e); + } + + return key; + } } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ActivatableEntityController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ActivatableEntityController.java index cc80b9f7..80d50421 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ActivatableEntityController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ActivatableEntityController.java @@ -205,50 +205,6 @@ public abstract class ActivatableEntityController entities = new ArrayList<>(); -// final Set errors = new HashSet<>(); -// final BulkAction bulkAction = new BulkAction( -// (active) ? BulkActionType.ACTIVATE : BulkActionType.DEACTIVATE, -// entityType, -// entities); -// -// Arrays.asList(StringUtils.split(ids, Constants.LIST_SEPARATOR)) -// .stream() -// .forEach(modelId -> { -// this.entityDAO -// .byModelId(modelId) -// .flatMap(this.authorization::checkWrite) -// .flatMap(entity -> validForActivation(entity, active)) -// .map(Entity::getEntityKey) -// .onSuccess(entities::add) -// .onError(error -> errors.add(new ErrorEntry( -// new EntityKey(modelId, entityType), -// APIMessage.ErrorMessage.UNAUTHORIZED.of(error)))); -// }); -// -// return this.bulkActionService -// .createReport(bulkAction) -// .map(report -> { -// if (!errors.isEmpty()) { -// errors.addAll(report.errors); -// return new EntityProcessingReport(report.source, report.results, errors, report.bulkActionType); -// } else { -// return report; -// } -// }); - } - private Result setActiveSingle(final String modelId, final boolean active) { final EntityType entityType = this.entityDAO.entityType(); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/AuthorizationServerConfig.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/AuthorizationServerConfig.java index 2aa63173..a96d0e59 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/AuthorizationServerConfig.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/AuthorizationServerConfig.java @@ -8,11 +8,16 @@ package ch.ethz.seb.sebserver.webservice.weblayer.oauth; +import javax.servlet.http.HttpServletResponse; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; +import org.springframework.http.MediaType; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.oauth2.config.annotation.configurers.ClientDetailsServiceConfigurer; @@ -26,6 +31,7 @@ import org.springframework.security.oauth2.provider.token.TokenStore; import org.springframework.security.oauth2.provider.token.store.JwtAccessTokenConverter; import ch.ethz.seb.sebserver.WebSecurityConfig; +import ch.ethz.seb.sebserver.gbl.Constants; import ch.ethz.seb.sebserver.gbl.profile.WebServiceProfile; import ch.ethz.seb.sebserver.webservice.weblayer.WebServiceSecurityConfig; import ch.ethz.seb.sebserver.webservice.weblayer.WebServiceUserDetails; @@ -42,6 +48,8 @@ import ch.ethz.seb.sebserver.webservice.weblayer.WebServiceUserDetails; @Order(100) public class AuthorizationServerConfig extends AuthorizationServerConfigurerAdapter { + private static final Logger log = LoggerFactory.getLogger(AuthorizationServerConfig.class); + @Autowired private AccessTokenConverter accessTokenConverter; @Autowired(required = true) @@ -66,7 +74,16 @@ public class AuthorizationServerConfig extends AuthorizationServerConfigurerAdap oauthServer .tokenKeyAccess("permitAll()") .checkTokenAccess("isAuthenticated()") - .passwordEncoder(this.clientPasswordEncoder); + .passwordEncoder(this.clientPasswordEncoder) + .authenticationEntryPoint((request, response, exception) -> { + response.setContentType(MediaType.APPLICATION_JSON_VALUE); + response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + log.warn( + "Unauthorized Request: {}, {}", + request, + exception != null ? exception.getMessage() : Constants.EMPTY_NOTE); + response.getOutputStream().println("{ \"error\": \"" + exception.getMessage() + "\" }"); + }); } @Override diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/RevokeTokenEndpoint.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/RevokeTokenEndpoint.java index ea3c0c5a..9a46e86c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/RevokeTokenEndpoint.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/RevokeTokenEndpoint.java @@ -69,6 +69,18 @@ public class RevokeTokenEndpoint { } } + @EventListener(RevokeExamTokenEvent.class) + void revokeExamAccessToken(final RevokeExamTokenEvent event) { + final Collection tokens = this.tokenStore + .findTokensByClientId(event.clientId); + + if (tokens != null) { + for (final OAuth2AccessToken token : tokens) { + this.tokenStore.removeAccessToken(token); + } + } + } + public static final class RevokeTokenEvent extends ApplicationEvent { private static final long serialVersionUID = 5776699085388043743L; @@ -82,4 +94,17 @@ public class RevokeTokenEndpoint { } + public static final class RevokeExamTokenEvent extends ApplicationEvent { + + private static final long serialVersionUID = 5776699085388043743L; + + public final String clientId; + + public RevokeExamTokenEvent(final String clientId) { + super(clientId); + this.clientId = clientId; + } + + } + } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/WebClientDetailsService.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/WebClientDetailsService.java index ea1585f2..487163d4 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/WebClientDetailsService.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/oauth/WebClientDetailsService.java @@ -11,7 +11,7 @@ package ch.ethz.seb.sebserver.webservice.weblayer.oauth; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.annotation.Lazy; -import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.ClientDetailsService; import org.springframework.security.oauth2.provider.ClientRegistrationException; @@ -67,7 +67,7 @@ public class WebClientDetailsService implements ClientDetailsService { return getForExamClientAPI(clientId) .get(t -> { log.error("Active ClientConfig not found: {} cause: {}", clientId, t.getMessage()); - throw new AccessDeniedException(t.getMessage()); + throw new UsernameNotFoundException(t.getMessage()); }); } diff --git a/src/main/resources/config/application-dev.properties b/src/main/resources/config/application-dev.properties index 8b11fa8d..649a771d 100644 --- a/src/main/resources/config/application-dev.properties +++ b/src/main/resources/config/application-dev.properties @@ -10,7 +10,7 @@ server.tomcat.uri-encoding=UTF-8 logging.level.ROOT=INFO logging.level.ch=INFO logging.level.ch.ethz.seb.sebserver.webservice.datalayer=INFO -logging.level.org.springframework.cache=INFO +logging.level.org.springframework.cache=DEBUG logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl=DEBUG logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.session=DEBUG logging.level.ch.ethz.seb.sebserver.webservice.servicelayer.session.impl.proctoring=INFO