Skip to content

Commit

Permalink
Upgrade jwt and keyfunc (#1005)
Browse files Browse the repository at this point in the history
* Renovate Update module github.com/golang-jwt/jwt/v4 to v5

* Update use of jwt and keyfunc packages

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
  • Loading branch information
hawx and renovate[bot] authored Jan 30, 2024
1 parent cf8c512 commit 9f76740
Show file tree
Hide file tree
Showing 12 changed files with 77 additions and 72 deletions.
2 changes: 1 addition & 1 deletion cmd/mlpa/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"github.com/aws/aws-sdk-go-v2/aws"
v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/golang-jwt/jwt/v4"
"github.com/golang-jwt/jwt/v5"
"github.com/gorilla/handlers"
"github.com/gorilla/sessions"
"github.com/ministryofjustice/opg-go-common/env"
Expand Down
6 changes: 3 additions & 3 deletions cmd/mock-onelogin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"strings"
"time"

"github.com/golang-jwt/jwt/v4"
"github.com/golang-jwt/jwt/v5"
"github.com/ministryofjustice/opg-go-common/env"
)

Expand Down Expand Up @@ -85,8 +85,8 @@ func jwks() http.HandlerFunc {
"use": "sig",
"crv": "P-256",
"kid": tokenSigningKid,
"x": base64.URLEncoding.EncodeToString(publicKey.X.Bytes()),
"y": base64.URLEncoding.EncodeToString(publicKey.Y.Bytes()),
"x": base64.RawURLEncoding.EncodeToString(publicKey.X.Bytes()),
"y": base64.RawURLEncoding.EncodeToString(publicKey.Y.Bytes()),
"alg": "ES256",
},
},
Expand Down
6 changes: 4 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ module github.com/ministryofjustice/opg-modernising-lpa
go 1.21

