feat: OAuth2/OIDC metadata discovery for jwt authenticator

[martin31821]

Related issue(s)

closes #619

Checklist

• I agree to follow this project's Code of Conduct.
• I have read, and I am following this repository's Contributing Guidelines.
• I have read the Security Policy.
• I have referenced an issue describing the bug/feature request.
• I have added tests that prove the correctness of my implementation.
• I have updated the documentation.

Description

As discussed in discord and also here, this PR extends the configuration of the jwt and the oauth2_introspection authenticators as follows:

Both have received a new metadata_endpoint property, which allows configuration of the [RFC 8414 - OAuth 2.0 Authorization Server Metadata] endpoint to discover the capabilities of the ouath2/OIDC server. With the RFC 8414 in place the OpenID Connect Discovery 1.0 specification is a OIDC specific profile for that RFC. Since the implementation doesn't make use of any OIDC specifics here, the chosen name adheres to OAuth2 definitions (actually even the referenced OIDC spec talks about metadata endpoint, even the document itself is named "discovery"). Either this new, or the already available endpoints must be configured.

The old endpoints were updated to support templating of endpoint path parts, which becomes very handy in multi tenancy setups. The new metadata_endpoint supports that as well. Templating is only allowed for those endpoints, which are explicitly configured. E.g. if the new metadata_endpoint is configured, one can make use of templates (see examples below), but the usage of templates in endpoints retrieved from the OAuth2 server metadata is prohibited and will result in an error if template statements will appear in those. If one configures the old endpoints directly, templating is allowed.

In both allowed cases a new object TokenIssuer is available to the template. In case of the oauth2_introspection authenticator, there is however a limitation: the token issuer information is only available if the token format is a JWT.

Here an example:

Given the external metadata endpoint of an auth server of the form https://my-fancy-auth-server/realms/customer1/.well-known/openid-configuration and the accest token in JWT format carrying the iss claim with the value https://my-fancy-auth-server/realms/customer1, one can configure the endpoints like shown below to resolve the proper customer specific auth server profile:

url: https://auth-server.svc.cluster.local/realms/{{ trimPrefix "https://my-fancy-auth-server/realms/" .TokenIssuer }}/.well-known/openid-configuration

That way internally you would use the https://auth-server.svc.cluster.local/realms/customer1/.well-known/openid-configuration endpoint to get metadata information and verify the token. This does not only reduce network hops, but also allows for seamless dispatching to the proper customer specific oauth server profile/realm for the given token.

Since the verification of the issuer reference can become a security concern, this PR introduces a new disable_issuer_identifier_verification property on the level of metadata_endpoint, defaulting to false and ensuring that value of the issuer claim in the obtained metadata matches the URL patterns to communicate to the metadata endpoint as required by both, the RFC and the OIDC spec referenced above. The above example with the external and internal urls, would clearly violate such requirements, as long as the contents of the discovery document is not domain aware. In such cases, you would need to set the disable_issuer_identifier_verification property to true and hopefully implement other measures in you infrastructure preventing spoofing of the auth servers in place.

BREAKING CHANGE
In addition, this PR introduces a breaking change related to http cache configuration and usage. Before this PR, all endpoints allowed enabling or disabling of http cache header usage via the enable_http_cache property. Since many server misbehave or just do not set the corresponding headers (see RFC 7234 for details), there is a need to support proper defaults to avoid useless bandwidth abuse. For that reason, the above said property has been refactored to an object, named http_cache, supporting now two properties:

• enabled - which can be used to enable or disable the http cache
• default_ttl - which allows setting http cache ttl if the server does not set any cache related header

The defaults are context dependent. E.g. for the new metadata_endpoint, these are true and 5m. In general, http caching is however disabled. Another place, where http caching plays a role is in the http endpoint provider to load rule sets from remote locations. Here the default is the enabled cache, but without default ttl.

The following code snippet summarize the differences

id: some authenticator
type: jwt # or oauth2_introspection
config:
  # new property supporting everything an endpoint type support
  # method and and Accept header are set to the values specified by the
  # specs referenced above. Useful http cache settings (see also below)
  # are applied as well
  metadata_endpoint: 
    url: ...
    # new property, with false being the default
    disable_issuer_identifier_verification: false 
    http_cache: # this is a breaking change
       enabled: true
       default_ttl: 5m

  # other properties, previously available, like e.g. the `introspection_endpoint`, etc

