diff --git a/CHANGELOG.md b/CHANGELOG.md index 84b505a4..cef75996 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,31 +3,29 @@ ## 22.0-SNAPSHOT ### 💥 Breaking Changes (upgrade difficulty: 🟢 LOW) -* All `Timer`, `Cache`, and `CachedValue` object require a 'name' property. This property was -previously optional in many cases, but is now required in order to support new cluster features, -logging, and admin tools. The new `BaseService.resources` property now will give access to all -resources by name, if needed and replaces `BaseService.timers`. -* `BaseService` methods `getIMap()`, `getReplicatedMap()` and `getISet()` have been changed to - `createIMap()`, `createReplicatedMap()` and `createISet()`, respectively. This change provides - a consistent interface for all resources on BaseService and is not expected to impact most - applications. +* Updated `Timer`, `Cache`, and `CachedValue` objects to require a `name` property. Names are now + mandatory to better support new cluster features, logging, and Admin Console tooling. +* Migrated `BaseService` methods `getIMap()`, `getReplicatedMap()` and `getISet()` to + `createIMap()`, `createReplicatedMap()` and `createISet()`, respectively. Not expected to impact + most apps, as these APIs are new and only used for distributed, multi-instance data. ### 🎁 New Features -* `Cache` and `CachedValue` should now be created using a factory on `BaseService`. This streamlined -interface reduces boilerplate, and provides a consistent interface with `Timer`. -### ⚙️ Technical - -* Improvements to `Timer` to avoid extra executions when primary instance changes. +* Added new `BaseService` factories to create `Cache` and `CachedValue` objects. This streamlined + interface reduces boilerplate and is consistent with `Timer` creation. +* Improved `Timer` to maintain consistent execution timing across primary instance changes. +* Improved `RestController` to support domain objects linked to a non-primary `DataSource`. -* Updated `ClusterService` to use Hoist's `InstanceNotFoundException` class to designate routine. +### ⚙️ Technical * Exposed `/xh/ping` as whitelisted route for basic uptime/reachability checks. Retained legacy `/ping` alias, but prefer this new path going forward. - -* Improvements to `RestController` to better support editing Domain Objects defined with secondary - domain objects. +* Improved handling + rendering of exceptions during authentication and authorization requests. +* Updated `ClusterService` to use Hoist's `InstanceNotFoundException`, ensuring that common errors + thrown due to instance changes are marked as routine and don't spam error reporting. +* Added new `BaseService.resources` property to track and provide access to `Cache` objects and + `Timer`s by name, replacing `BaseService.timers`. ## 21.0.1 - 2024-09-05 diff --git a/grails-app/controllers/io/xh/hoist/security/AccessInterceptor.groovy b/grails-app/controllers/io/xh/hoist/security/AccessInterceptor.groovy index cd653a58..7b081c3b 100644 --- a/grails-app/controllers/io/xh/hoist/security/AccessInterceptor.groovy +++ b/grails-app/controllers/io/xh/hoist/security/AccessInterceptor.groovy @@ -13,6 +13,8 @@ import io.xh.hoist.exception.NotAuthorizedException import io.xh.hoist.exception.NotFoundException import io.xh.hoist.log.LogSupport import io.xh.hoist.user.IdentityService +import io.xh.hoist.websocket.HoistWebSocketConfigurer + import java.lang.reflect.Method import static org.springframework.util.ReflectionUtils.findMethod @@ -28,66 +30,55 @@ class AccessInterceptor implements LogSupport { } boolean before() { - - // Ignore Websockets -- these are destined for a non-controller based endpoint - // established via a spring-websocket configuration mapping. (Note this is *not* currently - // built into Hoist but is checked / allowed for here.) - if (isWebSocketHandshake()) { - return true - } - - // Get controller method, or 404 - Class clazz = controllerClass?.clazz - String actionNm = actionName ?: controllerClass?.defaultAction - Method method = clazz && actionNm ? findMethod(clazz, actionNm) : null - if (!method) return handleNotFound() - - // Eval method annotations, and return true or 401 - def access = method.getAnnotation(Access) ?: - method.getAnnotation(AccessAll) ?: - clazz.getAnnotation(Access) as Access ?: - clazz.getAnnotation(AccessAll) as AccessAll - - if (access instanceof AccessAll || - (access instanceof Access && identityService.user.hasAllRoles(access.value())) - ) { - return true + try { + // Ignore websockets - these are destined for a non-controller based endpoint + // established via a spring-websocket configuration mapping. + // Note that websockets are not always enabled by Hoist apps but must be supported here. + if (isWebSocketHandshake()) { + return true + } + + // Get controller method, or throw 404 (Not Found). + Class clazz = controllerClass?.clazz + String actionNm = actionName ?: controllerClass?.defaultAction + Method method = clazz && actionNm ? findMethod(clazz, actionNm) : null + if (!method) throw new NotFoundException() + + // Eval @Access annotations, return true if allowed, or throw 403 (Forbidden). + def access = method.getAnnotation(Access) ?: + method.getAnnotation(AccessAll) ?: + clazz.getAnnotation(Access) as Access ?: + clazz.getAnnotation(AccessAll) as AccessAll + + if (access instanceof AccessAll || + (access instanceof Access && identityService.user.hasAllRoles(access.value())) + ) { + return true + } + + def username = identityService.username ?: 'UNKNOWN' + throw new NotAuthorizedException( + "You do not have the required role(s) for this action. Currently logged in as: $username." + ) + } catch (Exception e) { + xhExceptionHandler.handleException( + exception: e, + logMessage: [controller: controllerClass?.name, action: actionName], + logTo: this, + renderTo: response + ) } - - return handleUnauthorized() } //------------------------ // Implementation //------------------------ - private boolean handleUnauthorized() { - def username = identityService.username ?: 'UNKNOWN', - ex = new NotAuthorizedException(""" - You do not have the application role(s) required. - Currently logged in as: $username. - """) - xhExceptionHandler.handleException( - exception: ex, - logTo: this, - logMessage: [controller: controllerClass?.name, action: actionName], - renderTo: response - ) - return false - } - - private boolean handleNotFound() { - xhExceptionHandler.handleException( - exception: new NotFoundException(), - logTo: this, - logMessage: [controller: controllerClass?.name, action: actionName], - renderTo: response - ) - return false - } - private boolean isWebSocketHandshake() { - def upgradeHeader = request?.getHeader('upgrade') - return upgradeHeader == 'websocket' + def req = getRequest(), + upgradeHeader = req?.getHeader('upgrade'), + uri = req?.requestURI + + return upgradeHeader == 'websocket' && uri?.endsWith(HoistWebSocketConfigurer.WEBSOCKET_PATH) } } diff --git a/src/main/groovy/io/xh/hoist/HoistFilter.groovy b/src/main/groovy/io/xh/hoist/HoistFilter.groovy index 07258997..1d63ef9a 100644 --- a/src/main/groovy/io/xh/hoist/HoistFilter.groovy +++ b/src/main/groovy/io/xh/hoist/HoistFilter.groovy @@ -10,13 +10,15 @@ package io.xh.hoist import groovy.transform.CompileStatic import io.xh.hoist.exception.InstanceNotAvailableException import io.xh.hoist.log.LogSupport -import io.xh.hoist.security.BaseAuthenticationService -import io.xh.hoist.util.Utils import javax.servlet.* import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse +import static io.xh.hoist.util.Utils.authenticationService +import static io.xh.hoist.util.Utils.exceptionHandler +import static io.xh.hoist.util.Utils.instanceReady + /** * Main Filter for all requests in Hoist. * @@ -34,27 +36,21 @@ class HoistFilter implements Filter, LogSupport { HttpServletRequest httpRequest = (HttpServletRequest) request HttpServletResponse httpResponse = (HttpServletResponse) response - // Need to be *ready* before even attempting auth. - if (!Utils.instanceReady) { - Utils.exceptionHandler.handleException( - exception: new InstanceNotAvailableException('Application may be initializing. Please try again shortly.'), - renderTo: httpResponse, - logTo: this - ) - return - } + try { + // Need to be *ready* before even attempting auth. + if (!instanceReady) { + throw new InstanceNotAvailableException('Application may be initializing. Please try again shortly.') + } - BaseAuthenticationService authSvc = Utils.authenticationService - if (authSvc.allowRequest(httpRequest, httpResponse)) { - try { + if (authenticationService.allowRequest(httpRequest, httpResponse)) { chain.doFilter(request, response) - } catch (Throwable t) { - Utils.exceptionHandler.handleException( - exception: t, - renderTo: httpResponse, - logTo: this - ) } + } catch (Throwable t) { + exceptionHandler.handleException( + exception: t, + renderTo: httpResponse, + logTo: this + ) } } } diff --git a/src/main/groovy/io/xh/hoist/exception/ExceptionHandler.groovy b/src/main/groovy/io/xh/hoist/exception/ExceptionHandler.groovy index 95dd25b7..91d3c8fd 100644 --- a/src/main/groovy/io/xh/hoist/exception/ExceptionHandler.groovy +++ b/src/main/groovy/io/xh/hoist/exception/ExceptionHandler.groovy @@ -9,6 +9,8 @@ package io.xh.hoist.exception import grails.util.GrailsUtil import groovy.transform.CompileStatic +import groovy.transform.NamedParam +import groovy.transform.NamedVariant import io.xh.hoist.json.JSONSerializer import io.xh.hoist.log.LogSupport @@ -37,25 +39,26 @@ class ExceptionHandler { * Used by BaseController, ClusterRequest, Timer, and AccessInterceptor to handle * otherwise unhandled exception. */ - void handleException(Map options) { - Throwable t = options.exception as Throwable - HttpServletResponse renderTo = options.renderTo as HttpServletResponse - LogSupport logTo = options.logTo as LogSupport - Object logMessage = options.logMessage - - t = preprocess(t) + @NamedVariant + void handleException( + @NamedParam(required = true) Throwable exception, + @NamedParam HttpServletResponse renderTo, + @NamedParam LogSupport logTo, + @NamedParam Object logMessage + ) { + exception = preprocess(exception) if (logTo) { if (logMessage) { - shouldLogDebug(t) ? logTo.logDebug(logMessage, t) : logTo.logError(logMessage, t) + shouldLogDebug(exception) ? logTo.logDebug(logMessage, exception) : logTo.logError(logMessage, exception) } else { - shouldLogDebug(t) ? logTo.logDebug(t) : logTo.logError(t) + shouldLogDebug(exception) ? logTo.logDebug(exception) : logTo.logError(exception) } } if (renderTo) { - renderTo.setStatus(getHttpStatus(t)) + renderTo.setStatus(getHttpStatus(exception)) renderTo.setContentType('application/json') - renderTo.writer.write(JSONSerializer.serialize(t)) + renderTo.writer.write(JSONSerializer.serialize(exception)) renderTo.flushBuffer() } } diff --git a/src/main/groovy/io/xh/hoist/exception/NotAuthorizedException.groovy b/src/main/groovy/io/xh/hoist/exception/NotAuthorizedException.groovy index 8148e86a..9a50deb2 100644 --- a/src/main/groovy/io/xh/hoist/exception/NotAuthorizedException.groovy +++ b/src/main/groovy/io/xh/hoist/exception/NotAuthorizedException.groovy @@ -12,10 +12,12 @@ import static org.apache.hc.core5.http.HttpStatus.SC_FORBIDDEN /** * Exception for use when the authenticated user does not have access to the resource in question. * - * This exception is thrown by Hoist's AccessInterceptor class if the user does not have roles - * required by a controller's @Access annotation. Applications may also throw this exception, or - * subclasses of it, directly in response to requests they cannot fulfill due to auth-related - * constraints. + * This exception is thrown by Hoist's {@link io.xh.hoist.security.BaseAuthenticationService} if + * an authenticated user is not found and by {@link io.xh.hoist.security.AccessInterceptor} if an + * authenticated user does not have roles required by a controller's `@Access` annotation. + * + * Applications may also throw this exception, or subclasses of it, directly in response to requests + * they cannot fulfill due to auth-related constraints. * * Instances of this exception will be sent to clients with HttpStatus 403 ('Forbidden'). */ diff --git a/src/main/groovy/io/xh/hoist/security/BaseAuthenticationService.groovy b/src/main/groovy/io/xh/hoist/security/BaseAuthenticationService.groovy index 0db054d6..b63f5a11 100644 --- a/src/main/groovy/io/xh/hoist/security/BaseAuthenticationService.groovy +++ b/src/main/groovy/io/xh/hoist/security/BaseAuthenticationService.groovy @@ -8,6 +8,7 @@ package io.xh.hoist.security import groovy.transform.CompileStatic import io.xh.hoist.BaseService +import io.xh.hoist.exception.HttpException import io.xh.hoist.exception.NotAuthorizedException import io.xh.hoist.user.HoistUser import io.xh.hoist.user.IdentityService @@ -15,7 +16,11 @@ import io.xh.hoist.user.IdentityService import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse +import static io.xh.hoist.util.Utils.getExceptionHandler import static java.util.Collections.emptyMap +import static org.apache.hc.core5.http.HttpStatus.SC_SERVER_ERROR + + /** * Abstract base service for processing and confirming user authentications and evaluating incoming @@ -95,24 +100,31 @@ abstract class BaseAuthenticationService extends BaseService { //-------------------- /** * Called once on every request to ensure request is authenticated before passing through to - * rest of the framework. Not typically overridden - see completeAuthentication() as main entry + * rest of the framework. Not for override - see completeAuthentication() as main entry * point for subclass implementation. */ boolean allowRequest(HttpServletRequest request, HttpServletResponse response) { - if (identityService.findAuthUser(request) || isWhitelist(request)) { - return true - } + try { + if (identityService.findAuthUser(request) || isWhitelist(request)) { + return true + } - def complete = completeAuthentication(request, response) - if (!complete) return false + if (!completeAuthentication(request, response)) { + return false + } - if (!identityService.findAuthUser(request)) { - response.setStatus(401) + if (!identityService.findAuthUser(request)) { + throw new NotAuthorizedException() + } + + return true + } catch (Throwable e) { + // Do *not* render auth exception to unverified client. Log and return opaque response + exceptionHandler.handleException(exception: e, logTo: this) + response.setStatus(e instanceof HttpException ? e.statusCode : SC_SERVER_ERROR) response.flushBuffer() return false } - - return true } /** diff --git a/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketConfigurer.groovy b/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketConfigurer.groovy index 90f7c715..cc0c3032 100644 --- a/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketConfigurer.groovy +++ b/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketConfigurer.groovy @@ -15,10 +15,12 @@ import org.springframework.web.socket.server.support.HttpSessionHandshakeInterce @EnableWebSocket class HoistWebSocketConfigurer implements WebSocketConfigurer { + static final String WEBSOCKET_PATH = '/xhWebSocket' + @Override void registerWebSocketHandlers(WebSocketHandlerRegistry registry) { def handler = new PerConnectionWebSocketHandler(HoistWebSocketHandler.class) - registry.addHandler(handler, '/xhWebSocket') + registry.addHandler(handler, WEBSOCKET_PATH) .addInterceptors(new HttpSessionHandshakeInterceptor()) .setAllowedOrigins('*') }