Skip to content

Commit

Permalink
oidc: ignore cancellation of remote key set context
Browse files Browse the repository at this point in the history
This has largely caused confusion. Detach the context from its parent
and just use as a bag-of-values for configuration.
  • Loading branch information
ericchiang committed Jul 8, 2024
1 parent 6b94bec commit 40aa1fa
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 4 deletions.
16 changes: 14 additions & 2 deletions oidc/jwks.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,28 @@ func newRemoteKeySet(ctx context.Context, jwksURL string, now func() time.Time)
if now == nil {
now = time.Now
}
return &RemoteKeySet{jwksURL: jwksURL, ctx: ctx, now: now}
return &RemoteKeySet{
jwksURL: jwksURL,
now: now,
// For historical reasons, this package uses contexts for configuration, not just
// cancellation. In hindsight, this was a bad idea.
//
// Attemps to reason about how cancels should work with background requests have
// largely lead to confusion. Use the context here as a config bag-of-values and
// ignore the cancel function.
ctx: context.WithoutCancel(ctx),
}
}

// RemoteKeySet is a KeySet implementation that validates JSON web tokens against
// a jwks_uri endpoint.
type RemoteKeySet struct {
jwksURL string
ctx context.Context
now func() time.Time

// Used for configuration. Cancelation is ignored.
ctx context.Context

// guard all other fields
mu sync.RWMutex

Expand Down
81 changes: 81 additions & 0 deletions oidc/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,3 +675,84 @@ func TestVerifierAlg(t *testing.T) {
}

}

func TestCanceledContext(t *testing.T) {
priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
t.Fatalf("generating random key: %v", err)
}
pub := priv.Public()
pubKey := jose.JSONWebKey{
Algorithm: "ES256",
Key: pub,
Use: "sign",
}

signingKey := jose.SigningKey{
Algorithm: jose.ES256,
Key: priv,
}

ts := &testIssuer{
algs: []string{"ES256"},
jwks: &jose.JSONWebKeySet{
Keys: []jose.JSONWebKey{
pubKey,
},
},
}
srv := httptest.NewServer(ts)
ts.baseURL = srv.URL

ctx, cancel := context.WithCancel(context.Background())

provider, err := NewProvider(ctx, srv.URL)
if err != nil {
t.Fatalf("creating provider: %v", err)
}

// Explicitly cancel the context.
cancel()

now := time.Now()

claims := struct {
Iss string `json:"iss"`
Sub string `json:"sub"`
Aud string `json:"aud"`
Exp int64 `json:"exp"`
Iat int64 `json:"iat"`
}{
Iss: srv.URL,
Sub: "test-user",
Aud: "test-client",
Exp: now.Add(time.Hour).Unix(),
Iat: now.Add(-time.Hour).Unix(),
}
payload, err := json.Marshal(claims)
if err != nil {
t.Fatalf("marshaling claims: %v", err)
}
signer, err := jose.NewSigner(signingKey, nil)
if err != nil {
t.Fatalf("creating signing key: %v", err)
}
jws, err := signer.Sign(payload)
if err != nil {
t.Fatalf("signing token: %v", err)
}
rawIDToken, err := jws.CompactSerialize()
if err != nil {
t.Fatalf("serializing token: %v", err)
}

config := &Config{
ClientID: "test-client",
}
verifier := provider.Verifier(config)

ctx = context.Background()
if _, err := verifier.Verify(ctx, rawIDToken); err != nil {
t.Fatalf("verifying token: %v", err)
}
}
4 changes: 2 additions & 2 deletions oidc/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ type Config struct {
}

// VerifierContext returns an IDTokenVerifier that uses the provider's key set to
// verify JWTs. As opposed to Verifier, the context is used for all requests to
// the upstream JWKs endpoint.
// verify JWTs. As opposed to Verifier, the context is used to configure requests
// to the upstream JWKs endpoint. The provided context's cancellation is ignored.
func (p *Provider) VerifierContext(ctx context.Context, config *Config) *IDTokenVerifier {
return p.newVerifier(NewRemoteKeySet(ctx, p.jwksURL), config)
}
Expand Down

0 comments on commit 40aa1fa

Please sign in to comment.