From 40297b82a05b29dbfdad4de4caa94741e55387f9 Mon Sep 17 00:00:00 2001 From: lestrrat <49281+lestrrat@users.noreply.github.com> Date: Mon, 19 Feb 2024 09:14:17 +0900 Subject: [PATCH] [WIP][v3] Deprecate global fetcher (#1071) * initial attempt to changing API * Add tests * appease linter * fix deps.bzl * run make tidy and gazelle-update-repos * Update leak test * appease linter * Streamline jwk.Fetcher * appease linter * appease linter * Change httprc URL to v2 * fix bazel deps * update dependabot.yml * Update dep --- .github/dependabot.yml | 14 +++ Changes-v3.md | 6 ++ bench/performance/go.mod | 2 +- bench/performance/go.sum | 4 +- cmd/jwx/go.mod | 2 +- cmd/jwx/go.sum | 4 +- deps.bzl | 13 +-- examples/go.mod | 2 +- examples/go.sum | 42 ++++++++- go.mod | 2 +- go.sum | 4 +- jwk/BUILD.bazel | 2 +- jwk/cache.go | 4 +- jwk/fetch.go | 133 +++++++++++---------------- jwk/jwk_test.go | 194 ++++++++++++++++++--------------------- jwk/whitelist.go | 2 +- jws/BUILD.bazel | 2 +- jws/jws_test.go | 20 ++-- jws/key_provider.go | 4 + jws/options.go | 36 +++++++- jwt/options.go | 17 +--- 21 files changed, 265 insertions(+), 244 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 32efae2c4..41e9f6f0f 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -5,6 +5,15 @@ version: 2 updates: + - package-ecosystem: "gomod" # See documentation for possible values + directory: "/" # Location of package manifests + schedule: + interval: "daily" + target-branch: "develop/v3" + labels: + - "go" + - "dependencies" + - "dependabot" - package-ecosystem: "gomod" # See documentation for possible values directory: "/" # Location of package manifests schedule: @@ -23,6 +32,11 @@ updates: - "go" - "dependencies" - "dependabot" + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "daily" + target-branch: "develop/v3" - package-ecosystem: "github-actions" directory: "/" schedule: diff --git a/Changes-v3.md b/Changes-v3.md index 87ff46b7f..68f5c8aa3 100644 --- a/Changes-v3.md +++ b/Changes-v3.md @@ -48,3 +48,9 @@ These are changes that are incompatible with the v2.x.x version. * Added `jwk/ecdsa` to keep track of which curves are available for ECDSA keys. * `(jwk.Key).Raw()` has been deprecated. Use `jwk.Export()` instead. + +* `jwk.SetGlobalFetcher` has been deprecated. The required version for `github.com/lestrrat-go/httprc` + has been upgraded, and thus we no longer have a pool of workers that need to be controlled. + +* `jwk.Fetcher` has been clearly marked as something that has limited + usage for `jws.WithVerifyAuto` \ No newline at end of file diff --git a/bench/performance/go.mod b/bench/performance/go.mod index 4cef22bf4..953b0c1dc 100644 --- a/bench/performance/go.mod +++ b/bench/performance/go.mod @@ -10,7 +10,7 @@ require ( github.com/goccy/go-json v0.10.2 // indirect github.com/lestrrat-go/blackmagic v1.0.2 // indirect github.com/lestrrat-go/httpcc v1.0.1 // indirect - github.com/lestrrat-go/httprc v1.0.4 // indirect + github.com/lestrrat-go/httprc/v2 v2.0.0 // indirect github.com/lestrrat-go/option v1.0.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/segmentio/asm v1.2.0 // indirect diff --git a/bench/performance/go.sum b/bench/performance/go.sum index c5798d2fe..0ca2189d6 100644 --- a/bench/performance/go.sum +++ b/bench/performance/go.sum @@ -9,8 +9,8 @@ github.com/lestrrat-go/blackmagic v1.0.2 h1:Cg2gVSc9h7sz9NOByczrbUvLopQmXrfFx//N 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.4 h1:bAZymwoZQb+Oq8MEbyipag7iSq6YIga8Wj6GOiJGdI8= -github.com/lestrrat-go/httprc v1.0.4/go.mod h1:mwwz3JMTPBjHUkkDv/IGJ39aALInZLrhBp0X7KGUZlo= +github.com/lestrrat-go/httprc/v2 v2.0.0 h1:dZia9gCSXkYYZN9YUe4U3KU4rvpKXzmGB4QTYDDrOU0= +github.com/lestrrat-go/httprc/v2 v2.0.0/go.mod h1:smhwnjMK58yn+xnN/hxtdSRW2PCi9vNTZDzB85bxj24= 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= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/cmd/jwx/go.mod b/cmd/jwx/go.mod index 0f27cc4a7..2b3ec4f09 100644 --- a/cmd/jwx/go.mod +++ b/cmd/jwx/go.mod @@ -14,7 +14,7 @@ require ( github.com/goccy/go-json v0.10.2 // indirect github.com/lestrrat-go/blackmagic v1.0.2 // indirect github.com/lestrrat-go/httpcc v1.0.1 // indirect - github.com/lestrrat-go/httprc v1.0.4 // indirect + github.com/lestrrat-go/httprc/v2 v2.0.0 // indirect github.com/lestrrat-go/option v1.0.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/segmentio/asm v1.2.0 // indirect diff --git a/cmd/jwx/go.sum b/cmd/jwx/go.sum index ae650a92f..822f1a1ba 100644 --- a/cmd/jwx/go.sum +++ b/cmd/jwx/go.sum @@ -10,8 +10,8 @@ github.com/lestrrat-go/blackmagic v1.0.2 h1:Cg2gVSc9h7sz9NOByczrbUvLopQmXrfFx//N 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.4 h1:bAZymwoZQb+Oq8MEbyipag7iSq6YIga8Wj6GOiJGdI8= -github.com/lestrrat-go/httprc v1.0.4/go.mod h1:mwwz3JMTPBjHUkkDv/IGJ39aALInZLrhBp0X7KGUZlo= +github.com/lestrrat-go/httprc/v2 v2.0.0 h1:dZia9gCSXkYYZN9YUe4U3KU4rvpKXzmGB4QTYDDrOU0= +github.com/lestrrat-go/httprc/v2 v2.0.0/go.mod h1:smhwnjMK58yn+xnN/hxtdSRW2PCi9vNTZDzB85bxj24= 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= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/deps.bzl b/deps.bzl index 8e0a51fac..7d46d6751 100644 --- a/deps.bzl +++ b/deps.bzl @@ -44,11 +44,11 @@ def go_dependencies(): version = "v1.0.1", ) go_repository( - name = "com_github_lestrrat_go_httprc", + name = "com_github_lestrrat_go_httprc_v2", build_file_proto_mode = "disable_global", - importpath = "github.com/lestrrat-go/httprc", - sum = "h1:bAZymwoZQb+Oq8MEbyipag7iSq6YIga8Wj6GOiJGdI8=", - version = "v1.0.4", + importpath = "github.com/lestrrat-go/httprc/v2", + sum = "h1:dZia9gCSXkYYZN9YUe4U3KU4rvpKXzmGB4QTYDDrOU0=", + version = "v2.0.0", ) go_repository( @@ -108,13 +108,8 @@ def go_dependencies(): name = "org_golang_x_crypto", build_file_proto_mode = "disable_global", importpath = "golang.org/x/crypto", -<<<<<<< HEAD sum = "h1:PGVlW0xEltQnzFZ55hkuX5+KLyrMYhHld1YHO4AKcdc=", version = "v0.18.0", -======= - sum = "h1:mMMrFzRSCF0GvB7Ne27XVtVAaXLrPmgPC7/v0tkwHaY=", - version = "v0.16.0", ->>>>>>> 2161b89f ([WIP] Make JWK Key Parsing pluggable (#991)) ) go_repository( diff --git a/examples/go.mod b/examples/go.mod index 067d39f85..9cf28912d 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -14,7 +14,7 @@ require ( github.com/goccy/go-json v0.10.2 // indirect github.com/lestrrat-go/blackmagic v1.0.2 // indirect github.com/lestrrat-go/httpcc v1.0.1 // indirect - github.com/lestrrat-go/httprc v1.0.4 // indirect + github.com/lestrrat-go/httprc/v2 v2.0.0 // indirect github.com/lestrrat-go/option v1.0.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/segmentio/asm v1.2.0 // indirect diff --git a/examples/go.sum b/examples/go.sum index 0cf50f4eb..69efc1552 100644 --- a/examples/go.sum +++ b/examples/go.sum @@ -13,8 +13,8 @@ github.com/lestrrat-go/blackmagic v1.0.2 h1:Cg2gVSc9h7sz9NOByczrbUvLopQmXrfFx//N 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.4 h1:bAZymwoZQb+Oq8MEbyipag7iSq6YIga8Wj6GOiJGdI8= -github.com/lestrrat-go/httprc v1.0.4/go.mod h1:mwwz3JMTPBjHUkkDv/IGJ39aALInZLrhBp0X7KGUZlo= +github.com/lestrrat-go/httprc/v2 v2.0.0 h1:dZia9gCSXkYYZN9YUe4U3KU4rvpKXzmGB4QTYDDrOU0= +github.com/lestrrat-go/httprc/v2 v2.0.0/go.mod h1:smhwnjMK58yn+xnN/hxtdSRW2PCi9vNTZDzB85bxj24= 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= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -26,10 +26,48 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= +golang.org/x/crypto v0.14.0/go.mod h1:MVFd36DqK4CsrnJYDkBA3VC4m2GkXAM0PvzMCn4JQf4= golang.org/x/crypto v0.18.0 h1:PGVlW0xEltQnzFZ55hkuX5+KLyrMYhHld1YHO4AKcdc= golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1mg= +golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= +golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= +golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/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-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= +golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= +golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= +golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= +golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= +golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/go.mod b/go.mod index 3dc14e07a..db4ac8f26 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 github.com/goccy/go-json v0.10.2 github.com/lestrrat-go/blackmagic v1.0.2 - github.com/lestrrat-go/httprc v1.0.4 + github.com/lestrrat-go/httprc/v2 v2.0.0 github.com/lestrrat-go/option v1.0.1 github.com/segmentio/asm v1.2.0 github.com/stretchr/testify v1.8.4 diff --git a/go.sum b/go.sum index c5798d2fe..0ca2189d6 100644 --- a/go.sum +++ b/go.sum @@ -9,8 +9,8 @@ github.com/lestrrat-go/blackmagic v1.0.2 h1:Cg2gVSc9h7sz9NOByczrbUvLopQmXrfFx//N 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.4 h1:bAZymwoZQb+Oq8MEbyipag7iSq6YIga8Wj6GOiJGdI8= -github.com/lestrrat-go/httprc v1.0.4/go.mod h1:mwwz3JMTPBjHUkkDv/IGJ39aALInZLrhBp0X7KGUZlo= +github.com/lestrrat-go/httprc/v2 v2.0.0 h1:dZia9gCSXkYYZN9YUe4U3KU4rvpKXzmGB4QTYDDrOU0= +github.com/lestrrat-go/httprc/v2 v2.0.0/go.mod h1:smhwnjMK58yn+xnN/hxtdSRW2PCi9vNTZDzB85bxj24= 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= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= diff --git a/jwk/BUILD.bazel b/jwk/BUILD.bazel index bb5efa8e2..f9d13f53a 100644 --- a/jwk/BUILD.bazel +++ b/jwk/BUILD.bazel @@ -37,7 +37,7 @@ go_library( "//jwa", "//jwk/ecdsa", "@com_github_lestrrat_go_blackmagic//:blackmagic", - "@com_github_lestrrat_go_httprc//:httprc", + "@com_github_lestrrat_go_httprc_v2//:httprc", "@com_github_lestrrat_go_option//:option", ], ) diff --git a/jwk/cache.go b/jwk/cache.go index b60ebd869..b36663709 100644 --- a/jwk/cache.go +++ b/jwk/cache.go @@ -7,7 +7,7 @@ import ( "net/http" "time" - "github.com/lestrrat-go/httprc" + "github.com/lestrrat-go/httprc/v2" ) type Transformer = httprc.Transformer @@ -17,7 +17,7 @@ type ErrSink = httprc.ErrSink // Whitelist describes a set of rules that allows users to access // a particular URL. By default all URLs are blocked for security // reasons. You will HAVE to provide some sort of whitelist. See -// the documentation for github.com/lestrrat-go/httprc for more details. +// the documentation for github.com/lestrrat-go/httprc/v2 for more details. type Whitelist = httprc.Whitelist // Cache is a container that keeps track of Set object by their source URLs. diff --git a/jwk/fetch.go b/jwk/fetch.go index a4315a562..558e47aaf 100644 --- a/jwk/fetch.go +++ b/jwk/fetch.go @@ -4,85 +4,56 @@ import ( "context" "fmt" "io" - "math" - "os" - "strconv" - "sync" - "sync/atomic" + "net/http" - "github.com/lestrrat-go/httprc" + "github.com/lestrrat-go/httprc/v2" ) +// Fetcher is an interface that represents an object that fetches a JWKS. +// Currently this is only used in the `jws.WithVerifyAuto` option. +// +// Particularly, do not confuse this as the backend to `jwk.Fetch()` function. +// If you need to control how `jwk.Fetch()` implements HTTP requests look into +// providing a custom `http.Client` object via `jwk.WithHTTPClient` option type Fetcher interface { Fetch(context.Context, string, ...FetchOption) (Set, error) } +// FetchFunc describes a type of Fetcher that is represented as a function. +// +// You can use this to wrap functions (e.g. `jwk.Fetch“) as a Fetcher object. type FetchFunc func(context.Context, string, ...FetchOption) (Set, error) -func (f FetchFunc) Fetch(ctx context.Context, u string, options ...FetchOption) (Set, error) { - return f(ctx, u, options...) +func (ff FetchFunc) Fetch(ctx context.Context, u string, options ...FetchOption) (Set, error) { + return ff(ctx, u, options...) } -var globalFetcher httprc.Fetcher -var muGlobalFetcher sync.Mutex -var fetcherChanged uint32 - -func init() { - atomic.StoreUint32(&fetcherChanged, 1) +// CachedFetcher wraps `jwk.Cache` so that it can be used as a `jwk.Fetcher`. +// +// One notable diffence from a general use fetcher is that `jwk.CachedFetcher` +// can only be used with JWKS URLs that have been registered with the cache. +// Please read the documentation fo `(jwk.CachedFetcher).Fetch` for more details. +// +// This object is intended to be used with `jws.WithVerifyAuto` option, specifically +// for a scenario where there is a very small number of JWKS URLs that are trusted +// and used to verify JWS messages. It is NOT meant to be used as a general purpose +// caching fetcher object. +type CachedFetcher struct { + cache *Cache } -func getGlobalFetcher() httprc.Fetcher { - if v := atomic.LoadUint32(&fetcherChanged); v == 0 { - return globalFetcher - } - - muGlobalFetcher.Lock() - defer muGlobalFetcher.Unlock() - if globalFetcher == nil { - var nworkers int - v := os.Getenv(`JWK_FETCHER_WORKER_COUNT`) - if c, err := strconv.ParseInt(v, 10, 64); err == nil { - if c > math.MaxInt { - nworkers = math.MaxInt - } else { - nworkers = int(c) - } - } - if nworkers < 1 { - nworkers = 3 - } - - globalFetcher = httprc.NewFetcher(context.Background(), httprc.WithFetcherWorkerCount(nworkers)) - } - - atomic.StoreUint32(&fetcherChanged, 0) - return globalFetcher +// Creates a new `jwk.CachedFetcher` object. +func NewCachedFetcher(cache *Cache) *CachedFetcher { + return &CachedFetcher{cache} } -// SetGlobalFetcher allows users to specify a custom global fetcher, -// which is used by the `Fetch` function. Assigning `nil` forces -// the default fetcher to be (re)created when the next call to -// `jwk.Fetch` occurs -// -// You only need to call this function when you want to -// either change the fetching behavior (for example, you want to change -// how the default whitelist is handled), or when you want to control -// the lifetime of the global fetcher, for example for tests -// that require a clean shutdown. -// -// If you do use this function to set a custom fetcher and you -// control its termination, make sure that you call `jwk.SetGlobalFetcher()` -// one more time (possibly with `nil`) to assign a valid fetcher. -// Otherwise, once the fetcher is invalidated, subsequent calls to `jwk.Fetch` -// may hang, causing very hard to debug problems. -// -// If you are sure you no longer need `jwk.Fetch` after terminating the -// fetcher, then you the above caution is not necessary. -func SetGlobalFetcher(f httprc.Fetcher) { - muGlobalFetcher.Lock() - globalFetcher = f - muGlobalFetcher.Unlock() - atomic.StoreUint32(&fetcherChanged, 1) +// Fetch fetches a JWKS from the cache. If the JWKS URL has not been registered with +// the cache, an error is returned. +func (f *CachedFetcher) Fetch(ctx context.Context, u string, _ ...FetchOption) (Set, error) { + if !f.cache.IsRegistered(u) { + return nil, fmt.Errorf(`jwk.CachedFetcher: url %q has not been registered`, u) + } + return f.cache.Get(ctx, u) } // Fetch fetches a JWK resource specified by a URL. The url must be @@ -93,17 +64,10 @@ func SetGlobalFetcher(f httprc.Fetcher) { // contents of the object with the data at the remote resource, // consider using `jwk.Cache`, which automatically refreshes // jwk.Set objects asynchronously. -// -// Please note that underneath the `jwk.Fetch` function, it uses a global -// object that spawns goroutines that are present until the go runtime -// exits. Initially this global variable is uninitialized, but upon -// calling `jwk.Fetch` once, it is initialized and goroutines are spawned. -// If you want to control the lifetime of these goroutines, you can -// call `jwk.SetGlobalFetcher` with a custom fetcher which is tied to -// a `context.Context` object that you can control. func Fetch(ctx context.Context, u string, options ...FetchOption) (Set, error) { - var hrfopts []httprc.FetchOption var parseOptions []ParseOption + var wl Whitelist = InsecureWhitelist{} + var client HTTPClient = http.DefaultClient for _, option := range options { if parseOpt, ok := option.(ParseOption); ok { parseOptions = append(parseOptions, parseOpt) @@ -113,21 +77,34 @@ func Fetch(ctx context.Context, u string, options ...FetchOption) (Set, error) { //nolint:forcetypeassert switch option.Ident() { case identHTTPClient{}: - hrfopts = append(hrfopts, httprc.WithHTTPClient(option.Value().(HTTPClient))) + client = option.Value().(HTTPClient) case identFetchWhitelist{}: - hrfopts = append(hrfopts, httprc.WithWhitelist(option.Value().(httprc.Whitelist))) + wl = option.Value().(httprc.Whitelist) } } - res, err := getGlobalFetcher().Fetch(ctx, u, hrfopts...) + if !wl.IsAllowed(u) { + return nil, fmt.Errorf(`jwk.Fetch: url %q has been rejected by whitelist`, u) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) + if err != nil { + return nil, fmt.Errorf(`jwk.Fetch: failed to create new request: %w`, err) + } + + res, err := client.Do(req) if err != nil { - return nil, fmt.Errorf(`failed to fetch %q: %w`, u, err) + return nil, fmt.Errorf(`jwk.Fetch: request failed: %w`, err) + } + + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf(`jwk.Fetch: request returned status %d, expected 200`, res.StatusCode) } buf, err := io.ReadAll(res.Body) defer res.Body.Close() if err != nil { - return nil, fmt.Errorf(`failed to read response body for %q: %w`, u, err) + return nil, fmt.Errorf(`jwk.Fetch: failed to read response body for %q: %w`, u, err) } return Parse(buf, parseOptions...) diff --git a/jwk/jwk_test.go b/jwk/jwk_test.go index 16f70a4b7..fb1703cbc 100644 --- a/jwk/jwk_test.go +++ b/jwk/jwk_test.go @@ -1888,6 +1888,14 @@ func TestSetWithPrivateParams(t *testing.T) { }) } +type DummyRoundTripper struct{} + +func (t *DummyRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusTeapot, + Body: io.NopCloser(bytes.NewReader([]byte(nil))), + }, nil +} func TestFetch(t *testing.T) { k1, err := jwxtest.GenerateRsaJwk() if !assert.NoError(t, err, `jwxtest.GenerateRsaJwk should succeed`) { @@ -1915,81 +1923,99 @@ func TestFetch(t *testing.T) { w.WriteHeader(http.StatusOK) w.Write(expected) })) + // This test is commented out because we did not want to include `go.uber.org/goleak` + // in our dependency. We agree that it's important to check for goroutine leaks, + // but 1) this is sort of expected in this library, and 2) we don't believe that + // forcing all of our users to use `go.uber.org/goleak` is prudent. + // + // defer goleak.VerifyNone(t) defer srv.Close() - testcases := []struct { - Name string - Whitelist func() jwk.Whitelist - Error bool - }{ - { - Name: `InsecureWhitelist`, - Whitelist: func() jwk.Whitelist { - return jwk.InsecureWhitelist{} + t.Run("HTTPClient", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + client := &http.Client{ + Transport: &DummyRoundTripper{}, + } + _, err := jwk.Fetch(ctx, `https://www.googleapis.com/oauth2/v3/certs`, jwk.WithHTTPClient(client)) + require.Error(t, err, `jwk.Fetch should fail`) + require.Contains(t, err.Error(), `418`, `error should contain 418`) + }) + t.Run("Whitelist", func(t *testing.T) { + testcases := []struct { + Name string + Whitelist func() jwk.Whitelist + Error bool + }{ + { + Name: `InsecureWhitelist`, + Whitelist: func() jwk.Whitelist { + return jwk.InsecureWhitelist{} + }, }, - }, - { - Name: `MapWhitelist`, - Error: true, - Whitelist: func() jwk.Whitelist { - return jwk.NewMapWhitelist(). - Add(`https://www.googleapis.com/oauth2/v3/certs`). - Add(srv.URL) + { + Name: `MapWhitelist`, + Error: true, + Whitelist: func() jwk.Whitelist { + return jwk.NewMapWhitelist(). + Add(`https://www.googleapis.com/oauth2/v3/certs`). + Add(srv.URL) + }, }, - }, - { - Name: `RegexpWhitelist`, - Error: true, - Whitelist: func() jwk.Whitelist { - return jwk.NewRegexpWhitelist(). - Add(regexp.MustCompile(regexp.QuoteMeta(srv.URL))) + { + Name: `RegexpWhitelist`, + Error: true, + Whitelist: func() jwk.Whitelist { + return jwk.NewRegexpWhitelist(). + Add(regexp.MustCompile(regexp.QuoteMeta(srv.URL))) + }, }, - }, - { - Name: `WhitelistFunc`, - Error: true, - Whitelist: func() jwk.Whitelist { - return jwk.WhitelistFunc(func(s string) bool { - return s == srv.URL - }) + { + Name: `WhitelistFunc`, + Error: true, + Whitelist: func() jwk.Whitelist { + return jwk.WhitelistFunc(func(s string) bool { + return s == srv.URL + }) + }, }, - }, - } + } - for _, tc := range testcases { - tc := tc - t.Run(tc.Name, func(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + for _, tc := range testcases { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() - wl := tc.Whitelist() + wl := tc.Whitelist() - _, err = jwk.Fetch(ctx, `https://github.com/lestrrat-go/jwx/`, jwk.WithFetchWhitelist(wl)) - if tc.Error { - if !assert.Error(t, err, `jwk.Fetch should fail`) { - return + _, err = jwk.Fetch(ctx, `https://github.com/lestrrat-go/jwx/`, jwk.WithFetchWhitelist(wl)) + if tc.Error { + if !assert.Error(t, err, `jwk.Fetch should fail`) { + return + } + if !assert.True(t, strings.Contains(err.Error(), `rejected by whitelist`), `error should be whitelist error`) { + t.Logf("error was %q", err.Error()) + return + } } - if !assert.True(t, strings.Contains(err.Error(), `rejected by whitelist`), `error should be whitelist error`) { - t.Logf("error was %q", err.Error()) + + fetched, err := jwk.Fetch(ctx, srv.URL, jwk.WithFetchWhitelist(wl)) + if !assert.NoError(t, err, `jwk.Fetch should succeed`) { return } - } - - fetched, err := jwk.Fetch(ctx, srv.URL, jwk.WithFetchWhitelist(wl)) - if !assert.NoError(t, err, `jwk.Fetch should succeed`) { - return - } - got, err := json.MarshalIndent(fetched, "", " ") - if !assert.NoError(t, err, `json.MarshalIndent should succeed`) { - return - } + got, err := json.MarshalIndent(fetched, "", " ") + if !assert.NoError(t, err, `json.MarshalIndent should succeed`) { + return + } - if !assert.Equal(t, expected, got, `data should match`) { - return - } - }) - } + if !assert.Equal(t, expected, got, `data should match`) { + return + } + }) + } + }) } func TestGH567(t *testing.T) { @@ -2174,52 +2200,6 @@ func TestECDSAPEM(t *testing.T) { } } -// This test is commented out because we did not want to include `go.uber.org/goleak` -// in our dependency. We agree that it's important to check for goroutine leaks, -// but 1) this is sort of expected in this library, and 2) we don't believe that -// forcing all of our users to use `go.uber.org/goleak` is prudent. -// -// However, we are leaving this test here so that users can learn how this function -// can be used. This is meant to show you the boilerplate code for when you want to make -// absolutely sure that no goroutine is left when you finish your program. -// -// For example, if you are writing an app, you can follow the pattern in the -// test below and stop the goroutines that are started by httprc.NewFetcher -// when your app terminates: -// -// func AppMain() { -// ctx, cancel := context.WithCancel(context.Background()) -// defer cancel() -// -// jwk.SetGlobalFetcher(http.NewFetcher(ctx)) -// // your app code goes here -// } -// -// Then, you can be sure that no goroutines are left when `AppMain()` is done. -// You can also verify this in your test: -// -// func TestApp(t *testing.T) { -// AppMain() -// goleak.VerifyNone(t) -// } -/* -func TestGH928(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - jwk.SetGlobalFetcher(httprc.NewFetcher(ctx)) - - // If you are using a custom fetcher like this in your test and you - // still have more tests to run, you probably want to include the - // following line to restore the default behavior after this test. - // Otherwise, calls to `jwk.Fetch` may hang. - defer jwk.SetGlobalFetcher(nil) // This must be included to restore default behavior - - cancel() // stop fetcher goroutines - - // At this point, not goroutines from httprc.Fetcher should be running - goleak.VerifyNone(t) -} -*/ - func TestGH947(t *testing.T) { // AS OP described it. Below case will panic if the problem exists, raw := []byte(`{"crv":"Ed25519","d":"","x":"","kty":"OKP"}`) diff --git a/jwk/whitelist.go b/jwk/whitelist.go index 6b0180d30..9b144dc4e 100644 --- a/jwk/whitelist.go +++ b/jwk/whitelist.go @@ -28,7 +28,7 @@ func (w *RegexpWhitelist) Add(pat *regexp.Regexp) *RegexpWhitelist { return w } -// IsAlloed returns true if any of the patterns in the whitelist +// IsAllowed returns true if any of the patterns in the whitelist // returns true. func (w *RegexpWhitelist) IsAllowed(u string) bool { for _, pat := range w.patterns { diff --git a/jws/BUILD.bazel b/jws/BUILD.bazel index 34ee8a96d..587bc8ca8 100644 --- a/jws/BUILD.bazel +++ b/jws/BUILD.bazel @@ -54,7 +54,7 @@ go_test( "//jwa", "//jwk", "//jwt", - "@com_github_lestrrat_go_httprc//:httprc", + "@com_github_lestrrat_go_httprc_v2//:httprc", "@com_github_stretchr_testify//assert", "@com_github_stretchr_testify//require", ], diff --git a/jws/jws_test.go b/jws/jws_test.go index 9eff521d6..bb9e48920 100644 --- a/jws/jws_test.go +++ b/jws/jws_test.go @@ -24,7 +24,7 @@ import ( "testing" "time" - "github.com/lestrrat-go/httprc" + "github.com/lestrrat-go/httprc/v2" "github.com/lestrrat-go/jwx/v3/internal/base64" "github.com/lestrrat-go/jwx/v3/internal/json" "github.com/lestrrat-go/jwx/v3/internal/jwxtest" @@ -1384,20 +1384,14 @@ func TestJKU(t *testing.T) { }, }, { - Name: "JWKFetcher", + Name: "Cache", Fetcher: func() jwk.Fetcher { c := jwk.NewCache(context.TODO()) - return jwk.FetchFunc(func(ctx context.Context, u string, options ...jwk.FetchOption) (jwk.Set, error) { - var cacheopts []jwk.RegisterOption - for _, option := range options { - cacheopts = append(cacheopts, option) - } - cacheopts = append(cacheopts, jwk.WithHTTPClient(srv.Client())) - cacheopts = append(cacheopts, jwk.WithFetchWhitelist(httprc.InsecureWhitelist{})) - c.Register(u, cacheopts...) - - return c.Get(ctx, u) - }) + c.Register(srv.URL, + jwk.WithHTTPClient(srv.Client()), + jwk.WithFetchWhitelist(httprc.InsecureWhitelist{}), + ) + return jwk.NewCachedFetcher(c) }, }, } diff --git a/jws/key_provider.go b/jws/key_provider.go index c5fbfc6af..e767cd60f 100644 --- a/jws/key_provider.go +++ b/jws/key_provider.go @@ -216,6 +216,10 @@ type jkuProvider struct { } func (kp jkuProvider) FetchKeys(ctx context.Context, sink KeySink, sig *Signature, _ *Message) error { + if kp.fetcher == nil { + kp.fetcher = jwk.FetchFunc(jwk.Fetch) + } + kid := sig.ProtectedHeaders().KeyID() if kid == "" { return fmt.Errorf(`use of "jku" requires that the payload contain a "kid" field in the protected header`) diff --git a/jws/options.go b/jws/options.go index 1a8b6cad8..a1d1c7e54 100644 --- a/jws/options.go +++ b/jws/options.go @@ -170,11 +170,39 @@ func WithKeySet(set jwk.Set, options ...WithKeySetSuboption) VerifyOption { }) } +// WithVerifyAuto enables automatic verification of the signature using the JWKS specified in +// the `jku` header. Note that by default this option will _reject_ any jku +// provided by the JWS message. Read on for details. +// +// The JWKS is retrieved by the `jwk.Fetcher` specified in the first argument. +// If the fetcher object is nil, the default fetcher, which is the `jwk.Fetch()` +// function (wrapped in the `jwk.FetchFunc` type) is used. +// +// The remaining arguments are passed to the `(jwk.Fetcher).Fetch` method +// when the JWKS is retrieved. +// +// jws.WithVerifyAuto(nil) // uses jwk.Fetch +// jws.WithVerifyAuto(jwk.NewCachedFetcher(...)) // uses cached fetcher +// jws.WithVerifyAuto(myFetcher) // use your custom fetcher +// +// By default a whitelist that disallows all URLs is added to the options +// passed to the fetcher. You must explicitly specify a whitelist that allows +// the URLs you trust. This default behavior is provided because by design +// of the JWS specification it is the/ caller's responsibility to verify if +// the URL specified in the `jku` header can be trusted -- thus by default +// we trust nothing. +// +// Users are free to specify an open whitelist if they so choose, but this must +// be explicitly done: +// +// jws.WithVerifyAuto(nil, jwk.WithFetchWhitelist(jwk.InsecureWhitelist())) +// +// You can also use `jwk.CachedFetcher` to use cached JWKS objects, but do note +// that this object is not really designed to accommodate a large set of +// arbitrary URLs. Use `jwk.CachedFetcher` as the first argument if you only +// have a small set of URLs that you trust. For anything more complex, you should +// implement your own `jwk.Fetcher` object. func WithVerifyAuto(f jwk.Fetcher, options ...jwk.FetchOption) VerifyOption { - if f == nil { - f = jwk.FetchFunc(jwk.Fetch) - } - // the option MUST start with a "disallow no whitelist" to force // users provide a whitelist options = append(append([]jwk.FetchOption(nil), jwk.WithFetchWhitelist(allowNoneWhitelist)), options...) diff --git a/jwt/options.go b/jwt/options.go index 61e0b638b..d1f27d179 100644 --- a/jwt/options.go +++ b/jwt/options.go @@ -282,22 +282,7 @@ func WithMinDelta(dur time.Duration, c1, c2 string) ValidateOption { // method available is to use the keys available in the JWKS URL pointed // in the `jku` field. // -// The first argument should either be `nil`, or your custom jwk.Fetcher -// object, which tells how the JWKS should be fetched. Leaving it to -// `nil` is equivalent to specifying that `jwk.Fetch` should be used. -// -// You can further pass options to customize the fetching behavior. -// -// One notable difference in the option available via the `jwt` -// package and the `jws.Verify()` or `jwk.Fetch()` functions is that -// by default all fetching is disabled unless you explicitly whitelist urls. -// Therefore, when you use this option you WILL have to specify at least -// the `jwk.WithFetchWhitelist()` suboption: as: -// -// jwt.Parse(data, jwt.WithVerifyAuto(nil, jwk.WithFetchWhitelist(...))) -// -// See the list of available options that you can pass to `jwk.Fetch()` -// in the `jwk` package +// Please read the documentation for `jws.VerifyAuto` for more details. func WithVerifyAuto(f jwk.Fetcher, options ...jwk.FetchOption) ParseOption { return &parseOption{option.New(identVerifyAuto{}, jws.WithVerifyAuto(f, options...))} }