Skip to content

SystemAuthenticator.begin()/end() affect other threads of the same HTTP session - #5704

Open
knstvk wants to merge 3 commits into
release_3_0from
5693-system-auth
Open

knstvk wants to merge 3 commits into
release_3_0from
5693-system-auth

Conversation

@knstvk

@knstvk knstvk commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a privilege escalation in SystemAuthenticator: begin()/end() modified the SecurityContext instance shared by all threads of an HTTP session, so a withSystem block in a background task made concurrent UI requests of the same session run as system, and two overlapping blocks could leave the session as system until logout. Authentication switches now affect the current thread only. Closes #5693.

What was done

  • SystemAuthenticator.begin() now installs a new SecurityContext for the current thread and end() reinstalls the previous instance. The previously current context is never modified (module core).
  • Added ThreadSecurityContextOverride, an optional capability of a SecurityContextHolderStrategy that lets SystemAuthenticator push a context that takes precedence for the current thread (module core), and SecurityContextHelper.installContext()/restoreContext() as the shared way to switch and restore a thread's context.
  • Added JmixSecurityContextHolderStrategy (module flowui), which wraps Vaadin's VaadinAwareSecurityContextHolderStrategy and implements the override. It is registered by FlowuiAutoConfiguration as the SecurityContextHolderStrategy bean, so Vaadin's own bean backs off and Vaadin's initializer installs the Jmix one.
  • UiAsyncTasks wrappers (DelegatingSecurityRunnable, DelegatingSecuritySupplier), BackgroundWorkerImpl and JmixOfficeIntegration no longer share or modify the session's context instance. Background task progress and done handlers always run under the authentication the task was started with.
  • UiEventPublisher runs handlers in a recipient's session under the security context stored in that session, resolved when the queued session.access() command actually runs, and falls back to runWithUser only when no context is stored.

How it works

Vaadin's strategy returns the HTTP session's SecurityContext whenever a Vaadin session is current, and ignores setContext(), so a plain thread-local swap would make withSystem a no-op on UI threads. The Jmix strategy keeps a per-thread stack of override contexts: getContext() returns the top of the stack when it is non-empty and delegates to Vaadin otherwise. begin() pushes a fresh context with the system or user authentication, end() pops it. setContext() inside an override replaces the top entry, so inline security wrappers keep the enclosing withSystem scope intact, and clearContext() drops the whole stack, so the request filter and the async wrappers cannot leak an unbalanced begin() to the next unit of work on a pooled thread. Without the Jmix strategy, for example in REST-only applications, SystemAuthenticator falls back to setContext(), which is sufficient for the default thread-local strategy.

How to use

No changes are required in applications. SystemAuthenticator.withSystem(), withUser(), begin() and end() keep their API and now behave as documented, affecting the current thread only.

Compatibility

Existing applications are not affected unless they define their own SecurityContextHolderStrategy bean, in which case the Jmix bean is not registered (@ConditionalOnMissingBean) and SystemAuthenticator falls back to plain setContext(). SecurityContextHelper.setAuthentication() still modifies the current context instance for its existing callers; its javadoc now warns about that and points to SystemAuthenticator.

@knstvk
knstvk marked this pull request as draft September 21, 2026 10:47
- Preserve enclosing authentication scopes when replacing contexts
- Install recipient authentication inside queued UI callbacks
- Clear unused worker contexts after asynchronous tasks
- Add regression tests for nested wrappers and thread cleanup
@knstvk
knstvk marked this pull request as ready for review September 21, 2026 12:02
@knstvk
knstvk requested a review from glebfox September 21, 2026 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SystemAuthenticator.begin()/end() affect other threads of the same HTTP session

2 participants