jwt_authn: Allow to extract keys from jwt struct claims

[Reflejo]

Commit Message: Allow to extract object from jwt struct claims
Additional Description:

Serialize non-primitive custom claim objects from JWT tokens. The JWT filter will now serialize these claim as JSON and encode it to Base64.

Risk Level: Low
Testing: unit test
Docs Changes: Updated docs/root/configuration/http/http_filters/jwt_authn_filter.rst
Release Notes: Updated changelogs/current.yaml

[repokitteh-read-only]

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @lizan
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #30377 was opened by Reflejo.

see: more, trace.

[htuch]

CC @kyessenov

[mattklein123]

At a high level this seems like a useful feature, however I'm wondering if it would be more future proof to have an option to pack the entire struct into the header, vs. being specific about a list.

For example, you could take the struct and base64 encode the JSON. Or pack it in a proto stuct and base64 that.

Thoughts @lizan @kyessenov ?

[lizan]

@Reflejo can you elaborate your exact use case for this?

I would like the API to be more generic / future proof and not specific about a list. But would like to know where this implementation is coming from.

/wait:any

[Reflejo]

@lizan The use-case is many authentication providers nowadays send custom claims on JWT tokens that are formatted as per my example:

"tenants": {"a": {}, "b": {}}

Basically they do this as a way of sending lists. Our specific use-case is, we need to extract the tenants from the jwt and forward them to the service as a header.

I wouldn't mind changing my PR to define a bool serialize_struct instead and send the JSON string in base64. Would that be better?

[kyessenov]

Unless there's a common RFC that requires this encoding, I think I mostly agree with Matt's proposal - either encoding the whole JWT sub-struct as a header, or a separate extension that computes the header from dynamic metadata would be better.

[Reflejo]

@kyessenov @lizan @mattklein123 updated the PR implementing the proposed solution (serializing objects). The reason I added the flag allow_serialize_object (instead of just serializing non-primitive types) is to not make this a breaking change (ie anyone relying on the fact that we ignore keys with object types right now).

[kyessenov]

The last change looks good to me. I'm not sure it deserves an API flag since it's a new feature with a reasonable default (don't expect this header to blow up since it's always a subset of JWT).

[kyessenov]

Related: #30072

[mattklein123]

The last change looks good to me. I'm not sure it deserves an API flag since it's a new feature with a reasonable default (don't expect this header to blow up since it's always a subset of JWT).

I had mentioned to @Reflejo offline that I could probably lose the API change as well, but I don't feel strongly either way. This is certainly safer. I suppose if we wanted to limit the API change we could runtime guard instead. I'm fine either way. This LGTM. @lizan?

[Reflejo]

ok removed the flag, this should be ready to go

[mattklein123]

Thanks!