Skip to content

Conversation

Spikhalskiy
Copy link
Contributor

@Spikhalskiy Spikhalskiy commented Aug 27, 2021

What was changed

Added support for AuthorizationTokenSupplier to simplify providing JWT or another kind of auth tokens.
Added WorkflowServiceStubsOptions#grpcClientInterceptors that allow providing any custom grpc interceptors.
Added WorkflowServiceStubsOptions#grpcMetadataProviders that provide a handier way to provide dynamic gRPC headers.

Why?

To improve extensibility of Temporal Grpc Stubs and allow Temporal users to provide JWT tokens in an easy and clean manner.

  1. Closes Add an easy way to provide JWT tokens to java-sdk for server calls #671

@Spikhalskiy Spikhalskiy marked this pull request as draft August 27, 2021 23:47
@Spikhalskiy Spikhalskiy added this to the v1.3.0 milestone Aug 28, 2021
@Spikhalskiy Spikhalskiy force-pushed the issue-671 branch 2 times, most recently from dd8c5ec to edf8023 Compare August 30, 2021 00:02
@Sushisource
Copy link
Member

Overall this approach looks totally fine to me as a way to generically attach JWTs -- really it's even just any authorization header. Maybe we should call these classes authorization, and then we could have JWTTokenSupplier be an implementation of AuthorizationSupplier or something like that, which handles slapping bearer on the front of the token.

I think adding JWT into there makes sense if we have a way to supply the JWT payload & secret info so that we actually handle creating the JWT token for them, but that's substantially more work than this, so maybe something for later. Not sure how far we were looking to go here. The easiest-to-use version of this would probably be allowing the user to specify the namespaces / other info you mentioned is in the JWT payload, and their secret (or an interface to access it) and then creating & signing the JWT for them and then passing it into the code you've added here to attach to the header.

@Spikhalskiy Spikhalskiy force-pushed the issue-671 branch 2 times, most recently from 9c7aabc to 6cf8353 Compare August 31, 2021 08:15
@Spikhalskiy Spikhalskiy marked this pull request as ready for review August 31, 2021 08:16
@Spikhalskiy
Copy link
Contributor Author

@Sushisource I discussed it with @sergeybykov and the generation/construction of JWT tokens shouldn't be a part of SDKs. Tokens are usually generated and provided by separate services and SDKs should just pass them through.

Copy link
Member

@Sushisource Sushisource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, LGTM

@Spikhalskiy Spikhalskiy merged commit 95d7e0a into temporalio:master Aug 31, 2021
@Spikhalskiy Spikhalskiy deleted the issue-671 branch April 15, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an easy way to provide JWT tokens to java-sdk for server calls

2 participants