Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using WithHTTPClient & WithCredentialsJSON: NewTransport error #1074

Closed
maxlever opened this issue Jun 16, 2021 · 3 comments
Closed

Using WithHTTPClient & WithCredentialsJSON: NewTransport error #1074

maxlever opened this issue Jun 16, 2021 · 3 comments
Assignees
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. status: investigating The issue is under investigation, which is determined to be non-trivial. type: question Request for information or clarification. Not an issue.

Comments

@maxlever
Copy link

What you're trying to do

Hello again! I'm trying to create a client with both timeout and service creds. Following up on #1072 -- thanks for your help @codyoss -- I tried the suggested syntax of passing in multiple client options. the syntax works, but it seems the issue is that it doesn't like using a custom client with custom credentials. What am I missing?

What code you've tried

httpClient := &http.Client{
Timeout: 20 * time.Second
}
credOptions := idtoken.WithCredentialsJSON(creds)
clientOptions := idtoken.WithHTTPClient(httpClient)
httpClient, err = idtoken.NewClient(ctx, aud, credOptions, clientOptions)

Any error messages you're getting

this error appears to come from here: https://github.com/googleapis/google-api-go-client/blob/master/transport/http/dial.go#L59. but i'm not sure what it means!

transport/http: WithHTTPClient passed to NewTransport
@maxlever maxlever added priority: p3 Desirable enhancement or fix. May not be included in next release. type: question Request for information or clarification. Not an issue. labels Jun 16, 2021
@codyoss codyoss added the status: investigating The issue is under investigation, which is determined to be non-trivial. label Jun 16, 2021
@codyoss
Copy link
Member

codyoss commented Jun 16, 2021

Hey @maxlever you are right, I think we could do a better job with the error message here. Usually client and cred options are not to be used at the same time as they can lead to undesirable behaviors; having an unauthenticated client. When passing in a HTTP client it is expected that this client is already set up to handle auth, which in this case it is not.

Here is a workaround that should allow you to create a authticated TokenSource and derive a client from that:

package main

import (
	"context"
	"log"
	"time"

	"golang.org/x/oauth2"
	"google.golang.org/api/idtoken"
)

func main() {
	ctx := context.Background()
        // read in your creds from somewhere
	credOptions := idtoken.WithCredentialsJSON([]byte(data))

	ts, err := idtoken.NewTokenSource(ctx, "aud", credOptions)
	if err != nil {
		log.Fatalf("unable to create TokenSource: %v", err)
	}
	authenticatedClient := oauth2.NewClient(ctx, ts)
	authenticatedClient.Timeout = 20 * time.Second
	// use it!
}

Please let me know if that does not work for you, but hope that helps!

@maxlever
Copy link
Author

maxlever commented Jun 16, 2021

Thank you @codyoss ! That code snippet appears to work! Thanks for letting me know about this:

Usually client and cred options are not to be used at the same time as they can lead to undesirable behaviors

And yes, updating the error name to clarify that information would be helpful!

@j0hnsmith
Copy link

I've just run into this, from the comment above it seems there's no legitimate use for idtoken.WithHTTPClient(), if so remove it?

t, err := htransport.NewTransport(ctx, http.DefaultTransport, opts...)
if err != nil {
return nil, err
}
return &http.Client{Transport: t}, nil

I wonder if something like this at then end of idtoken.NewClient() would work

        ...
	if ds.HTTPClient != nil {
		t, err := htransport.NewTransport(ctx, ds.HTTPClient.Transport, opts...)
		if err != nil {
			return nil, err
		}
		ds.HTTPClient.Transport = t
		return ds.HTTPClient, nil
	}

	t, err := htransport.NewTransport(ctx, http.DefaultTransport, opts...)
	if err != nil {
		return nil, err
	}
	return &http.Client{Transport: t}, nil
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. status: investigating The issue is under investigation, which is determined to be non-trivial. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

3 participants