Skip to content

Commit

Permalink
update session storage version from 5 to 6 due to fosite upgrade
Browse files Browse the repository at this point in the history
A small part of the session storage changed type in the latest version
of fosite compared to the old version of fosite that we were using.
Just to be safe, update our session storage version to invalidate
any pre-existing sessions upon upgrade of Pinniped.
  • Loading branch information
cfryanr committed Dec 4, 2023
1 parent 912335b commit f370d37
Show file tree
Hide file tree
Showing 12 changed files with 62 additions and 57 deletions.
30 changes: 15 additions & 15 deletions internal/controller/supervisorstorage/garbage_collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there are valid, expired authcode secrets which contain upstream refresh tokens", func() {
it.Before(func() {
activeOIDCAuthcodeSession := &authorizationcode.Session{
Version: "5",
Version: "6",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Expand Down Expand Up @@ -309,7 +309,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
r.NoError(kubeClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))

inactiveOIDCAuthcodeSession := &authorizationcode.Session{
Version: "5",
Version: "6",
Active: false,
Request: &fosite.Request{
ID: "request-id-2",
Expand Down Expand Up @@ -388,7 +388,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there are valid, expired authcode secrets which contain upstream access tokens", func() {
it.Before(func() {
activeOIDCAuthcodeSession := &authorizationcode.Session{
Version: "5",
Version: "6",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Expand Down Expand Up @@ -433,7 +433,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
r.NoError(kubeClient.Tracker().Add(activeOIDCAuthcodeSessionSecret))

inactiveOIDCAuthcodeSession := &authorizationcode.Session{
Version: "5",
Version: "6",
Active: false,
Request: &fosite.Request{
ID: "request-id-2",
Expand Down Expand Up @@ -512,7 +512,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there is an invalid, expired authcode secret", func() {
it.Before(func() {
invalidOIDCAuthcodeSession := &authorizationcode.Session{
Version: "5",
Version: "6",
Active: true,
Request: &fosite.Request{
ID: "", // it is invalid for there to be a missing request ID
Expand Down Expand Up @@ -581,7 +581,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there is a valid, expired authcode secret but its upstream name does not match any existing upstream", func() {
it.Before(func() {
wrongProviderNameOIDCAuthcodeSession := &authorizationcode.Session{
Version: "5",
Version: "6",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Expand Down Expand Up @@ -652,7 +652,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there is a valid, expired authcode secret but its upstream UID does not match any existing upstream", func() {
it.Before(func() {
wrongProviderNameOIDCAuthcodeSession := &authorizationcode.Session{
Version: "5",
Version: "6",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Expand Down Expand Up @@ -723,7 +723,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there is a valid, recently expired authcode secret but the upstream revocation fails", func() {
it.Before(func() {
activeOIDCAuthcodeSession := &authorizationcode.Session{
Version: "5",
Version: "6",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Expand Down Expand Up @@ -828,7 +828,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there is a valid, long-since expired authcode secret but the upstream revocation fails", func() {
it.Before(func() {
activeOIDCAuthcodeSession := &authorizationcode.Session{
Version: "5",
Version: "6",
Active: true,
Request: &fosite.Request{
ID: "request-id-1",
Expand Down Expand Up @@ -907,7 +907,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there are valid, expired access token secrets which contain upstream refresh tokens", func() {
it.Before(func() {
offlineAccessGrantedOIDCAccessTokenSession := &accesstoken.Session{
Version: "5",
Version: "6",
Request: &fosite.Request{
GrantedScope: fosite.Arguments{"scope1", "scope2", "offline_access"},
ID: "request-id-1",
Expand Down Expand Up @@ -952,7 +952,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
r.NoError(kubeClient.Tracker().Add(offlineAccessGrantedOIDCAccessTokenSessionSecret))

offlineAccessNotGrantedOIDCAccessTokenSession := &accesstoken.Session{
Version: "5",
Version: "6",
Request: &fosite.Request{
GrantedScope: fosite.Arguments{"scope1", "scope2"},
ID: "request-id-2",
Expand Down Expand Up @@ -1031,7 +1031,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there are valid, expired access token secrets which contain upstream access tokens", func() {
it.Before(func() {
offlineAccessGrantedOIDCAccessTokenSession := &accesstoken.Session{
Version: "5",
Version: "6",
Request: &fosite.Request{
GrantedScope: fosite.Arguments{"scope1", "scope2", "offline_access"},
ID: "request-id-1",
Expand Down Expand Up @@ -1076,7 +1076,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
r.NoError(kubeClient.Tracker().Add(offlineAccessGrantedOIDCAccessTokenSessionSecret))

offlineAccessNotGrantedOIDCAccessTokenSession := &accesstoken.Session{
Version: "5",
Version: "6",
Request: &fosite.Request{
GrantedScope: fosite.Arguments{"scope1", "scope2"},
ID: "request-id-2",
Expand Down Expand Up @@ -1155,7 +1155,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there are valid, expired refresh secrets which contain upstream refresh tokens", func() {
it.Before(func() {
oidcRefreshSession := &refreshtoken.Session{
Version: "5",
Version: "6",
Request: &fosite.Request{
ID: "request-id-1",
Client: &clientregistry.Client{},
Expand Down Expand Up @@ -1232,7 +1232,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
when("there are valid, expired refresh secrets which contain upstream access tokens", func() {
it.Before(func() {
oidcRefreshSession := &refreshtoken.Session{
Version: "5",
Version: "6",
Request: &fosite.Request{
ID: "request-id-1",
Client: &clientregistry.Client{},
Expand Down
3 changes: 2 additions & 1 deletion internal/fositestorage/accesstoken/accesstoken.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ const (
// Version 3 is when we added the Username field to the psession.CustomSessionData.
// Version 4 is when fosite added json tags to their openid.DefaultSession struct.
// Version 5 is when we added the UpstreamUsername and UpstreamGroups fields to psession.CustomSessionData.
accessTokenStorageVersion = "5"
// Version 6 is when we upgraded fosite in Dec 2023.
accessTokenStorageVersion = "6"
)

type RevocationStorage interface {
Expand Down
18 changes: 9 additions & 9 deletions internal/fositestorage/accesstoken/accesstoken_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestAccessTokenStorage(t *testing.T) {
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"5"}`),
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/access-token",
Expand Down Expand Up @@ -123,7 +123,7 @@ func TestAccessTokenStorageRevocation(t *testing.T) {
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"5"}`),
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","requestedAt":"0001-01-01T00:00:00Z","client":{"id":"pinny","redirect_uris":null,"grant_types":null,"response_types":null,"scopes":null,"audience":null,"public":true,"jwks_uri":"where","jwks":null,"token_endpoint_auth_method":"something","request_uris":null,"request_object_signing_alg":"","token_endpoint_auth_signing_alg":""},"scopes":null,"grantedScopes":null,"form":{"key":["val"]},"session":{"fosite":{"id_token_claims":null,"headers":null,"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","warnings":null,"oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"6"}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/access-token",
Expand Down Expand Up @@ -196,7 +196,7 @@ func TestWrongVersion(t *testing.T) {

_, err = storage.GetAccessTokenSession(ctx, "fancy-signature", nil)

require.EqualError(t, err, "access token request data has wrong version: access token session for fancy-signature has version not-the-right-version instead of 5")
require.EqualError(t, err, "access token request data has wrong version: access token session for fancy-signature has version not-the-right-version instead of 6")
}

func TestNilSessionRequest(t *testing.T) {
Expand All @@ -214,7 +214,7 @@ func TestNilSessionRequest(t *testing.T) {
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"nonsense-key": "nonsense-value","version":"5"}`),
"pinniped-storage-data": []byte(`{"nonsense-key": "nonsense-value","version":"6"}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/access-token",
Expand Down Expand Up @@ -298,13 +298,13 @@ func TestReadFromSecret(t *testing.T) {
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"id_token_claims":{"jti": "xyz"},"headers":{"extra":{"myheader": "foo"}},"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"5","active": true}`),
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1","session":{"fosite":{"id_token_claims":{"jti": "xyz"},"headers":{"extra":{"myheader": "foo"}},"expires_at":null,"username":"snorlax","subject":"panda"},"custom":{"username":"fake-username","upstreamUsername":"fake-upstream-username","upstreamGroups":["fake-upstream-group1","fake-upstream-group2"],"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token"}}}},"version":"6","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/access-token",
},
wantSession: &Session{
Version: "5",
Version: "6",
Request: &fosite.Request{
ID: "abcd-1",
Client: &clientregistry.Client{},
Expand Down Expand Up @@ -341,7 +341,7 @@ func TestReadFromSecret(t *testing.T) {
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1"},"version":"5","active": true}`),
"pinniped-storage-data": []byte(`{"request":{"id":"abcd-1"},"version":"6","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/not-access-token",
Expand All @@ -364,7 +364,7 @@ func TestReadFromSecret(t *testing.T) {
},
Type: "storage.pinniped.dev/access-token",
},
wantErr: "access token request data has wrong version: access token session has version wrong-version-here instead of 5",
wantErr: "access token request data has wrong version: access token session has version wrong-version-here instead of 6",
},
{
name: "missing request",
Expand All @@ -377,7 +377,7 @@ func TestReadFromSecret(t *testing.T) {
},
},
Data: map[string][]byte{
"pinniped-storage-data": []byte(`{"version":"5","active": true}`),
"pinniped-storage-data": []byte(`{"version":"6","active": true}`),
"pinniped-storage-version": []byte("1"),
},
Type: "storage.pinniped.dev/access-token",
Expand Down
5 changes: 3 additions & 2 deletions internal/fositestorage/authorizationcode/authorizationcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ const (
// Version 3 is when we added the Username field to the psession.CustomSessionData.
// Version 4 is when fosite added json tags to their openid.DefaultSession struct.
// Version 5 is when we added the UpstreamUsername and UpstreamGroups fields to psession.CustomSessionData.
authorizeCodeStorageVersion = "5"
// Version 6 is when we upgraded fosite in Dec 2023.
authorizeCodeStorageVersion = "6"
)

var _ oauth2.AuthorizeCodeStorage = &authorizeCodeStorage{}
Expand Down Expand Up @@ -380,5 +381,5 @@ const ExpectedAuthorizeCodeSessionJSONFromFuzzing = `{
"d鞕ȸ腿tʏƲ%}ſ¯Ɣ 籌Tǘ乚Ȥ2"
]
},
"version": "5"
"version": "6"
}`
Loading

0 comments on commit f370d37

Please sign in to comment.