temporalio / temporal

Temporal service
https://docs.temporal.io
MIT License
11.94k stars 843 forks source link

Allow configuration of expected audience value for Temporal authorization #5156

Open dzmitry-panamarenka-absa opened 11 months ago

dzmitry-panamarenka-absa commented 11 months ago

Author: Dima Ponomarenko

Summary of the feature being proposed

Extend global:authorization section of docker config to allow configuring of expected audience value. Currently, it's possible to configure JWTClaimMapper via config file. If defaultJWTClaimMapper is used, one of the existing checks there is to validate aud claim against the expected values received from audienceGetter.Audience. But the audienceGetter is not defined by default and the only way to set it is to use custom temporal build. If audienceGetter is not set, 'aud' is not validated at all. There is no way to configure it using docker config file unlike claimMapper and authorizer. Ideally, we should allow specifying the expected audience value in the docker config itself, but I'd like to hear if you can suggest a better way.

What value does this feature bring to Temporal?

This feature will simplify Temporal setup when JWT-based authentication is used. As an example, Azure Active Directory oAuth2. In's commonly used in enterprise environment, where you must check the aud because application within your organisation share the same JWT signing key. As a result, another application from your org can successfully authenticate and mimic your JWT claims to get access into your system, unless you check aud. https://learn.microsoft.com/en-us/entra/identity-platform/id-token-claims-reference#payload-claims

It's possible to build custom Temporal with your own audienceGetter backed in. But it requires more knowledge (including golang) and make is harder to maintain. As well as keeping your custom build up to date. But I believe the following security feature should be achievable using configuration file and without custom Temporal builds.

Are you willing to implement this feature yourself?

Yes

cretz commented 11 months ago

I am going to forward this to the Temporal server repo...

dzmitry-panamarenka-absa commented 8 months ago

Are there any plans to implement this change? Perhaps you would be willing to accept a contribution to this issue, if we can agree on the design first.

dzmitry-panamarenka-absa commented 8 months ago

I'd to highlight importance of aud check by putting this link to "JSON Web Token Best Current Practices" standard RFC 8725