AuthenticatedClient objects created too soon

[drnic]

Consider https://github.com/cloudfoundry-community/go-uaa/blob/377005ac1e67d387c4e4ba66000fd3f2f35bda8b/api.go#L145-L148

The AuthenticatedClient object is built with client := &http.Client{Transport: http.DefaultTransport}, and so does not allow an opportunity for the caller of NewWithPasswordCredentials to setup an alternate client (say with custom root CAs).

Perhaps AuthenticatedClient and UnauthenticatedClient() should be functions that are built as needed using ContextClient()?

For similar reasons, perhaps we do away with API.SkipSSLValidation and instead encourage construction of explicit clients? e.g.:

tr := &http.Transport{
	TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
}
api.UnauthenticatedClient = &http.Client{Transport: tr}
me, err := api.GetMe()

Or

rootCAs, _ := x509.SystemCertPool()
if rootCAs == nil {
	rootCAs = x509.NewCertPool()
}
rootCAs.AppendCertsFromPEM([]byte(opts.UAACACert))
tr := &http.Transport{
	TLSClientConfig: &tls.Config{
		RootCAs: rootCAs,
	},
}
api.UnauthenticatedClient = &http.Client{Transport: tr}

[joefitzgerald]

@drnic the fields are all exported and you are not in any way obliged to use the New... functions - nothing you need is unexported. Just construct your API as you wish.

As you note, the only magic is SkipSSLValidation which flips things in the client around depending on the value. I'd be willing to remove that and instead document how to skip SSL validation, but I felt like it's a common enough scenario that it might be worth doing some of the heavy lifting for the consumer of the client library.

[drnic]

Re SkipSSLValidation - I think there's a trend towards discouraging ppl from skipping ssl validation; and rather encouraging them to setup root CAs so that correct validation is performed. Perhaps we can make this custom certs story easier? (my example above shows it can be kinda verbose to pass in opts.UAACACert)

[drnic]

I'm still new to this library, to oauth2 library internals, and to golang http clients + context. Sorry if I'm off the mark in discussing this library. I'll keep working on my sample apps and when you've got time we can look at them, and chat about how to support them nicely in future versions of this lib.

[joefitzgerald]

This is fixed in v0.2.0, which includes a uaa.API.WithClient function.

[joefitzgerald]

It's probably worth opening up another issue regarding the use of certs vs. SkipSSLValidation.