diff --git a/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go.tmpl b/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go.tmpl index a53d6f53..798275a9 100644 --- a/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go.tmpl +++ b/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go.tmpl @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package v1alpha1 diff --git a/generated/1.17/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/1.17/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index a53d6f53..798275a9 100644 --- a/generated/1.17/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/generated/1.17/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package v1alpha1 diff --git a/generated/1.18/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/1.18/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index a53d6f53..798275a9 100644 --- a/generated/1.18/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/generated/1.18/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package v1alpha1 diff --git a/generated/1.19/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/1.19/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index a53d6f53..798275a9 100644 --- a/generated/1.19/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/generated/1.19/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package v1alpha1 diff --git a/generated/1.20/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/1.20/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index a53d6f53..798275a9 100644 --- a/generated/1.20/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/generated/1.20/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package v1alpha1 diff --git a/generated/latest/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go b/generated/latest/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go index a53d6f53..798275a9 100644 --- a/generated/latest/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go +++ b/generated/latest/apis/supervisor/idp/v1alpha1/types_oidcidentityprovider.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package v1alpha1 diff --git a/hack/header.txt b/hack/header.txt index f6970c2b..8ebfbf38 100644 --- a/hack/header.txt +++ b/hack/header.txt @@ -1,2 +1,2 @@ -Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. SPDX-License-Identifier: Apache-2.0 diff --git a/internal/controller/kubecertagent/mocks/mockdynamiccert.go b/internal/controller/kubecertagent/mocks/mockdynamiccert.go index 030d3e07..fda36b65 100644 --- a/internal/controller/kubecertagent/mocks/mockdynamiccert.go +++ b/internal/controller/kubecertagent/mocks/mockdynamiccert.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // diff --git a/internal/controller/kubecertagent/mocks/mockpodcommandexecutor.go b/internal/controller/kubecertagent/mocks/mockpodcommandexecutor.go index 637f0927..f9aef2d0 100644 --- a/internal/controller/kubecertagent/mocks/mockpodcommandexecutor.go +++ b/internal/controller/kubecertagent/mocks/mockpodcommandexecutor.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // diff --git a/internal/fositestorage/accesstoken/accesstoken_test.go b/internal/fositestorage/accesstoken/accesstoken_test.go index 5b503371..ab6f7ee6 100644 --- a/internal/fositestorage/accesstoken/accesstoken_test.go +++ b/internal/fositestorage/accesstoken/accesstoken_test.go @@ -53,7 +53,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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/access-token", @@ -122,7 +122,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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/access-token", diff --git a/internal/fositestorage/authorizationcode/authorizationcode.go b/internal/fositestorage/authorizationcode/authorizationcode.go index e7c6655b..f2504d90 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode.go +++ b/internal/fositestorage/authorizationcode/authorizationcode.go @@ -371,32 +371,34 @@ const ExpectedAuthorizeCodeSessionJSONFromFuzzing = `{ "providerType": "ɥ闣ʬ橳(ý綃ʃʚƟ覣k眐4", "oidc": { "upstreamRefreshToken": "tC嵽痊w", - "upstreamSubject": "a紽ǒ|鰽ŋ猊I", - "upstreamIssuer": "妬\u003e6鉢緋uƴŤȱʀ" + "upstreamAccessToken": "a紽ǒ|鰽ŋ猊I", + "upstreamSubject": "妬\u003e6鉢緋uƴŤȱʀ", + "upstreamIssuer": ":設虝27就伒犘c" }, "ldap": { - "userDN": "Â?墖\u003cƬb獭潜Ʃ饾k|鬌R蜚蠣", + "userDN": "ɏȫ齁š%Op", "extraRefreshAttributes": { - "ȱ藚ɏ¬Ê蒭堜]ȗ韚ʫ": "鷞aŚB碠k9帴ʘ赱" + "T妼É4İ\u003e×1": "ʥ笿0D", + "÷驣7Ʀ澉1æɽ誮": "ʫ繕ȫ", + "ŚB碠k9": "i磊ůď逳鞪?3)藵睋邔\u0026Ű" } }, "activedirectory": { - "userDN": "瑹xȢ~1Įx欼笝?úT妼", + "userDN": "s", "extraRefreshAttributes": { - "iYn": "麹Œ颛", - "İ\u003e×1飞O+î艔垎0OƉǢIȽ齤士": "ȐĨf跞@)¿,ɭS隑ip偶" + "ƉǢIȽ齤士bEǎ儯惝IozŁ5rƖ螼": "偶宾儮猷V麹Œ颛Ė應,Ɣ鬅X¤" } } } }, "requestedAudience": [ - "應,Ɣ鬅X¤", - "¤.岵骘胲ƤkǦ" + "tO灞浛a齙\\蹼偦歛ơ", + "皦pSǬŝ社Vƅȭǝ*" ], "grantedAudience": [ - "鸖I¶媁y衑拁Ȃ", - "社Vƅȭǝ*擦28Dž 甍 ć", - "bņ抰蛖a³2ʫ承dʬ)ġ,TÀqy_" + "ĝ\"zvưã置bņ抰蛖a³2ʫ", + "Ŷɽ蔒PR}Ųʓl{鼐jÃ轘屔挝", + "Œų崓ļ憽-蹐È_¸]fś" ] }, "version": "2" diff --git a/internal/fositestorage/authorizationcode/authorizationcode_test.go b/internal/fositestorage/authorizationcode/authorizationcode_test.go index 72c4cfba..fe30e4d9 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode_test.go +++ b/internal/fositestorage/authorizationcode/authorizationcode_test.go @@ -65,7 +65,7 @@ func TestAuthorizationCodeStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"active":true,"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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"active":true,"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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/authcode", @@ -84,7 +84,7 @@ func TestAuthorizationCodeStorage(t *testing.T) { }, }, Data: map[string][]byte{ - "pinniped-storage-data": []byte(`{"active":false,"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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "pinniped-storage-data": []byte(`{"active":false,"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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/authcode", @@ -389,11 +389,12 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) { validSessionJSONBytes, err := json.MarshalIndent(validSession, "", "\t") require.NoError(t, err) authorizeCodeSessionJSONFromFuzzing := string(validSessionJSONBytes) - t.Log(authorizeCodeSessionJSONFromFuzzing) // the fuzzed session and storage session should have identical JSON require.JSONEq(t, authorizeCodeSessionJSONFromFuzzing, authorizeCodeSessionJSONFromStorage) + t.Log("actual value from fuzzing", authorizeCodeSessionJSONFromFuzzing) // can be useful when updating expected value + // while the fuzzer will panic if AuthorizeRequest changes in a way that cannot be fuzzed, // if it adds a new field that can be fuzzed, this check will fail // thus if AuthorizeRequest changes, we will detect it here (though we could possibly miss an omitempty field) diff --git a/internal/fositestorage/openidconnect/openidconnect_test.go b/internal/fositestorage/openidconnect/openidconnect_test.go index ac680a60..03ae3626 100644 --- a/internal/fositestorage/openidconnect/openidconnect_test.go +++ b/internal/fositestorage/openidconnect/openidconnect_test.go @@ -52,7 +52,7 @@ func TestOpenIdConnectStorage(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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/oidc", diff --git a/internal/fositestorage/pkce/pkce_test.go b/internal/fositestorage/pkce/pkce_test.go index b84798ce..58921725 100644 --- a/internal/fositestorage/pkce/pkce_test.go +++ b/internal/fositestorage/pkce/pkce_test.go @@ -52,7 +52,7 @@ func TestPKCEStorage(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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/pkce", diff --git a/internal/fositestorage/refreshtoken/refreshtoken_test.go b/internal/fositestorage/refreshtoken/refreshtoken_test.go index ce74793a..36af0559 100644 --- a/internal/fositestorage/refreshtoken/refreshtoken_test.go +++ b/internal/fositestorage/refreshtoken/refreshtoken_test.go @@ -52,7 +52,7 @@ func TestRefreshTokenStorage(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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/refresh-token", @@ -122,7 +122,7 @@ func TestRefreshTokenStorageRevocation(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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), + "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":{"Claims":null,"Headers":null,"ExpiresAt":null,"Username":"snorlax","Subject":"panda"},"custom":{"providerUID":"fake-provider-uid","providerName":"fake-provider-name","providerType":"fake-provider-type","oidc":{"upstreamRefreshToken":"fake-upstream-refresh-token","upstreamAccessToken":"","upstreamSubject":"some-subject","upstreamIssuer":"some-issuer"}}},"requestedAudience":null,"grantedAudience":null},"version":"2"}`), "pinniped-storage-version": []byte("1"), }, Type: "storage.pinniped.dev/refresh-token", diff --git a/internal/mocks/credentialrequestmocks/credentialrequestmocks.go b/internal/mocks/credentialrequestmocks/credentialrequestmocks.go index 68ff7f2e..db5c5cce 100644 --- a/internal/mocks/credentialrequestmocks/credentialrequestmocks.go +++ b/internal/mocks/credentialrequestmocks/credentialrequestmocks.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // diff --git a/internal/mocks/issuermocks/issuermocks.go b/internal/mocks/issuermocks/issuermocks.go index 045fbed3..737b9ab1 100644 --- a/internal/mocks/issuermocks/issuermocks.go +++ b/internal/mocks/issuermocks/issuermocks.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // diff --git a/internal/mocks/mockkeyset/mockkeyset.go b/internal/mocks/mockkeyset/mockkeyset.go index a2cb28e6..81f4b012 100644 --- a/internal/mocks/mockkeyset/mockkeyset.go +++ b/internal/mocks/mockkeyset/mockkeyset.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // diff --git a/internal/mocks/mockldapconn/mockldapconn.go b/internal/mocks/mockldapconn/mockldapconn.go index 24b0b1aa..b8839a04 100644 --- a/internal/mocks/mockldapconn/mockldapconn.go +++ b/internal/mocks/mockldapconn/mockldapconn.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // diff --git a/internal/mocks/mocksecrethelper/mocksecrethelper.go b/internal/mocks/mocksecrethelper/mocksecrethelper.go index 051c7548..d7520b20 100644 --- a/internal/mocks/mocksecrethelper/mocksecrethelper.go +++ b/internal/mocks/mocksecrethelper/mocksecrethelper.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // diff --git a/internal/mocks/mocktokenauthenticator/mocktokenauthenticator.go b/internal/mocks/mocktokenauthenticator/mocktokenauthenticator.go index 31349d33..6946e967 100644 --- a/internal/mocks/mocktokenauthenticator/mocktokenauthenticator.go +++ b/internal/mocks/mocktokenauthenticator/mocktokenauthenticator.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // diff --git a/internal/mocks/mocktokenauthenticatorcloser/mocktokenauthenticatorcloser.go b/internal/mocks/mocktokenauthenticatorcloser/mocktokenauthenticatorcloser.go index b8c7c28c..3f651eb1 100644 --- a/internal/mocks/mocktokenauthenticatorcloser/mocktokenauthenticatorcloser.go +++ b/internal/mocks/mocktokenauthenticatorcloser/mocktokenauthenticatorcloser.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // diff --git a/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go b/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go index 2a2a689e..c27c0184 100644 --- a/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go +++ b/internal/mocks/mockupstreamoidcidentityprovider/mockupstreamoidcidentityprovider.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // @@ -14,12 +14,11 @@ import ( reflect "reflect" gomock "github.com/golang/mock/gomock" - oauth2 "golang.org/x/oauth2" - types "k8s.io/apimachinery/pkg/types" - nonce "go.pinniped.dev/pkg/oidcclient/nonce" oidctypes "go.pinniped.dev/pkg/oidcclient/oidctypes" pkce "go.pinniped.dev/pkg/oidcclient/pkce" + oauth2 "golang.org/x/oauth2" + types "k8s.io/apimachinery/pkg/types" ) // MockUpstreamOIDCIdentityProviderI is a mock of UpstreamOIDCIdentityProviderI interface. @@ -186,6 +185,20 @@ func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) GetUsernameClaim() *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUsernameClaim", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).GetUsernameClaim)) } +// HasUserInfoURL mocks base method. +func (m *MockUpstreamOIDCIdentityProviderI) HasUserInfoURL() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HasUserInfoURL") + ret0, _ := ret[0].(bool) + return ret0 +} + +// HasUserInfoURL indicates an expected call of HasUserInfoURL. +func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) HasUserInfoURL() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasUserInfoURL", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).HasUserInfoURL)) +} + // PasswordCredentialsGrantAndValidateTokens mocks base method. func (m *MockUpstreamOIDCIdentityProviderI) PasswordCredentialsGrantAndValidateTokens(arg0 context.Context, arg1, arg2 string) (*oidctypes.Token, error) { m.ctrl.T.Helper() @@ -230,17 +243,17 @@ func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) RevokeRefreshToken(arg0 return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevokeRefreshToken", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).RevokeRefreshToken), arg0, arg1) } -// ValidateToken mocks base method. -func (m *MockUpstreamOIDCIdentityProviderI) ValidateTokenAndMergeWithUserInfo(arg0 context.Context, arg1 *oauth2.Token, arg2 nonce.Nonce, arg3 bool) (*oidctypes.Token, error) { +// ValidateTokenAndMergeWithUserInfo mocks base method. +func (m *MockUpstreamOIDCIdentityProviderI) ValidateTokenAndMergeWithUserInfo(arg0 context.Context, arg1 *oauth2.Token, arg2 nonce.Nonce, arg3, arg4 bool) (*oidctypes.Token, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ValidateTokenAndMergeWithUserInfo", arg0, arg1, arg2, arg3) + ret := m.ctrl.Call(m, "ValidateTokenAndMergeWithUserInfo", arg0, arg1, arg2, arg3, arg4) ret0, _ := ret[0].(*oidctypes.Token) ret1, _ := ret[1].(error) return ret0, ret1 } -// ValidateToken indicates an expected call of ValidateToken. -func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) ValidateToken(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +// ValidateTokenAndMergeWithUserInfo indicates an expected call of ValidateTokenAndMergeWithUserInfo. +func (mr *MockUpstreamOIDCIdentityProviderIMockRecorder) ValidateTokenAndMergeWithUserInfo(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateTokenAndMergeWithUserInfo", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).ValidateTokenAndMergeWithUserInfo), arg0, arg1, arg2, arg3) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateTokenAndMergeWithUserInfo", reflect.TypeOf((*MockUpstreamOIDCIdentityProviderI)(nil).ValidateTokenAndMergeWithUserInfo), arg0, arg1, arg2, arg3, arg4) } diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index b08d39bc..4f281ad8 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -174,16 +174,6 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( fosite.ErrAccessDenied.WithDebug(err.Error()), true) // WithDebug hides the error from the client } - if token.RefreshToken == nil || token.RefreshToken.Token == "" { - plog.Warning("refresh token not returned by upstream provider during password grant, "+ - "please check configuration of OIDCIdentityProvider and the client in the upstream provider's API/UI", - "upstreamName", oidcUpstream.GetName(), - "scopes", oidcUpstream.GetScopes()) - return writeAuthorizeError(w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHint( - "Refresh token not returned by upstream provider during password grant."), true) - } - subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(oidcUpstream, token.IDToken.Claims) if err != nil { // Return a user-friendly error for this case which is entirely within our control. @@ -191,31 +181,14 @@ func handleAuthRequestForOIDCUpstreamPasswordGrant( fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, ) } - upstreamSubject, err := downstreamsession.ExtractStringClaimValue(oidc.IDTokenSubjectClaim, oidcUpstream.GetName(), token.IDToken.Claims) + + customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(oidcUpstream, token) if err != nil { - // Return a user-friendly error for this case which is entirely within our control. - return writeAuthorizeError(w, oauthHelper, authorizeRequester, - fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, - ) - } - upstreamIssuer, err := downstreamsession.ExtractStringClaimValue(oidc.IDTokenIssuerClaim, oidcUpstream.GetName(), token.IDToken.Claims) - if err != nil { - // Return a user-friendly error for this case which is entirely within our control. return writeAuthorizeError(w, oauthHelper, authorizeRequester, fosite.ErrAccessDenied.WithHintf("Reason: %s.", err.Error()), true, ) } - customSessionData := &psession.CustomSessionData{ - ProviderUID: oidcUpstream.GetResourceUID(), - ProviderName: oidcUpstream.GetName(), - ProviderType: psession.ProviderTypeOIDC, - OIDC: &psession.OIDCSessionData{ - UpstreamRefreshToken: token.RefreshToken.Token, - UpstreamIssuer: upstreamIssuer, - UpstreamSubject: upstreamSubject, - }, - } return makeDownstreamSessionAndReturnAuthcodeRedirect(r, w, oauthHelper, authorizeRequester, subject, username, groups, customSessionData) } diff --git a/internal/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 845e39b4..0198c83a 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -56,6 +56,7 @@ func TestAuthorizationEndpoint(t *testing.T) { oidcUpstreamUsernameClaim = "the-user-claim" oidcUpstreamGroupsClaim = "the-groups-claim" oidcPasswordGrantUpstreamRefreshToken = "some-opaque-token" //nolint: gosec + oidcUpstreamAccessToken = "some-access-token" downstreamIssuer = "https://my-downstream-issuer.com/some-path" downstreamRedirectURI = "http://127.0.0.1/callback" @@ -154,9 +155,15 @@ func TestAuthorizationEndpoint(t *testing.T) { "state": happyState, } - fositeAccessDeniedWithMissingRefreshTokenErrorQuery = map[string]string{ + fositeAccessDeniedWithMissingAccessTokenErrorQuery = map[string]string{ "error": "access_denied", - "error_description": "The resource owner or authorization server denied the request. Refresh token not returned by upstream provider during password grant.", + "error_description": "The resource owner or authorization server denied the request. Reason: neither access token nor refresh token returned by upstream provider.", + "state": happyState, + } + + fositeAccessDeniedWithMissingUserInfoEndpointErrorQuery = map[string]string{ + "error": "access_denied", + "error_description": "The resource owner or authorization server denied the request. Reason: access token was returned by upstream provider but there was no userinfo endpoint.", "state": happyState, } @@ -475,6 +482,17 @@ func TestAuthorizationEndpoint(t *testing.T) { }, } + expectedHappyOIDCPasswordGrantCustomSessionWithAccessToken := &psession.CustomSessionData{ + ProviderUID: oidcPasswordGrantUpstreamResourceUID, + ProviderName: oidcPasswordGrantUpstreamName, + ProviderType: psession.ProviderTypeOIDC, + OIDC: &psession.OIDCSessionData{ + UpstreamAccessToken: oidcUpstreamAccessToken, + UpstreamSubject: oidcUpstreamSubject, + UpstreamIssuer: oidcUpstreamIssuer, + }, + } + // Note that fosite puts the granted scopes as a param in the redirect URI even though the spec doesn't seem to require it happyAuthcodeDownstreamRedirectLocationRegexp := downstreamRedirectURI + `\?code=([^&]+)&scope=openid&state=` + happyState @@ -873,6 +891,50 @@ func TestAuthorizationEndpoint(t *testing.T) { wantUpstreamStateParamInLocationHeader: true, wantBodyStringWithLocationInHref: true, }, + { + name: "OIDC password grant happy path when upstream IDP returned empty refresh token but it did return an access token and has a userinfo endpoint", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithUserInfoURL().Build()), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), + customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword), + wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantRedirectLocationRegexp: happyAuthcodeDownstreamRedirectLocationRegexp, + wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, + wantDownstreamIDTokenUsername: oidcUpstreamUsername, + wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamRedirectURI: downstreamRedirectURI, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSessionWithAccessToken, + }, + { + name: "OIDC password grant happy path when upstream IDP did not return a refresh token but it did return an access token and has a userinfo endpoint", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithUserInfoURL().Build()), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), + customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword), + wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, + wantStatus: http.StatusFound, + wantContentType: htmlContentType, + wantRedirectLocationRegexp: happyAuthcodeDownstreamRedirectLocationRegexp, + wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, + wantDownstreamIDTokenUsername: oidcUpstreamUsername, + wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamRedirectURI: downstreamRedirectURI, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSessionWithAccessToken, + }, { name: "error during upstream LDAP authentication", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithLDAP(&erroringUpstreamLDAPIdentityProvider), @@ -1015,8 +1077,8 @@ func TestAuthorizationEndpoint(t *testing.T) { wantBodyString: "", }, { - name: "return an error when upstream IDP did not return a refresh token", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().Build()), + name: "password grant returns an error when upstream IDP returns no refresh token with an access token but has no userinfo endpoint", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithoutUserInfoURL().Build()), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), @@ -1024,12 +1086,12 @@ func TestAuthorizationEndpoint(t *testing.T) { wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, wantStatus: http.StatusFound, wantContentType: "application/json; charset=utf-8", - wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingRefreshTokenErrorQuery), + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUserInfoEndpointErrorQuery), wantBodyString: "", }, { - name: "return an error when upstream IDP did not return a refresh token", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().Build()), + name: "password grant returns an error when upstream IDP returns empty refresh token with an access token but has no userinfo endpoint", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithoutUserInfoURL().Build()), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), @@ -1037,7 +1099,59 @@ func TestAuthorizationEndpoint(t *testing.T) { wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, wantStatus: http.StatusFound, wantContentType: "application/json; charset=utf-8", - wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingRefreshTokenErrorQuery), + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingUserInfoEndpointErrorQuery), + wantBodyString: "", + }, + { + name: "password grant returns an error when upstream IDP returns empty refresh token and empty access token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithEmptyAccessToken().Build()), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), + customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword), + wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingAccessTokenErrorQuery), + wantBodyString: "", + }, + { + name: "password grant returns an error when upstream IDP returns no refresh and no access token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithoutAccessToken().Build()), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), + customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword), + wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingAccessTokenErrorQuery), + wantBodyString: "", + }, + { + name: "password grant returns an error when upstream IDP returns no refresh token and empty access token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithEmptyAccessToken().Build()), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), + customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword), + wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingAccessTokenErrorQuery), + wantBodyString: "", + }, + { + name: "password grant returns an error when upstream IDP returns empty refresh token and no access token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithoutAccessToken().Build()), + method: http.MethodGet, + path: happyGetRequestPath, + customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), + customPasswordHeader: pointer.StringPtr(oidcUpstreamPassword), + wantPasswordGrantCall: happyUpstreamPasswordGrantMockExpectation, + wantStatus: http.StatusFound, + wantContentType: "application/json; charset=utf-8", + wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithMissingAccessTokenErrorQuery), wantBodyString: "", }, { diff --git a/internal/oidc/callback/callback_handler.go b/internal/oidc/callback/callback_handler.go index 1a5a3aee..fbf13728 100644 --- a/internal/oidc/callback/callback_handler.go +++ b/internal/oidc/callback/callback_handler.go @@ -19,7 +19,6 @@ import ( "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/oidc/provider/formposthtml" "go.pinniped.dev/internal/plog" - "go.pinniped.dev/internal/psession" ) func NewHandler( @@ -69,39 +68,17 @@ func NewHandler( return httperr.New(http.StatusBadGateway, "error exchanging and validating upstream tokens") } - if token.RefreshToken == nil || token.RefreshToken.Token == "" { - plog.Warning("refresh token not returned by upstream provider during authcode exchange, "+ - "please check configuration of OIDCIdentityProvider and the client in the upstream provider's API/UI", - "upstreamName", upstreamIDPConfig.GetName(), - "scopes", upstreamIDPConfig.GetScopes(), - "additionalParams", upstreamIDPConfig.GetAdditionalAuthcodeParams()) - return httperr.New(http.StatusUnprocessableEntity, "refresh token not returned by upstream provider during authcode exchange") - } - subject, username, groups, err := downstreamsession.GetDownstreamIdentityFromUpstreamIDToken(upstreamIDPConfig, token.IDToken.Claims) if err != nil { return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) } - upstreamSubject, err := downstreamsession.ExtractStringClaimValue(oidc.IDTokenSubjectClaim, upstreamIDPConfig.GetName(), token.IDToken.Claims) - if err != nil { - return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) - } - upstreamIssuer, err := downstreamsession.ExtractStringClaimValue(oidc.IDTokenIssuerClaim, upstreamIDPConfig.GetName(), token.IDToken.Claims) + customSessionData, err := downstreamsession.MakeDownstreamOIDCCustomSessionData(upstreamIDPConfig, token) if err != nil { return httperr.Wrap(http.StatusUnprocessableEntity, err.Error(), err) } - openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, &psession.CustomSessionData{ - ProviderUID: upstreamIDPConfig.GetResourceUID(), - ProviderName: upstreamIDPConfig.GetName(), - ProviderType: psession.ProviderTypeOIDC, - OIDC: &psession.OIDCSessionData{ - UpstreamRefreshToken: token.RefreshToken.Token, - UpstreamSubject: upstreamSubject, - UpstreamIssuer: upstreamIssuer, - }, - }) + openIDSession := downstreamsession.MakeDownstreamSession(subject, username, groups, customSessionData) authorizeResponder, err := oauthHelper.NewAuthorizeResponse(r.Context(), authorizeRequester, openIDSession) if err != nil { diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index d14c159f..a43b2926 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -31,6 +31,7 @@ const ( oidcUpstreamIssuer = "https://my-upstream-issuer.com" oidcUpstreamRefreshToken = "test-refresh-token" + oidcUpstreamAccessToken = "test-access-token" oidcUpstreamSubject = "abc123-some guid" // has a space character which should get escaped in URL oidcUpstreamSubjectQueryEscaped = "abc123-some+guid" oidcUpstreamUsername = "test-pinniped-username" @@ -83,6 +84,16 @@ var ( UpstreamSubject: oidcUpstreamSubject, }, } + happyDownstreamAccessTokenCustomSessionData = &psession.CustomSessionData{ + ProviderUID: happyUpstreamIDPResourceUID, + ProviderName: happyUpstreamIDPName, + ProviderType: psession.ProviderTypeOIDC, + OIDC: &psession.OIDCSessionData{ + UpstreamAccessToken: oidcUpstreamAccessToken, + UpstreamIssuer: oidcUpstreamIssuer, + UpstreamSubject: oidcUpstreamSubject, + }, + } ) func TestCallbackEndpoint(t *testing.T) { @@ -200,6 +211,29 @@ func TestCallbackEndpoint(t *testing.T) { args: happyExchangeAndValidateTokensArgs, }, }, + { + name: "GET with authcode exchange that returns an access token but no refresh token when there is a userinfo endpoint returns 303 to downstream client callback with its state and code", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithUserInfoURL().Build()), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusSeeOther, + wantRedirectLocationRegexp: happyDownstreamRedirectLocationRegexp, + wantBody: "", + wantDownstreamIDTokenSubject: oidcUpstreamIssuer + "?sub=" + oidcUpstreamSubjectQueryEscaped, + wantDownstreamIDTokenUsername: oidcUpstreamUsername, + wantDownstreamIDTokenGroups: oidcUpstreamGroupMembership, + wantDownstreamRequestedScopes: happyDownstreamScopesRequested, + wantDownstreamGrantedScopes: happyDownstreamScopesGranted, + wantDownstreamNonce: downstreamNonce, + wantDownstreamPKCEChallenge: downstreamPKCEChallenge, + wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, + wantDownstreamCustomSessionData: happyDownstreamAccessTokenCustomSessionData, + wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ + performedByUpstreamName: happyUpstreamIDPName, + args: happyExchangeAndValidateTokensArgs, + }, + }, { name: "upstream IDP provides no username or group claim configuration, so we use default username claim and skip groups", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( @@ -323,28 +357,70 @@ func TestCallbackEndpoint(t *testing.T) { }, }, { - name: "return an error when upstream IDP did not return a refresh token", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithoutRefreshToken().Build()), + name: "return an error when upstream IDP returned no refresh token with an access token when there is no userinfo endpoint", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken).WithoutUserInfoURL().Build()), method: http.MethodGet, path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, wantContentType: htmlContentType, - wantBody: "Unprocessable Entity: refresh token not returned by upstream provider during authcode exchange\n", + wantBody: "Unprocessable Entity: access token was returned by upstream provider but there was no userinfo endpoint\n", wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, }, }, { - name: "return an error when upstream IDP returned an empty refresh token", - idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().Build()), + name: "return an error when upstream IDP returned no refresh token and no access token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithoutRefreshToken().WithoutAccessToken().Build()), method: http.MethodGet, path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, wantStatus: http.StatusUnprocessableEntity, wantContentType: htmlContentType, - wantBody: "Unprocessable Entity: refresh token not returned by upstream provider during authcode exchange\n", + wantBody: "Unprocessable Entity: neither access token nor refresh token returned by upstream provider\n", + wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ + performedByUpstreamName: happyUpstreamIDPName, + args: happyExchangeAndValidateTokensArgs, + }, + }, + { + name: "return an error when upstream IDP returned an empty refresh token and empty access token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().WithEmptyAccessToken().Build()), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, + wantBody: "Unprocessable Entity: neither access token nor refresh token returned by upstream provider\n", + wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ + performedByUpstreamName: happyUpstreamIDPName, + args: happyExchangeAndValidateTokensArgs, + }, + }, + { + name: "return an error when upstream IDP returned no refresh token and empty access token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithoutRefreshToken().WithEmptyAccessToken().Build()), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, + wantBody: "Unprocessable Entity: neither access token nor refresh token returned by upstream provider\n", + wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ + performedByUpstreamName: happyUpstreamIDPName, + args: happyExchangeAndValidateTokensArgs, + }, + }, + { + name: "return an error when upstream IDP returned an empty refresh token and no access token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().WithoutAccessToken().Build()), + method: http.MethodGet, + path: newRequestPath().WithState(happyState).String(), + csrfCookie: happyCSRFCookie, + wantStatus: http.StatusUnprocessableEntity, + wantContentType: htmlContentType, + wantBody: "Unprocessable Entity: neither access token nor refresh token returned by upstream provider\n", wantAuthcodeExchangeCall: &expectedAuthcodeExchange{ performedByUpstreamName: happyUpstreamIDPName, args: happyExchangeAndValidateTokensArgs, diff --git a/internal/oidc/downstreamsession/downstream_session.go b/internal/oidc/downstreamsession/downstream_session.go index 265ed5c1..64ac1418 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/internal/oidc/downstreamsession/downstream_session.go @@ -5,6 +5,7 @@ package downstreamsession import ( + "errors" "fmt" "net/url" "time" @@ -19,6 +20,7 @@ import ( "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/psession" + "go.pinniped.dev/pkg/oidcclient/oidctypes" ) const ( @@ -58,6 +60,55 @@ func MakeDownstreamSession(subject string, username string, groups []string, cus return openIDSession } +func MakeDownstreamOIDCCustomSessionData(oidcUpstream provider.UpstreamOIDCIdentityProviderI, token *oidctypes.Token) (*psession.CustomSessionData, error) { + upstreamSubject, err := ExtractStringClaimValue(oidc.IDTokenSubjectClaim, oidcUpstream.GetName(), token.IDToken.Claims) + if err != nil { + return nil, err + } + upstreamIssuer, err := ExtractStringClaimValue(oidc.IDTokenIssuerClaim, oidcUpstream.GetName(), token.IDToken.Claims) + if err != nil { + return nil, err + } + + customSessionData := &psession.CustomSessionData{ + ProviderUID: oidcUpstream.GetResourceUID(), + ProviderName: oidcUpstream.GetName(), + ProviderType: psession.ProviderTypeOIDC, + OIDC: &psession.OIDCSessionData{ + UpstreamIssuer: upstreamIssuer, + UpstreamSubject: upstreamSubject, + }, + } + + const pleaseCheck = "please check configuration of OIDCIdentityProvider and the client in the " + + "upstream provider's API/UI and try to get a refresh token if possible" + logKV := []interface{}{ + "upstreamName", oidcUpstream.GetName(), + "scopes", oidcUpstream.GetScopes(), + "additionalParams", oidcUpstream.GetAdditionalAuthcodeParams(), + } + + hasRefreshToken := token.RefreshToken != nil && token.RefreshToken.Token != "" + hasAccessToken := token.AccessToken != nil && token.AccessToken.Token != "" + switch { + case hasRefreshToken: // we prefer refresh tokens, so check for this first + customSessionData.OIDC.UpstreamRefreshToken = token.RefreshToken.Token + case hasAccessToken: // as a fallback, we can use the access token as long as there is a userinfo endpoint + if !oidcUpstream.HasUserInfoURL() { + plog.Warning("access token was returned by upstream provider during login without a refresh token "+ + "and there was no userinfo endpoint available on the provider. "+pleaseCheck, logKV...) + return nil, errors.New("access token was returned by upstream provider but there was no userinfo endpoint") + } + plog.Info("refresh token not returned by upstream provider during login, using access token instead. "+pleaseCheck, logKV...) + customSessionData.OIDC.UpstreamAccessToken = token.AccessToken.Token + default: + plog.Warning("refresh token and access token not returned by upstream provider during login. "+pleaseCheck, logKV...) + return nil, errors.New("neither access token nor refresh token returned by upstream provider") + } + + return customSessionData, nil +} + // GrantScopesIfRequested auto-grants the scopes for which we do not require end-user approval, if they were requested. func GrantScopesIfRequested(authorizeRequester fosite.AuthorizeRequester) { oidc.GrantScopeIfRequested(authorizeRequester, coreosoidc.ScopeOpenID) diff --git a/internal/oidc/provider/dynamic_upstream_idp_provider.go b/internal/oidc/provider/dynamic_upstream_idp_provider.go index 24b821e7..c471955f 100644 --- a/internal/oidc/provider/dynamic_upstream_idp_provider.go +++ b/internal/oidc/provider/dynamic_upstream_idp_provider.go @@ -31,6 +31,9 @@ type UpstreamOIDCIdentityProviderI interface { // GetAuthorizationURL returns the Authorization Endpoint fetched from discovery. GetAuthorizationURL() *url.URL + // HasUserInfoURL returns whether there is a non-empty value for userinfo_endpoint fetched from discovery. + HasUserInfoURL() bool + // GetScopes returns the scopes to request in authorization (authcode or password grant) flow. GetScopes() []string @@ -74,7 +77,7 @@ type UpstreamOIDCIdentityProviderI interface { // ValidateTokenAndMergeWithUserInfo will validate the ID token. It will also merge the claims from the userinfo endpoint response // into the ID token's claims, if the provider offers the userinfo endpoint. It returns the validated/updated // tokens, or an error. - ValidateTokenAndMergeWithUserInfo(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, requireIDToken bool) (*oidctypes.Token, error) + ValidateTokenAndMergeWithUserInfo(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, requireIDToken bool, requireUserInfo bool) (*oidctypes.Token, error) } type UpstreamLDAPIdentityProviderI interface { diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index e0fc4408..56200aad 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -11,12 +11,14 @@ import ( "github.com/ory/fosite" "github.com/ory/x/errorsx" + "golang.org/x/oauth2" "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/oidc" "go.pinniped.dev/internal/oidc/provider" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/psession" + "go.pinniped.dev/pkg/oidcclient/oidctypes" ) var ( @@ -101,7 +103,15 @@ func upstreamRefresh(ctx context.Context, accessRequest fosite.AccessRequester, func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, providerCache oidc.UpstreamIdentityProvidersLister) error { s := session.Custom - if s.OIDC == nil || s.OIDC.UpstreamRefreshToken == "" { + if s.OIDC == nil { + return errorsx.WithStack(errMissingUpstreamSessionInternalError) + } + + accessTokenStored := s.OIDC.UpstreamAccessToken != "" + refreshTokenStored := s.OIDC.UpstreamRefreshToken != "" + + exactlyOneTokenStored := (accessTokenStored || refreshTokenStored) && !(accessTokenStored && refreshTokenStored) + if !exactlyOneTokenStored { return errorsx.WithStack(errMissingUpstreamSessionInternalError) } @@ -113,63 +123,88 @@ func upstreamOIDCRefresh(ctx context.Context, session *psession.PinnipedSession, plog.Debug("attempting upstream refresh request", "providerName", s.ProviderName, "providerType", s.ProviderType, "providerUID", s.ProviderUID) - refreshedTokens, err := p.PerformRefresh(ctx, s.OIDC.UpstreamRefreshToken) - if err != nil { - return errorsx.WithStack(errUpstreamRefreshError.WithHint( - "Upstream refresh failed.", - ).WithWrap(err).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + var tokens *oauth2.Token + if refreshTokenStored { + tokens, err = p.PerformRefresh(ctx, s.OIDC.UpstreamRefreshToken) + if err != nil { + return errorsx.WithStack(errUpstreamRefreshError.WithHint( + "Upstream refresh failed.", + ).WithWrap(err).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + } + } else { + tokens = &oauth2.Token{AccessToken: s.OIDC.UpstreamAccessToken} } // Upstream refresh may or may not return a new ID token. From the spec: // "the response body is the Token Response of Section 3.1.3.3 except that it might not contain an id_token." // https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse - _, hasIDTok := refreshedTokens.Extra("id_token").(string) + _, hasIDTok := tokens.Extra("id_token").(string) // The spec is not 100% clear about whether an ID token from the refresh flow should include a nonce, and at // least some providers do not include one, so we skip the nonce validation here (but not other validations). - validatedTokens, err := p.ValidateTokenAndMergeWithUserInfo(ctx, refreshedTokens, "", hasIDTok) + validatedTokens, err := p.ValidateTokenAndMergeWithUserInfo(ctx, tokens, "", hasIDTok, accessTokenStored) if err != nil { return errorsx.WithStack(errUpstreamRefreshError.WithHintf( "Upstream refresh returned an invalid ID token or UserInfo response.").WithWrap(err).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } - claims := validatedTokens.IDToken.Claims - // if we have any claims at all, we better have a subject, and it better match the previous value. - // but it's possible that we don't because both returning a new refresh token on refresh and having a userinfo - // endpoint are optional. - if len(validatedTokens.IDToken.Claims) != 0 { //nolint:nestif - newSub, hasSub := getString(claims, oidc.IDTokenSubjectClaim) - if !hasSub { - return errorsx.WithStack(errUpstreamRefreshError.WithHintf( - "Upstream refresh failed.").WithWrap(errors.New("subject in upstream refresh not found")).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) - } - if s.OIDC.UpstreamSubject != newSub { - return errorsx.WithStack(errUpstreamRefreshError.WithHintf( - "Upstream refresh failed.").WithWrap(errors.New("subject in upstream refresh does not match previous value")).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) - } - usernameClaim := p.GetUsernameClaim() - newUsername, hasUsername := getString(claims, usernameClaim) - oldUsername := session.Fosite.Claims.Extra[oidc.DownstreamUsernameClaim] - // its possible this won't be returned. - // but if it is, verify that it hasn't changed. - if hasUsername && oldUsername != newUsername { - return errorsx.WithStack(errUpstreamRefreshError.WithHintf( - "Upstream refresh failed.").WithWrap(errors.New("username in upstream refresh does not match previous value")).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) - } - newIssuer, hasIssuer := getString(claims, oidc.IDTokenIssuerClaim) - if hasIssuer && s.OIDC.UpstreamIssuer != newIssuer { - return errorsx.WithStack(errUpstreamRefreshError.WithHintf( - "Upstream refresh failed.").WithWrap(errors.New("issuer in upstream refresh does not match previous value")).WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) - } + err = validateIdentityUnchangedSinceInitialLogin(validatedTokens, session, p.GetUsernameClaim()) + if err != nil { + return err } // Upstream refresh may or may not return a new refresh token. If we got a new refresh token, then update it in // the user's session. If we did not get a new refresh token, then keep the old one in the session by avoiding // overwriting the old one. - if refreshedTokens.RefreshToken != "" { - plog.Debug("upstream refresh request did not return a new refresh token", + if tokens.RefreshToken != "" { + plog.Debug("upstream refresh request returned a new refresh token", "providerName", s.ProviderName, "providerType", s.ProviderType, "providerUID", s.ProviderUID) - s.OIDC.UpstreamRefreshToken = refreshedTokens.RefreshToken + s.OIDC.UpstreamRefreshToken = tokens.RefreshToken + } + + return nil +} + +func validateIdentityUnchangedSinceInitialLogin(validatedTokens *oidctypes.Token, session *psession.PinnipedSession, usernameClaimName string) error { + s := session.Custom + mergedClaims := validatedTokens.IDToken.Claims + + // If we have any claims at all, we better have a subject, and it better match the previous value. + // but it's possible that we don't because both returning a new id token on refresh and having a userinfo + // endpoint are optional. + if len(mergedClaims) == 0 { + return nil + } + + newSub, hasSub := getString(mergedClaims, oidc.IDTokenSubjectClaim) + if !hasSub { + return errorsx.WithStack(errUpstreamRefreshError.WithHintf( + "Upstream refresh failed.").WithWrap(errors.New("subject in upstream refresh not found")). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + } + if s.OIDC.UpstreamSubject != newSub { + return errorsx.WithStack(errUpstreamRefreshError.WithHintf( + "Upstream refresh failed.").WithWrap(errors.New("subject in upstream refresh does not match previous value")). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + } + + newUsername, hasUsername := getString(mergedClaims, usernameClaimName) + oldUsername := session.Fosite.Claims.Extra[oidc.DownstreamUsernameClaim] + // It's possible that a username wasn't returned by the upstream provider during refresh, + // but if it is, verify that it hasn't changed. + if hasUsername && oldUsername != newUsername { + return errorsx.WithStack(errUpstreamRefreshError.WithHintf( + "Upstream refresh failed.").WithWrap(errors.New("username in upstream refresh does not match previous value")). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) + } + + newIssuer, hasIssuer := getString(mergedClaims, oidc.IDTokenIssuerClaim) + // It's possible that an issuer wasn't returned by the upstream provider during refresh, + // but if it is, verify that it hasn't changed. + if hasIssuer && s.OIDC.UpstreamIssuer != newIssuer { + return errorsx.WithStack(errUpstreamRefreshError.WithHintf( + "Upstream refresh failed.").WithWrap(errors.New("issuer in upstream refresh does not match previous value")). + WithDebugf("provider name: %q, provider type: %q", s.ProviderName, s.ProviderType)) } return nil diff --git a/internal/oidc/token/token_handler_test.go b/internal/oidc/token/token_handler_test.go index 2a8f039a..11405ed4 100644 --- a/internal/oidc/token/token_handler_test.go +++ b/internal/oidc/token/token_handler_test.go @@ -225,7 +225,7 @@ type expectedUpstreamRefresh struct { type expectedUpstreamValidateTokens struct { performedByUpstreamName string - args *oidctestutil.ValidateTokenArgs + args *oidctestutil.ValidateTokenAndMergeWithUserInfoArgs } type tokenEndpointResponseExpectedValues struct { @@ -881,6 +881,7 @@ func TestRefreshGrant(t *testing.T) { oidcUpstreamInitialRefreshToken = "initial-upstream-refresh-token" oidcUpstreamRefreshedIDToken = "fake-refreshed-id-token" oidcUpstreamRefreshedRefreshToken = "fake-refreshed-refresh-token" + oidcUpstreamAccessToken = "fake-upstream-access-token" //nolint:gosec ldapUpstreamName = "some-ldap-idp" ldapUpstreamResourceUID = "ldap-resource-uid" @@ -904,7 +905,7 @@ func TestRefreshGrant(t *testing.T) { WithResourceUID(oidcUpstreamResourceUID) } - initialUpstreamOIDCCustomSessionData := func() *psession.CustomSessionData { + initialUpstreamOIDCRefreshTokenCustomSessionData := func() *psession.CustomSessionData { return &psession.CustomSessionData{ ProviderName: oidcUpstreamName, ProviderUID: oidcUpstreamResourceUID, @@ -917,8 +918,21 @@ func TestRefreshGrant(t *testing.T) { } } + initialUpstreamOIDCAccessTokenCustomSessionData := func() *psession.CustomSessionData { + return &psession.CustomSessionData{ + ProviderName: oidcUpstreamName, + ProviderUID: oidcUpstreamResourceUID, + ProviderType: oidcUpstreamType, + OIDC: &psession.OIDCSessionData{ + UpstreamAccessToken: oidcUpstreamAccessToken, + UpstreamSubject: goodUpstreamSubject, + UpstreamIssuer: goodIssuer, + }, + } + } + upstreamOIDCCustomSessionDataWithNewRefreshToken := func(newRefreshToken string) *psession.CustomSessionData { - sessionData := initialUpstreamOIDCCustomSessionData() + sessionData := initialUpstreamOIDCRefreshTokenCustomSessionData() sessionData.OIDC.UpstreamRefreshToken = newRefreshToken return sessionData } @@ -957,13 +971,15 @@ func TestRefreshGrant(t *testing.T) { } } - happyUpstreamValidateTokenCall := func(expectedTokens *oauth2.Token) *expectedUpstreamValidateTokens { + happyUpstreamValidateTokenCall := func(expectedTokens *oauth2.Token, requireIDToken bool) *expectedUpstreamValidateTokens { return &expectedUpstreamValidateTokens{ performedByUpstreamName: oidcUpstreamName, - args: &oidctestutil.ValidateTokenArgs{ + args: &oidctestutil.ValidateTokenAndMergeWithUserInfoArgs{ Ctx: nil, // this will be filled in with the actual request context by the test below Tok: expectedTokens, ExpectedIDTokenNonce: "", // always expect empty string + RequireUserInfo: false, + RequireIDToken: requireIDToken, }, } } @@ -986,7 +1002,7 @@ func TestRefreshGrant(t *testing.T) { // Should always try to perform an upstream refresh. want.wantUpstreamRefreshCall = happyOIDCUpstreamRefreshCall() if expectToValidateToken != nil { - want.wantUpstreamOIDCValidateTokenCall = happyUpstreamValidateTokenCall(expectToValidateToken) + want.wantUpstreamOIDCValidateTokenCall = happyUpstreamValidateTokenCall(expectToValidateToken, true) } return want } @@ -1049,7 +1065,7 @@ func TestRefreshGrant(t *testing.T) { { name: "happy path refresh grant with openid scope granted (id token returned)", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "sub": goodUpstreamSubject, @@ -1057,9 +1073,9 @@ func TestRefreshGrant(t *testing.T) { }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ want: happyRefreshTokenResponseForOpenIDAndOfflineAccess( @@ -1071,7 +1087,7 @@ func TestRefreshGrant(t *testing.T) { { name: "refresh grant with unchanged username claim", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithUsernameClaim("username-claim").WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithUsernameClaim("username-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "some-claim": "some-value", @@ -1081,9 +1097,9 @@ func TestRefreshGrant(t *testing.T) { }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ want: happyRefreshTokenResponseForOpenIDAndOfflineAccess( @@ -1092,23 +1108,64 @@ func TestRefreshGrant(t *testing.T) { ), }, }, + { + name: "refresh grant when the customsessiondata has a stored access token and no stored refresh token", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( + upstreamOIDCIdentityProviderBuilder().WithUsernameClaim("username-claim"). + WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ + IDToken: &oidctypes.IDToken{ + Claims: map[string]interface{}{ + "some-claim": "some-value", + "sub": goodUpstreamSubject, + "username-claim": goodUsername, + }, + }, + AccessToken: &oidctypes.AccessToken{ + Token: oidcUpstreamAccessToken, + }, + }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), + authcodeExchange: authcodeExchangeInputs{ + customSessionData: initialUpstreamOIDCAccessTokenCustomSessionData(), + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCAccessTokenCustomSessionData()), + }, // do not want upstreamRefreshRequest??? + refreshRequest: refreshRequestInputs{ + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "id_token", "access_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + wantUpstreamOIDCValidateTokenCall: &expectedUpstreamValidateTokens{ + oidcUpstreamName, + &oidctestutil.ValidateTokenAndMergeWithUserInfoArgs{ + Ctx: nil, // this will be filled in with the actual request context by the test below + Tok: &oauth2.Token{AccessToken: oidcUpstreamAccessToken}, // only the old access token + ExpectedIDTokenNonce: "", // always expect empty string + RequireIDToken: false, + RequireUserInfo: true, + }, + }, + wantCustomSessionDataStored: initialUpstreamOIDCAccessTokenCustomSessionData(), // doesn't change when we refresh + }, + }, + }, { name: "happy path refresh grant without openid scope granted (no id token returned)", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{}, }, - }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), + }).WithRefreshedTokens(refreshedUpstreamTokensWithRefreshTokenWithoutIDToken()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "offline_access") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"refresh_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"offline_access"}, wantGrantedScopes: []string{"offline_access"}, - wantCustomSessionDataStored: initialUpstreamOIDCCustomSessionData(), + wantCustomSessionDataStored: initialUpstreamOIDCRefreshTokenCustomSessionData(), }, }, refreshRequest: refreshRequestInputs{ @@ -1118,7 +1175,7 @@ func TestRefreshGrant(t *testing.T) { wantRequestedScopes: []string{"offline_access"}, wantGrantedScopes: []string{"offline_access"}, wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), - wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens()), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithRefreshTokenWithoutIDToken(), false), wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), }, }, @@ -1126,27 +1183,32 @@ func TestRefreshGrant(t *testing.T) { { name: "happy path refresh grant when the upstream refresh does not return a new ID token", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{}, }, }).WithRefreshedTokens(refreshedUpstreamTokensWithRefreshTokenWithoutIDToken()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ - want: happyRefreshTokenResponseForOpenIDAndOfflineAccess( - upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), - refreshedUpstreamTokensWithRefreshTokenWithoutIDToken(), // expect ValidateTokenAndMergeWithUserInfo is called - ), + want: tokenEndpointResponseExpectedValues{ + wantStatus: http.StatusOK, + wantSuccessBodyFields: []string{"refresh_token", "access_token", "id_token", "token_type", "expires_in", "scope"}, + wantRequestedScopes: []string{"openid", "offline_access"}, + wantGrantedScopes: []string{"openid", "offline_access"}, + wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithRefreshTokenWithoutIDToken(), false), + wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), + }, }, }, { name: "happy path refresh grant when the upstream refresh does not return a new refresh token", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "sub": goodUpstreamSubject, @@ -1154,13 +1216,13 @@ func TestRefreshGrant(t *testing.T) { }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDTokenWithoutRefreshToken()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ want: happyRefreshTokenResponseForOpenIDAndOfflineAccess( - initialUpstreamOIDCCustomSessionData(), // still has the initial refresh token stored + initialUpstreamOIDCRefreshTokenCustomSessionData(), // still has the initial refresh token stored refreshedUpstreamTokensWithIDTokenWithoutRefreshToken(), ), }, @@ -1168,7 +1230,7 @@ func TestRefreshGrant(t *testing.T) { { name: "when the refresh request adds a new scope to the list of requested scopes then it is ignored", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "sub": goodUpstreamSubject, @@ -1176,9 +1238,9 @@ func TestRefreshGrant(t *testing.T) { }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { @@ -1193,7 +1255,7 @@ func TestRefreshGrant(t *testing.T) { { name: "when the refresh request removes a scope which was originally granted from the list of requested scopes then it is granted anyway", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "sub": goodUpstreamSubject, @@ -1201,14 +1263,14 @@ func TestRefreshGrant(t *testing.T) { }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access pinniped:request-audience") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"id_token", "refresh_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"openid", "offline_access", "pinniped:request-audience"}, wantGrantedScopes: []string{"openid", "offline_access", "pinniped:request-audience"}, - wantCustomSessionDataStored: initialUpstreamOIDCCustomSessionData(), + wantCustomSessionDataStored: initialUpstreamOIDCRefreshTokenCustomSessionData(), }, }, refreshRequest: refreshRequestInputs{ @@ -1221,7 +1283,7 @@ func TestRefreshGrant(t *testing.T) { wantRequestedScopes: []string{"openid", "offline_access", "pinniped:request-audience"}, wantGrantedScopes: []string{"openid", "offline_access", "pinniped:request-audience"}, wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), - wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens()), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), wantCustomSessionDataStored: upstreamOIDCCustomSessionDataWithNewRefreshToken(oidcUpstreamRefreshedRefreshToken), }, }, @@ -1229,7 +1291,7 @@ func TestRefreshGrant(t *testing.T) { { name: "when the refresh request does not include a scope param then it gets all the same scopes as the original authorization request", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "sub": goodUpstreamSubject, @@ -1237,9 +1299,9 @@ func TestRefreshGrant(t *testing.T) { }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ modifyTokenRequest: func(r *http.Request, refreshToken string, accessToken string) { @@ -1255,14 +1317,14 @@ func TestRefreshGrant(t *testing.T) { name: "when a bad refresh token is sent in the refresh request", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "offline_access") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"refresh_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"offline_access"}, wantGrantedScopes: []string{"offline_access"}, - wantCustomSessionDataStored: initialUpstreamOIDCCustomSessionData(), + wantCustomSessionDataStored: initialUpstreamOIDCRefreshTokenCustomSessionData(), }, }, refreshRequest: refreshRequestInputs{ @@ -1279,14 +1341,14 @@ func TestRefreshGrant(t *testing.T) { name: "when the access token is sent as if it were a refresh token", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "offline_access") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"refresh_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"offline_access"}, wantGrantedScopes: []string{"offline_access"}, - wantCustomSessionDataStored: initialUpstreamOIDCCustomSessionData(), + wantCustomSessionDataStored: initialUpstreamOIDCRefreshTokenCustomSessionData(), }, }, refreshRequest: refreshRequestInputs{ @@ -1303,14 +1365,14 @@ func TestRefreshGrant(t *testing.T) { name: "when the wrong client ID is included in the refresh request", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "offline_access") }, want: tokenEndpointResponseExpectedValues{ wantStatus: http.StatusOK, wantSuccessBodyFields: []string{"refresh_token", "access_token", "token_type", "expires_in", "scope"}, wantRequestedScopes: []string{"offline_access"}, wantGrantedScopes: []string{"offline_access"}, - wantCustomSessionDataStored: initialUpstreamOIDCCustomSessionData(), + wantCustomSessionDataStored: initialUpstreamOIDCRefreshTokenCustomSessionData(), }, }, refreshRequest: refreshRequestInputs{ @@ -1474,7 +1536,7 @@ func TestRefreshGrant(t *testing.T) { }, }, { - name: "when there is no OIDC refresh token in custom session data found in the session storage during the refresh request", + name: "when there is no OIDC refresh token nor access token in custom session data found in the session storage during the refresh request", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder().Build()), authcodeExchange: authcodeExchangeInputs{ customSessionData: &psession.CustomSessionData{ @@ -1482,7 +1544,8 @@ func TestRefreshGrant(t *testing.T) { ProviderUID: oidcUpstreamResourceUID, ProviderType: oidcUpstreamType, OIDC: &psession.OIDCSessionData{ - UpstreamRefreshToken: "", // this should not happen in practice + UpstreamRefreshToken: "", // this should not happen in practice. we should always have exactly one of these. + UpstreamAccessToken: "", }, }, modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, @@ -1493,6 +1556,7 @@ func TestRefreshGrant(t *testing.T) { ProviderType: oidcUpstreamType, OIDC: &psession.OIDCSessionData{ UpstreamRefreshToken: "", // this should not happen in practice + UpstreamAccessToken: "", }, }, ), @@ -1573,9 +1637,9 @@ func TestRefreshGrant(t *testing.T) { idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder(). WithPerformRefreshError(errors.New("some upstream refresh error")).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ @@ -1595,17 +1659,17 @@ func TestRefreshGrant(t *testing.T) { idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder(). WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()). // This is the current format of the errors returned by the production code version of ValidateTokenAndMergeWithUserInfo, see ValidateTokenAndMergeWithUserInfo in upstreamoidc.go - WithValidateTokenError(httperr.Wrap(http.StatusBadRequest, "some validate error", errors.New("some validate cause"))). + WithValidateTokenAndMergeWithUserInfoError(httperr.Wrap(http.StatusBadRequest, "some validate error", errors.New("some validate cause"))). Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), - wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens()), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), wantStatus: http.StatusUnauthorized, wantErrorResponseBody: here.Doc(` { @@ -1621,7 +1685,7 @@ func TestRefreshGrant(t *testing.T) { idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(upstreamOIDCIdentityProviderBuilder(). WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()). // This is the current format of the errors returned by the production code version of ValidateTokenAndMergeWithUserInfo, see ValidateTokenAndMergeWithUserInfo in upstreamoidc.go - WithValidatedTokens(&oidctypes.Token{ + WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "sub": "something-different", @@ -1630,14 +1694,14 @@ func TestRefreshGrant(t *testing.T) { }). Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), - wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens()), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), wantStatus: http.StatusUnauthorized, wantErrorResponseBody: here.Doc(` { @@ -1651,7 +1715,7 @@ func TestRefreshGrant(t *testing.T) { { name: "refresh grant with claims but not the subject claim", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "some-claim": "some-value", @@ -1659,14 +1723,14 @@ func TestRefreshGrant(t *testing.T) { }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), - wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens()), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), wantStatus: http.StatusUnauthorized, wantErrorResponseBody: here.Doc(` { @@ -1680,7 +1744,7 @@ func TestRefreshGrant(t *testing.T) { { name: "refresh grant with changed username claim", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithUsernameClaim("username-claim").WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithUsernameClaim("username-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "some-claim": "some-value", @@ -1690,14 +1754,14 @@ func TestRefreshGrant(t *testing.T) { }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), - wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens()), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), wantStatus: http.StatusUnauthorized, wantErrorResponseBody: here.Doc(` { @@ -1711,7 +1775,7 @@ func TestRefreshGrant(t *testing.T) { { name: "refresh grant with changed issuer claim", idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC( - upstreamOIDCIdentityProviderBuilder().WithUsernameClaim("username-claim").WithValidatedTokens(&oidctypes.Token{ + upstreamOIDCIdentityProviderBuilder().WithUsernameClaim("username-claim").WithValidatedAndMergedWithUserInfoTokens(&oidctypes.Token{ IDToken: &oidctypes.IDToken{ Claims: map[string]interface{}{ "some-claim": "some-value", @@ -1721,14 +1785,14 @@ func TestRefreshGrant(t *testing.T) { }, }).WithRefreshedTokens(refreshedUpstreamTokensWithIDAndRefreshTokens()).Build()), authcodeExchange: authcodeExchangeInputs{ - customSessionData: initialUpstreamOIDCCustomSessionData(), + customSessionData: initialUpstreamOIDCRefreshTokenCustomSessionData(), modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access") }, - want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCCustomSessionData()), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccess(initialUpstreamOIDCRefreshTokenCustomSessionData()), }, refreshRequest: refreshRequestInputs{ want: tokenEndpointResponseExpectedValues{ wantUpstreamRefreshCall: happyOIDCUpstreamRefreshCall(), - wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens()), + wantUpstreamOIDCValidateTokenCall: happyUpstreamValidateTokenCall(refreshedUpstreamTokensWithIDAndRefreshTokens(), true), wantStatus: http.StatusUnauthorized, wantErrorResponseBody: here.Doc(` { diff --git a/internal/psession/pinniped_session.go b/internal/psession/pinniped_session.go index bbcc4317..3ed39ee0 100644 --- a/internal/psession/pinniped_session.go +++ b/internal/psession/pinniped_session.go @@ -61,9 +61,27 @@ const ( // OIDCSessionData is the additional data needed by Pinniped when the upstream IDP is an OIDC provider. type OIDCSessionData struct { + // UpstreamRefreshToken will contain the refresh token from the upstream OIDC provider, if the upstream provider + // returned a refresh token during initial authorization. Otherwise, this field should be empty + // and the UpstreamAccessToken should be non-empty. We may not get a refresh token from the upstream provider, + // but we should always get an access token. However, when we do get a refresh token there is no need to + // also store the access token, since storing unnecessary tokens makes it possible for them to be leaked and + // creates more upstream revocation HTTP requests when it comes time to revoke the stored tokens. UpstreamRefreshToken string `json:"upstreamRefreshToken"` - UpstreamSubject string `json:"upstreamSubject"` - UpstreamIssuer string `json:"upstreamIssuer"` + + // UpstreamAccessToken will contain the access token returned by the upstream OIDC provider during initial + // authorization, but only when the provider did not also return a refresh token. When UpstreamRefreshToken is + // non-empty, then this field should be empty, indicating that we should use the upstream refresh token during + // downstream refresh. + UpstreamAccessToken string `json:"upstreamAccessToken"` + + // UpstreamSubject is the "sub" claim from the upstream identity provider from the user's initial login. We store this + // so that we can validate that it does not change upon refresh. + UpstreamSubject string `json:"upstreamSubject"` + + // UpstreamIssuer is the "iss" claim from the upstream identity provider from the user's initial login. We store this + // so that we can validate that it does not change upon refresh. + UpstreamIssuer string `json:"upstreamIssuer"` } // LDAPSessionData is the additional data needed by Pinniped when the upstream IDP is an LDAP provider. diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index aa682081..bed4685b 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidctestutil @@ -75,12 +75,14 @@ type RevokeRefreshTokenArgs struct { RefreshToken string } -// ValidateTokenArgs is used to spy on calls to -// TestUpstreamOIDCIdentityProvider.ValidateTokenFunc(). -type ValidateTokenArgs struct { +// ValidateTokenAndMergeWithUserInfoArgs is used to spy on calls to +// TestUpstreamOIDCIdentityProvider.ValidateTokenAndMergeWithUserInfoFunc(). +type ValidateTokenAndMergeWithUserInfoArgs struct { Ctx context.Context Tok *oauth2.Token ExpectedIDTokenNonce nonce.Nonce + RequireIDToken bool + RequireUserInfo bool } type ValidateRefreshArgs struct { @@ -150,6 +152,7 @@ type TestUpstreamOIDCIdentityProvider struct { ClientID string ResourceUID types.UID AuthorizationURL url.URL + UserInfoURL bool RevocationURL *url.URL UsernameClaim string GroupsClaim string @@ -174,7 +177,7 @@ type TestUpstreamOIDCIdentityProvider struct { RevokeRefreshTokenFunc func(ctx context.Context, refreshToken string) error - ValidateTokenFunc func(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) + ValidateTokenAndMergeWithUserInfoFunc func(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) exchangeAuthcodeAndValidateTokensCallCount int exchangeAuthcodeAndValidateTokensArgs []*ExchangeAuthcodeAndValidateTokenArgs @@ -184,8 +187,8 @@ type TestUpstreamOIDCIdentityProvider struct { performRefreshArgs []*PerformRefreshArgs revokeRefreshTokenCallCount int revokeRefreshTokenArgs []*RevokeRefreshTokenArgs - validateTokenCallCount int - validateTokenArgs []*ValidateTokenArgs + validateTokenAndMergeWithUserInfoCallCount int + validateTokenAndMergeWithUserInfoArgs []*ValidateTokenAndMergeWithUserInfoArgs } var _ provider.UpstreamOIDCIdentityProviderI = &TestUpstreamOIDCIdentityProvider{} @@ -210,6 +213,10 @@ func (u *TestUpstreamOIDCIdentityProvider) GetAuthorizationURL() *url.URL { return &u.AuthorizationURL } +func (u *TestUpstreamOIDCIdentityProvider) HasUserInfoURL() bool { + return u.UserInfoURL +} + func (u *TestUpstreamOIDCIdentityProvider) GetRevocationURL() *url.URL { return u.RevocationURL } @@ -318,28 +325,30 @@ func (u *TestUpstreamOIDCIdentityProvider) RevokeRefreshTokenArgs(call int) *Rev return u.revokeRefreshTokenArgs[call] } -func (u *TestUpstreamOIDCIdentityProvider) ValidateTokenAndMergeWithUserInfo(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, requireIDToken bool) (*oidctypes.Token, error) { - if u.validateTokenArgs == nil { - u.validateTokenArgs = make([]*ValidateTokenArgs, 0) +func (u *TestUpstreamOIDCIdentityProvider) ValidateTokenAndMergeWithUserInfo(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, requireIDToken bool, requireUserInfo bool) (*oidctypes.Token, error) { + if u.validateTokenAndMergeWithUserInfoArgs == nil { + u.validateTokenAndMergeWithUserInfoArgs = make([]*ValidateTokenAndMergeWithUserInfoArgs, 0) } - u.validateTokenCallCount++ - u.validateTokenArgs = append(u.validateTokenArgs, &ValidateTokenArgs{ + u.validateTokenAndMergeWithUserInfoCallCount++ + u.validateTokenAndMergeWithUserInfoArgs = append(u.validateTokenAndMergeWithUserInfoArgs, &ValidateTokenAndMergeWithUserInfoArgs{ Ctx: ctx, Tok: tok, ExpectedIDTokenNonce: expectedIDTokenNonce, + RequireIDToken: requireIDToken, + RequireUserInfo: requireUserInfo, }) - return u.ValidateTokenFunc(ctx, tok, expectedIDTokenNonce) + return u.ValidateTokenAndMergeWithUserInfoFunc(ctx, tok, expectedIDTokenNonce) } -func (u *TestUpstreamOIDCIdentityProvider) ValidateTokenCallCount() int { - return u.validateTokenCallCount +func (u *TestUpstreamOIDCIdentityProvider) ValidateTokenAndMergeWithUserInfoCallCount() int { + return u.validateTokenAndMergeWithUserInfoCallCount } -func (u *TestUpstreamOIDCIdentityProvider) ValidateTokenArgs(call int) *ValidateTokenArgs { - if u.validateTokenArgs == nil { - u.validateTokenArgs = make([]*ValidateTokenArgs, 0) +func (u *TestUpstreamOIDCIdentityProvider) ValidateTokenAndMergeWithUserInfoArgs(call int) *ValidateTokenAndMergeWithUserInfoArgs { + if u.validateTokenAndMergeWithUserInfoArgs == nil { + u.validateTokenAndMergeWithUserInfoArgs = make([]*ValidateTokenAndMergeWithUserInfoArgs, 0) } - return u.validateTokenArgs[call] + return u.validateTokenAndMergeWithUserInfoArgs[call] } type UpstreamIDPListerBuilder struct { @@ -524,18 +533,18 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyZeroCallsToPerformRefresh(t *te func (b *UpstreamIDPListerBuilder) RequireExactlyOneCallToValidateToken( t *testing.T, expectedPerformedByUpstreamName string, - expectedArgs *ValidateTokenArgs, + expectedArgs *ValidateTokenAndMergeWithUserInfoArgs, ) { t.Helper() - var actualArgs *ValidateTokenArgs + var actualArgs *ValidateTokenAndMergeWithUserInfoArgs var actualNameOfUpstreamWhichMadeCall string actualCallCountAcrossAllOIDCUpstreams := 0 for _, upstreamOIDC := range b.upstreamOIDCIdentityProviders { - callCountOnThisUpstream := upstreamOIDC.validateTokenCallCount + callCountOnThisUpstream := upstreamOIDC.validateTokenAndMergeWithUserInfoCallCount actualCallCountAcrossAllOIDCUpstreams += callCountOnThisUpstream if callCountOnThisUpstream == 1 { actualNameOfUpstreamWhichMadeCall = upstreamOIDC.Name - actualArgs = upstreamOIDC.validateTokenArgs[0] + actualArgs = upstreamOIDC.validateTokenAndMergeWithUserInfoArgs[0] } } require.Equal(t, 1, actualCallCountAcrossAllOIDCUpstreams, @@ -551,7 +560,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyZeroCallsToValidateToken(t *tes t.Helper() actualCallCountAcrossAllOIDCUpstreams := 0 for _, upstreamOIDC := range b.upstreamOIDCIdentityProviders { - actualCallCountAcrossAllOIDCUpstreams += upstreamOIDC.validateTokenCallCount + actualCallCountAcrossAllOIDCUpstreams += upstreamOIDC.validateTokenAndMergeWithUserInfoCallCount } require.Equal(t, 0, actualCallCountAcrossAllOIDCUpstreams, "expected exactly zero calls to ValidateTokenAndMergeWithUserInfo()", @@ -600,24 +609,26 @@ func NewUpstreamIDPListerBuilder() *UpstreamIDPListerBuilder { } type TestUpstreamOIDCIdentityProviderBuilder struct { - name string - resourceUID types.UID - clientID string - scopes []string - idToken map[string]interface{} - refreshToken *oidctypes.RefreshToken - usernameClaim string - groupsClaim string - refreshedTokens *oauth2.Token - validatedTokens *oidctypes.Token - authorizationURL url.URL - additionalAuthcodeParams map[string]string - allowPasswordGrant bool - authcodeExchangeErr error - passwordGrantErr error - performRefreshErr error - revokeRefreshTokenErr error - validateTokenErr error + name string + resourceUID types.UID + clientID string + scopes []string + idToken map[string]interface{} + refreshToken *oidctypes.RefreshToken + accessToken *oidctypes.AccessToken + usernameClaim string + groupsClaim string + refreshedTokens *oauth2.Token + validatedAndMergedWithUserInfoTokens *oidctypes.Token + authorizationURL url.URL + hasUserInfoURL bool + additionalAuthcodeParams map[string]string + allowPasswordGrant bool + authcodeExchangeErr error + passwordGrantErr error + performRefreshErr error + revokeRefreshTokenErr error + validateTokenAndMergeWithUserInfoErr error } func (u *TestUpstreamOIDCIdentityProviderBuilder) WithName(value string) *TestUpstreamOIDCIdentityProviderBuilder { @@ -640,6 +651,16 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAuthorizationURL(value url return u } +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithUserInfoURL() *TestUpstreamOIDCIdentityProviderBuilder { + u.hasUserInfoURL = true + return u +} + +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithoutUserInfoURL() *TestUpstreamOIDCIdentityProviderBuilder { + u.hasUserInfoURL = false + return u +} + func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAllowPasswordGrant(value bool) *TestUpstreamOIDCIdentityProviderBuilder { u.allowPasswordGrant = value return u @@ -703,6 +724,20 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithoutRefreshToken() *TestUps return u } +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAccessToken(token string) *TestUpstreamOIDCIdentityProviderBuilder { + u.accessToken = &oidctypes.AccessToken{Token: token} + return u +} + +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithEmptyAccessToken() *TestUpstreamOIDCIdentityProviderBuilder { + u.accessToken = &oidctypes.AccessToken{Token: ""} + return u +} +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithoutAccessToken() *TestUpstreamOIDCIdentityProviderBuilder { + u.accessToken = nil + return u +} + func (u *TestUpstreamOIDCIdentityProviderBuilder) WithUpstreamAuthcodeExchangeError(err error) *TestUpstreamOIDCIdentityProviderBuilder { u.authcodeExchangeErr = err return u @@ -723,13 +758,13 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithPerformRefreshError(err er return u } -func (u *TestUpstreamOIDCIdentityProviderBuilder) WithValidatedTokens(tokens *oidctypes.Token) *TestUpstreamOIDCIdentityProviderBuilder { - u.validatedTokens = tokens +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithValidatedAndMergedWithUserInfoTokens(tokens *oidctypes.Token) *TestUpstreamOIDCIdentityProviderBuilder { + u.validatedAndMergedWithUserInfoTokens = tokens return u } -func (u *TestUpstreamOIDCIdentityProviderBuilder) WithValidateTokenError(err error) *TestUpstreamOIDCIdentityProviderBuilder { - u.validateTokenErr = err +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithValidateTokenAndMergeWithUserInfoError(err error) *TestUpstreamOIDCIdentityProviderBuilder { + u.validateTokenAndMergeWithUserInfoErr = err return u } @@ -748,18 +783,19 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) Build() *TestUpstreamOIDCIdent Scopes: u.scopes, AllowPasswordGrant: u.allowPasswordGrant, AuthorizationURL: u.authorizationURL, + UserInfoURL: u.hasUserInfoURL, AdditionalAuthcodeParams: u.additionalAuthcodeParams, ExchangeAuthcodeAndValidateTokensFunc: func(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) { if u.authcodeExchangeErr != nil { return nil, u.authcodeExchangeErr } - return &oidctypes.Token{IDToken: &oidctypes.IDToken{Claims: u.idToken}, RefreshToken: u.refreshToken}, nil + return &oidctypes.Token{IDToken: &oidctypes.IDToken{Claims: u.idToken}, RefreshToken: u.refreshToken, AccessToken: u.accessToken}, nil }, PasswordCredentialsGrantAndValidateTokensFunc: func(ctx context.Context, username, password string) (*oidctypes.Token, error) { if u.passwordGrantErr != nil { return nil, u.passwordGrantErr } - return &oidctypes.Token{IDToken: &oidctypes.IDToken{Claims: u.idToken}, RefreshToken: u.refreshToken}, nil + return &oidctypes.Token{IDToken: &oidctypes.IDToken{Claims: u.idToken}, RefreshToken: u.refreshToken, AccessToken: u.accessToken}, nil }, PerformRefreshFunc: func(ctx context.Context, refreshToken string) (*oauth2.Token, error) { if u.performRefreshErr != nil { @@ -770,11 +806,11 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) Build() *TestUpstreamOIDCIdent RevokeRefreshTokenFunc: func(ctx context.Context, refreshToken string) error { return u.revokeRefreshTokenErr }, - ValidateTokenFunc: func(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) { - if u.validateTokenErr != nil { - return nil, u.validateTokenErr + ValidateTokenAndMergeWithUserInfoFunc: func(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce) (*oidctypes.Token, error) { + if u.validateTokenAndMergeWithUserInfoErr != nil { + return nil, u.validateTokenAndMergeWithUserInfoErr } - return u.validatedTokens, nil + return u.validatedAndMergedWithUserInfoTokens, nil }, } } diff --git a/internal/upstreamoidc/upstreamoidc.go b/internal/upstreamoidc/upstreamoidc.go index b9d371ed..94ce7502 100644 --- a/internal/upstreamoidc/upstreamoidc.go +++ b/internal/upstreamoidc/upstreamoidc.go @@ -61,6 +61,19 @@ func (p *ProviderConfig) GetRevocationURL() *url.URL { return p.RevocationURL } +func (p *ProviderConfig) HasUserInfoURL() bool { + providerJSON := &struct { + UserInfoURL string `json:"userinfo_endpoint"` + }{} + if err := p.Provider.Claims(providerJSON); err != nil { + // This should never happen in practice because we should have already successfully + // parsed these claims when p.Provider was created. + return false + } + + return len(providerJSON.UserInfoURL) > 0 +} + func (p *ProviderConfig) GetAdditionalAuthcodeParams() map[string]string { return p.AdditionalAuthcodeParams } @@ -113,7 +126,7 @@ func (p *ProviderConfig) PasswordCredentialsGrantAndValidateTokens(ctx context.C // There is no nonce to validate for a resource owner password credentials grant because it skips using // the authorize endpoint and goes straight to the token endpoint. const skipNonceValidation nonce.Nonce = "" - return p.ValidateTokenAndMergeWithUserInfo(ctx, tok, skipNonceValidation, true) + return p.ValidateTokenAndMergeWithUserInfo(ctx, tok, skipNonceValidation, true, false) } func (p *ProviderConfig) ExchangeAuthcodeAndValidateTokens(ctx context.Context, authcode string, pkceCodeVerifier pkce.Code, expectedIDTokenNonce nonce.Nonce, redirectURI string) (*oidctypes.Token, error) { @@ -127,7 +140,7 @@ func (p *ProviderConfig) ExchangeAuthcodeAndValidateTokens(ctx context.Context, return nil, err } - return p.ValidateTokenAndMergeWithUserInfo(ctx, tok, expectedIDTokenNonce, true) + return p.ValidateTokenAndMergeWithUserInfo(ctx, tok, expectedIDTokenNonce, true, false) } func (p *ProviderConfig) PerformRefresh(ctx context.Context, refreshToken string) (*oauth2.Token, error) { @@ -239,7 +252,7 @@ func (p *ProviderConfig) tryRevokeRefreshToken( // ValidateTokenAndMergeWithUserInfo will validate the ID token. It will also merge the claims from the userinfo endpoint response, // if the provider offers the userinfo endpoint. -func (p *ProviderConfig) ValidateTokenAndMergeWithUserInfo(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, requireIDToken bool) (*oidctypes.Token, error) { +func (p *ProviderConfig) ValidateTokenAndMergeWithUserInfo(ctx context.Context, tok *oauth2.Token, expectedIDTokenNonce nonce.Nonce, requireIDToken bool, requireUserInfo bool) (*oidctypes.Token, error) { var validatedClaims = make(map[string]interface{}) var idTokenExpiry time.Time @@ -255,7 +268,7 @@ func (p *ProviderConfig) ValidateTokenAndMergeWithUserInfo(ctx context.Context, if len(idTokenSubject) > 0 || !requireIDToken { // only fetch userinfo if the ID token has a subject or if we are ignoring the id token completely. // otherwise, defer to existing ID token validation - if err := p.maybeFetchUserInfoAndMergeClaims(ctx, tok, validatedClaims, requireIDToken); err != nil { + if err := p.maybeFetchUserInfoAndMergeClaims(ctx, tok, validatedClaims, requireIDToken, requireUserInfo); err != nil { return nil, httperr.Wrap(http.StatusInternalServerError, "could not fetch user info claims", err) } } @@ -309,10 +322,10 @@ func (p *ProviderConfig) validateIDToken(ctx context.Context, tok *oauth2.Token, return idTokenExpiry, idTok, nil } -func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, tok *oauth2.Token, claims map[string]interface{}, requireIDToken bool) error { +func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, tok *oauth2.Token, claims map[string]interface{}, requireIDToken bool, requireUserInfo bool) error { idTokenSubject, _ := claims[oidc.IDTokenSubjectClaim].(string) - userInfo, err := p.maybeFetchUserInfo(ctx, tok) + userInfo, err := p.maybeFetchUserInfo(ctx, tok, requireUserInfo) if err != nil { return err } @@ -355,17 +368,13 @@ func (p *ProviderConfig) maybeFetchUserInfoAndMergeClaims(ctx context.Context, t return nil } -func (p *ProviderConfig) maybeFetchUserInfo(ctx context.Context, tok *oauth2.Token) (*coreosoidc.UserInfo, error) { - providerJSON := &struct { - UserInfoURL string `json:"userinfo_endpoint"` - }{} - if err := p.Provider.Claims(providerJSON); err != nil { - // this should never happen because we should have already parsed these claims at an earlier stage - return nil, httperr.Wrap(http.StatusInternalServerError, "could not unmarshal discovery JSON", err) - } - - // implementing the user info endpoint is not required, skip this logic when it is absent - if len(providerJSON.UserInfoURL) == 0 { +func (p *ProviderConfig) maybeFetchUserInfo(ctx context.Context, tok *oauth2.Token, requireUserInfo bool) (*coreosoidc.UserInfo, error) { + // implementing the user info endpoint is not required by the OIDC spec, but we may require it in certain situations. + if !p.HasUserInfoURL() { + if requireUserInfo { + // TODO should these all be http errors? + return nil, httperr.New(http.StatusInternalServerError, "userinfo endpoint not found, but is required") + } return nil, nil } diff --git a/internal/upstreamoidc/upstreamoidc_test.go b/internal/upstreamoidc/upstreamoidc_test.go index 90dc6f1f..811b2300 100644 --- a/internal/upstreamoidc/upstreamoidc_test.go +++ b/internal/upstreamoidc/upstreamoidc_test.go @@ -40,6 +40,9 @@ func TestProviderConfig(t *testing.T) { Endpoint: oauth2.Endpoint{AuthURL: "https://example.com"}, Scopes: []string{"scope1", "scope2"}, }, + Provider: &mockProvider{ + rawClaims: []byte(`{"userinfo_endpoint": "https://example.com/userinfo"}`), + }, } require.Equal(t, "test-name", p.GetName()) require.Equal(t, "test-client-id", p.GetClientID()) @@ -54,6 +57,16 @@ func TestProviderConfig(t *testing.T) { require.True(t, p.AllowsPasswordGrant()) p.AllowPasswordGrant = false require.False(t, p.AllowsPasswordGrant()) + + require.True(t, p.HasUserInfoURL()) + p.Provider = &mockProvider{ + rawClaims: []byte(`{"some_other_endpoint": "https://example.com/blah"}`), + } + require.False(t, p.HasUserInfoURL()) + p.Provider = &mockProvider{ + rawClaims: []byte(`{`), + } + require.False(t, p.HasUserInfoURL()) }) const ( @@ -609,6 +622,7 @@ func TestProviderConfig(t *testing.T) { tok *oauth2.Token nonce nonce.Nonce requireIDToken bool + requireUserInfo bool userInfo *oidc.UserInfo rawClaims []byte userInfoErr error @@ -694,6 +708,34 @@ func TestProviderConfig(t *testing.T) { }, }, }, + { + name: "userinfo is required, token with id, access and refresh tokens, valid nonce, and userinfo with a value that doesn't exist in the id token", + tok: testTokenWithoutIDToken.WithExtra(map[string]interface{}{"id_token": goodIDToken}), + nonce: "some-nonce", + requireIDToken: true, + requireUserInfo: true, + rawClaims: []byte(`{"userinfo_endpoint": "not-empty"}`), + userInfo: forceUserInfoWithClaims("some-subject", `{"name": "Pinny TheSeal", "sub": "some-subject"}`), + wantMergedTokens: &oidctypes.Token{ + AccessToken: &oidctypes.AccessToken{ + Token: "test-access-token", + Type: "test-token-type", + Expiry: metav1.NewTime(expiryTime), + }, + RefreshToken: &oidctypes.RefreshToken{ + Token: "test-initial-refresh-token", + }, + IDToken: &oidctypes.IDToken{ + Token: goodIDToken, + Claims: map[string]interface{}{ + "iss": "some-issuer", + "nonce": "some-nonce", + "sub": "some-subject", + "name": "Pinny TheSeal", + }, + }, + }, + }, { name: "claims from userinfo override id token claims", tok: testTokenWithoutIDToken.WithExtra(map[string]interface{}{"id_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJzb21lLXN1YmplY3QiLCJuYW1lIjoiSm9obiBEb2UiLCJpc3MiOiJzb21lLWlzc3VlciIsIm5vbmNlIjoic29tZS1ub25jZSJ9.sBWi3_4cfGwrmMFZWkCghw4uvCnHN35h9xNX1gkwOtj6Oz_yKqpj7wfO4AqeWsRyrDGnkmIZbVuhAAJqPSi4GlNzN4NU8zh53PGDUpFlpDI1dvqDjIRb9iIEJpRIj34--Sz41H0ooxviIzvUdZFvQlaSzLOqgjR3ddHe2urhbtUuz_DsabP84AWo2DSg0y3ull6DRvk_DvzC6HNN8JwVi08fFvvV9BVq8kjdVeob7gajJkuGSTjsxNZGs5rbBuxBx0MZTQ8boR1fDNdG70GoIb4SsCoBSs7pZxtmGZPHInteY1SilHDDDmpQuE-LvSmvvPN_Cyk1d3eS-IR7hBbCAA"}), @@ -748,6 +790,32 @@ func TestProviderConfig(t *testing.T) { }, }, }, + { + name: "token with id, access and refresh tokens and valid nonce, but no userinfo endpoint from discovery and it's not required", + tok: testTokenWithoutIDToken.WithExtra(map[string]interface{}{"id_token": goodIDToken}), + nonce: "some-nonce", + requireIDToken: true, + requireUserInfo: false, + rawClaims: []byte(`{"not_the_userinfo_endpoint": "some-other-endpoint"}`), + wantMergedTokens: &oidctypes.Token{ + AccessToken: &oidctypes.AccessToken{ + Token: "test-access-token", + Type: "test-token-type", + Expiry: metav1.NewTime(expiryTime), + }, + RefreshToken: &oidctypes.RefreshToken{ + Token: "test-initial-refresh-token", + }, + IDToken: &oidctypes.IDToken{ + Token: goodIDToken, + Claims: map[string]interface{}{ + "iss": "some-issuer", + "nonce": "some-nonce", + "sub": "some-subject", + }, + }, + }, + }, { name: "token with no id token but valid userinfo", tok: testTokenWithoutIDToken, @@ -838,6 +906,23 @@ func TestProviderConfig(t *testing.T) { userInfo: forceUserInfoWithClaims("some-subject", `{"name": "Pinny TheSeal", "sub": "some-subject"}`), wantErr: "received response missing ID token", }, + { + name: "expected to have userinfo, but doesn't", + tok: testTokenWithoutIDToken, + nonce: "some-other-nonce", + requireUserInfo: true, + rawClaims: []byte(`{}`), + wantErr: "could not fetch user info claims: userinfo endpoint not found, but is required", + }, + { + name: "expected to have id token and userinfo, but doesn't have either", + tok: testTokenWithoutIDToken, + nonce: "some-other-nonce", + requireUserInfo: true, + requireIDToken: true, + rawClaims: []byte(`{}`), + wantErr: "received response missing ID token", + }, { name: "mismatched access token hash", tok: testTokenWithoutIDToken, @@ -898,7 +983,7 @@ func TestProviderConfig(t *testing.T) { userInfoErr: tt.userInfoErr, }, } - gotTok, err := p.ValidateTokenAndMergeWithUserInfo(context.Background(), tt.tok, tt.nonce, tt.requireIDToken) + gotTok, err := p.ValidateTokenAndMergeWithUserInfo(context.Background(), tt.tok, tt.nonce, tt.requireIDToken, tt.requireUserInfo) if tt.wantErr != "" { require.Error(t, err) require.Equal(t, tt.wantErr, err.Error()) @@ -982,6 +1067,36 @@ func TestProviderConfig(t *testing.T) { rawClaims: []byte(`{}`), // user info not supported wantUserInfoCalled: false, }, + { + name: "valid but userinfo endpoint could not be found due to parse error", + authCode: "valid", + returnIDTok: validIDToken, + wantToken: oidctypes.Token{ + AccessToken: &oidctypes.AccessToken{ + Token: "test-access-token", + Expiry: metav1.Time{}, + }, + RefreshToken: &oidctypes.RefreshToken{ + Token: "test-refresh-token", + }, + IDToken: &oidctypes.IDToken{ + Token: validIDToken, + Expiry: metav1.Time{}, + Claims: map[string]interface{}{ + "foo": "bar", + "bat": "baz", + "aud": "test-client-id", + "iat": 1.606768593e+09, + "jti": "test-jti", + "nbf": 1.606768593e+09, + "sub": "test-user", + }, + }, + }, + // cannot be parsed as json, but note that in this case constructing a real provider would have failed + rawClaims: []byte(`{`), + wantUserInfoCalled: false, + }, { name: "valid", authCode: "valid", @@ -1011,13 +1126,6 @@ func TestProviderConfig(t *testing.T) { rawClaims: []byte(`{}`), // user info not supported wantUserInfoCalled: false, }, - { - name: "user info discovery parse error", - authCode: "valid", - returnIDTok: validIDToken, - rawClaims: []byte(`junk`), // user info discovery fails - wantErr: "could not fetch user info claims: could not unmarshal discovery JSON: invalid character 'j' looking for beginning of value", - }, { name: "user info fetch error", authCode: "valid", diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index 325ec4a4..d9364689 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package oidcclient implements a CLI OIDC login flow. @@ -822,7 +822,7 @@ func (h *handlerState) handleRefresh(ctx context.Context, refreshToken *oidctype // The spec is not 100% clear about whether an ID token from the refresh flow should include a nonce, and at least // some providers do not include one, so we skip the nonce validation here (but not other validations). - return upstreamOIDCIdentityProvider.ValidateTokenAndMergeWithUserInfo(ctx, refreshed, "", true) + return upstreamOIDCIdentityProvider.ValidateTokenAndMergeWithUserInfo(ctx, refreshed, "", true, false) } func (h *handlerState) handleAuthCodeCallback(w http.ResponseWriter, r *http.Request) (err error) { diff --git a/pkg/oidcclient/login_test.go b/pkg/oidcclient/login_test.go index e55f8e03..8ee920d7 100644 --- a/pkg/oidcclient/login_test.go +++ b/pkg/oidcclient/login_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package oidcclient @@ -406,7 +406,7 @@ func TestLogin(t *testing.T) { // nolint:gocyclo h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) provider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). - ValidateToken(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true). + ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). Return(&testToken, nil) mock.EXPECT(). PerformRefresh(gomock.Any(), testToken.RefreshToken.Token). @@ -453,7 +453,7 @@ func TestLogin(t *testing.T) { // nolint:gocyclo h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) provider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). - ValidateToken(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true). + ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). Return(nil, fmt.Errorf("some validation error")) mock.EXPECT(). PerformRefresh(gomock.Any(), "test-refresh-token-returning-invalid-id-token"). @@ -1648,7 +1648,7 @@ func TestLogin(t *testing.T) { // nolint:gocyclo h.getProvider = func(config *oauth2.Config, provider *oidc.Provider, client *http.Client) provider.UpstreamOIDCIdentityProviderI { mock := mockUpstream(t) mock.EXPECT(). - ValidateToken(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true). + ValidateTokenAndMergeWithUserInfo(gomock.Any(), HasAccessToken(testToken.AccessToken.Token), nonce.Nonce(""), true, false). Return(&testToken, nil) mock.EXPECT(). PerformRefresh(gomock.Any(), testToken.RefreshToken.Token). diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 29a46e79..5fdd6060 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -129,6 +129,40 @@ func TestSupervisorLogin(t *testing.T) { wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Username) + "$" }, wantDownstreamIDTokenGroups: env.SupervisorUpstreamOIDC.ExpectedGroups, }, + { + name: "oidc without refresh token", + maybeSkip: func(t *testing.T) { + // never need to skip this test + }, + createIDP: func(t *testing.T) string { + t.Helper() + oidcIDP := testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ + Issuer: env.SupervisorUpstreamOIDC.Issuer, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamOIDC.CABundle)), + }, + Client: idpv1alpha1.OIDCClient{ + SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, + }, + Claims: idpv1alpha1.OIDCClaims{ + Username: env.SupervisorUpstreamOIDC.UsernameClaim, + Groups: env.SupervisorUpstreamOIDC.GroupsClaim, + }, + AuthorizationConfig: idpv1alpha1.OIDCAuthorizationConfig{ + AdditionalScopes: []string{"email"}, // does not ask for offline_access. + }, + }, idpv1alpha1.PhaseReady) + return oidcIDP.Name + }, + requestAuthorization: requestAuthorizationUsingBrowserAuthcodeFlow, + breakRefreshSessionData: func(t *testing.T, pinnipedSession *psession.PinnipedSession, _, _ string) { + fositeSessionData := pinnipedSession.Fosite + fositeSessionData.Claims.Extra["username"] = "some-incorrect-username" + }, + wantDownstreamIDTokenSubjectToMatch: "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Issuer+"?sub=") + ".+", + wantDownstreamIDTokenUsernameToMatch: func(_ string) string { return "^" + regexp.QuoteMeta(env.SupervisorUpstreamOIDC.Username) + "$" }, + wantDownstreamIDTokenGroups: env.SupervisorUpstreamOIDC.ExpectedGroups, + }, { name: "oidc with CLI password flow", maybeSkip: func(t *testing.T) {