---

feat: OAuth2 metadata discovery for oauth2_introspection authenticator
refactor!: Endpoint specific HTTP cache settings refactored to allow cache ttl definition

[codecov]

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (57da7ec) 89.20% compared to head (fc5e420) 89.65%.

Files	Patch %	Lines
...les/mechanisms/authenticators/jwt_authenticator.go	92.07%	6 Missing and 2 partials ⚠️
...ernal/rules/mechanisms/oauth2/metadata_endpoint.go	96.00%	3 Missing and 1 partial ⚠️
...al/rules/mechanisms/oauth2/mapstructure_decoder.go	25.00%	3 Missing ⚠️
internal/rules/mechanisms/oauth2/audience.go	66.66%	1 Missing ⚠️
internal/rules/mechanisms/oauth2/scopes.go	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1043      +/-   ##
==========================================
+ Coverage   89.20%   89.65%   +0.45%     
==========================================
  Files         246      249       +3     
  Lines       10367    10615     +248     
==========================================
+ Hits         9248     9517     +269     
+ Misses        881      862      -19     
+ Partials      238      236       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dadrus]

Unfortunatelly, it took much longer then anticipated. One of the reasons - I'm still COVID positive.

First of all - great job! Thank you for that!

During the review, I encountered some challenges/issues which I would like to discuss with you in Discord:

1. you forgot to copy the setting about oidc discovery usage on creation of the actual mechanism instance for the rule. So I assume, you have not yet tested it in a real integration setup. That's peanuts though. ;)
2. the way you made use of assertions allowed remote code (the information in the discovery document) overriding the provided configuration. IMHO, situations like this should be avoided, even we're actually talking about the source of truth here. Much better would be to let the user decide, whether it wants rely on the issuer info provided in the discovery/metadata document, or have more control about that.
3. Since the usage of the discovery/metadata document heavily relies on http caching enabled to reduce traffic, the default setting should be "enabled". Related to that the next question would be how to behave, when the oauth server does not set caching headers at all?
4. The old implementation used to calculate the cache key over the key id and the jwks url used to fetch the key from. This behavior is broken with the implementation in the PR. Depending on the configured endpoint, we might end up in situations where the same key is returned from the cache, even it does not belong to the actual server.
5. Given the way, how the PR is currently implemented, I fear, it will be challenging to add support for Support getting of JWKS from local file system for JWT verification purposes #614 in the future. There is a need for some type of abstraction.
6. Even many people talk about OIDC discovery service as specified by the corresponding spec, there is also the RFC8414 - OAuth 2.0 Authorization Server Metadata, which actually supersedes the OIDC discovery. To be more precise, with that RFC in place, the OIDC discovery becomes kind of a profile of that RFC. Since the current implementation does not make use of any OIDC specifics (from the "OIDC discovery profile"), I personally would vote to rename oidc_discovery property to metadata_endpoint. That way we can avoid confusion when people use a pure OAuth2 service.
7. According to both, the RFC and the OIDC discovery spec, the client making use of the discovery/metadata functionality must ensure, the "issuer" value returned must be identical to the authorization server's issuer identifier value into which the well-known URI string was inserted to create the URL used to retrieve the metadata. Otherwise the response must not be used. This check has not been implemented so far. I wonder, whether there are some limitations e.g. with KeyCloack or you just forgot that.
8. Even the JWT authenticator as well as the OAuth2 Introspection authenticator can now make use of the new JWT, respectively Token objects to render issuer specific URL paths, I wonder if the implemented approach is not too much. I mean, beyond the value of the iss claim nothing else could be used in a meaningful way.
9. Related to the previous bullet point, I wonder, whether we should support url templating if the endpoint information comes from the metadata/discovery endpoint. I personally vote for "no" as it would open potential security wholes.

To overcome some of the above said challenges, I allowed myself to do some refactoring. You can find that code in the https://github.com/dadrus/heimdall/tree/feature/oidc_autodiscovery branch. I wanted to talk about all that before pushing it to your PR. BTW, that PR is not complete as yours, it just refactors the discovery/metadata part to make it more future proof and simplify the code base. From the two authenticators, only the jwt one has be updated incl tests. The other one just compiles. So no JSON schema update, no docs update, etc

