connect icon indicating copy to clipboard operation
connect copied to clipboard

Add interpolated fields to JWT claims for http auth

Open alexthecarp opened this issue 2 years ago • 4 comments

In this PR I'm adding support for interpolated fields for JWT claims. This is more or less necessary to make this usable in many current scenarios because some common claims are dynamic (iat and exp use timestamps; jti needs to be unique; it's also possible to need to set the sub dynamically).

This is a very cautious implementation, careful to preserve existing semantics: it will only look at fields of string type. It will attempt to interpolate them only if NumDynamicExpressions() says there is an interpolation. If no fields are interpolated it will just fallback to the existing implementation. If only SOME of the fields are interpolated, it will only attempt to interpolate those and preserve the current logic for the others.

NOTES:

  • The current http processor is in the middle of a refactor from the old config to the new one, which is a much larger change and I'm struggling to wrap my head around it. Possibly other reactors might be needed (OAuth2 is completely different from everything else). I've tried to keep the logic the same as before to avoid getting in the way of a refactor.
  • The general auth code is shared between multiple components (websocket, confluent schema registry). I've left those untouched, as they're much more static (don't work with batches, for example) and I'm not sure if interpolated fields make sense there.
  • I've NOT updated the documentation since it seems shared between all the components. I'm not sure what to do here, looking for guidance.

alexthecarp avatar Jan 16 '23 23:01 alexthecarp

Thanks @alexthecarp this looks great. These components are all internal now so I'm happy to rework the request signer so that the constructor is passed a manager (in order to access the bloblang env and create mappings) and that the Sign function is provided a reference message. I think it'd make the codebase a bit cleaner as we'd avoid the fork in logic at the HTTP component side.

If you're happy to make those changes let me know, otherwise I'll rework it as I merge it (when I have a bit of time to spare).

Jeffail avatar Jan 30 '23 17:01 Jeffail

@Jeffail I've actually tried to do it the right way but I couldn't figure it out. The two existing paths using the same closure really threw me off. It also feels like the HTTP client is fairly old and it interfaces with the config in unusual ways, so I'd rather let you do it.

(i'm happy to try again, but I'd need some guidance on what ideally would need to change, since I tried like 3-4 different approaches and nothing "looked good" to me).

alexthecarp avatar Jan 31 '23 16:01 alexthecarp

No worries, I'll do a bit of a tidy up of the HTTP stuff as I get this done.

Jeffail avatar Feb 05 '23 16:02 Jeffail

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


alexthecarp seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Jan 27 '25 14:01 CLAassistant