Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: upgrade to jose v4 library #824

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

chore: upgrade to jose v4 library #824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion authorize_request_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"net/http"
"strings"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"
"go.opentelemetry.io/otel/trace"

"github.com/ory/fosite/i18n"
Expand Down
2 changes: 1 addition & 1 deletion authorize_request_handler_oidc_request_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (

"github.com/pkg/errors"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down
2 changes: 1 addition & 1 deletion client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
package fosite

import (
"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"
)

// Client represents a client or an app.
Expand Down
2 changes: 1 addition & 1 deletion client_authentication.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (

"github.com/ory/x/errorsx"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"
"github.com/pkg/errors"

"github.com/ory/fosite/token/jwt"
Expand Down
2 changes: 1 addition & 1 deletion client_authentication_jwks_strategy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

"github.com/ory/x/errorsx"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"
)

const defaultJWKSFetcherStrategyCachePrefix = "github.com/ory/fosite.DefaultJWKSFetcherStrategy:"
Expand Down
2 changes: 1 addition & 1 deletion client_authentication_jwks_strategy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

"github.com/ory/fosite/internal/gen"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down
2 changes: 1 addition & 1 deletion client_authentication_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

"github.com/ory/fosite/internal/gen"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down
20 changes: 11 additions & 9 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2
github.com/cristalhq/jwt/v4 v4.0.2
github.com/dgraph-io/ristretto v0.1.1
github.com/go-jose/go-jose/v3 v3.0.3
github.com/go-jose/go-jose/v4 v4.0.4
github.com/golang/mock v1.6.0
github.com/google/uuid v1.3.0
github.com/gorilla/mux v1.8.0
Expand All @@ -25,13 +25,13 @@ require (
github.com/ory/x v0.0.575
github.com/parnurzeal/gorequest v0.2.15
github.com/pkg/errors v0.9.1
github.com/stretchr/testify v1.8.4
github.com/stretchr/testify v1.9.0
github.com/tidwall/gjson v1.14.3
go.opentelemetry.io/otel/trace v1.16.0
golang.org/x/crypto v0.21.0
golang.org/x/net v0.23.0
golang.org/x/crypto v0.25.0
golang.org/x/net v0.25.0
golang.org/x/oauth2 v0.10.0
golang.org/x/text v0.14.0
golang.org/x/text v0.16.0
)

require (
Expand Down Expand Up @@ -83,9 +83,9 @@ require (
go.opentelemetry.io/otel/metric v1.16.0 // indirect
go.opentelemetry.io/otel/sdk v1.16.0 // indirect
go.opentelemetry.io/proto/otlp v1.0.0 // indirect
golang.org/x/mod v0.12.0 // indirect
golang.org/x/sys v0.20.0 // indirect
golang.org/x/tools v0.11.1 // indirect
golang.org/x/mod v0.17.0 // indirect
golang.org/x/sys v0.22.0 // indirect
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20230731193218-e0aa005b6bdf // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20230731193218-e0aa005b6bdf // indirect
Expand All @@ -96,4 +96,6 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
)

go 1.20
go 1.21

toolchain go1.23.1
67 changes: 42 additions & 25 deletions go.sum
View file
Open in desktop

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions handler/rfc7523/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import (

"github.com/ory/fosite/handler/oauth2"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v3/jwt"
"github.com/go-jose/go-jose/v4"
"github.com/go-jose/go-jose/v4/jwt"

"github.com/ory/fosite"
fositeJWT "github.com/ory/fosite/token/jwt"
"github.com/ory/x/errorsx"
)

Expand Down Expand Up @@ -51,7 +52,7 @@ func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite
return errorsx.WithStack(fosite.ErrInvalidRequest.WithHintf("The assertion request parameter must be set when using grant_type of '%s'.", grantTypeJWTBearer))
}

token, err := jwt.ParseSigned(assertion)
token, err := jwt.ParseSigned(assertion, fositeJWT.SupportedSignatureAlgorithms)
if err != nil {
return errorsx.WithStack(fosite.ErrInvalidGrant.
WithHint("Unable to parse JSON Web Token passed in \"assertion\" request parameter.").
Expand Down
6 changes: 3 additions & 3 deletions handler/rfc7523/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (

"github.com/ory/fosite/handler/oauth2"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v3/jwt"
"github.com/go-jose/go-jose/v4"
"github.com/go-jose/go-jose/v4/jwt"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/suite"

Expand Down Expand Up @@ -760,7 +760,7 @@ func (s *AuthorizeJWTGrantRequestHandlerTestSuite) createTestAssertion(cl jwt.Cl
s.FailNowf("failed to create test assertion", "failed to create signer: %s", err.Error())
}

raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize()
raw, err := jwt.Signed(sig).Claims(cl).Serialize()
if err != nil {
s.FailNowf("failed to create test assertion", "failed to sign assertion: %s", err.Error())
}
Expand Down
2 changes: 1 addition & 1 deletion handler/rfc7523/storage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"context"
"time"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"
)

// RFC7523KeyStorage holds information needed to validate jwt assertion in authorization grants.
Expand Down
2 changes: 1 addition & 1 deletion integration/authorize_jwt_bearer_required_iat_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

"github.com/stretchr/testify/require"

"github.com/go-jose/go-jose/v3/jwt"
"github.com/go-jose/go-jose/v4/jwt"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
Expand Down
2 changes: 1 addition & 1 deletion integration/authorize_jwt_bearer_required_jti_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"testing"
"time"

"github.com/go-jose/go-jose/v3/jwt"
"github.com/go-jose/go-jose/v4/jwt"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
Expand Down
2 changes: 1 addition & 1 deletion integration/authorize_jwt_bearer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

"github.com/stretchr/testify/require"

"github.com/go-jose/go-jose/v3/jwt"
"github.com/go-jose/go-jose/v4/jwt"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
Expand Down
6 changes: 3 additions & 3 deletions integration/clients/jwt_bearer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
"net/url"
"strings"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v3/jwt"
"github.com/go-jose/go-jose/v4"
"github.com/go-jose/go-jose/v4/jwt"
)

// #nosec:gosec G101 - False Positive
Expand Down Expand Up @@ -69,7 +69,7 @@ func (c *JWTBearer) GetToken(ctx context.Context, payloadData *JWTBearerPayload,
Claims(payloadData.Claims).
Claims(payloadData.PrivateClaims)

assertion, err := builder.CompactSerialize()
assertion, err := builder.Serialize()
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion integration/helper_setup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/ory/fosite/internal"
"github.com/ory/fosite/internal/gen"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"
"github.com/gorilla/mux"
goauth "golang.org/x/oauth2"
"golang.org/x/oauth2/clientcredentials"
Expand Down
2 changes: 1 addition & 1 deletion integration/introspect_jwt_bearer_token_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (

"github.com/stretchr/testify/require"

"github.com/go-jose/go-jose/v3/jwt"
"github.com/go-jose/go-jose/v4/jwt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"

Expand Down
2 changes: 1 addition & 1 deletion internal/oauth2_auth_jwt_storage.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion storage/memory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"sync"
"time"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"

"github.com/ory/fosite"
"github.com/ory/fosite/internal"
Expand Down
2 changes: 1 addition & 1 deletion token/jwt/jwt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"crypto/sha256"
"strings"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"

"github.com/ory/x/errorsx"

Expand Down
2 changes: 1 addition & 1 deletion token/jwt/jwt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"testing"
"time"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v4"

"github.com/ory/fosite/internal/gen"

Expand Down
2 changes: 1 addition & 1 deletion token/jwt/map_claims.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"errors"
"time"

jjson "github.com/go-jose/go-jose/v3/json"
jjson "github.com/go-jose/go-jose/v4/json"

"github.com/ory/x/errorsx"
)
Expand Down
16 changes: 11 additions & 5 deletions token/jwt/token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"fmt"
"reflect"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v3/jwt"
"github.com/go-jose/go-jose/v4"
"github.com/go-jose/go-jose/v4/jwt"

"github.com/ory/x/errorsx"
)
Expand All @@ -37,6 +37,12 @@ const (
JWTHeaderTypeValue = "JWT"
)

var SupportedSignatureAlgorithms = []jose.SignatureAlgorithm{
SigningMethodNone,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have missed some here that would cause some breaking changes in existing systems because the validation has changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I listed everything standardized and available and secure. In the worst case though, this is a publicly accessible variable in Go, so people can extend it if we did miss something. Not nice API though.

But as I mentioned in my PR description, I really think we should do a backwards incompatible change after this, to address current code rot around JWTs in fosite and address some vulnerabilities available exactly because we allow for example to try all algorithms and not just the one expected. But this is probably something only you can decide on how much breaking change would you approve (not in terms of external users changes, but in terms of API changes for fosite integrators - so some fosite structs we could just get rid of and delegate to go-jose).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure we validate the signing method somewhere, do we not?

jose.EdDSA, jose.HS256, jose.HS384, jose.HS512, jose.RS256, jose.RS384,
jose.RS512, jose.ES256, jose.ES384, jose.ES512, jose.PS256, jose.PS384, jose.PS512,
}

type unsafeNoneMagicConstant string

// Valid informs if the token was verified against a given verification key
Expand Down Expand Up @@ -96,10 +102,10 @@ func (t *Token) SignedString(k interface{}) (rawToken string, err error) {

// A explicit conversion from type alias MapClaims
// to map[string]interface{} is required because the
// go-jose CompactSerialize() only support explicit maps
// go-jose Serialize() only support explicit maps
// as claims or structs but not type aliases from maps.
claims := map[string]interface{}(t.Claims)
rawToken, err = jwt.Signed(signer).Claims(claims).CompactSerialize()
rawToken, err = jwt.Signed(signer).Claims(claims).Serialize()
if err != nil {
err = &ValidationError{Errors: ValidationErrorClaimsInvalid, Inner: err}
return
Expand Down Expand Up @@ -163,7 +169,7 @@ func Parse(tokenString string, keyFunc Keyfunc) (*Token, error) {
// If everything is kosher, err will be nil
func ParseWithClaims(rawToken string, claims MapClaims, keyFunc Keyfunc) (*Token, error) {
// Parse the token.
parsedToken, err := jwt.ParseSigned(rawToken)
parsedToken, err := jwt.ParseSigned(rawToken, SupportedSignatureAlgorithms)
if err != nil {
return &Token{}, &ValidationError{Errors: ValidationErrorMalformed, text: err.Error()}
}
Expand Down
8 changes: 4 additions & 4 deletions token/jwt/token_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import (

"github.com/ory/fosite/internal/gen"

"github.com/go-jose/go-jose/v3"
"github.com/go-jose/go-jose/v3/jwt"
"github.com/go-jose/go-jose/v4"
"github.com/go-jose/go-jose/v4/jwt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -54,7 +54,7 @@ func TestUnsignedToken(t *testing.T) {
parts := strings.Split(rawToken, ".")
require.Len(t, parts, 3)
require.Empty(t, parts[2])
tk, err := jwt.ParseSigned(rawToken)
tk, err := jwt.ParseSigned(rawToken, SupportedSignatureAlgorithms)
require.NoError(t, err)
require.Len(t, tk.Headers, 1)
require.Equal(t, tc.expectedType, tk.Headers[0].ExtraHeaders[jose.HeaderKey("typ")])
Expand Down Expand Up @@ -82,7 +82,7 @@ func TestJWTHeaders(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
rawToken := makeSampleTokenWithCustomHeaders(nil, jose.RS256, tc.jwtHeaders, gen.MustRSAKey())
tk, err := jwt.ParseSigned(rawToken)
tk, err := jwt.ParseSigned(rawToken, SupportedSignatureAlgorithms)
require.NoError(t, err)
require.Len(t, tk.Headers, 1)
require.Equal(t, tk.Headers[0].Algorithm, "RS256")
Expand Down
Loading