Fix issues with long-held reference to context passed to NewProvider

[misberner]

Instead of using a context to bound key fetch requests, allow the user to pass
a custom HTTP client to control timeout behavior. There are no behavioral
changes for the now-deprecated NewRemoteKeySet function.

Change NewProvider to only use the passed context for the discovery HTTP
request and for extracting an HTTP client to be used in the remote key set.

Enforce a default HTTP request timeout of 30 seconds if the client for remote
key fetches does not specify a timeout.

Fixes #214

NOTE: See #214 for a discussion of the issue. This change aims at being minimally disruptive (of course some behavioral change is inevitable) without taking any features that specifying a context entails away. This motivates the choice to pass an *http.Client to NewRemoteKeySetWithClient in favor of a simpler option such as a simple timeout time.Duration.

[ericchiang]

I think this could actually be fixed by adding a method to construct a Verifier with a context:

func (p *Provider) VerifierContext(ctx context.Context, config *Config) (*IDTokenVerifier) {}

...or NewVerifier if that's a better name. So if you did:

p, err := oidc.NewProvider(pCtx, issuerURL)
if err != nil {
    // ...
}
v, err := p.VerifierContext(vCtx, config)

Then NewProvider would be bounded by pCtx, and the created verifier would use vCtx.

[misberner]

Hmm, this would solve the problem of not being able to effectively bound NewProvider via the context, but it doesn't really address the anti-pattern of having a long-lived context in the struct - I can't think of a reason why I would pass anything other than context.Background() to VerifierContext, given that (*IDTokenVerifier).Verify already takes a context param. It also doesn't make controlling the timeout for the background fetch any easier.

[ericchiang]

There are two properties that need to hold:

• IDTokenVerifier needs to be able to be shut down, either by canceling a context or by calling a Close method.
• Canceling a context associated with a Verify() shouldn't impact the background routine refreshing the keys.

I know the long running context isn't idiomatic, but we can't change the behavior of the package for existing users without going to v3. e.g. this code needs to have the same behavior no matter what for v3:

ctx, cancel := context.WithCancel(context.Background())
p, err := oidc.NewProvider(ctx, issuerURL)
if err != nil {
    // ...
}
v := p.Verifier(config)
cancel()

[ericchiang]

Fixed by #262 which will be on the v3 branch.