[dadrus]

Joint Review Results:

1. Has been indeed forgotten
2. The approach suggested by myself will be implemented
3. Caching will be enabled by default if not explicitly disabled. Related to that there is
   open question: What to do, when the server does not set cache header. Suggestion is to use 5min as http cache ttl in such cases. Many server do however disable cache via cache-control header explicitly, which is actually weird, as the content of the discovery/metadata document is static and will rarely change
4. Has been overlooked and will be implemented
5. A strategy pattern based approach is considered to be a pretty good abstraction option to reduce code complexity and allow extensions in the future. The code will be updated accordingly
6. open question: Renaming from oidc_discover to metadata_endpoint might be misleading, as people may be not used to it. Maybe something more specific, like authorization_server_metadata_endpoint would be better.
7. The issuer check required by the spec has not been implemented by intention, as internal and external urls might differ. That would break functionality. We could add it, with an option for opt-in.
   open question: What is the est way to address that?
8. Instead of making the entire Token/JWT available to the endpoint rendering context, we should rather limit the available property to the actual issuer only. It is pretty easy to see, whether the configuration uses templates and do the required token parsing/issuer extracting then.
   open question: Does somebody see any limitations with that approach (by providing only the TokenIssuer to the template)?
9. Definitely no. Endpoints resolved via metadata/discovery endpoint, should not allow templating

[dadrus]

I updated the title to wip again as there are still some opens to be discussed and implemented if there will be deviations from suggestions. Some tests are also still missing and I have not yet touched the documentation (waiting for the discussion of the opens)

[dadrus]

To have a better focused discussion, here only the opens using the same numbers as above (everything else is added/updated by the recent commits):

• (3) Caching will be enabled by default if not explicitly disabled. Related to that there is
  open question: What to do, when the server does not set cache header. Suggestion is to use 5min as http cache ttl in such cases. Many server do however disable cache via cache-control header explicitly, which is actually weird, as the content of the discovery/metadata document is static and will rarely change

  The updates I introduced today come with a breaking change in this regard:
  the previous boolean property enable_http_caching on the endpoint level has been replaced by an object http_cache supporting

  • enabled - which can be used to enable or disable the http cache
  • default_ttl - which allows setting http cache ttl if the server does not set any cache related header

  The defaults are context dependent. E.g. for the new metadata_endpoint, these would be true and 5m given the above suggestion (personally, I would even go for 30m or even longer - we are only using the issuer claim and the two jwks_uri and introspection_endpoint claims. These will never change through out the entire lifecycle of an auth server. And if somebody needs it shorter, there is an option to adjust it). In general, http caching is however disabled. Another place, where http caching plays a role is in the http endpoint provider to load rule sets from remote locations. Here the default would be the enabled cache, but no default ttl if server does not set any cache related header. These two are also the primary discussion topics of this open.

• (6) open question: Renaming from oidc_discover to metadata_endpoint might be misleading, as people may be not used to it. Maybe something more specific, like authorization_server_metadata_endpoint would be better.

  As of now the new endpoints are named metadata_endpoint for both the jwt, as well as the oauth2 introspection authenticators.

• (7) The issuer check required by the spec has not been implemented by intention, as internal and external urls might differ. That would break functionality. We could add it, with an option for opt-in.
  open question: What is the best way to address that?

  As of now this is solved by the introduction of the new disable_issuer_identifier_verification property, on the config level of the two afore said authenticators. Setting it to true will disable the verification mandated by the RFC and also the OIDC discovery spec.

• (8) Instead of making the entire Token/JWT available to the endpoint rendering context, we should rather limit the available property to the actual issuer only. It is pretty easy to see, whether the configuration uses templates and do the required token parsing/issuer extracting then.
  open question: Does somebody see any limitations with that approach (by providing only the TokenIssuer to the template)?

  At least, this is now implemented

[dadrus]

As long as there is no feedback to the opens from #1043 (comment), the PR is feature complete.

I personally would however like to change the default HTTP cache ttl for the metadata_endpoint to 30m