From 7c70811bfebca70c8b7ca791d8722be01d4a48ae Mon Sep 17 00:00:00 2001 From: Giannis Katsanos Date: Thu, 22 Feb 2024 18:09:38 +0200 Subject: [PATCH] feat: Clock interface as a time source Verifying a session JWT includes time sensitive validations, like checking token expiration, or that the token can be used now ('nbf' claim). The HTTP middleware caches JSON web keys for one hour. After that, the cache is considered invalid and a new request to fetch the JSON web key set will be issued. These are both flows that depend on the current time and time comparisons. Various system architectures with a distributed nature might not be synchronized. A clock skew is expected. In such cases it's common to share a "time source" so that all nodes can coordinate on what the perceived current time is. This commit adds a new clerk.Clock interface which can be passed as a dependency in the http middleware functions and the jwt.Verify function. We also add a dependency to the clockwork project, in order to use a fake clock for our tests. --- clerk.go | 26 +++++++++++++++ go.mod | 1 + go.sum | 2 ++ http/middleware.go | 34 +++++++++++++++---- http/middleware_test.go | 72 +++++++++++++++++++++++++++++++++++++++++ jwt/jwt.go | 10 +++++- v2_migration_guide.md | 1 + 7 files changed, 138 insertions(+), 8 deletions(-) diff --git a/clerk.go b/clerk.go index cd4652b2..15cb9520 100644 --- a/clerk.go +++ b/clerk.go @@ -448,6 +448,32 @@ type ClientConfig struct { BackendConfig } +// Clock is an interface that can be used with the library instead +// of the [time] package. +// The interface is useful for testing time sensitive paths or +// feeding the library with an authoritative source of time, like +// an external time generator. +type Clock interface { + Now() time.Time +} + +// A default implementation of a Clock, keeping the real time by +// using the [time] package directly. +type defaultClock struct{} + +// Now returns the current time. +func (c *defaultClock) Now() time.Time { + return time.Now() +} + +// NewClock returns a default clock implementation which calls +// the [time] package internally. +// Please note that the return type is an interface because the +// Clock is not supposed to be used directly. +func NewClock() Clock { + return &defaultClock{} +} + // Regular expression that matches multiple backslashes in a row. var extraBackslashesRE = regexp.MustCompile("([^:])//+") diff --git a/go.mod b/go.mod index ec39be71..e04dfcf6 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect + github.com/jonboulle/clockwork v0.4.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index 998ac782..f139f123 100644 --- a/go.sum +++ b/go.sum @@ -5,6 +5,8 @@ github.com/go-jose/go-jose/v3 v3.0.1 h1:pWmKFVtt+Jl0vBZTIpz/eAKwsm6LkIxDVVbFHKkc github.com/go-jose/go-jose/v3 v3.0.1/go.mod h1:RNkWWRld676jZEYoV3+XK8L2ZnNSvIsxFMht0mSX+u8= github.com/google/go-cmp v0.5.0 h1:/QaMHBdZ26BB3SSst0Iwl10Epc+xhTquomWX0oZEB6w= github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/jonboulle/clockwork v0.4.0 h1:p4Cf1aMWXnXAUh8lVfewRBx1zaTSYKrKMF2g3ST4RZ4= +github.com/jonboulle/clockwork v0.4.0/go.mod h1:xgRqUGwRcjKCO1vbZUEtSLrqKoPSsUpK7fnezOII0kc= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/http/middleware.go b/http/middleware.go index d339f9a2..12b3dd5f 100644 --- a/http/middleware.go +++ b/http/middleware.go @@ -46,6 +46,9 @@ func WithHeaderAuthorization(opts ...AuthorizationOption) func(http.Handler) htt return } } + if params.Clock == nil { + params.Clock = clerk.NewClock() + } authorization := strings.TrimSpace(r.Header.Get("Authorization")) if authorization == "" { @@ -60,7 +63,7 @@ func WithHeaderAuthorization(opts ...AuthorizationOption) func(http.Handler) htt return } if params.JWK == nil { - params.JWK, err = getJWK(r.Context(), params.JWKSClient, decoded.KeyID) + params.JWK, err = getJWK(r.Context(), params.JWKSClient, decoded.KeyID, params.Clock) if err != nil { w.WriteHeader(http.StatusUnauthorized) return @@ -84,20 +87,20 @@ func WithHeaderAuthorization(opts ...AuthorizationOption) func(http.Handler) htt // Tries a cached value first, but if there's no value or the entry // has expired, it will fetch the JWK set from the API and cache the // value. -func getJWK(ctx context.Context, jwksClient *jwks.Client, kid string) (*clerk.JSONWebKey, error) { +func getJWK(ctx context.Context, jwksClient *jwks.Client, kid string, clock clerk.Clock) (*clerk.JSONWebKey, error) { if kid == "" { return nil, fmt.Errorf("missing jwt kid header claim") } jwk := getCache().Get(kid) - if jwk == nil { + if jwk == nil || !getCache().IsValid(kid, clock.Now().UTC()) { var err error jwk, err = forceGetJWK(ctx, jwksClient, kid) if err != nil { return nil, err } } - getCache().Set(kid, jwk, time.Now().UTC().Add(time.Hour)) + getCache().Set(kid, jwk, clock.Now().UTC().Add(time.Hour)) return jwk, nil } @@ -169,6 +172,17 @@ func AuthorizedPartyMatches(parties ...string) AuthorizationOption { } } +// Clock allows to pass a clock implementation that will be the +// authority for time related operations. +// You can use a custom clock for testing purposes, or to +// eliminate clock skew if your code runs on different servers. +func Clock(c clerk.Clock) AuthorizationOption { + return func(params *AuthorizationParams) error { + params.Clock = c + return nil + } +} + // CustomClaimsConstructor allows to pass a constructor function // which returns a pointer to a type (struct) to hold custom token // claims. @@ -280,6 +294,15 @@ type cacheEntry struct { expiresAt time.Time } +// IsValid returns true if a non-expired entry exists in the cache +// for the provided key, false otherwise. +func (c *jwkCache) IsValid(key string, t time.Time) bool { + c.mu.RLock() + defer c.mu.RUnlock() + entry, ok := c.entries[key] + return ok && entry != nil && entry.expiresAt.After(t) +} + // Get fetches the JSON Web Key for the provided key, unless the // entry has expired. func (c *jwkCache) Get(key string) *clerk.JSONWebKey { @@ -289,9 +312,6 @@ func (c *jwkCache) Get(key string) *clerk.JSONWebKey { if !ok || entry == nil { return nil } - if entry.expiresAt.Before(time.Now().UTC()) { - return nil - } return entry.value } diff --git a/http/middleware_test.go b/http/middleware_test.go index 02c38ba9..9886ea3b 100644 --- a/http/middleware_test.go +++ b/http/middleware_test.go @@ -1,11 +1,15 @@ package http import ( + "fmt" "net/http" "net/http/httptest" "testing" + "time" "github.com/clerk/clerk-sdk-go/v2" + "github.com/clerk/clerk-sdk-go/v2/clerktest" + "github.com/jonboulle/clockwork" "github.com/stretchr/testify/require" ) @@ -63,6 +67,74 @@ func TestRequireHeaderAuthorization_InvalidAuthorization(t *testing.T) { require.Equal(t, http.StatusForbidden, res.StatusCode) } +func TestWithHeaderAuthorization_Caching(t *testing.T) { + kid := "kid" + clock := clockwork.NewFakeClockAt(time.Now().UTC()) + + // Mock the Clerk API server. We expect requests to GET /v1/jwks. + totalJWKSRequests := 0 + clerkAPI := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v1/jwks" && r.Method == http.MethodGet { + // Count the number of requests to the JWKS endpoint + totalJWKSRequests++ + _, err := w.Write([]byte( + fmt.Sprintf( + `{"keys":[{"use":"sig","kty":"RSA","kid":"%s","alg":"RS256","n":"ypsS9Iq26F71B3lPjT_IMtglDXo8Dko9h5UBmrvkWo6pdH_4zmMjeghozaHY1aQf1dHUBLsov_XvG_t-1yf7tFfO_ImC1JqSQwdSjrXZp3oMNFHwdwAknvtlBg3sBxJ8nM1WaCWaTlb2JhEmczIji15UG6V0M2cAp2VK_brcylQROaJLC2zVa4usGi4AHzAHaRUTv6XB9bGYMvkM-ZniuXgp9dPurisIIWg25DGrTaH-kg8LPaqGwa54eLEnvfAe0ZH_MvA4_bn_u_iDkQ9ZI_CD1vwf0EDnzLgd9ZG1khGsqmXY_4WiLRGsPqZe90HzaBJma9sAxXB4qj_aNnwD5w","e":"AQAB"}]}`, + kid, + ), + )) + require.NoError(t, err) + return + } + })) + defer clerkAPI.Close() + + // Mock the clerk backend + clerk.SetBackend(clerk.NewBackend(&clerk.BackendConfig{ + HTTPClient: clerkAPI.Client(), + URL: &clerkAPI.URL, + })) + + // This is the user's server, guarded by Clerk's http middleware. + ts := httptest.NewServer(WithHeaderAuthorization(Clock(clock))(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, err := w.Write([]byte("{}")) + require.NoError(t, err) + }))) + defer ts.Close() + + // Generate a token with the claims below. + tokenClaims := map[string]any{ + "sid": "sess_123", + "sub": "user_123", + "iss": "https://clerk.com", + } + token, _ := clerktest.GenerateJWT(t, tokenClaims, kid) + // The first request needs to fetch the JSON web key set, because + // the cache is empty. + req, err := http.NewRequest(http.MethodGet, ts.URL, nil) + req.Header.Set("Authorization", "Bearer "+token) + require.NoError(t, err) + _, err = ts.Client().Do(req) + require.NoError(t, err) + require.Equal(t, 1, totalJWKSRequests) + + // The next request will use the cached value + _, err = ts.Client().Do(req) + require.NoError(t, err) + require.Equal(t, 1, totalJWKSRequests) + + // If we move past the cache's expiry date, the JWKS will be fetched again. + clock.Advance(2 * time.Hour) + _, err = ts.Client().Do(req) + require.NoError(t, err) + require.Equal(t, 2, totalJWKSRequests) + + // The next time the JWKS will be cached again. + _, err = ts.Client().Do(req) + require.NoError(t, err) + require.Equal(t, 2, totalJWKSRequests) +} + func TestAuthorizedPartyFunc(t *testing.T) { t.Parallel() for _, tc := range []struct { diff --git a/jwt/jwt.go b/jwt/jwt.go index a5b364c9..4819995f 100644 --- a/jwt/jwt.go +++ b/jwt/jwt.go @@ -26,6 +26,10 @@ type VerifyParams struct { // JWK the custom JSON Web Key that will be used to verify the // Token with. Required. JWK *clerk.JSONWebKey + // Clock can be used to keep track of time and will replace usage of + // the [time] package. Pass a custom Clock to control the source of + // time or facilitate testing chronologically sensitive flows. + Clock clerk.Clock // CustomClaimsConstructor will be called when parsing the Token's // claims. It's useful for parsing custom claims into user-defined // types. @@ -77,7 +81,11 @@ func Verify(ctx context.Context, params *VerifyParams) (*clerk.SessionClaims, er return nil, err } - err = claims.ValidateWithLeeway(time.Now().UTC(), params.Leeway) + clock := params.Clock + if clock == nil { + clock = clerk.NewClock() + } + err = claims.ValidateWithLeeway(clock.Now().UTC(), params.Leeway) if err != nil { return nil, err } diff --git a/v2_migration_guide.md b/v2_migration_guide.md index e1c42c23..81d02cc4 100644 --- a/v2_migration_guide.md +++ b/v2_migration_guide.md @@ -324,6 +324,7 @@ Name in v1 | Name in v2 `WithSatelliteDomain` | `Satellite` `WithProxyURL` | `ProxyURL` `WithCustomClaims` | `CustomClaimsConstructor` +n/a | `Clock` n/a | `JWKSClient` ## Verify tokens