diff --git a/CHANGELOG.md b/CHANGELOG.md index 12aeca1a..b8597af9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## 41.0-SNAPSHOT - unreleased +### ⚙️ Technical + +* Reworked identity resolution onto an explicit per-thread `HoistIdentity` cache, installed at + every framework thread-entry point (`HoistFilter`, `HoistWebSocketHandler`, async `task` workers + via a new `IdentityPropagatingPromiseFactory`, and `ClusterTask`). Identity accessors + (`identityService.username`/`authUsername`/etc.) no longer dereference the live servlet request + or session on each call. Eliminates `IllegalStateException: request object has been recycled` + errors when identity is resolved on async continuations, propagates identity into Grails + `task {}` workers automatically, and makes `identityService` usable inside WebSocket message + handlers. See [docs/planning/identity-async-safety.md](docs/planning/identity-async-safety.md). + ## 40.0.1 - 2026-05-14 ### 💥 Breaking Changes (upgrade difficulty: 🟢 LOW - updates to new metrics APIs) diff --git a/docs/planning/identity-async-safety.md b/docs/planning/identity-async-safety.md new file mode 100644 index 00000000..3681afa1 --- /dev/null +++ b/docs/planning/identity-async-safety.md @@ -0,0 +1,195 @@ +--- +status: active +type: implementation-plan +description: Plan to make hoist-core identity/request access safe across async boundaries (post-recycle threads, auto-instrumented spans). +created: 2026-05-19 +--- + +# Plan: Make hoist-core identity/request access safe across async boundaries + +## Background + +`RequestFacade.checkFacade()` throws `IllegalStateException` once Tomcat recycles the +underlying request. Hoist-core hits this when async or post-response code paths dereference +the live request indirectly — currently observed via `TraceService.createSpan` → +`hoistTags()` → `IdentityService.getAuthUsername` → `request.getSession()`. The same shape +applies to `TagSpanProcessor.onStart` (every auto-instrumented span) and +`TrackService.parseSubmittedEntry` (userAgent/browser/device). + +Root cause: framework accessors re-read identity from the session on every call, and +`WebPromises.task` propagates a live `GrailsWebRequest` reference into worker threads that +outlive the original request. + +## Guiding principle + +The session is the durable source of truth for identity, but it is read at session-resume +points only — i.e., when a thread starts processing on behalf of a user. After that, identity +is fixed for the thread and should live in thread-local state. No accessor should hit the +session on every call; nothing outside the request thread should touch a `RequestFacade` at +all. + +--- + +## Phase 1 — Thread-scoped identity cache + defensive guards (minor release) + +**Goal:** Make identity a thread property sourced from the session once at thread entry, +eliminate per-call session reads, and add defensive guards on the remaining request-touching +paths. + +### Changes + +1. **Introduce `HoistIdentity` — single immutable POGO holding `username` and `authUsername`.** + Lives in `io.xh.hoist.user`. Constructed once at thread entry; never mutated. All identity + reads return fields off this object. + +2. **`IdentityService` uses a single `ThreadLocal` as primary identity source.** + Replaces the existing `threadUsername` / `threadAuthUsername` ThreadLocals (the legacy pair + may be kept populated for one release for BC, or removed if no external callers exist). + All public accessors (`getUsername`, `getAuthUsername`, `getUser`, `getAuthUser`, + `findHoistUsername`) read from the cache. Session is no longer touched from accessors. + +3. **Cache populated at request entry.** + `HoistFilter` (post-auth) reads session attributes once, constructs a `HoistIdentity`, + installs it on the ThreadLocal, clears in `finally` when the filter exits. One session + read per request, on the request thread, where the facade is live. + +4. **Identity-mutating operations replace the cached `HoistIdentity` in lock-step with the session.** + `login()`, `logout()`, `impersonate()`, `endImpersonate()` already write to the session — + they additionally construct and install a fresh `HoistIdentity` on the ThreadLocal. + Finite, audit-able set of mutation sites. + +5. **New `IdentityPropagatingPromiseFactory` (modeled on `ContextPropagatingPromiseFactory`).** + At task creation: capture the originating thread's `HoistIdentity`. On the worker: + install it on the ThreadLocal before the closure runs; clear in `finally`. Installed at + startup, composed with the OTel context-propagating factory. + +6. **`ClusterTask` constructs and installs a `HoistIdentity`** rather than setting the raw + ThreadLocals. Same behavior; unified accessor surface. + +7. **Defensive guards** on remaining request-touching paths, for any code path that bypasses + the identity cache: + - `IdentityService.getSessionIfExists` — wrap `request?.getSession(false)` in + `try/catch (IllegalStateException) → null`. A recycled facade is semantically equivalent + to "no session," which already has correct fallback behavior. + - `TrackService.parseSubmittedEntry` — wrap `currentRequest?.getHeader(...)`, + `getBrowser(currentRequest)`, `getDevice(currentRequest)` in the same guard. These + fields are best-effort observability; null is acceptable. + +### Effect + +- `TraceService.hoistTags()`, `TagSpanProcessor.onStart`, and any other identity consumer no + longer touch a request or session. The observed bug is gone. +- Async work spawned via `task {}` (web or plain) receives the originating user's identity + automatically via the decorator. +- Any future or app-side code that bypasses the cache and hits the recycled facade fails + cleanly (null fallback) rather than throwing. + +### Risk + +Low. Identity accessors keep their signatures and return values. The behavioral shift is +"session is read once at request start instead of N times during the request," which is a +perf improvement, not a semantic change. The defensive guards are pure null-where-it-already- +fails-null. + +### Ships as + +Minor release. + +--- + +## Phase 2 — *Optional* — Eliminate live-request propagation into worker threads + +**Goal:** Defense-in-depth. After Phase 1 the observed bug is fixed, but any future or +app-side code that calls `Utils.currentRequest`/`request.X` inside an async block still +holds a live-but-doomed facade. Phase 2 closes that door. + +**Status:** Optional. Evaluate after Phase 1 ships and we see whether real-world reports +surface code paths Phase 1 doesn't already cover. + +### Why it's separate + +`BaseController.runAsync` is built on `WebPromises.task`, which propagates +`RequestContextHolder` + GORM session binding to the worker. That's exactly what +`WebPromises.task` is designed for, and it's plausibly used by downstream apps for **async +response rendering** (start work on a worker, eventually `render`/`renderJSON` to the +still-open response). Switching the factory under existing callers would break that use +case. The current `runAsync` exception handler itself reads `actionName` (a `webRequest` +lookup), so even the framework's own code assumes web propagation. + +### Recommended shape: add a sibling, do not change `runAsync` + +1. **New `BaseController.runDetached(Closure)`.** + Built on plain `Promises.task` + the identity propagation decorator from Phase 1. On + the worker: + - Identity is available via `identityService.authUsername` (from decorator). + - `RequestContextHolder.resetRequestAttributes()` is called so `Utils.currentRequest` + returns null — no live facade reference on the worker. + - Exception handler snapshots needed controller state (`actionName`) on the request + thread into the closure. + +2. **Documentation:** + - `runAsync` — "Use for async response handling. Worker thread has access to + request/response. Do not use for work that outlives the response." + - `runDetached` — "Use for fire-and-forget background work that outlives the response. + Identity propagates; request/response are not accessible." + +3. **Optional deprecation pass.** If telemetry or code inspection shows that `runAsync` + is overwhelmingly used for fire-and-forget rather than async response handling, + consider deprecating `runAsync` in favor of explicit `runAsync` / `runDetached` + naming in a future major. + +### Risk if pursued + +Low — additive API. The risk we avoided is the one of changing `runAsync` semantics, which +this approach explicitly sidesteps. + +### Ships as + +Separate minor release. Only if real-world need emerges. + +--- + +## What this plan deliberately does *not* do + +- Does **not** introduce a multi-field request snapshot POGO. Identity is the only state + that needs to survive thread transitions; non-identity request fields are either + observability best-effort (handled by the Phase 1 defensive guards) or genuinely scoped + to the request thread. +- Does **not** change `TraceService.hoistTags` or `TagSpanProcessor` directly. They benefit + automatically once identity comes from cache. +- Does **not** change `WebPromises.task` propagation semantics. Phase 2 (if pursued) adds + a parallel API rather than mutating the existing one. +- Does **not** deprecate any current public API. The Phase 1 ThreadLocals may be unified + internally but the existing ones can stay populated for BC if needed. + +--- + +## Verification + +For Phase 1: + +- Unit: `IdentityService` accessors read from the thread cache, populate from session only + at filter entry, update on login/logout/impersonate. +- Unit: `IdentityPropagatingPromiseFactory` captures on task creation, installs on worker, + clears in finally, handles nesting. +- Integration: a controller that does + `runAsync { Thread.sleep(200); identityService.authUsername }` returns the correct + username with no `IllegalStateException` after the response is rendered. +- Integration: same scenario hitting `trackService.track(...)` and opening a manual span — + confirms tags resolve, no exception. +- Regression: existing impersonation flow continues to work (session write + cache update + on the same thread). + +For Phase 2 (if pursued): + +- Confirm `Utils.currentRequest` returns null inside a `runDetached` body. +- Confirm `runAsync` still has live request/response for async response rendering. + +--- + +## Sequencing + +| Phase | Ships as | When | +|---|---|---| +| 1 | Minor release | Now | +| 2 | Minor release | Optional. Only if Phase 1 leaves uncovered cases in practice. | diff --git a/grails-app/services/io/xh/hoist/track/TrackService.groovy b/grails-app/services/io/xh/hoist/track/TrackService.groovy index 5b0a7e5d..d078c0c3 100644 --- a/grails-app/services/io/xh/hoist/track/TrackService.groovy +++ b/grails-app/services/io/xh/hoist/track/TrackService.groovy @@ -19,6 +19,7 @@ import io.xh.hoist.util.Utils import static io.xh.hoist.browser.Utils.getBrowser import static io.xh.hoist.browser.Utils.getDevice +import static io.xh.hoist.browser.Utils.safeHeader import static io.xh.hoist.json.JSONSerializer.serialize import static io.xh.hoist.util.InstanceConfigUtils.getInstanceConfig import static grails.async.Promises.task @@ -232,7 +233,7 @@ class TrackService extends BaseService { // From request/context instance : ClusterService.instanceName, appEnvironment: Utils.appEnvironment, - userAgent : currentRequest?.getHeader('User-Agent'), + userAgent : safeHeader(currentRequest, 'User-Agent'), browser : getBrowser(currentRequest), device : getDevice(currentRequest) ] diff --git a/grails-app/services/io/xh/hoist/user/IdentityService.groovy b/grails-app/services/io/xh/hoist/user/IdentityService.groovy index 4573a1c6..070386bc 100644 --- a/grails-app/services/io/xh/hoist/user/IdentityService.groovy +++ b/grails-app/services/io/xh/hoist/user/IdentityService.groovy @@ -7,6 +7,7 @@ package io.xh.hoist.user +import grails.async.Promises import groovy.transform.CompileStatic import io.xh.hoist.BaseService import io.xh.hoist.config.ConfigService @@ -15,58 +16,69 @@ import io.xh.hoist.track.TrackService import static io.xh.hoist.util.Utils.getCurrentRequest import jakarta.servlet.http.HttpServletRequest -import jakarta.servlet.http.HttpSession +import org.springframework.web.socket.WebSocketSession /** * Primary service for retrieving the logged-in HoistUser (aka the application user), with support * for impersonation. This service powers the getUser() and getUsername() methods in Hoist's * BaseService and BaseController classes. * - * The implementation of this service uses the session to hold the authenticated username (and - * potentially a distinct "apparent" username when impersonation is active). It depends on the app's - * AuthenticationService to initialize the authenticated user via noteUserAuthenticated(), and - * likewise delegates to the app's UserService to resolve usernames to actual HoistUser objects. - * * This service is *not* intended for override or customization at the application level. + * + * Implementation notes: + * The session is the durable source of truth for identity, but accessors read from a per-thread + * {@link HoistIdentity} cache installed explicitly at each thread entry point: + * {@link io.xh.hoist.HoistFilter} (HTTP request), {@link io.xh.hoist.websocket.HoistWebSocketHandler} + * (WS lifecycle callbacks), {@link IdentityPropagatingPromiseFactory} (async {@code task} workers), + * and {@link io.xh.hoist.cluster.ClusterTask} (cluster RPC). Mutating operations + * ({@link #login}, {@link #logout}, {@link #impersonate}, {@link #endImpersonate}, + * {@link #noteUserAuthenticated}) update the session and the cache together. */ @CompileStatic class IdentityService extends BaseService { - ThreadLocal threadUsername = new ThreadLocal() - ThreadLocal threadAuthUsername = new ThreadLocal() + final ThreadLocal threadIdentity = new ThreadLocal() - static public String AUTH_USER_KEY = 'xhAuthUser' - static public String APPARENT_USER_KEY = 'xhApparentUser' + private static final String AUTH_USER_KEY = 'xhAuthUser' + private static final String APPARENT_USER_KEY = 'xhApparentUser' BaseAuthenticationService authenticationService BaseUserService userService TrackService trackService ConfigService configService + void init() { + super.init() + installIdentityPromisePropagation() + } + //------------------------------------ // Implementation of IdentitySupport //------------------------------------- - HoistUser getUser() { - findHoistUser(APPARENT_USER_KEY) + String getUsername() { + threadIdentity.get()?.username } - String getUsername() { - findHoistUsername(APPARENT_USER_KEY) + String getAuthUsername() { + threadIdentity.get()?.authUsername } - HoistUser getAuthUser() { - findHoistUser(AUTH_USER_KEY) + HoistUser getUser() { + def name = username + name ? userService.find(name) : null } - String getAuthUsername() { - findHoistUsername(AUTH_USER_KEY) + HoistUser getAuthUser() { + def name = authUsername + name ? userService.find(name) : null } /** * Is the authorized user currently impersonating someone else? */ boolean isImpersonating() { - return username != authUsername + def identity = threadIdentity.get() + identity && identity.username != identity.authUsername } /** @@ -106,7 +118,7 @@ class IdentityService extends BaseService { // first explicitly end any existing impersonation session -- important for tracking. if (impersonating) endImpersonate() - request.session[APPARENT_USER_KEY] = targetUser.username + setIdentity(targetUser.username, authUser.username) trackImpersonate('Started impersonation', [target: targetUser.username]) logInfo("User '$authUser.username' has started impersonating user '$targetUser.username'") @@ -124,7 +136,7 @@ class IdentityService extends BaseService { if (apparentUser != authUser) { trackImpersonate("Stopped impersonation", [target: apparentUser.username]) logInfo("User '$authUser.username' has stopped impersonating user '$apparentUser.username'") - currentRequest.session[APPARENT_USER_KEY] = authUser.username + setIdentity(authUser.username, authUser.username) } } @@ -165,8 +177,7 @@ class IdentityService extends BaseService { */ boolean logout() { if (authenticationService.logout()) { - def session = getSessionIfExists() - if (session) session[APPARENT_USER_KEY] = session[AUTH_USER_KEY] = null + clearIdentity() return true } @@ -178,23 +189,72 @@ class IdentityService extends BaseService { * Called by authenticationService when HoistUser has first been established for this session. */ void noteUserAuthenticated(HttpServletRequest request, HoistUser user) { - def session = request.session - session[APPARENT_USER_KEY] = session[AUTH_USER_KEY] = user.username + setIdentity(user.username, user.username) } + + //----------------- + // Framework + //---------------- + /** - * Entry Point for AuthenticationService - * Called by authenticationService to determine if user has been set on this session + * Install the given identity on the current thread (or clear it, if null). Used by + * {@link IdentityPropagatingPromiseFactory} and {@link io.xh.hoist.cluster.ClusterTask} + * to propagate identity captured/trampolined from an originating thread or node. + * + * @internal - not for application use */ - HoistUser findAuthUser(HttpServletRequest request) { - HttpSession session = getSessionIfExists(request) - String username = session ? session[AUTH_USER_KEY] : null - username ? userService.find(username) : null + void installThreadIdentity(HoistIdentity identity) { + if (identity == null) { + threadIdentity.remove() + } else { + threadIdentity.set(identity) + } + } + + /** + * Install thread identity from the given request's existing session (does not create one). + * Called by {@link io.xh.hoist.HoistFilter} at request entry. + * + * @internal - not for application use + */ + void installIdentityFromRequest(HttpServletRequest request) { + def session = request?.getSession(false) + installThreadIdentity(session ? new HoistIdentity( + session.getAttribute(APPARENT_USER_KEY) as String, + session.getAttribute(AUTH_USER_KEY) as String + ) : null) + } + + /** + * Install thread identity from a WebSocket session's handshake-captured attribute map. + * Called by {@link io.xh.hoist.websocket.HoistWebSocketHandler} on each lifecycle callback. + * + * @internal - not for application use + */ + void installIdentityFromWebSocketSession(WebSocketSession session) { + installThreadIdentity(session ? new HoistIdentity( + session.attributes[APPARENT_USER_KEY] as String, + session.attributes[AUTH_USER_KEY] as String + ) : null) } //---------------------- // Implementation //---------------------- + private void setIdentity(String username, String authUsername) { + def session = currentRequest.session + session[APPARENT_USER_KEY] = username + session[AUTH_USER_KEY] = authUsername + threadIdentity.set(new HoistIdentity(username, authUsername)) + } + + private void clearIdentity() { + def session = currentRequest?.getSession(false) + if (session) session[APPARENT_USER_KEY] = session[AUTH_USER_KEY] = null + threadIdentity.remove() + } + private void checkImpersonationEnabled() { if (!configService.getBool('xhEnableImpersonation')) { throw new RuntimeException('Impersonation is disabled for this app.') @@ -207,21 +267,13 @@ class IdentityService extends BaseService { severity: 'WARN', msg: msg, data: data - ); + ) } - private HoistUser findHoistUser(String key) { - String username = findHoistUsername(key) - username ? userService.find(username) : null - } - - private String findHoistUsername(String key) { - HttpSession session = getSessionIfExists() - session ? session[key] : (key == AUTH_USER_KEY ? threadAuthUsername.get() : threadUsername.get()) - } - private HttpSession getSessionIfExists(HttpServletRequest request = currentRequest) { - // Do *not* create session for simple, early checks (avoid DOS attack) - request?.getSession(false) + private void installIdentityPromisePropagation() { + if (!(Promises.promiseFactory instanceof IdentityPropagatingPromiseFactory)) { + Promises.promiseFactory = new IdentityPropagatingPromiseFactory(Promises.promiseFactory, this) + } } } diff --git a/src/main/groovy/io/xh/hoist/HoistFilter.groovy b/src/main/groovy/io/xh/hoist/HoistFilter.groovy index 0c11f178..859109f5 100644 --- a/src/main/groovy/io/xh/hoist/HoistFilter.groovy +++ b/src/main/groovy/io/xh/hoist/HoistFilter.groovy @@ -20,6 +20,7 @@ import jakarta.servlet.http.HttpServletResponse import static io.opentelemetry.api.trace.SpanKind.SERVER import static io.xh.hoist.util.Utils.authenticationService +import static io.xh.hoist.util.Utils.identityService import static io.xh.hoist.util.Utils.traceService import static io.xh.hoist.util.Utils.traceContextService import static io.xh.hoist.util.Utils.getClusterService @@ -45,11 +46,16 @@ class HoistFilter implements Filter, LogSupport { HttpServletRequest httpRequest = (HttpServletRequest) request HttpServletResponse httpResponse = (HttpServletResponse) response + identityService.installIdentityFromRequest(httpRequest) + // Always restore trace context, but conditionally add span here. try (def scope = traceContextService.restoreContextFromRequest(httpRequest)) { shouldTrace(httpRequest) ? handleTraced(httpRequest, httpResponse, chain) : handleUntraced(httpRequest, httpResponse, chain) + } finally { + // Prevent identity leaking between requests when Tomcat returns the thread to the pool. + identityService.installThreadIdentity(null) } } diff --git a/src/main/groovy/io/xh/hoist/browser/Utils.groovy b/src/main/groovy/io/xh/hoist/browser/Utils.groovy index 638986be..1368d878 100644 --- a/src/main/groovy/io/xh/hoist/browser/Utils.groovy +++ b/src/main/groovy/io/xh/hoist/browser/Utils.groovy @@ -16,18 +16,31 @@ class Utils { static Browser getBrowser(HttpServletRequest request) { if (!request) return null - def ua = request.getHeader('User-Agent'), - uaHints = request.getHeader('Sec-Ch-UA') + def ua = safeHeader(request, 'User-Agent'), + uaHints = safeHeader(request, 'Sec-Ch-UA') findMatch(uaHints, BROWSER_MATCHERS) ?: findMatch(ua, BROWSER_MATCHERS) ?: Browser.OTHER } static Device getDevice(HttpServletRequest request) { if (!request) return null - def ua = request.getHeader('User-Agent'), - uaPlatformHints = request.getHeader('Sec-Ch-UA-Platform') + def ua = safeHeader(request, 'User-Agent'), + uaPlatformHints = safeHeader(request, 'Sec-Ch-UA-Platform') findMatch(uaPlatformHints, DEVICE_MATCHERS) ?: findMatch(ua, DEVICE_MATCHERS) ?: Device.OTHER } + /** + * Read a header from a servlet request, returning null if the underlying RequestFacade + * has been recycled by Tomcat. Tolerates identity/observability calls that arrive on + * threads whose request reference is no longer valid (e.g. async continuations). + */ + static String safeHeader(HttpServletRequest request, String name) { + try { + return request?.getHeader(name) + } catch (IllegalStateException ignored) { + return null + } + } + //-------------------- // Implementation //-------------------- diff --git a/src/main/groovy/io/xh/hoist/cluster/ClusterTask.groovy b/src/main/groovy/io/xh/hoist/cluster/ClusterTask.groovy index 02de2fdc..0e5c7b89 100644 --- a/src/main/groovy/io/xh/hoist/cluster/ClusterTask.groovy +++ b/src/main/groovy/io/xh/hoist/cluster/ClusterTask.groovy @@ -8,6 +8,7 @@ package io.xh.hoist.cluster import io.xh.hoist.BaseService import io.xh.hoist.log.LogSupport +import io.xh.hoist.user.HoistIdentity import io.xh.hoist.util.Utils import java.util.concurrent.Callable @@ -45,8 +46,7 @@ class ClusterTask implements Callable, LogSupport { } ClusterResult call() { - identityService.threadUsername.set(username) - identityService.threadAuthUsername.set(authUsername) + identityService.installThreadIdentity(new HoistIdentity(username, authUsername)) try (def traceScope = traceContextService.restoreContextFromTraceparent(traceparent)) { clusterService.ensureRunning() @@ -65,8 +65,7 @@ class ClusterTask implements Callable, LogSupport { ) return new ClusterResult(exception: new ClusterTaskException(t)) } finally { - identityService.threadUsername.remove() - identityService.threadAuthUsername.remove() + identityService.installThreadIdentity(null) } } } diff --git a/src/main/groovy/io/xh/hoist/security/BaseAuthenticationService.groovy b/src/main/groovy/io/xh/hoist/security/BaseAuthenticationService.groovy index 8d578641..0891a77e 100644 --- a/src/main/groovy/io/xh/hoist/security/BaseAuthenticationService.groovy +++ b/src/main/groovy/io/xh/hoist/security/BaseAuthenticationService.groovy @@ -105,7 +105,7 @@ abstract class BaseAuthenticationService extends BaseService { */ boolean allowRequest(HttpServletRequest request, HttpServletResponse response) { try { - if (identityService.findAuthUser(request) || isWhitelist(request)) { + if (identityService.authUser || isWhitelist(request)) { return true } @@ -113,7 +113,7 @@ abstract class BaseAuthenticationService extends BaseService { return false } - if (!identityService.findAuthUser(request)) { + if (!identityService.authUser) { throw new NotAuthenticatedException() } diff --git a/src/main/groovy/io/xh/hoist/user/HoistIdentity.groovy b/src/main/groovy/io/xh/hoist/user/HoistIdentity.groovy new file mode 100644 index 00000000..7607dde2 --- /dev/null +++ b/src/main/groovy/io/xh/hoist/user/HoistIdentity.groovy @@ -0,0 +1,32 @@ +/* + * This file belongs to Hoist, an application development toolkit + * developed by Extremely Heavy Industries (www.xh.io | info@xh.io) + * + * Copyright © 2026 Extremely Heavy Industries Inc. + */ + +package io.xh.hoist.user + +import groovy.transform.CompileStatic +import groovy.transform.Immutable + +/** + * Immutable snapshot of the identity associated with a thread of execution. + * + * Held in a {@link ThreadLocal} by {@link IdentityService} as the authoritative per-thread + * identity source. Sourced from the HTTP session on the request thread, captured and + * propagated across async boundaries (Grails {@code task {}}, {@code ClusterTask}) so that + * code running on worker threads can resolve the originating user without touching the + * (potentially recycled) servlet request. + * + * Either or both fields may be null if no user is associated with the thread. + */ +@CompileStatic +@Immutable +class HoistIdentity { + /** Apparent user — the user the app appears to be running as (impersonated user, if any). */ + String username + + /** Authenticated user — always the actual logged-in user, regardless of impersonation. */ + String authUsername +} diff --git a/src/main/groovy/io/xh/hoist/user/IdentityPropagatingPromiseFactory.groovy b/src/main/groovy/io/xh/hoist/user/IdentityPropagatingPromiseFactory.groovy new file mode 100644 index 00000000..e1e58bce --- /dev/null +++ b/src/main/groovy/io/xh/hoist/user/IdentityPropagatingPromiseFactory.groovy @@ -0,0 +1,88 @@ +/* + * This file belongs to Hoist, an application development toolkit + * developed by Extremely Heavy Industries (www.xh.io | info@xh.io) + * + * Copyright © 2026 Extremely Heavy Industries Inc. + */ +package io.xh.hoist.user + +import grails.async.Promise +import grails.async.PromiseFactory +import groovy.transform.CompileStatic + +/** + * Delegating {@link PromiseFactory} that propagates the originating thread's + * {@link HoistIdentity} to worker threads spawned by Grails {@code task {}} calls. + * + * Installed once at startup by {@link IdentityService}. Captures identity at task creation + * time (still on the originating thread) and installs it on the worker before the wrapped + * closure runs, then restores the prior value in a {@code finally}. With no identity to + * propagate (e.g. system background threads), the wrapping is a no-op. + * + * Composes with {@link io.xh.hoist.telemetry.trace.ContextPropagatingPromiseFactory}: both + * decorate the underlying factory, each propagating its own slice of context. + */ +@CompileStatic +class IdentityPropagatingPromiseFactory implements PromiseFactory { + + @Delegate PromiseFactory delegate + private final IdentityService identityService + + IdentityPropagatingPromiseFactory(PromiseFactory delegate, IdentityService identityService) { + this.delegate = delegate + this.identityService = identityService + } + + @Override + Promise createPromise(Closure... closures) { + delegate.createPromise(wrapClosures(closures as List>) as Closure[]) as Promise + } + + @Override + Promise createPromise(Closure closure, List decorators) { + delegate.createPromise(wrapClosure(closure), decorators) + } + + @Override + Promise> createPromise(List> closures) { + delegate.createPromise(wrapClosures(closures)) + } + + @Override + Promise> createPromise(List> closures, List decorators) { + delegate.createPromise(wrapClosures(closures), decorators) + } + + @Override + Promise> createPromise(Map> promises) { + delegate.createPromise(wrapPromises(promises)) as Promise> + } + + @Override + Promise> createPromise(Map> promises, List decorators) { + delegate.createPromise(wrapPromises(promises), decorators) as Promise> + } + + //-------------------------- + // Implementation + //-------------------------- + private Closure wrapClosure(Closure closure) { + def captured = identityService.threadIdentity.get() + return { -> + identityService.installThreadIdentity(captured) + try { + return closure.call() + } finally { + identityService.installThreadIdentity(null) + } + } as Closure + } + + private List> wrapClosures(List> closures) { + closures.collect { wrapClosure(it) } + } + + private Map> wrapPromises(Map> promises) { + promises.collectEntries { k, v -> [k, wrapClosure(v)] } as Map> + } +} diff --git a/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketChannel.groovy b/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketChannel.groovy index bbe16038..811f2704 100644 --- a/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketChannel.groovy +++ b/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketChannel.groovy @@ -12,7 +12,6 @@ import io.xh.hoist.cluster.ClusterService import io.xh.hoist.json.JSONFormat import io.xh.hoist.log.LogSupport import io.xh.hoist.user.HoistUser -import io.xh.hoist.user.IdentityService import org.springframework.util.MultiValueMap import org.springframework.web.socket.CloseStatus import org.springframework.web.socket.TextMessage @@ -23,6 +22,7 @@ import org.springframework.web.util.UriComponentsBuilder import java.time.Instant import static io.xh.hoist.util.Utils.configService +import static io.xh.hoist.util.Utils.identityService import static io.xh.hoist.util.Utils.userService import static java.util.UUID.randomUUID @@ -61,8 +61,8 @@ class HoistWebSocketChannel implements JSONFormat, LogSupport { logDebug("Creating managed socket session", [sendTimeLimit: sendTimeLimit, bufferSizeLimit: bufferSizeLimit]) session = new ConcurrentWebSocketSessionDecorator(webSocketSession, sendTimeLimit, bufferSizeLimit) - authUsername = getAuthUsernameFromSession() - apparentUsername = getApparentUsernameFromSession() + authUsername = identityService.authUsername ?: 'unknownUser' + apparentUsername = identityService.username ?: 'unknownUser' appVersion = queryParams.getFirst('appVersion') appBuild = queryParams.getFirst('appBuild') loadId = queryParams.getFirst('loadId') @@ -101,14 +101,6 @@ class HoistWebSocketChannel implements JSONFormat, LogSupport { //------------------------ // Implementation //------------------------ - private String getAuthUsernameFromSession() { - return (String) session.attributes[IdentityService.AUTH_USER_KEY] ?: 'unknownUser' - } - - private String getApparentUsernameFromSession() { - return (String) session.attributes[IdentityService.APPARENT_USER_KEY] ?: 'unknownUser' - } - private MultiValueMap getQueryParams(URI uri) { UriComponentsBuilder.fromUri(uri).build().queryParams } diff --git a/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketHandler.groovy b/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketHandler.groovy index a662d388..58c521fb 100644 --- a/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketHandler.groovy +++ b/src/main/groovy/io/xh/hoist/websocket/HoistWebSocketHandler.groovy @@ -12,29 +12,47 @@ import org.springframework.web.socket.CloseStatus import org.springframework.web.socket.TextMessage import org.springframework.web.socket.WebSocketSession import org.springframework.web.socket.handler.TextWebSocketHandler + +import static io.xh.hoist.util.Utils.identityService import static io.xh.hoist.util.Utils.getWebSocketService /** * Helper class to relay events from the Spring websocket infrastructure to the Hoist * WebSocketService. Must be wired by the main Application.groovy class - see XH-provided * template apps for examples. + * + * Installs the connection's identity onto the current thread for the duration of each + * lifecycle callback, mirroring the HTTP filter pattern. Downstream code (channel + * construction, message handlers, etc.) can therefore resolve identity through the + * standard {@code identityService} accessors. */ @CompileStatic class HoistWebSocketHandler extends TextWebSocketHandler { @Override void afterConnectionEstablished(WebSocketSession session) throws Exception { - webSocketService.registerSession(session) + withIdentity(session) { webSocketService.registerSession(session) } } @Override void handleTextMessage(WebSocketSession session, TextMessage message) throws Exception { - webSocketService.onMessage(session, message) + withIdentity(session) { webSocketService.onMessage(session, message) } } @Override void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception { - webSocketService.unregisterSession(session, closeStatus) + withIdentity(session) { webSocketService.unregisterSession(session, closeStatus) } } + //------------------------ + // Implementation + //------------------------ + private static void withIdentity(WebSocketSession session, Closure block) { + identityService.installIdentityFromWebSocketSession(session) + try { + block() + } finally { + identityService.installThreadIdentity(null) + } + } }