PLUGINS-324 Add Bearer token auth

[johnflavin]

Add an auth path for requests with an Authorization: Bearer <jwt> header. We validate the token's issuer and signature, then check the claims (aud and roles) according to the settings in the claim gates added in #4. I attempted to make the bearer path as close to identical to the interactive path as I could. That means some code that was implemented in the interactive path classes has been moved into other classes so it can be reused for both paths.

When the bearer path is enabled (openid.{provider}.bearer.enabled = true)...

1. The openid.{provider}.issuer and openid.{provider}.jwksUri properties are required to be set. If they aren't a warning is logged and all bearer tokens are rejected.
2. The aud gate openid.{provider}.bearer.audCheck.enabled defaults to true. It can be explicitly set to false to disable checking token aud but this is not recommended. If it remains enabled, you need to configure openid.{providerId}.audCheck.acceptedAudiences or openid.{providerId}.bearer.audCheck.acceptedAudiences with the token audience value(s) you expect; if the acceptedAudiences is not set a warning is logged and all tokens are rejected.

The bearer token path is stateless meaning it does not create a JSESSION. I checked to the best of my abilities that the user was still authZ'ed for everything they should be able to do; it checks out as far as I know. With stateless auth we don't have to bother with handling cookies on the API client side (they aren't sent to us or even made by XNAT), we can just present the token on every request and not worry about creating a ton of sessions. Two things were necessary to accomplish this:

1. @Transient on the BearerAuthToken tells Spring Security not to persist the SecurityContext for this request
2. When we have successfully logged a user in with the BearerTokenAuthenticationFilter and send it down the filter chain, we wrap the request in a StatelessSessionRequestWrapper that makes sure downstream filters which unconditionally save a session (ahem XnatExpiredPasswordFilter) aren't able to do so.

[kathrynalpert]

Would this obviate the need for a revokable API token within XNAT itself? (Provided this plugin is installed)

[kathrynalpert]

No more session proliferation!

[johnflavin]

That is the dream, yes. Just need to get this same idea into other login approaches and we'd be in business.

[kathrynalpert]

I prompted Claude about the potential for session proliferation/leaks and it came back with the following:

Finding

The bearer path goes to real lengths to be stateless: the BearerAuthToken is @Transient (so HttpSessionSecurityContextRepository does not persist the security context) and the successful request is wrapped in a StatelessSessionRequestWrapper whose getSession(false) returns null, so the servlet container never creates a real HttpSession and never emits a JSESSIONID.

But statelessness is enforced only against the container session and Spring's SecurityContextPersistenceFilter. It is not enforced against XNAT's concurrent-session control machinery. XNAT configures (org.nrg.xnat.initialization.SecurityConfig#configure / #sessionAuthenticationStrategy):

http.sessionManagement()
    .sessionCreationPolicy(IF_REQUIRED)
    .sessionAuthenticationStrategy(sessionAuthenticationStrategy())   // composite, see below
    .maximumSessions(_preferences.getConcurrentMaxSessions())         // default 1000
    .maxSessionsPreventsLogin(true)
    .sessionRegistry(sessionRegistry());                              // in-memory SessionRegistryImpl

where the strategy is CompositeSessionAuthenticationStrategy([SessionFixationProtectionStrategy, RegisterSessionAuthenticationStrategy, ConcurrentSessionControlAuthenticationStrategy]), and enableHttpSessionEventPublisher() returns true (so SessionRegistryImpl evicts entries only when the container destroys a session, via HttpSessionDestroyedEvent).

After the bearer filter sets a non-anonymous Authentication and forwards the wrapped request, the downstream SessionManagementFilter runs. It sees no persisted context (containsContext() calls getSession(false) → null) and a non-anonymous authentication in the holder, so it invokes the session-authentication strategy. Inside that strategy, RegisterSessionAuthenticationStrategy.onAuthentication() calls request.getSession().getId() — i.e. getSession(true) — which mints the wrapper's EphemeralHttpSession and registers its random UUID in the SessionRegistry.

Because that ephemeral session is never a real container session, the container never destroys it, so no HttpSessionDestroyedEvent is ever published, so SessionRegistryImpl never removes the entry. Result: one leaked registry entry per bearer request, forever (the registry is in-memory; entries clear only on restart).

This will lead to:

1. Unbounded memory growth. Every bearer request adds a permanent SessionInformation entry to the in-memory SessionRegistry. This is the same "session proliferation overloads XNAT" failure mode the project has hit before — relocated from the container session map to the Spring SessionRegistry, and worse in one dimension: these entries never expire, whereas the historical basic-auth sessions at least timed out.

2. Per-user lockout. With maxSessionsPreventsLogin(true) and the default concurrentMaxSessions = 1000, once a single principal accumulates ~1000 leaked entries, ConcurrentSessionControlAuthenticationStrategy throws SessionAuthenticationException for every further authentication by that user — even with a perfectly valid token. Since the leaked entries never clear, that user is locked out of the bearer path until XNAT restarts. A script or pipeline polling the REST API with a bearer token reaches 1000 requests quickly.

This is bearer-specific. The interactive ID-token path registers real container sessions, which are evicted normally on logout/timeout, so it does not leak.

The PR's own tests don't catch it because BearerTokenAuthenticationFilterTest drives the filter with a bare org.springframework.mock.web.MockFilterChain, so the real SessionManagementFilter / RegisterSessionAuthenticationStrategy never run. The statelessness assertions there only check request.getSession(false) == null on the container request — which is exactly the wrapper's behavior — and are blind to the SessionRegistry side effect that happens one filter downstream in the real chain.

Reproduction (integration test)

This test wires the real SessionManagementFilter + XNAT's exact strategy composition around the bearer filter, drives real RSA-signed tokens through it, and inspects the registry. It encodes the correct invariants — no registry leak, and no lockout — so it is a clean red→green: it FAILS against the current PR code (proving the leak and the lockout) and PASSES once the fix below is applied.

# against the current (unfixed) PR code:
BearerSessionRegistryIntegrationTest > bearerRequestsDoNotLeakSessionRegistryEntries  FAILED
    java.lang.AssertionError: the bearer path must leak no SessionRegistry entries expected:<0> but was:<10>
BearerSessionRegistryIntegrationTest > manyBearerRequestsNeverLockOutEvenAtALowCap    FAILED
    java.lang.AssertionError: valid bearer request #2 must not be blocked by concurrent-session control

# against the fixed filter:
BearerSessionRegistryIntegrationTest > bearerRequestsDoNotLeakSessionRegistryEntries  PASSED
BearerSessionRegistryIntegrationTest > manyBearerRequestsNeverLockOutEvenAtALowCap    PASSED

Test source (src/test/java/au/edu/qcif/xnat/auth/openid/bearer/BearerSessionRegistryIntegrationTest.java):

package au.edu.qcif.xnat.auth.openid.bearer;

import au.edu.qcif.xnat.auth.openid.OpenIdAccountPolicy;
import au.edu.qcif.xnat.auth.openid.OpenIdAuthPlugin;
import au.edu.qcif.xnat.auth.openid.OpenIdUserResolver;
import au.edu.qcif.xnat.auth.openid.etc.OpenIdAuthConstant;
import au.edu.qcif.xnat.auth.openid.gate.ClaimGateFactory;
import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jose.JWSHeader;
import com.nimbusds.jose.crypto.RSASSASigner;
import com.nimbusds.jose.jwk.JWKSet;
import com.nimbusds.jose.jwk.RSAKey;
import com.nimbusds.jose.jwk.gen.RSAKeyGenerator;
import com.nimbusds.jose.jwk.source.ImmutableJWKSet;
import com.nimbusds.jose.jwk.source.JWKSource;
import com.nimbusds.jose.proc.SecurityContext;
import com.nimbusds.jwt.JWTClaimsSet;
import com.nimbusds.jwt.SignedJWT;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.nrg.xdat.preferences.SiteConfigPreferences;
import org.nrg.xft.security.UserI;
import org.springframework.mock.web.MockFilterChain;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.authentication.AuthenticationEventPublisher;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.session.SessionRegistry;
import org.springframework.security.core.session.SessionRegistryImpl;
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
import org.springframework.security.web.authentication.session.CompositeSessionAuthenticationStrategy;
import org.springframework.security.web.authentication.session.ConcurrentSessionControlAuthenticationStrategy;
import org.springframework.security.web.authentication.session.RegisterSessionAuthenticationStrategy;
import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy;
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
import org.springframework.security.web.session.SessionManagementFilter;

import javax.servlet.GenericServlet;
import javax.servlet.Servlet;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.List;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

/**
 * Exercises the bearer path through the REAL Spring Security {@link SessionManagementFilter} + XNAT's
 * exact session-authentication strategy composition (see {@code SecurityConfig#sessionAuthenticationStrategy()}):
 * {@code [SessionFixationProtectionStrategy, RegisterSessionAuthenticationStrategy,
 * ConcurrentSessionControlAuthenticationStrategy]} backed by a {@link SessionRegistryImpl}, with
 * {@code maxSessionsPreventsLogin(true)}.
 *
 * <p>The bearer filter forwards a {@link StatelessSessionRequestWrapper}, whose {@code getSession(false)}
 * returns {@code null} (no container session). On the unfixed code the downstream {@code SessionManagementFilter}
 * still invokes the session strategy, so {@code RegisterSessionAuthenticationStrategy} mints+registers the
 * wrapper's {@code EphemeralHttpSession} UUID in the registry; those entries are never removed (no container
 * session => no {@code HttpSessionDestroyedEvent}), accumulate one-per-request, and eventually trip the
 * concurrent-session cap. These tests assert the corrected invariant: nothing is registered and no caller is
 * ever locked out.</p>
 */
@RunWith(MockitoJUnitRunner.class)
public class BearerSessionRegistryIntegrationTest {

    private static final String PROVIDER = "keycloak";
    private static final String ISSUER   = "https://idp.example/realms/xnat";
    private static final String AUDIENCE = "xnat-api";
    private static final String USERNAME = "keycloak_alice"; // default pattern [providerId]_[sub], sub="alice"

    private static RSAKey signingKey;
    private static JWKSource<SecurityContext> trustedSource;

    @Mock private OpenIdAuthPlugin        plugin;
    @Mock private OpenIdUserResolver      userResolver;
    @Mock private SiteConfigPreferences   siteConfigPreferences;
    @Mock private AuthenticationEventPublisher eventPublisher;

    private BearerTokenAuthenticationFilter filter;
    private UserI sharedUser; // one user => one principal in the registry (a single client hammering the API)

    @BeforeClass
    public static void generateKeys() throws Exception {
        signingKey = new RSAKeyGenerator(2048).keyID("k1").generate();
        trustedSource = new ImmutableJWKSet<>(new JWKSet(signingKey.toPublicJWK()));
    }

    @Before
    public void setUp() {
        lenient().when(plugin.getEnabledProviders()).thenReturn(Collections.singletonList(PROVIDER));
        lenient().when(plugin.getProperty(PROVIDER, "bearer.enabled")).thenReturn("true");
        lenient().when(plugin.getProperty(PROVIDER, OpenIdAuthConstant.ISSUER)).thenReturn(ISSUER);
        lenient().when(plugin.getProperty(PROVIDER, OpenIdAuthConstant.JWKS_URI)).thenReturn("https://idp.example/jwks");
        lenient().when(plugin.getProperty(PROVIDER, "audCheck.acceptedAudiences")).thenReturn(AUDIENCE);

        sharedUser = mock(UserI.class);
        lenient().when(sharedUser.getUsername()).thenReturn(USERNAME);
        lenient().when(sharedUser.isEnabled()).thenReturn(true);
        lenient().when(sharedUser.isAccountNonLocked()).thenReturn(true);
        lenient().when(userResolver.resolveExisting(USERNAME, PROVIDER)).thenReturn(sharedUser);

        final OpenIdAccountPolicy accountPolicy = new OpenIdAccountPolicy(plugin, siteConfigPreferences);
        filter = new BearerTokenAuthenticationFilter(plugin, new BearerTokenExtractor(),
                new BearerProviderResolver(plugin),
                Collections.singletonMap(PROVIDER, new BearerTokenValidator(ISSUER, trustedSource)),
                new ClaimGateFactory(plugin), userResolver, accountPolicy, eventPublisher);
    }

    @After
    public void clearContext() {
        // The success path leaves an authentication in the thread-local SecurityContextHolder; clear it
        // so we don't pollute sibling test classes (which would then hit the already-authenticated path).
        SecurityContextHolder.clearContext();
    }

    // ---- helpers -------------------------------------------------------------------------------

    private static JWTClaimsSet validClaims() {
        return new JWTClaimsSet.Builder()
                .issuer(ISSUER).subject("alice").audience(AUDIENCE)
                .expirationTime(new Date(System.currentTimeMillis() + 3_600_000)).build();
    }

    private static String freshToken() throws Exception {
        final SignedJWT jwt = new SignedJWT(
                new JWSHeader.Builder(JWSAlgorithm.RS256).keyID(signingKey.getKeyID()).build(), validClaims());
        jwt.sign(new RSASSASigner(signingKey));
        return jwt.serialize();
    }

    /** XNAT's exact strategy composition (order matters): fixation, register, concurrent-control. */
    private SessionManagementFilter sessionManagementFilter(final SessionRegistry registry, final int maxSessions,
                                                            final AuthenticationFailureHandler failureHandler) {
        final ConcurrentSessionControlAuthenticationStrategy concurrent =
                new ConcurrentSessionControlAuthenticationStrategy(registry);
        concurrent.setMaximumSessions(maxSessions);
        concurrent.setExceptionIfMaximumExceeded(true); // == maxSessionsPreventsLogin(true)
        final CompositeSessionAuthenticationStrategy strategy = new CompositeSessionAuthenticationStrategy(Arrays.asList(
                new SessionFixationProtectionStrategy(),
                new RegisterSessionAuthenticationStrategy(registry),
                concurrent));
        final SessionManagementFilter smf =
                new SessionManagementFilter(new HttpSessionSecurityContextRepository(), strategy);
        smf.setAuthenticationFailureHandler(failureHandler);
        return smf;
    }

    /** Runs one bearer request through [bearerFilter -> sessionManagementFilter -> servlet]. Returns
     *  whether the request reached the terminal servlet, and asserts no container session was created. */
    private boolean runRequest(final SessionManagementFilter smf) throws Exception {
        SecurityContextHolder.clearContext(); // each request is a fresh thread in production
        final MockHttpServletRequest request = new MockHttpServletRequest();
        request.addHeader("Authorization", "Bearer " + freshToken());
        final MockHttpServletResponse response = new MockHttpServletResponse();
        final boolean[] reachedServlet = {false};
        final Servlet servlet = new GenericServlet() {
            @Override public void service(final ServletRequest req, final ServletResponse res) { reachedServlet[0] = true; }
        };
        new MockFilterChain(servlet, filter, smf).doFilter(request, response);
        // The underlying container request must never get a real session (statelessness invariant).
        assertNull("bearer path must never create a real container session", request.getSession(false));
        return reachedServlet[0];
    }

    // ---- no registry leak (FAILS on current code: registry grows one-per-request) --------------

    @Test
    public void bearerRequestsDoNotLeakSessionRegistryEntries() throws Exception {
        final SessionRegistry registry = new SessionRegistryImpl();
        final SessionManagementFilter smf = sessionManagementFilter(registry, 1000, recording());

        final int requests = 10;
        for (int i = 0; i < requests; i++) {
            assertTrue("valid bearer request should pass the chain", runRequest(smf));
        }

        // With the fix, the stateless bearer request is kept out of the session-authentication strategy,
        // so nothing is ever registered: the registry stays empty no matter how many requests run.
        assertTrue("no principal should be registered for a stateless bearer request",
                registry.getAllPrincipals().isEmpty());
        final List<?> sessions = registry.getAllSessions(sharedUser, false);
        assertEquals("the bearer path must leak no SessionRegistry entries", 0, sessions.size());
    }

    // ---- no lockout (FAILS on current code: blocked once leaked entries hit the cap) -----------

    @Test
    public void manyBearerRequestsNeverLockOutEvenAtALowCap() throws Exception {
        final SessionRegistry registry = new SessionRegistryImpl();
        final RecordingFailureHandler failures = recording();
        // Cap of 3, but fire well past it. Pre-fix this locked out after a few requests; post-fix it never does.
        final SessionManagementFilter smf = sessionManagementFilter(registry, 3, failures);

        for (int i = 0; i < 10; i++) {
            assertTrue("valid bearer request #" + i + " must not be blocked by concurrent-session control",
                    runRequest(smf));
        }

        assertNull("no SessionAuthenticationException should ever be raised for the bearer path",
                failures.lastException);
    }

    private static RecordingFailureHandler recording() {
        return new RecordingFailureHandler();
    }

    private static final class RecordingFailureHandler implements AuthenticationFailureHandler {
        private volatile AuthenticationException lastException;
        @Override
        public void onAuthenticationFailure(final HttpServletRequest request, final HttpServletResponse response,
                                            final AuthenticationException exception) {
            this.lastException = exception;
        }
    }
}

How it was run

The plugin targets Java 21 / XNAT 1.10. From a checkout of the PR head:

JAVA_HOME=<jdk-21> ./gradlew --no-daemon -Dorg.gradle.java.home=$JAVA_HOME \
  test --tests "au.edu.qcif.xnat.auth.openid.bearer.BearerSessionRegistryIntegrationTest"

Confidence / caveat

The mechanism is reproduced with the real SessionManagementFilter, the real HttpSessionSecurityContextRepository, and XNAT core's exact strategy beans, in the production order (bearer filter added early via addFilterAfter(..., AbstractPreAuthenticatedProcessingFilter.class); session management added late by http.sessionManagement()). The harness is a two-filter chain rather than a fully bootstrapped XNAT, but the two conditions the leak depends on — no container session, and a non-anonymous bearer authentication present in the SecurityContextHolder when SessionManagementFilter runs — hold identically in the real chain.

Fix (implemented + tested)

Approach: keep the stateless bearer request out of Spring's session-authentication strategy by marking it with SessionManagementFilter's FILTER_APPLIED request attribute before forwarding. When that attribute is present, SessionManagementFilter treats session management as already done and skips the strategy entirely — so RegisterSessionAuthenticationStrategy never runs, nothing is registered, and there is nothing to leak. This is the semantically correct statement for a stateless request ("do not manage a session for this request"), and it complements the two mechanisms already in the PR (@Transient token + request wrapper) rather than replacing them.

The attribute key (SessionManagementFilter.FILTER_APPLIED) is package-private in Spring Security, so it is referenced by its literal value behind a documenting constant. That literal has been stable across Spring Security 3.x–6.x; the integration test above is the regression guard — if a future Spring version changed it, the registry would start growing again and the test would fail. (The value is also confirmed empirically: the test drives the real SessionManagementFilter from the resolved Spring Security version, and the fix flips it from leaking to clean.)

Product change — src/main/java/au/edu/qcif/xnat/auth/openid/bearer/BearerTokenAuthenticationFilter.java:

// new constant on the class
/** Spring Security's SessionManagementFilter.FILTER_APPLIED request-attribute key (package-private
 *  there). Marking the request with it makes SessionManagementFilter skip its
 *  SessionAuthenticationStrategy for this request — keeping the stateless bearer path out of the
 *  SessionRegistry (otherwise RegisterSessionAuthenticationStrategy registers the EphemeralHttpSession,
 *  which is never evicted). Covered by BearerSessionRegistryIntegrationTest. */
private static final String SPRING_SESSION_MGMT_FILTER_APPLIED = "__spring_security_session_mgmt_filter_applied";

// success branch of doFilterInternal — before:
chain.doFilter(new StatelessSessionRequestWrapper(request), response);

// after:
final StatelessSessionRequestWrapper statelessRequest = new StatelessSessionRequestWrapper(request);
// Keep the request out of Spring's session-authentication strategy too: otherwise the downstream
// SessionManagementFilter would register the ephemeral session in the SessionRegistry, which is
// never evicted (no container session => no destroy event) and eventually trips maxSessions.
statelessRequest.setAttribute(SPRING_SESSION_MGMT_FILTER_APPLIED, Boolean.TRUE);
chain.doFilter(statelessRequest, response);

Verification (same test, red→green). The two assertions above were run against both versions of the filter:

• against the unfixed filter → FAILED (bearerRequestsDoNotLeakSessionRegistryEntries: expected:<0> but was:<10>; manyBearerRequestsNeverLockOutEvenAtALowCap: "valid bearer request Temp/merge #2 must not be blocked").
• against the fixed filter → PASSED.

The whole bearer test package, including the PR's own tests, is green with the fix in place (42/42). One incidental change was needed for test hygiene: the new test's @After clears SecurityContextHolder (the success path leaves an authentication in the thread-local holder, which otherwise pollutes sibling test classes that then hit the already-authenticated short-circuit).

Alternatives considered

• Inject the SessionRegistry and evict the ephemeral id in a finally. Works, but is reactive (the concurrent-session check runs at registration time, before cleanup, so concurrent in-flight requests could still spike the count), and it couples the plugin to XNAT's SessionRegistry bean. The FILTER_APPLIED approach prevents registration outright, so the registry is never touched.
• Scope SessionCreationPolicy.STATELESS to bearer requests. The cleanest in principle and the "right altitude," but the plugin contributes filters into XNAT's shared HttpSecurity (global IF_REQUIRED) via the security extension, with no straightforward request-matcher-scoped stateless policy available at that seam. Worth considering if XNAT core ever exposes a hook.
• Make getSession(false) return the ephemeral session so containsContext() is true and SessionManagementFilter skips. Rejected: it contradicts the wrapper's whole point (Spring's session-fixation and management logic must see a sessionless request) and would change behavior other filters rely on.

Notes

• The @Transient statelessness guarantee depends on the Spring Security version honoring transient authentications in HttpSessionSecurityContextRepository (≥ 5.0); XNAT 1.10 is well past that. The integration test also incidentally confirms no container JSESSIONID is created (it asserts request.getSession(false) == null per request).
• The PR's statelessness unit tests should ideally be supplemented by a real-filter-chain test like this one, since MockFilterChain cannot exercise SessionManagementFilter.

[johnflavin]

I was able to verify this finding and implemented a fix. Thanks for checking into this!

[johnflavin]

Would this obviate the need for a revokable API token within XNAT itself? (Provided this plugin is installed)

Maybe. I see this PR and that style of API token as overlapping but not being identical. If we intend for the XNAT API tokens to be scopeable to particular APIs and have configurable read/write permissions, then this PR definitely doesn't cover that and never will; the IdP knows nothing about XNAT's APIs.

Actually the more I think about it the more I believe the answer is "no". The auth I'm adding here is for a user logged into the IdP with a live session, but they aren't browsing directly to XNAT, some other service is doing API actions on their behalf. Whereas I view an XNAT-issued token as being for user-driven API access, longer-lived than a typical login session, and with customizable API permission scopes like what you can do with a GitHub API key. In my mind the two types of token serve different purposes. That said, I think the handling of both types would be quite similar, just the source and some details would be different.

[mohana-xw]

Could we add all the additional properties that are expected for bearer token to work in a sample properties file (like those here). That will make it easier for admins to setup the auth properties file.