golang-samples icon indicating copy to clipboard operation
golang-samples copied to clipboard

Tokens expire after 1 hour - use Tokensource instead.

Open jankrynauw opened this issue 3 years ago • 2 comments

https://github.com/GoogleCloudPlatform/golang-samples/blob/42a8597d5beb089c313eb618ac1363af5e404be0/run/grpc-ping/connection.go#L30

The current example manually adds a token to each client request. For some longer running services, this becomes an issue since the client is usually instantiated when launching the service, requests made after 1 hour of the initial client instantiation will fail since the token will have expired by then.

Rather use the TokenSource methodology where the underlying token is validated and only refreshed when needed.

Here is a revised NewConn method illustrating a potential solution to the above:

type grpcTokenSource struct {
	oauth.TokenSource
}

// NewConn creates a new gRPC connection.
// host should be of the form domain:port, e.g., example.com:443
func NewConn(ctx context.Context, host string, insecure bool) (*grpc.ClientConn, error) {
	var opts []grpc.DialOption
	if host != "" {
		opts = append(opts, grpc.WithAuthority(host))
	}

	if insecure {
		opts = append(opts, grpc.WithInsecure())
	} else {
		systemRoots, err := x509.SystemCertPool()
		if err != nil {
			return nil, err
		}
		cred := credentials.NewTLS(&tls.Config{
			RootCAs: systemRoots,
		})
		opts = append(opts, grpc.WithTransportCredentials(cred))
	}

	// use a tokenSource to automatically inject tokens with each underlying client request
	audience := "https://" + strings.Split(host, ":")[0]
	tokenSource, err := idtoken.NewTokenSource(ctx, audience, option.WithAudiences(audience))
	if err != nil {
		return nil, status.Errorf(
			codes.Unauthenticated,
			"NewTokenSource: %s", err,
		)
	}
	opts = append(opts, grpc.WithPerRPCCredentials(grpcTokenSource{
		TokenSource: oauth.TokenSource{
			tokenSource,
		},
	}))

	return grpc.Dial(host, opts...)
}

jankrynauw avatar Jul 08 '21 07:07 jankrynauw

Hello jankrynauw,

Thanks for the issue and coding up a proposed improvement, that is very generous with your time.

To ensure I understand what you're saying, you think this code sample should properly handle token refreshing, a limitation of the current code pointed out in:

https://github.com/GoogleCloudPlatform/golang-samples/blob/42a8597d5beb089c313eb618ac1363af5e404be0/run/grpc-ping/request_auth.go#L39-L42

That seems very reasonable. To keep the sample aligned with the documentation I'd like to see the changes made within the request_auth.go file, and not inside the NewConn() function.

Do you want to make this a formalized contribution to the samples? I am happy to expedite as a reviewer if so. If not, I will carry forward the change. I will start working on this on Friday morning to give you some time to react in case you do want to contribute.

Thanks again for helping us improve the code samples.

grayside avatar Jul 13 '21 22:07 grayside

Thank you @grayside - I created a pull request (my first ever...): #2153 Let's continue the discussion there.

jankrynauw avatar Jul 15 '21 22:07 jankrynauw