Skip to content

Commit

Permalink
Merge pull request #2158 from vmware-tanzu/upgrade_fosite
Browse files Browse the repository at this point in the history
upgrade fosite to v0.49.0 and handle its API changes
  • Loading branch information
joshuatcasey authored Dec 13, 2024
2 parents 57fc177 + 90c9586 commit acbe9ce
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 38 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ require (
github.com/joshlf/go-acl v0.0.0-20200411065538-eae00ae38531
github.com/migueleliasweb/go-github-mock v1.1.0
github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826
github.com/ory/fosite v0.48.1-0.20241204153806-6c26dc54eb64
github.com/ory/fosite v0.49.0
github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c
github.com/pkg/errors v0.9.1
github.com/sclevine/spec v1.4.0
Expand All @@ -43,7 +43,7 @@ require (
github.com/tdewolff/minify/v2 v2.21.2
go.uber.org/mock v0.5.0
go.uber.org/zap v1.27.0
golang.org/x/crypto v0.30.0
golang.org/x/crypto v0.31.0
golang.org/x/net v0.32.0
golang.org/x/oauth2 v0.24.0
golang.org/x/sync v0.10.0
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,8 @@ github.com/openzipkin/zipkin-go v0.4.2 h1:zjqfqHjUpPmB3c1GlCvvgsM1G4LkvqQbBDueDO
github.com/openzipkin/zipkin-go v0.4.2/go.mod h1:ZeVkFjuuBiSy13y8vpSDCjMi9GoI3hPpCJSBx/EYFhY=
github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde h1:x0TT0RDC7UhAVbbWWBzr41ElhJx5tXPWkIHA2HWPRuw=
github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzbfBr3hhjoZnS66nKrHmduYNpc34ny7RK4z5/HM0=
github.com/ory/fosite v0.48.1-0.20241204153806-6c26dc54eb64 h1:EFceZAYrvvkh/ODW4EpNWLbVoRbNv2tDmxKDGqhrpS8=
github.com/ory/fosite v0.48.1-0.20241204153806-6c26dc54eb64/go.mod h1:M+C+Ng1UDNgwX4SaErnuZwEw26uDN7I3kNUt0WyValI=
github.com/ory/fosite v0.49.0 h1:KNqO7RVt/1X8F08/UI0Y+GRvcpscCWgjqvpLBQPRovo=
github.com/ory/fosite v0.49.0/go.mod h1:FAn7IY+I6DjT1r29wMouPeRYq63DWUuBj++96uOS4mE=
github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe h1:rvu4obdvqR0fkSIJ8IfgzKOWwZ5kOT2UNfLq81Qk7rc=
github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe/go.mod h1:z4n3u6as84LbV4YmgjHhnwtccQqzf4cZlSk9f1FhygI=
github.com/ory/go-convenience v0.1.0 h1:zouLKfF2GoSGnJwGq+PE/nJAE6dj2Zj5QlTgmMTsTS8=
Expand Down Expand Up @@ -680,8 +680,8 @@ golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0
golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58=
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.30.0 h1:RwoQn3GkWiMkzlX562cLB7OxWvjH1L8xutO2WoJcRoY=
golang.org/x/crypto v0.30.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
golang.org/x/crypto v0.31.0 h1:ihbySMvVjLAeSH1IbfcRTkD/iNscyz8rGzjF/E5hV6U=
golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=
Expand Down
28 changes: 21 additions & 7 deletions internal/federationdomain/endpoints/token/token_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,20 @@ var (
}
`)

fositeInvalidRefreshTokenErrorBody = here.Doc(`
{
"error": "invalid_grant",
"error_description": "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. The refresh token is malformed or not valid."
}
`)

fositeExpiredRefreshTokenErrorBody = here.Doc(`
{
"error": "invalid_grant",
"error_description": "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. The refresh token expired."
}
`)

fositeReusedAuthCodeErrorBody = here.Doc(`
{
"error": "invalid_grant",
Expand Down Expand Up @@ -3766,7 +3780,7 @@ func TestRefreshGrant(t *testing.T) {
refreshRequest: refreshRequestInputs{
want: tokenEndpointResponseExpectedValues{
wantStatus: http.StatusBadRequest,
wantErrorResponseBody: fositeInvalidAuthCodeErrorBody,
wantErrorResponseBody: fositeExpiredRefreshTokenErrorBody,
},
},
},
Expand All @@ -3793,7 +3807,7 @@ func TestRefreshGrant(t *testing.T) {
},
want: tokenEndpointResponseExpectedValues{
wantStatus: http.StatusBadRequest,
wantErrorResponseBody: fositeInvalidAuthCodeErrorBody,
wantErrorResponseBody: fositeInvalidRefreshTokenErrorBody,
},
},
},
Expand All @@ -3820,7 +3834,7 @@ func TestRefreshGrant(t *testing.T) {
},
want: tokenEndpointResponseExpectedValues{
wantStatus: http.StatusBadRequest,
wantErrorResponseBody: fositeInvalidAuthCodeErrorBody,
wantErrorResponseBody: fositeInvalidRefreshTokenErrorBody,
},
},
},
Expand Down Expand Up @@ -4831,7 +4845,7 @@ func TestRefreshGrant(t *testing.T) {
session.Fosite = &openid.DefaultSession{}
err = oauthStore.DeleteRefreshTokenSession(context.Background(), refreshTokenSignature)
require.NoError(t, err)
err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, firstRequester)
err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, "ignored", firstRequester)
require.NoError(t, err)
},
refreshRequest: refreshRequestInputs{
Expand Down Expand Up @@ -4869,7 +4883,7 @@ func TestRefreshGrant(t *testing.T) {
delete(session.Fosite.Claims.Extra, "groups")
err = oauthStore.DeleteRefreshTokenSession(context.Background(), refreshTokenSignature)
require.NoError(t, err)
err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, firstRequester)
err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, "ignored", firstRequester)
require.NoError(t, err)
},
refreshRequest: refreshRequestInputs{
Expand Down Expand Up @@ -4907,7 +4921,7 @@ func TestRefreshGrant(t *testing.T) {
session.Custom.Username = ""
err = oauthStore.DeleteRefreshTokenSession(context.Background(), refreshTokenSignature)
require.NoError(t, err)
err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, firstRequester)
err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, "ignored", firstRequester)
require.NoError(t, err)
},
refreshRequest: refreshRequestInputs{
Expand Down Expand Up @@ -4989,7 +5003,7 @@ func TestRefreshGrant(t *testing.T) {
session.Fosite.Claims = fositeSessionClaims
err = oauthStore.DeleteRefreshTokenSession(context.Background(), refreshTokenSignature)
require.NoError(t, err)
err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, firstRequester)
err = oauthStore.CreateRefreshTokenSession(context.Background(), refreshTokenSignature, "ignored", firstRequester)
require.NoError(t, err)
},
refreshRequest: refreshRequestInputs{
Expand Down
16 changes: 11 additions & 5 deletions internal/federationdomain/storage/kube_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (k KubeStorage) RevokeAccessToken(ctx context.Context, requestID string) er
//
// These are keyed by the signature of the refresh token.
//
// Fosite will create these in the token endpoint whenever it wants to hand out an refresh token, including the original
// Fosite will create these in the token endpoint whenever it wants to hand out a refresh token, including the original
// authcode redemption and also during refresh. Refresh tokens are only handed out when the user requested the
// offline_access scope on the original authorization request.
//
Expand All @@ -169,8 +169,8 @@ func (k KubeStorage) RevokeAccessToken(ctx context.Context, requestID string) er
// refresh token will never be deleted.
//

func (k KubeStorage) CreateRefreshTokenSession(ctx context.Context, signatureOfRefreshToken string, request fosite.Requester) (err error) {
return k.refreshTokenStorage.CreateRefreshTokenSession(ctx, signatureOfRefreshToken, request)
func (k KubeStorage) CreateRefreshTokenSession(ctx context.Context, signatureOfRefreshToken string, accessTokenSignature string, request fosite.Requester) (err error) {
return k.refreshTokenStorage.CreateRefreshTokenSession(ctx, signatureOfRefreshToken, accessTokenSignature, request)
}

func (k KubeStorage) GetRefreshTokenSession(ctx context.Context, signatureOfRefreshToken string, session fosite.Session) (request fosite.Requester, err error) {
Expand All @@ -185,8 +185,14 @@ func (k KubeStorage) RevokeRefreshToken(ctx context.Context, requestID string) e
return k.refreshTokenStorage.RevokeRefreshToken(ctx, requestID)
}

func (k KubeStorage) RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, requestID string, signature string) error {
return k.refreshTokenStorage.RevokeRefreshTokenMaybeGracePeriod(ctx, requestID, signature)
func (k KubeStorage) RotateRefreshToken(ctx context.Context, requestID string, _refreshTokenSignature string) error {
// RotateRefreshToken was added in fosite v0.49.0, replacing RevokeRefreshTokenMaybeGracePeriod.
// Confusingly, its job is to both revoke the old refresh token and also revoke the old access token.
// See their sample storage implementation here: https://github.com/ory/fosite/blob/v0.49.0/storage/memory.go#L497-L504
if err := k.refreshTokenStorage.RevokeRefreshToken(ctx, requestID); err != nil {
return err
}
return k.accessTokenStorage.RevokeAccessToken(ctx, requestID)
}

//
Expand Down
6 changes: 3 additions & 3 deletions internal/federationdomain/storage/null_storage.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package storage
Expand Down Expand Up @@ -39,15 +39,15 @@ func (NullStorage) RevokeRefreshToken(_ context.Context, _ string) error {
return errNullStorageNotImplemented
}

func (NullStorage) RevokeRefreshTokenMaybeGracePeriod(_ context.Context, _ string, _ string) error {
func (NullStorage) RotateRefreshToken(_ context.Context, _ string, _ string) error {
return errNullStorageNotImplemented
}

func (NullStorage) RevokeAccessToken(_ context.Context, _ string) error {
return errNullStorageNotImplemented
}

func (NullStorage) CreateRefreshTokenSession(_ context.Context, _ string, _ fosite.Requester) (err error) {
func (NullStorage) CreateRefreshTokenSession(_ context.Context, _ string, _ string, _ fosite.Requester) (err error) {
return nil
}

Expand Down
8 changes: 4 additions & 4 deletions internal/fositestorage/refreshtoken/refreshtoken.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const (
type RevocationStorage interface {
fositeoauth2.RefreshTokenStorage
RevokeRefreshToken(ctx context.Context, requestID string) error
RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, requestID string, signature string) error
RotateRefreshToken(ctx context.Context, requestID string, refreshTokenSignature string) error
}

var _ RevocationStorage = &refreshTokenStorage{}
Expand Down Expand Up @@ -82,12 +82,12 @@ func (a *refreshTokenStorage) RevokeRefreshToken(ctx context.Context, requestID
return a.storage.DeleteByLabel(ctx, fositestorage.StorageRequestIDLabelName, requestID)
}

func (a *refreshTokenStorage) RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, requestID string, _signature string) error {
// We don't support a grace period, so always call the regular RevokeRefreshToken().
func (a *refreshTokenStorage) RotateRefreshToken(ctx context.Context, requestID string, _refreshTokenSignature string) error {
// Rotation is called to revoke an old token during a refresh, so we can always call RevokeRefreshToken().
return a.RevokeRefreshToken(ctx, requestID)
}

func (a *refreshTokenStorage) CreateRefreshTokenSession(ctx context.Context, signature string, requester fosite.Requester) error {
func (a *refreshTokenStorage) CreateRefreshTokenSession(ctx context.Context, signature string, _accessTokenSignature string, requester fosite.Requester) error {
request, err := fositestorage.ValidateAndExtractAuthorizeRequest(requester)
if err != nil {
return err
Expand Down
18 changes: 9 additions & 9 deletions internal/fositestorage/refreshtoken/refreshtoken_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestRefreshTokenStorage(t *testing.T) {
RequestedAudience: nil,
GrantedAudience: nil,
}
err := storage.CreateRefreshTokenSession(ctx, "fancy-signature", request)
err := storage.CreateRefreshTokenSession(ctx, "fancy-signature", "ignored", request)
require.NoError(t, err)
require.Equal(t, 1, storageLifetimeFuncCallCount)
require.Equal(t, request, storageLifetimeFuncCallRequesterArg)
Expand Down Expand Up @@ -172,7 +172,7 @@ func TestRefreshTokenStorageRevocation(t *testing.T) {
Form: url.Values{"key": []string{"val"}},
Session: testutil.NewFakePinnipedSession(),
}
err := storage.CreateRefreshTokenSession(ctx, "fancy-signature", request)
err := storage.CreateRefreshTokenSession(ctx, "fancy-signature", "ignored", request)
require.NoError(t, err)

// Revoke the request ID of the session that we just created
Expand Down Expand Up @@ -227,12 +227,12 @@ func TestRefreshTokenStorageRevokeRefreshTokenMaybeGracePeriod(t *testing.T) {
Form: url.Values{"key": []string{"val"}},
Session: testutil.NewFakePinnipedSession(),
}
err := storage.CreateRefreshTokenSession(ctx, "fancy-signature", request)
err := storage.CreateRefreshTokenSession(ctx, "fancy-signature", "ignored", request)
require.NoError(t, err)

// Revoke the request ID of the session that we just created. We don't support grace periods, so this
// Revoke the request ID of the session that we just created. This
// should work exactly like the regular RevokeRefreshToken() function.
err = storage.RevokeRefreshTokenMaybeGracePeriod(ctx, "abcd-1", "fancy-signature")
err = storage.RotateRefreshToken(ctx, "abcd-1", "fancy-signature")
require.NoError(t, err)

testutil.LogActualJSONFromCreateAction(t, client, 0) // makes it easier to update expected values when needed
Expand Down Expand Up @@ -306,7 +306,7 @@ func TestNilSessionRequest(t *testing.T) {
func TestCreateWithNilRequester(t *testing.T) {
ctx, _, _, storage := makeTestSubject(lifetimeFunc)

err := storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", nil)
err := storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", "ignored", nil)
require.EqualError(t, err, "requester must be of type fosite.Request")
}

Expand All @@ -317,14 +317,14 @@ func TestCreateWithWrongRequesterDataTypes(t *testing.T) {
Session: nil,
Client: &clientregistry.Client{},
}
err := storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", request)
err := storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", "ignored", request)
require.EqualError(t, err, "requester's session must be of type PinnipedSession")

request = &fosite.Request{
Session: &psession.PinnipedSession{},
Client: nil,
}
err = storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", request)
err = storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", "ignored", request)
require.EqualError(t, err, "requester's client must be of type clientregistry.Client")
}

Expand All @@ -336,7 +336,7 @@ func TestCreateWithoutRequesterID(t *testing.T) {
Session: &psession.PinnipedSession{},
Client: &clientregistry.Client{},
}
err := storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", request)
err := storage.CreateRefreshTokenSession(ctx, "signature-doesnt-matter", "ignored", request)
require.NoError(t, err)

// the blank ID was filled in with an auto-generated ID
Expand Down
4 changes: 2 additions & 2 deletions test/integration/supervisor_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3201,7 +3201,7 @@ func testSupervisorLogin(
// Then save the mutated Secret back to Kubernetes.
// There is no update function, so delete and create again at the same name.
require.NoError(t, oauthStore.DeleteRefreshTokenSession(ctx, signatureOfLatestRefreshToken))
require.NoError(t, oauthStore.CreateRefreshTokenSession(ctx, signatureOfLatestRefreshToken, storedRefreshSession))
require.NoError(t, oauthStore.CreateRefreshTokenSession(ctx, signatureOfLatestRefreshToken, "ignored", storedRefreshSession))
}

// Use the refresh token to get new tokens by calling the token endpoint again.
Expand Down Expand Up @@ -3275,7 +3275,7 @@ func testSupervisorLogin(
// Then save the mutated Secret back to Kubernetes.
// There is no update function, so delete and create again at the same name.
require.NoError(t, oauthStore.DeleteRefreshTokenSession(ctx, signatureOfLatestRefreshToken))
require.NoError(t, oauthStore.CreateRefreshTokenSession(ctx, signatureOfLatestRefreshToken, storedRefreshSession))
require.NoError(t, oauthStore.CreateRefreshTokenSession(ctx, signatureOfLatestRefreshToken, "ignored", storedRefreshSession))

// Now try to perform a downstream refresh again, knowing that the corresponding upstream refresh should fail.
_, err = downstreamOAuth2Config.TokenSource(oidcHTTPClientContext, &oauth2.Token{RefreshToken: latestRefreshToken}).Token()
Expand Down
4 changes: 2 additions & 2 deletions test/integration/supervisor_warnings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
pinnipedSession.Fosite.Claims.Extra["groups"] = []string{"some-wrong-group", "some-other-group"} // update downstream groups

require.NoError(t, oauthStore.DeleteRefreshTokenSession(ctx, refreshTokenSignature))
require.NoError(t, oauthStore.CreateRefreshTokenSession(ctx, refreshTokenSignature, storedRefreshSession))
require.NoError(t, oauthStore.CreateRefreshTokenSession(ctx, refreshTokenSignature, "ignored", storedRefreshSession))

// remove the credential cache, which includes the cached cert, so it won't be reused and the refresh flow will be triggered.
err = os.Remove(credentialCachePath)
Expand Down Expand Up @@ -529,7 +529,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
pinnipedSession.Fosite.Claims.Extra["groups"] = []string{"some-wrong-group", "some-other-group"}

require.NoError(t, oauthStore.DeleteRefreshTokenSession(ctx, refreshTokenSignature))
require.NoError(t, oauthStore.CreateRefreshTokenSession(ctx, refreshTokenSignature, storedRefreshSession))
require.NoError(t, oauthStore.CreateRefreshTokenSession(ctx, refreshTokenSignature, "ignored", storedRefreshSession))

// remove the credential cache, which includes the cached cert, so it won't be reused and the refresh flow will be triggered.
err = os.Remove(credentialCachePath)
Expand Down

0 comments on commit acbe9ce

Please sign in to comment.