Skip to content

Support customization of JwtAuthenticationConverter #9096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
AbstractConcept opened this issue Oct 6, 2020 · 8 comments
Closed

Support customization of JwtAuthenticationConverter #9096

AbstractConcept opened this issue Oct 6, 2020 · 8 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@AbstractConcept
Copy link

AbstractConcept commented Oct 6, 2020

This issue is related to @jzheaux as he requested some of the changes from PR #9005 to be moved to a separate ticket.
On-hold until 9005 is merged.

Expected Behavior

It should be possible to override / customize JwtAuthenticationProvider inside JwtIssuerAuthenticationManagerResolver class, in a multi-tenant environment, so that the end-user can set non-standard behavior that may be desired (for example, custom JWT parsing).

Current Behavior

Customization is not possible at all, end-user is forced to use predefined implementations inside JwtIssuerAuthenticationManagerResolver, and this leads to errors if JWTs contain something uncommon.
Context

Using an external oauth2ResourceServer, in a multi-tenant environment, it should be possible to override / select a custom JwtAuthenticationProvider with a custom JwtAuthenticationConverter, as not all of the JWT tokens are the same, and this leads to errors.

Please also see #9005 and #8535 for more information and the timeline of changes.

@AbstractConcept AbstractConcept added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Oct 6, 2020
@AbstractConcept AbstractConcept changed the title Support customization of JwtAuthenticationProvider in multi-tenant systems Support customization of JwtAuthenticationConverter Oct 6, 2020
@jzheaux
Copy link
Contributor

jzheaux commented Oct 6, 2020

Thanks for logging this ticket, @AbstractConcept

I agree that customizing how the underlying AuthenticationManager is resolved may be valuable. But, I think that exposing JwtAuthenticationConverter is too narrow. What if I need to customize the way the JwtDecoder is constructed?

Instead of taking a JwtAuthenticationConverter in the constructor, it would address more use cases to take an AuthenticationManagerResolver<String>.

So, instead of

new JwtIssuerAuthenticationManagerResolver(myTrustedIssuers,
    myAuthenticationConverter)

you'd do:

new JwtIssuerAuthenticationManagerResolver(myTrustedIssuers,
    (issuer) -> {
        JwtDecoder decoder = JwtDecoders.fromIssuerLocation(issuer);
        JwtAuthenticationProvider provider = new JwtAuthenticationProvider(decoder);
        provider.setJwtAuthenticationConverter(myAuthenticationConverter);
        return provider::authenticate;
    });

While slightly more verbose, it allows applications to specify a wider range of customizations.

I think it would also be good to adjust a parameter name in another constructor, giving this class four constructors like so:

(String... trustedIssuers)
(Collection<String> trustedIssuers)
(Collection<String> trustedIssuers, AuthenticationManagerResolver<String> issuerAuthenticationManagerResolver)
(AuthenticationManagerResolver<String> trustedIssuerAuthenticationManagerResolver)

@AbstractConcept
Copy link
Author

@jzheaux Should this one still be completed, or did #9168 also took care of this?

@jzheaux
Copy link
Contributor

jzheaux commented Nov 4, 2020

#9168 did not take care of this ticket, thanks for double-checking.

@AbstractConcept
Copy link
Author

I should be available to complete this one then, but if someone can do it faster, by all means, go ahead.

@fabiangr
Copy link

This affects a project I'm currently working on. We need both a custom JWTConverter and multi-tenancy. Our workaround is a configuration that looks like this:

public OAuth2SecurityConfiguration(@Value("${security.issuer-uris}") String issuerUris) {
  String[] trustedIssuers = issuerUris.split(",");
  OAuth2JwtAuthenticationConverter jwtAuthenticationConverter = new OAuth2JwtAuthenticationConverter();
  Arrays.stream(trustedIssuers).forEach(issuer -> {
    JwtAuthenticationProvider authenticationProvider = new JwtAuthenticationProvider(JwtDecoders.fromOidcIssuerLocation(issuer));
    authenticationProvider.setJwtAuthenticationConverter(jwtAuthenticationConverter);
    authenticationManagers.put(issuer, authenticationProvider::authenticate);
  });
}

Is this issue still going to be adressed?

@jzheaux
Copy link
Contributor

jzheaux commented Sep 15, 2021

Thanks for checking, @fabiangr. I think it's open for someone with time to contribute a PR.

@AbstractConcept, is this something you are still able to contribute?

@cselagea
Copy link

cselagea commented Nov 6, 2021

@jzheaux @fabiangr @AbstractConcept I didn't see any work in progress on this, and I find it useful as well, so I opened a PR.

cselagea added a commit to cselagea/spring-security that referenced this issue Nov 11, 2021
…r trusted JWT issuer

JwtIssuer(Reactive)AuthenticationManagerResolver did not provide a simple means of
 customizing, for example, the JWT authentication converter or decoder.
 This commit introduces a new constructor that accepts a collection of
 trusted issuers and a strategy for resolving the (Reactive)AuthenticationManager
 given a trusted issuer, which offers the ability to make the
 aforementioned customizations.

Closes spring-projectsgh-9096
@jzheaux
Copy link
Contributor

jzheaux commented Nov 18, 2021

After reviewing this in conjunction with #10002, I don't think we want to add more constructors to JwtIssuerAuthenticationManagerResolver.

Instead, please consider a custom resolver like so:

@Component
public class MyAuthenticationManagerResolver implements AuthenticationManagerResolver<String> {
  private final Collection<String> validIssuers;

  // ...

  @Override
  @Cacheable(unless="#result==null")
  public AuthenticationManager resolve(String issuer) {
    if (!this.validIssuers.contains(issuer)) {
      return null;
    }
    JwtAuthenticationProvider provider = new JwtAuthenticationProvider(
            JwtDecoders.fromIssuerLocation(issuer));
    provider.setAuthenticationConverter(myConverter);
    return provider::authenticate;
  }
}

// ...

@Bean 
JwtIssuerAuthenticationManagerResolver multitenancy(
        AuthenticationManagerResolver<String> resolver) {
  return new JwtIssuerAuthenticationManagerResolver(resolver);
}

Or, with the introduction of SupplierJwtDecoder, computation can be deferred a bit more simply:

@Bean 
JwtIssuerAuthenticationManagerResolver byIssuer(MyJwtConverter converter) {
    Map<String, AuthenticationManager> managers = new HashMap<>();
    for (String issuer : issuers) {
        JwtDecoder decoder = new SupplierJwtDecoder(() -> JwtDecoders.fromIssuerLocation(issuer));
        JwtAuthenticationProvider provider = new JwtAuthenticationProvider(decoder);
        provider.setJwtAuthenticationConverter(converter);
        managers.put(issuer, provider::authenticate);
    }
    return new JwtIssuerAuthenticationManagerResolver(managers::get);
}

@jzheaux jzheaux closed this as completed Nov 18, 2021
@jzheaux jzheaux self-assigned this Nov 18, 2021
@jzheaux jzheaux added status: declined A suggestion or change that we don't feel we should currently apply in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
4 participants