Add BearerTokenAuthenticationConverter

[franticticktick]

Closes gh-14750

[jzheaux]

Will you please change BearerTokenAuthenticationFilter to accept an AuthenticationConverter? Also, OAuth2ResourceServerConfigurer should configure this in the same way as BearerTokenResolver. Will you please add that?

Finally, please add tests both for the new authentication converter and corresponding tests in OAuth2ResourceServerConfigurerTests.

[jzheaux]

I'd prefer to keep these object hierarchies separate. It reduces the possibility of package tangles. Will you please remove this abstract class?

[franticticktick]

Hi @jzheaux! Thanks for your feedback! I would like some advice on the best way to do this. I can add AuthenticationConverter to BearerTokenAuthenticationFilter and change doFilterInternal method:

        @Override
	protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
			throws ServletException, IOException {
		try {
			Authentication authentication = authenticationConverter.convert(request);
			if (authentication == null) {
				this.logger.trace("Did not process request since did not find bearer token");
				filterChain.doFilter(request, response);
				return;
			}
			AuthenticationManager authenticationManager = this.authenticationManagerResolver.resolve(request);
			Authentication authenticationResult = authenticationManager.authenticate(authentication);
			SecurityContext context = this.securityContextHolderStrategy.createEmptyContext();
			context.setAuthentication(authenticationResult);
			this.securityContextHolderStrategy.setContext(context);
			this.securityContextRepository.saveContext(context, request, response);
			if (this.logger.isDebugEnabled()) {
				this.logger.debug(LogMessage.format("Set SecurityContextHolder to %s", authenticationResult));
			}
			filterChain.doFilter(request, response);
		} catch (AuthenticationException failed) {
			this.securityContextHolderStrategy.clearContext();
			this.logger.trace("Failed to process authentication request", failed);
			this.authenticationFailureHandler.onAuthenticationFailure(request, response, failed);
		}
	}

But there are several problems.
Firstly, it is necessary to save backward compatibility with BearerTokenResolver, which were set through OAuth2ResourceServerConfigurer, for example:

SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
			// @formatter:off
			http
					.authorizeRequests()
					.anyRequest().authenticated()
					.and()
					.oauth2ResourceServer()
					.bearerTokenResolver(customResolver())
					.jwt();
			return http.build();

Even if i declare method bearerTokenResolver as deprecated it should still work. And it seems hard to do, i could try adding a BearerTokenResolverAuthenticationConverterAdapter which would do something like this:

	private static final class BearerTokenResolverAuthenticationConverterAdapter implements AuthenticationConverter {

		private final BearerTokenResolver bearerTokenResolver;

		BearerTokenResolverAuthenticationConverterAdapter(BearerTokenResolver bearerTokenResolver) {
			Assert.notNull(bearerTokenResolver, "bearerTokenResolver cant be null");
			this.bearerTokenResolver = bearerTokenResolver;
		}

		@Override
		public Authentication convert(HttpServletRequest request) {
			String token = this.bearerTokenResolver.resolve(request);
			if (StringUtils.hasText(token)) {
				return new BearerTokenAuthenticationToken(token);
			}
			return null;
		}

	}

And use it only when a custom BearerTokenResolver is specified, for example:

BearerTokenResolver getBearerTokenResolver() {
		if (this.bearerTokenResolver == null) {
			if (this.context.getBeanNamesForType(BearerTokenResolver.class).length > 0) {
				this.bearerTokenResolver = this.context.getBean(BearerTokenResolver.class);
			} else {
				this.bearerTokenResolver = new DefaultBearerTokenResolver();
				return this.bearerTokenResolver;
			}
		}
		this.authenticationConverter = new BearerTokenResolverAuthenticationConverterAdapter(this.bearerTokenResolver);
		return this.bearerTokenResolver;
	}

Not the most beautiful method, but it partially solves the problem. Next you will need to do something with BearerTokenRequestMatcher, of course it can be changed to:

                 @Override
		public boolean matches(HttpServletRequest request) {
			try {
				Authentication authentication = authenticationConverter.convert(request);
				return authentication != null;
			} catch (OAuth2AuthenticationException ex) {
				return false;
			}
		}

But I'm not sure about this solution. In addition, there may be a problem with AuthenticationDetailsSource, in BearerTokenAuthenticationFilter it can be changed, but now it must be changed only through AuthenticationConverter.

