From b67792097093386c667d940327d859b3dc7e5e32 Mon Sep 17 00:00:00 2001 From: Chris Tran Date: Thu, 24 Oct 2024 12:04:08 -0500 Subject: [PATCH] fix: updates jwx library to use its thread-safe jwks cache (#88) * fix: updates jwx library to use its thread-safe jwks cache * ci: adds race detector test to ci and replaces redundant test steps with a workflow call * chore: removes unnecessary comment --- .github/workflows/deploy.yml | 14 +++---------- .github/workflows/on-pull-request.yml | 6 +++++- app.go | 22 ++++++++++++-------- app_test.go | 30 +++++++++++++++++++++++++++ authentication.go | 28 ++++++++++++------------- authentication_test.go | 2 +- go.mod | 5 +---- go.sum | 18 +++++++--------- 8 files changed, 74 insertions(+), 51 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index c135294..bc4e803 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -13,21 +13,13 @@ env: PASSAGE_AUTH_TOKEN: ${{ secrets.PASSAGE_AUTH_TOKEN }} jobs: + run-test-workflow: + uses: ./.github/workflows/on-pull-request.yml + build: name: Create Release runs-on: ubuntu-latest steps: - - name: Install Go - uses: actions/setup-go@v4 - with: - go-version: '1.20.14' - - - name: Checkout code - uses: actions/checkout@v2 - - - name: Test - run: go test ./... - - name: Checkout code uses: actions/checkout@v2 with: diff --git a/.github/workflows/on-pull-request.yml b/.github/workflows/on-pull-request.yml index 296dfa1..8c13888 100644 --- a/.github/workflows/on-pull-request.yml +++ b/.github/workflows/on-pull-request.yml @@ -1,6 +1,7 @@ name: Run Tests -on: +on: + workflow_call: workflow_dispatch: pull_request: @@ -26,3 +27,6 @@ jobs: - name: Test run: go test ./... + + - name: Test with race detector + run: go test -race -run TestAppJWKSCacheWriteConcurrency diff --git a/app.go b/app.go index 01fd014..f36f7dd 100644 --- a/app.go +++ b/app.go @@ -2,20 +2,24 @@ package passage import ( "context" + "fmt" - jwkLibrary "github.com/lestrrat-go/jwx/jwk" + "github.com/lestrrat-go/jwx/v2/jwk" ) +const jwksUrl = "https://auth.passage.id/v1/apps/%v/.well-known/jwks.json" + type Config struct { APIKey string HeaderAuth bool } type App struct { - ID string - JWKS jwkLibrary.Set - Config *Config - client *ClientWithResponses + ID string + JWKS jwk.Set + Config *Config + client *ClientWithResponses + jwksCache *jwk.Cache } func New(appID string, config *Config) (*App, error) { @@ -38,16 +42,16 @@ func New(appID string, config *Config) (*App, error) { client: client, } - app.JWKS, err = app.fetchJWKS() - if err != nil { + app.jwksCache = jwk.NewCache(context.Background()) + app.jwksCache.Register(fmt.Sprintf(jwksUrl, appID)) + + if err := app.refreshJWKSCache(); err != nil { return nil, err } return &app, nil } -var jwkCache map[string]jwkLibrary.Set = make(map[string]jwkLibrary.Set) - // GetApp gets information about an app // returns App on success, error on failure func (a *App) GetApp() (*AppInfo, error) { diff --git a/app_test.go b/app_test.go index 08d1f88..64ea5ad 100644 --- a/app_test.go +++ b/app_test.go @@ -1,6 +1,7 @@ package passage_test import ( + "sync" "testing" "github.com/passageidentity/passage-go" @@ -38,3 +39,32 @@ func TestGetApp(t *testing.T) { assert.Equal(t, PassageAppID, appInfo.ID) } + +func TestAppNewJWKSCache(t *testing.T) { + psg, err := passage.New(PassageAppID, &passage.Config{ + APIKey: PassageApiKey, // An API_KEY environment variable is required for testing. + }) + require.Nil(t, err) + assert.NotNil(t, psg.JWKS) +} + +// should be run with the -race flag, i.e. `go test -race -run TestAppJWKSCacheWriteConcurrency` +func TestAppJWKSCacheWriteConcurrency(t *testing.T) { + goRoutineCount := 2 + + var wg sync.WaitGroup + wg.Add(goRoutineCount) + + for i := 0; i < goRoutineCount; i++ { + go func() { + defer wg.Done() + + _, err := passage.New(PassageAppID, &passage.Config{ + APIKey: PassageApiKey, // An API_KEY environment variable is required for testing. + }) + require.Nil(t, err) + }() + } + + wg.Wait() +} diff --git a/authentication.go b/authentication.go index dc61100..f01d6bc 100644 --- a/authentication.go +++ b/authentication.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/golang-jwt/jwt" - jwkLibrary "github.com/lestrrat-go/jwx/jwk" ) // AuthenticateRequest determines whether or not to authenticate via header or cookie authentication @@ -42,33 +41,32 @@ func (a *App) getPublicKey(token *jwt.Token) (interface{}, error) { return nil, Error{Message: "expecting JWT header to have string kid"} } - key, ok := jwkCache[a.ID].LookupKeyID(keyID) + key, ok := a.JWKS.LookupKeyID(keyID) // if key doesn't exist, re-fetch one more time to see if this jwk was just added if !ok { - a.fetchJWKS() - key, ok := jwkCache[a.ID].LookupKeyID(keyID) + if err := a.refreshJWKSCache(); err != nil { + return nil, err + } + + key, ok = a.JWKS.LookupKeyID(keyID) if !ok { return nil, Error{Message: fmt.Sprintf("unable to find key %q", keyID)} } - - var pubKey interface{} - err := key.Raw(&pubKey) - return pubKey, err } var pubKey interface{} err := key.Raw(&pubKey) + return pubKey, err } -// fetchJWKS returns the JWKS for the current app -func (a *App) fetchJWKS() (jwkLibrary.Set, error) { - jwks, err := jwkLibrary.Fetch(context.Background(), fmt.Sprintf("https://auth.passage.id/v1/apps/%v/.well-known/jwks.json", a.ID)) - if err != nil { - return nil, Error{Message: "failed to fetch jwks"} +func (a *App) refreshJWKSCache() error { + var err error + if a.JWKS, err = a.jwksCache.Refresh(context.Background(), fmt.Sprintf(jwksUrl, a.ID)); err != nil { + return Error{Message: "failed to fetch jwks"} } - jwkCache[a.ID] = jwks - return jwks, nil + + return nil } // AuthenticateRequestWithCookie fetches a cookie from the request and uses it to authenticate diff --git a/authentication_test.go b/authentication_test.go index f35af07..163815d 100644 --- a/authentication_test.go +++ b/authentication_test.go @@ -85,8 +85,8 @@ func TestAuthenticationWithHeader(t *testing.T) { func TestAuthenticateToken(t *testing.T) { psg, err := passage.New(PassageAppID, nil) - require.Nil(t, err) + t.Run("valid auth token", func(t *testing.T) { _, success := psg.ValidateAuthToken(PassageAuthToken) assert.True(t, success) diff --git a/go.mod b/go.mod index 7c344fb..62f004c 100644 --- a/go.mod +++ b/go.mod @@ -5,16 +5,13 @@ go 1.16 require ( github.com/golang-jwt/jwt v3.2.2+incompatible github.com/joho/godotenv v1.5.1 - github.com/lestrrat-go/jwx v1.2.29 github.com/oapi-codegen/runtime v1.1.1 github.com/stretchr/testify v1.9.0 ) require ( - github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0 // indirect - github.com/goccy/go-json v0.10.3 // indirect github.com/google/uuid v1.6.0 // indirect github.com/kr/pretty v0.3.1 // indirect + github.com/lestrrat-go/jwx/v2 v2.1.1 github.com/rogpeppe/go-internal v1.12.0 // indirect - golang.org/x/crypto v0.25.0 // indirect ) diff --git a/go.sum b/go.sum index 59f8aba..ba6ae71 100644 --- a/go.sum +++ b/go.sum @@ -41,7 +41,6 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/decred/dcrd/crypto/blake256 v1.0.1/go.mod h1:2OfgNZ5wDpcsFmHmCK5gZTPcCXqlm2ArzUIkw9czNJo= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0/go.mod h1:v57UDF4pDQJcEfFUCRop3lJL149eHGSe9Jvczhzjo/0= github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0 h1:rpfIENRNNilwHwZeG5+P150SMrnNEcHYvcCuK6dPZSg= github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0/go.mod h1:v57UDF4pDQJcEfFUCRop3lJL149eHGSe9Jvczhzjo/0= github.com/dgraph-io/badger/v2 v2.2007.4/go.mod h1:vSw/ax2qojzbN6eXHIx6KPKtCSHJN/Uz0X0VPruTIhk= @@ -153,16 +152,16 @@ github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/labstack/echo/v4 v4.11.4/go.mod h1:noh7EvLwqDsmh/X/HWKPUl1AjzJrhyptRyEbQJfxen8= github.com/labstack/gommon v0.4.2/go.mod h1:QlUFxVM+SNXhDL/Z7YhocGIBYOiwB0mXm1+1bAPHPyU= github.com/leodido/go-urn v1.2.4/go.mod h1:7ZrI8mTSeBSHl/UaRyKQW1qZeMgak41ANeCNaVckg+4= -github.com/lestrrat-go/backoff/v2 v2.0.8 h1:oNb5E5isby2kiro9AgdHLv5N5tint1AnDVVf2E2un5A= -github.com/lestrrat-go/backoff/v2 v2.0.8/go.mod h1:rHP/q/r9aT27n24JQLa7JhSQZCKBBOiM/uP402WwN8Y= github.com/lestrrat-go/blackmagic v1.0.2 h1:Cg2gVSc9h7sz9NOByczrbUvLopQmXrfFx//N+AkAr5k= github.com/lestrrat-go/blackmagic v1.0.2/go.mod h1:UrEqBzIR2U6CnzVyUtfM6oZNMt/7O7Vohk2J0OGSAtU= github.com/lestrrat-go/httpcc v1.0.1 h1:ydWCStUeJLkpYyjLDHihupbn2tYmZ7m22BGkcvZZrIE= github.com/lestrrat-go/httpcc v1.0.1/go.mod h1:qiltp3Mt56+55GPVCbTdM9MlqhvzyuL6W/NMDA8vA5E= +github.com/lestrrat-go/httprc v1.0.6 h1:qgmgIRhpvBqexMJjA/PmwSvhNk679oqD1RbovdCGW8k= +github.com/lestrrat-go/httprc v1.0.6/go.mod h1:mwwz3JMTPBjHUkkDv/IGJ39aALInZLrhBp0X7KGUZlo= github.com/lestrrat-go/iter v1.0.2 h1:gMXo1q4c2pHmC3dn8LzRhJfP1ceCbgSiT9lUydIzltI= github.com/lestrrat-go/iter v1.0.2/go.mod h1:Momfcq3AnRlRjI5b5O8/G5/BvpzrhoFTZcn06fEOPt4= -github.com/lestrrat-go/jwx v1.2.29 h1:QT0utmUJ4/12rmsVQrJ3u55bycPkKqGYuGT4tyRhxSQ= -github.com/lestrrat-go/jwx v1.2.29/go.mod h1:hU8k2l6WF0ncx20uQdOmik/Gjg6E3/wIRtXSNFeZuB8= +github.com/lestrrat-go/jwx/v2 v2.1.1 h1:Y2ltVl8J6izLYFs54BVcpXLv5msSW4o8eXwnzZLI32E= +github.com/lestrrat-go/jwx/v2 v2.1.1/go.mod h1:4LvZg7oxu6Q5VJwn7Mk/UwooNRnTHUpXBj2C4j3HNx0= github.com/lestrrat-go/option v1.0.0/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I= github.com/lestrrat-go/option v1.0.1 h1:oAzP2fvZGQKWkvHa1/SAcFolBEca1oN+mQ7eooNBEYU= github.com/lestrrat-go/option v1.0.1/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I= @@ -239,8 +238,6 @@ github.com/pelletier/go-toml/v2 v2.0.9/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdU github.com/pkg/diff v0.0.0-20200914180035-5b29258ca4f7/go.mod h1:zO8QMzTeZd5cpnIkz/Gn6iK0jDfGicM1nynOkkPIl28= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= -github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v0.0.0-20151028094244-d8ed2627bdf0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -254,6 +251,8 @@ github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sanity-io/litter v1.5.5/go.mod h1:9gzJgR2i4ZpjZHsKvUXIRQVk7P+yM3e+jAF7bU2UI5U= github.com/schollz/closestmatch v2.1.0+incompatible/go.mod h1:RtP1ddjLong6gTkbtmuhtR2uUrrJOpYzYRvbcPAid+g= +github.com/segmentio/asm v1.2.0 h1:9BQrFxC+YOHJlTlHGkTrFWf59nbL3XnCoFLTwDCI7ys= +github.com/segmentio/asm v1.2.0/go.mod h1:BqMnlJP91P8d+4ibuonYZw9mfnzI9HfxselHZr5aAcs= github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo= github.com/shirou/gopsutil/v3 v3.23.8/go.mod h1:7hmCaBn+2ZwaZOr6jmPBZDfawwMGuo1id3C6aM8EDqQ= github.com/shoenig/go-m1cpu v0.1.6/go.mod h1:1JJMcUBvfNwpq05QDQVAnx3gUHr9IYF7GNg9SUEw2VQ= @@ -338,7 +337,6 @@ golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliY golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= -golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8= golang.org/x/crypto v0.25.0 h1:ypSNr+bnYL2YhwoMt2zPxHFmbAN1KZs/njMG3hxUp30= golang.org/x/crypto v0.25.0/go.mod h1:T+wALwcMOSE0kXgUAnPAHqTLW+XHgcELELW8VaDgm/M= @@ -414,6 +412,7 @@ golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220319134239-a9b59b0215f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220422013727-9388b58f7150/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -433,8 +432,8 @@ golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= @@ -451,7 +450,6 @@ golang.org/x/term v0.11.0/go.mod h1:zC9APTIj3jG3FdV/Ons+XE1riIZXG4aZ4GTHiPZJPIU= golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= -golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY= golang.org/x/term v0.22.0/go.mod h1:F3qCibpT5AMpCRfhfT53vVJwhLtIVHhB9XDjfFvnMI4= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=