require (
github.com/MicahParks/keyfunc v1.9.0
github.com/MicahParks/jwkset v0.5.7
github.com/MicahParks/keyfunc/v3 v3.2.4
github.com/aws/aws-lambda-go v1.45.0
github.com/aws/aws-sdk-go-v2 v1.24.1
github.com/aws/aws-sdk-go-v2/config v1.26.6
Expand All @@ -17,7 +18,7 @@ require (
github.com/dustin/go-humanize v1.0.1
github.com/felixge/httpsnoop v1.0.4
github.com/gabriel-vasile/mimetype v1.4.3
github.com/golang-jwt/jwt/v4 v4.5.0
github.com/golang-jwt/jwt/v5 v5.2.0
github.com/google/uuid v1.6.0
github.com/gorilla/handlers v1.5.2
github.com/gorilla/sessions v1.2.2
Expand Down Expand Up @@ -101,6 +102,7 @@ require (
golang.org/x/sys v0.16.0 // indirect
golang.org/x/term v0.16.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/time v0.5.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20231106174013-bbf56f31fb17 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20231106174013-bbf56f31fb17 // indirect
google.golang.org/protobuf v1.32.0 // indirect
Expand Down
13 changes: 8 additions & 5 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3f
dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
github.com/MicahParks/keyfunc v1.9.0 h1:lhKd5xrFHLNOWrDc4Tyb/Q1AJ4LCzQ48GVJyVIID3+o=
github.com/MicahParks/keyfunc v1.9.0/go.mod h1:IdnCilugA0O/99dW+/MkvlyrsX8+L8+x95xuVNtM5jw=
github.com/MicahParks/jwkset v0.5.7 h1:eHUGJrSO8EetbNnSb7xhlYQ9mX0vQ7Ga9wN1HhGL3i4=
github.com/MicahParks/jwkset v0.5.7/go.mod h1:q8ptTGn/Z9c4MwbcfeCDssADeVQb3Pk7PnVxrvi+2QY=
github.com/MicahParks/keyfunc/v3 v3.2.4 h1:SuFGdd3HvlwEceJvlEEfjJjvOiq69hS0wqM5iMbTlaA=
github.com/MicahParks/keyfunc/v3 v3.2.4/go.mod h1:xXfj5uRVZsOMGpjKH/AYn7V8OJb0UMQlJLP1GRBuizg=
github.com/aws/aws-lambda-go v1.45.0 h1:3xS35Dlc8ffmcwfcKTyqJGiMuL0UDvkQaVUrI5yHycI=
github.com/aws/aws-lambda-go v1.45.0/go.mod h1:dpMpZgvWx5vuQJfBt0zqBha60q7Dd7RfgJv23DymV8A=
github.com/aws/aws-sdk-go-v2 v1.24.1 h1:xAojnj+ktS95YZlDf0zxWBkbFtymPeDP+rvUQIH3uAU=
Expand Down Expand Up @@ -148,9 +150,8 @@ github.com/gobwas/httphead v0.0.0-20180130184737-2c6c146eadee/go.mod h1:L0fX3K22
github.com/gobwas/pool v0.2.0/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw=
github.com/gobwas/ws v1.0.2/go.mod h1:szmBTxLgaFppYjEmNtny/v3w89xOydFnnZMcgRRu/EM=
github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/golang-jwt/jwt/v4 v4.4.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg=
github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
github.com/golang-jwt/jwt/v5 v5.2.0 h1:d/ix8ftRUorsN+5eMIlF4T6J8CAt9rch3My2winC1Jw=
github.com/golang-jwt/jwt/v5 v5.2.0/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/golang/glog v1.1.2 h1:DVjP2PbBOzHyzA+dn3WhHIq4NdVu3Q+pvivFICf/7fo=
github.com/golang/glog v1.1.2/go.mod h1:zR+okUeTbrL6EL3xHUDxZuEtGv04p5shwip1+mL/rLQ=
Expand Down Expand Up @@ -540,6 +541,8 @@ golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk=
golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=
Expand Down
2 changes: 1 addition & 1 deletion internal/lpastore/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"net/http"
"time"

"github.com/golang-jwt/jwt/v4"
"github.com/golang-jwt/jwt/v5"
"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
"github.com/ministryofjustice/opg-modernising-lpa/internal/date"
"github.com/ministryofjustice/opg-modernising-lpa/internal/place"
Expand Down
2 changes: 1 addition & 1 deletion internal/notify/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"strings"
"time"

"github.com/golang-jwt/jwt/v4"
"github.com/golang-jwt/jwt/v5"
)

type Doer interface {
Expand Down
34 changes: 22 additions & 12 deletions internal/onelogin/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import (
"encoding/json"
"errors"
"net/http"
"net/url"
"time"

"github.com/MicahParks/keyfunc"
"github.com/golang-jwt/jwt/v4"
"github.com/MicahParks/jwkset"
"github.com/MicahParks/keyfunc/v3"
"github.com/golang-jwt/jwt/v5"
)

const (
Expand Down Expand Up @@ -38,7 +40,7 @@ type configurationClient struct {
refreshRequest chan (struct{})

currentConfiguration *openidConfiguration
currentJwks *keyfunc.JWKS
currentJwks keyfunc.Keyfunc
}

func getConfiguration(ctx context.Context, logger Logger, httpClient *http.Client, issuer string) *configurationClient {
Expand Down Expand Up @@ -126,18 +128,26 @@ func (c *configurationClient) refresh() error {
return err
}

c.currentConfiguration = &v
c.currentJwks, err = keyfunc.Get(c.currentConfiguration.JwksURI, keyfunc.Options{
Client: c.httpClient,
Ctx: c.ctx,
RefreshErrorHandler: func(err error) {
uri, err := url.Parse(v.JwksURI)
if err != nil {
return err
}

storage, err := jwkset.NewStorageFromHTTP(uri, jwkset.HTTPClientStorageOptions{
Ctx: c.ctx,
Client: c.httpClient,
RefreshInterval: refreshInterval,
HTTPTimeout: refreshTimeout,
RefreshErrorHandler: func(_ context.Context, err error) {
c.logger.Print("error refreshing jwks: ", err)
},
RefreshInterval: refreshInterval,
RefreshRateLimit: refreshRateLimit,
RefreshTimeout: refreshTimeout,
RefreshUnknownKID: true,
})
if err != nil {
return err
}

c.currentConfiguration = &v
c.currentJwks, err = keyfunc.New(keyfunc.Options{Ctx: c.ctx, Storage: storage})

return err
}
Expand Down
7 changes: 4 additions & 3 deletions internal/onelogin/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"testing"
"time"

"github.com/MicahParks/keyfunc"
"github.com/MicahParks/keyfunc/v3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)
Expand Down Expand Up @@ -105,7 +105,7 @@ func TestConfigurationClientForExchange(t *testing.T) {
TokenEndpoint: "TokenEndpoint",
Issuer: "Issuer",
},
currentJwks: &keyfunc.JWKS{},
currentJwks: &mockKeyfunc{},
}

tokenEndpoint, keyfunc, issuer, err := client.ForExchange()
Expand All @@ -117,10 +117,11 @@ func TestConfigurationClientForExchange(t *testing.T) {

func TestConfigurationClientForExchangeWhenMissing(t *testing.T) {
ch := make(chan struct{}, 1)
keys, _ := keyfunc.New(keyfunc.Options{})

testcases := map[string]*configurationClient{
"configuration": {
currentJwks: &keyfunc.JWKS{},
currentJwks: keys,
refreshRequest: ch,
},
"jwks": {
Expand Down
17 changes: 6 additions & 11 deletions internal/onelogin/exchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"strings"
"time"

"github.com/golang-jwt/jwt/v4"
"github.com/golang-jwt/jwt/v5"
"github.com/ministryofjustice/opg-modernising-lpa/internal/secrets"
)

Expand Down Expand Up @@ -95,23 +95,18 @@ func (c *Client) Exchange(ctx context.Context, code, nonce string) (idToken, acc
}

func (c *Client) validateToken(keyfunc jwt.Keyfunc, issuer, idToken, nonce string) error {
token, err := jwt.ParseWithClaims(idToken, jwt.MapClaims{}, keyfunc)
token, err := jwt.ParseWithClaims(idToken, jwt.MapClaims{}, keyfunc,
jwt.WithIssuer(issuer),
jwt.WithAudience(c.clientID),
jwt.WithIssuedAt())
if err != nil {
return err
}

if !token.Valid {
return fmt.Errorf("idToken not valid")
}

claims := token.Claims.(jwt.MapClaims)
if !claims.VerifyIssuer(issuer, true) {
return jwt.ErrTokenInvalidIssuer
}
if !claims.VerifyAudience(c.clientID, true) {
return jwt.ErrTokenInvalidAudience
}
if claims["nonce"] != nonce {
if claims := token.Claims.(jwt.MapClaims); claims["nonce"] != nonce {
return fmt.Errorf("nonce is invalid")
}

Expand Down
29 changes: 14 additions & 15 deletions internal/onelogin/exchange_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,28 @@ import (
"crypto/x509"
"encoding/json"
"encoding/pem"
"io/ioutil"
"io"
"math/rand"
"net/http"
"testing"
"time"

"github.com/MicahParks/keyfunc"
"github.com/golang-jwt/jwt/v4"
"github.com/MicahParks/jwkset"
"github.com/golang-jwt/jwt/v5"
"github.com/ministryofjustice/opg-modernising-lpa/internal/secrets"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)

var ctx = context.Background()

type mockKeyfunc struct{}

func (*mockKeyfunc) Keyfunc(*jwt.Token) (any, error) { return []byte("my-key"), nil }
func (*mockKeyfunc) Storage() jwkset.Storage { return (jwkset.Storage)(nil) }

func TestExchange(t *testing.T) {
privateKey, _ := rsa.GenerateKey(rand.New(rand.NewSource(99)), 2048)
jwks := keyfunc.NewGiven(map[string]keyfunc.GivenKey{
"myKey": keyfunc.NewGivenHMAC([]byte("my-key")),
})

token, err := (&jwt.Token{
Header: map[string]interface{}{
Expand Down Expand Up @@ -85,7 +87,7 @@ func TestExchange(t *testing.T) {
})).
Return(&http.Response{
StatusCode: http.StatusOK,
Body: ioutil.NopCloser(bytes.NewReader(data)),
Body: io.NopCloser(bytes.NewReader(data)),
}, nil)

client := &Client{
Expand All @@ -96,7 +98,7 @@ func TestExchange(t *testing.T) {
Issuer: "http://issuer",
TokenEndpoint: "http://token",
},
currentJwks: jwks,
currentJwks: &mockKeyfunc{},
},
clientID: "client-id",
redirectURL: "http://redirect",
Expand Down Expand Up @@ -128,7 +130,7 @@ func TestExchangeWhenPrivateKeyError(t *testing.T) {
secretsClient: secretsClient,
openidConfiguration: &configurationClient{
currentConfiguration: &openidConfiguration{},
currentJwks: &keyfunc.JWKS{},
currentJwks: &mockKeyfunc{},
},
}

Expand Down Expand Up @@ -161,7 +163,7 @@ func TestExchangeWhenTokenRequestError(t *testing.T) {
currentConfiguration: &openidConfiguration{
TokenEndpoint: "http://token",
},
currentJwks: &keyfunc.JWKS{},
currentJwks: &mockKeyfunc{},
},
randomString: func(i int) string { return "this-is-random" },
}
Expand All @@ -172,9 +174,6 @@ func TestExchangeWhenTokenRequestError(t *testing.T) {

func TestExchangeWhenInvalidToken(t *testing.T) {
privateKey, _ := rsa.GenerateKey(rand.New(rand.NewSource(99)), 2048)
jwks := keyfunc.NewGiven(map[string]keyfunc.GivenKey{
"myKey": keyfunc.NewGivenHMAC([]byte("my-key")),
})

testCases := map[string]struct {
claims jwt.MapClaims
Expand Down Expand Up @@ -307,7 +306,7 @@ func TestExchangeWhenInvalidToken(t *testing.T) {
})).
Return(&http.Response{
StatusCode: http.StatusOK,
Body: ioutil.NopCloser(bytes.NewReader(data)),
Body: io.NopCloser(bytes.NewReader(data)),
}, nil)

client := &Client{
Expand All @@ -318,7 +317,7 @@ func TestExchangeWhenInvalidToken(t *testing.T) {
Issuer: "http://issuer",
TokenEndpoint: "http://token",
},
currentJwks: jwks,
currentJwks: &mockKeyfunc{},
},
clientID: "client-id",
redirectURL: "http://redirect",
Expand Down
10 changes: 6 additions & 4 deletions internal/onelogin/user_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import (
"strings"
"time"

"github.com/golang-jwt/jwt/v4"
"github.com/golang-jwt/jwt/v5"
"github.com/ministryofjustice/opg-modernising-lpa/internal/date"
"github.com/ministryofjustice/opg-modernising-lpa/internal/identity"
)

var ErrMissingCoreIdentityJWT = errors.New("UserInfo missing CoreIdentityJWT property")

type UserInfo struct {
Sub string `json:"sub"`
Email string `json:"email"`
Expand Down Expand Up @@ -121,16 +123,16 @@ func (c *Client) ParseIdentityClaim(ctx context.Context, u UserInfo) (identity.U
}

if u.CoreIdentityJWT == "" {
return identity.UserData{}, errors.New("UserInfo missing CoreIdentityJWT property")
return identity.UserData{}, ErrMissingCoreIdentityJWT
}

token, err := jwt.ParseWithClaims(u.CoreIdentityJWT, &CoreIdentityClaims{}, func(token *jwt.Token) (any, error) {
if _, ok := token.Method.(*jwt.SigningMethodECDSA); !ok {
return nil, jwt.NewValidationError(fmt.Sprintf("signing method %v is invalid", token.Header["alg"]), jwt.ValidationErrorSignatureInvalid)
return nil, fmt.Errorf("signing method %v is invalid", token.Header["alg"])
}

return publicKey, nil
})
}, jwt.WithIssuedAt())
if err != nil {
return identity.UserData{}, err
}
Expand Down
Loading

0 comments on commit 9f76740

Please sign in to comment.