[franticticktick]

Hi @jzheaux! I have added authenticationConverter to OAuth2ResourceServerConfigurer and to BearerTokenAuthenticationFilter. To ensure backward compatibility with bearerTokenResolver, several changes had to be made. First, i changed BearerTokenRequestMatcher:

private static final class BearerTokenRequestMatcher implements RequestMatcher {

		private BearerTokenResolver bearerTokenResolver;

		private AuthenticationConverter authenticationConverter;

		@Override
		public boolean matches(HttpServletRequest request) {
			try {
				if (this.bearerTokenResolver != null) {
					return this.bearerTokenResolver.resolve(request) != null;
				}
				return this.authenticationConverter.convert(request) != null;
			}
			catch (OAuth2AuthenticationException ex) {
				return false;
			}
		}

		void setBearerTokenResolver(BearerTokenResolver tokenResolver) {
			Assert.notNull(tokenResolver, "resolver cannot be null");
			this.bearerTokenResolver = tokenResolver;
		}

		void setAuthenticationConverter(AuthenticationConverter authenticationConverter) {
			Assert.notNull(authenticationConverter, "authenticationConverter cannot be null");
			this.authenticationConverter = authenticationConverter;
		}

	}

Now it works with authenticationConverter or with bearerTokenResolver, but not with both. If a bearerTokenResolver was specified, then the BearerTokenAuthenticationFilter will be used along with the authenticationDetailsSource. By default, authenticationConverter will be used if bearerTokenResolver is not specified:

        @Override
	public void configure(H http) {
		AuthenticationManagerResolver resolver = this.authenticationManagerResolver;
		if (resolver == null) {
			AuthenticationManager authenticationManager = getAuthenticationManager(http);
			resolver = (request) -> authenticationManager;
		}
		BearerTokenAuthenticationFilter filter = new BearerTokenAuthenticationFilter(resolver);

		BearerTokenResolver bearerTokenResolver = getBearerTokenResolver();
		if (bearerTokenResolver != null) {
			this.requestMatcher.setBearerTokenResolver(bearerTokenResolver);
			filter.setBearerTokenResolver(bearerTokenResolver);
		}
		else {
			AuthenticationConverter converter = getAuthenticationConverter();
			this.requestMatcher.setAuthenticationConverter(converter);
			filter.setAuthenticationConverter(converter);
		}

		filter.setAuthenticationConverter(getAuthenticationConverter());
		filter.setAuthenticationEntryPoint(this.authenticationEntryPoint);
		filter.setSecurityContextHolderStrategy(getSecurityContextHolderStrategy());
		filter = postProcess(filter);
		http.addFilter(filter);
	}

In BearerTokenAuthenticationFilter authenticationConverter will work if bearerTokenResolver is not set:

                 Authentication authentication;
		try {
			if (this.bearerTokenResolver != null) {
				String token = this.bearerTokenResolver.resolve(request);
				if (!StringUtils.hasText(token)) {
					this.logger.trace("Did not process request since did not find bearer token");
					return;
				}
				authentication = bearerTokenAuthenticationToken(token, request);
			}
			else {
				authentication = this.authenticationConverter.convert(request);
			}
		}
		catch (OAuth2AuthenticationException invalid) {
			this.logger.trace("Sending to authentication entry point since failed to resolve bearer token", invalid);
			this.authenticationEntryPoint.commence(request, response, invalid);
			return;
		}

		if (authentication == null) {
			this.logger.trace("Failed to convert authentication request");
			filterChain.doFilter(request, response);
			return;
		}

Tests work as is. AuthenticationDetailsSource can only be set if bearerTokenResolver has been set. I think bearerTokenResolver can be noted as deprecated.

[franticticktick]

Hi @jzheaux ! Maybe it’s worth adding BearerTokenAuthenticationConverter to the BearerTokenAuthenticationFilter as a separate issue? This issue does not seem too easy and may affect the current functionality.

[jzheaux]

@CrazyParanoid, thanks for your patience as I have been out of town.

We certainly want to remain passive and don't want to change behavior unnecessarily. Also, though, we don't typically add filter components that cannot be used by existing filters.

I'll take a look at the PR this week and see if I can find a way to simplify things. Thank you for all your research details.