From b0ea7063c710359e5c1fc1c7818b43a058a7064b Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Tue, 18 Jan 2022 15:34:19 -0800 Subject: [PATCH 1/4] Supervisor should emit a warning when access token lifetime is too short --- .../accesstoken/accesstoken_test.go | 4 +- .../authorizationcode/authorizationcode.go | 35 +++++++++------- .../authorizationcode_test.go | 4 +- .../openidconnect/openidconnect_test.go | 2 +- internal/fositestorage/pkce/pkce_test.go | 2 +- .../refreshtoken/refreshtoken_test.go | 4 +- internal/oidc/auth/auth_handler_test.go | 42 +++++++++++++++++-- .../oidc/callback/callback_handler_test.go | 39 ++++++++++++++++- .../downstreamsession/downstream_session.go | 8 ++++ internal/oidc/token/token_handler.go | 9 ++++ internal/psession/pinniped_session.go | 4 ++ .../testutil/oidctestutil/oidctestutil.go | 5 ++- test/deploy/tools/dex.yaml | 4 +- 13 files changed, 130 insertions(+), 32 deletions(-) diff --git a/internal/fositestorage/accesstoken/accesstoken_test.go b/internal/fositestorage/accesstoken/accesstoken_test.go index ab6f7ee6..ddf30727 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","upstreamAccessToken":"","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","warnings":null,"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","upstreamAccessToken":"","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","warnings":null,"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 f2504d90..ecfad7be 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode.go +++ b/internal/fositestorage/authorizationcode/authorizationcode.go @@ -369,36 +369,41 @@ const ExpectedAuthorizeCodeSessionJSONFromFuzzing = `{ "providerUID": "Ĝ眧Ĭ", "providerName": "ʼn2ƋŢ觛ǂ焺nŐǛ", "providerType": "ɥ闣ʬ橳(ý綃ʃʚƟ覣k眐4", + "warnings": [ + "掘ʃƸ澺淗a紽ǒ|鰽ŋ猊", + "毇妬\u003e6鉢緋uƴŤȱʀļÂ?" + ], "oidc": { - "upstreamRefreshToken": "tC嵽痊w", - "upstreamAccessToken": "a紽ǒ|鰽ŋ猊I", - "upstreamSubject": "妬\u003e6鉢緋uƴŤȱʀ", - "upstreamIssuer": ":設虝27就伒犘c" + "upstreamRefreshToken": "\u003cƬb", + "upstreamAccessToken": "犘c钡ɏȫ", + "upstreamSubject": "鬌", + "upstreamIssuer": "%OpKȱ藚ɏ¬Ê蒭堜" }, "ldap": { - "userDN": "ɏȫ齁š%Op", + "userDN": "ȗ韚ʫ繕ȫ碰+", "extraRefreshAttributes": { - "T妼É4İ\u003e×1": "ʥ笿0D", - "÷驣7Ʀ澉1æɽ誮": "ʫ繕ȫ", - "ŚB碠k9": "i磊ůď逳鞪?3)藵睋邔\u0026Ű" + "+î艔垎0": "ĝ", + "4İ": "墀jMʥ", + "k9帴": "磊ůď逳鞪?3)藵睋邔\u0026Ű惫蜀Ģ" } }, "activedirectory": { - "userDN": "s", + "userDN": "%Ä摱ìÓȐĨf跞@)¿,ɭS隑i", "extraRefreshAttributes": { - "ƉǢIȽ齤士bEǎ儯惝IozŁ5rƖ螼": "偶宾儮猷V麹Œ颛Ė應,Ɣ鬅X¤" + " 皦pSǬŝ社Vƅȭǝ*擦28Dž": "vư", + "艱iYn面@yȝƋ鬯犦獢9c5¤.岵": "浛a齙\\蹼偦歛" } } } }, "requestedAudience": [ - "tO灞浛a齙\\蹼偦歛ơ", - "皦pSǬŝ社Vƅȭǝ*" + "置b", + "筫MN\u0026錝D肁Ŷɽ蔒PR}Ųʓl{" ], "grantedAudience": [ - "ĝ\"zvưã置bņ抰蛖a³2ʫ", - "Ŷɽ蔒PR}Ųʓl{鼐jÃ轘屔挝", - "Œų崓ļ憽-蹐È_¸]fś" + "jÃ轘屔挝", + "Œų崓ļ憽-蹐È_¸]fś", + "ɵʮGɃɫ囤1+,Ȳ" ] }, "version": "2" diff --git a/internal/fositestorage/authorizationcode/authorizationcode_test.go b/internal/fositestorage/authorizationcode/authorizationcode_test.go index 182aa2cf..dd007317 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","upstreamAccessToken":"","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","warnings":null,"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","upstreamAccessToken":"","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","warnings":null,"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", diff --git a/internal/fositestorage/openidconnect/openidconnect_test.go b/internal/fositestorage/openidconnect/openidconnect_test.go index 03ae3626..10979e9c 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","upstreamAccessToken":"","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","warnings":null,"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 58921725..06e3db6b 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","upstreamAccessToken":"","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","warnings":null,"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 36af0559..6dd5fa69 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","upstreamAccessToken":"","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","warnings":null,"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","upstreamAccessToken":"","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","warnings":null,"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/oidc/auth/auth_handler_test.go b/internal/oidc/auth/auth_handler_test.go index 0198c83a..51a810de 100644 --- a/internal/oidc/auth/auth_handler_test.go +++ b/internal/oidc/auth/auth_handler_test.go @@ -14,11 +14,13 @@ import ( "regexp" "strings" "testing" + "time" "github.com/gorilla/securecookie" "github.com/ory/fosite" "github.com/stretchr/testify/require" "golang.org/x/oauth2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/client-go/kubernetes/fake" v1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -893,7 +895,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { 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()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken, metav1.NewTime(time.Now().Add(9*time.Hour))).WithUserInfoURL().Build()), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), @@ -913,9 +915,41 @@ func TestAuthorizationEndpoint(t *testing.T) { wantDownstreamPKCEChallengeMethod: downstreamPKCEChallengeMethod, wantDownstreamCustomSessionData: expectedHappyOIDCPasswordGrantCustomSessionWithAccessToken, }, + { + name: "OIDC password grant happy path when upstream IDP returned empty refresh token and an access token that has a short lifetime", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken, metav1.NewTime(time.Now().Add(1*time.Hour))).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: &psession.CustomSessionData{ + ProviderUID: oidcPasswordGrantUpstreamResourceUID, + ProviderName: oidcPasswordGrantUpstreamName, + ProviderType: psession.ProviderTypeOIDC, + Warnings: []string{"Access token from identity provider has lifetime of less than 3 hours. Expect frequent prompts to log in."}, + OIDC: &psession.OIDCSessionData{ + UpstreamAccessToken: oidcUpstreamAccessToken, + UpstreamSubject: oidcUpstreamSubject, + UpstreamIssuer: oidcUpstreamIssuer, + }, + }, + }, { 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()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken, metav1.NewTime(time.Now().Add(9*time.Hour))).WithUserInfoURL().Build()), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), @@ -1078,7 +1112,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { 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()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken, metav1.NewTime(time.Now().Add(9*time.Hour))).WithoutUserInfoURL().Build()), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), @@ -1091,7 +1125,7 @@ func TestAuthorizationEndpoint(t *testing.T) { }, { 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()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(passwordGrantUpstreamOIDCIdentityProviderBuilder().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken, metav1.NewTime(time.Now().Add(9*time.Hour))).WithoutUserInfoURL().Build()), method: http.MethodGet, path: happyGetRequestPath, customUsernameHeader: pointer.StringPtr(oidcUpstreamUsername), diff --git a/internal/oidc/callback/callback_handler_test.go b/internal/oidc/callback/callback_handler_test.go index a43b2926..a9021749 100644 --- a/internal/oidc/callback/callback_handler_test.go +++ b/internal/oidc/callback/callback_handler_test.go @@ -11,9 +11,11 @@ import ( "net/url" "strings" "testing" + "time" "github.com/gorilla/securecookie" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" "go.pinniped.dev/internal/oidc" @@ -213,7 +215,7 @@ func TestCallbackEndpoint(t *testing.T) { }, { 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()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken, metav1.NewTime(time.Now().Add(9*time.Hour))).WithUserInfoURL().Build()), method: http.MethodGet, path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, @@ -234,6 +236,39 @@ func TestCallbackEndpoint(t *testing.T) { args: happyExchangeAndValidateTokensArgs, }, }, + { + name: "GET with authcode exchange that returns an access token but no refresh token but has a short token lifetime which is stored as a warning in the session", + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithEmptyRefreshToken().WithAccessToken(oidcUpstreamAccessToken, metav1.NewTime(time.Now().Add(1*time.Hour))).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: &psession.CustomSessionData{ + ProviderUID: happyUpstreamIDPResourceUID, + ProviderName: happyUpstreamIDPName, + ProviderType: psession.ProviderTypeOIDC, + Warnings: []string{"Access token from identity provider has lifetime of less than 3 hours. Expect frequent prompts to log in."}, + OIDC: &psession.OIDCSessionData{ + UpstreamAccessToken: oidcUpstreamAccessToken, + UpstreamIssuer: oidcUpstreamIssuer, + UpstreamSubject: oidcUpstreamSubject, + }, + }, + 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( @@ -358,7 +393,7 @@ func TestCallbackEndpoint(t *testing.T) { }, { 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()), + idps: oidctestutil.NewUpstreamIDPListerBuilder().WithOIDC(happyUpstream().WithoutRefreshToken().WithAccessToken(oidcUpstreamAccessToken, metav1.NewTime(time.Now().Add(9*time.Hour))).WithoutUserInfoURL().Build()), method: http.MethodGet, path: newRequestPath().WithState(happyState).String(), csrfCookie: happyCSRFCookie, diff --git a/internal/oidc/downstreamsession/downstream_session.go b/internal/oidc/downstreamsession/downstream_session.go index ec85f07e..195aae00 100644 --- a/internal/oidc/downstreamsession/downstream_session.go +++ b/internal/oidc/downstreamsession/downstream_session.go @@ -14,6 +14,7 @@ import ( "github.com/ory/fosite" "github.com/ory/fosite/handler/openid" "github.com/ory/fosite/token/jwt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/oidc" @@ -101,6 +102,13 @@ func MakeDownstreamOIDCCustomSessionData(oidcUpstream provider.UpstreamOIDCIdent } plog.Info("refresh token not returned by upstream provider during login, using access token instead. "+pleaseCheck, logKV...) customSessionData.OIDC.UpstreamAccessToken = token.AccessToken.Token + // When we are in a flow where we will be performing access token based refresh, issue a warning to the client if the access + // token lifetime is very short, since that would mean that the user's session is very short. + // The warnings are stored here and will be processed by the token handler. + threeHoursFromNow := metav1.NewTime(time.Now().Add(3 * time.Hour)) + if !token.AccessToken.Expiry.IsZero() && token.AccessToken.Expiry.Before(&threeHoursFromNow) { + customSessionData.Warnings = append(customSessionData.Warnings, "Access token from identity provider has lifetime of less than 3 hours. Expect frequent prompts to log in.") + } 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") diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index 4820a6d6..81902df8 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -12,6 +12,7 @@ import ( "github.com/ory/fosite" "github.com/ory/x/errorsx" "golang.org/x/oauth2" + "k8s.io/apiserver/pkg/warning" "go.pinniped.dev/internal/httputil/httperr" "go.pinniped.dev/internal/oidc" @@ -63,6 +64,14 @@ func NewHandler( } } + // When we are in the authorization code flow, check if we have any warnings that previous handlers want us + // to send to the client to be printed on the CLI. + if accessRequest.GetGrantTypes().ExactOne("authorization_code") { + for _, warningText := range session.Custom.Warnings { + warning.AddWarning(r.Context(), "", warningText) + } + } + accessResponse, err := oauthHelper.NewAccessResponse(r.Context(), accessRequest) if err != nil { plog.Info("token response error", oidc.FositeErrorForLog(err)...) diff --git a/internal/psession/pinniped_session.go b/internal/psession/pinniped_session.go index 3ed39ee0..665d0a90 100644 --- a/internal/psession/pinniped_session.go +++ b/internal/psession/pinniped_session.go @@ -43,6 +43,10 @@ type CustomSessionData struct { // Used during a downstream refresh to decide which upstream to refresh. ProviderType ProviderType `json:"providerType"` + // Warnings that were encountered at some point during login that should be emitted to the client. + // These will be RFC 2616-formatted errors with error code 299. + Warnings []string `json:"warnings"` + // Only used when ProviderType == "oidc". OIDC *OIDCSessionData `json:"oidc,omitempty"` diff --git a/internal/testutil/oidctestutil/oidctestutil.go b/internal/testutil/oidctestutil/oidctestutil.go index 92cc582c..66e915db 100644 --- a/internal/testutil/oidctestutil/oidctestutil.go +++ b/internal/testutil/oidctestutil/oidctestutil.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/oauth2" "gopkg.in/square/go-jose.v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/fake" @@ -726,8 +727,8 @@ func (u *TestUpstreamOIDCIdentityProviderBuilder) WithoutRefreshToken() *TestUps return u } -func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAccessToken(token string) *TestUpstreamOIDCIdentityProviderBuilder { - u.accessToken = &oidctypes.AccessToken{Token: token} +func (u *TestUpstreamOIDCIdentityProviderBuilder) WithAccessToken(token string, expiry metav1.Time) *TestUpstreamOIDCIdentityProviderBuilder { + u.accessToken = &oidctypes.AccessToken{Token: token, Expiry: expiry} return u } diff --git a/test/deploy/tools/dex.yaml b/test/deploy/tools/dex.yaml index fe581be9..ceb7543d 100644 --- a/test/deploy/tools/dex.yaml +++ b/test/deploy/tools/dex.yaml @@ -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 #@ load("@ytt:data", "data") @@ -15,6 +15,8 @@ web: https: 0.0.0.0:8443 tlsCert: /var/certs/dex.pem tlsKey: /var/certs/dex-key.pem +expiry: + idTokens: 20m #! this is the lifetime for the id token as well as the access token. oauth2: skipApprovalScreen: true #! Allow the resource owner password grant, which Dex implements to also return ID tokens. From 38d184fe8140358c9786d6124bc72b8788a46d21 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Wed, 19 Jan 2022 13:20:49 -0800 Subject: [PATCH 2/4] Integration test + making sure we get the session correctly in token handler --- internal/oidc/token/token_handler.go | 8 ++++++-- test/integration/e2e_test.go | 21 +++++++++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/internal/oidc/token/token_handler.go b/internal/oidc/token/token_handler.go index 81902df8..29b61ea5 100644 --- a/internal/oidc/token/token_handler.go +++ b/internal/oidc/token/token_handler.go @@ -67,8 +67,12 @@ func NewHandler( // When we are in the authorization code flow, check if we have any warnings that previous handlers want us // to send to the client to be printed on the CLI. if accessRequest.GetGrantTypes().ExactOne("authorization_code") { - for _, warningText := range session.Custom.Warnings { - warning.AddWarning(r.Context(), "", warningText) + storedSession := accessRequest.GetSession().(*psession.PinnipedSession) + customSessionData := storedSession.Custom + if customSessionData != nil { + for _, warningText := range customSessionData.Warnings { + warning.AddWarning(r.Context(), "", warningText) + } } } diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index f152b54a..34e315dc 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_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 integration @@ -290,6 +290,20 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo Resource: "namespaces", }) + var additionalScopes []string + // If we're using dex, we will test that we see a warning when the access token + // lifetime is too short (we have it set to 20 minutes) and it's using access token based refresh. + // To ensure that access token refresh happens rather than refresh token, don't ask for the offline_access scope. + // In other environments, test the refresh token based flow. + if len(env.ToolsNamespace) == 0 { + additionalScopes = env.SupervisorUpstreamOIDC.AdditionalScopes + } else { + for _, additionalScope := range env.SupervisorUpstreamOIDC.AdditionalScopes { + if additionalScope != "offline_access" { + additionalScopes = append(additionalScopes, additionalScope) + } + } + } // Create upstream OIDC provider and wait for it to become ready. testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ Issuer: env.SupervisorUpstreamOIDC.Issuer, @@ -297,7 +311,7 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamOIDC.CABundle)), }, AuthorizationConfig: idpv1alpha1.OIDCAuthorizationConfig{ - AdditionalScopes: env.SupervisorUpstreamOIDC.AdditionalScopes, + AdditionalScopes: additionalScopes, }, Claims: idpv1alpha1.OIDCClaims{ Username: env.SupervisorUpstreamOIDC.UsernameClaim, @@ -369,6 +383,9 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo // Ignore any errors returned because there is always an error on linux. kubectlOutputBytes, _ := ioutil.ReadAll(ptyFile) requireKubectlGetNamespaceOutput(t, env, string(kubectlOutputBytes)) + if len(env.ToolsNamespace) > 0 { + require.Contains(t, string(kubectlOutputBytes), "Access token from identity provider has lifetime of less than 3 hours. Expect frequent prompts to log in.") + } t.Logf("first kubectl command took %s", time.Since(start).String()) From acd23c4c373deae80bfd4095d828cd3e9d00dc20 Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Thu, 20 Jan 2022 08:52:16 -0800 Subject: [PATCH 3/4] Separate test for access token refresh --- test/integration/e2e_test.go | 136 ++++++++++++++++++++++++++++++----- 1 file changed, 119 insertions(+), 17 deletions(-) diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index 34e315dc..76c9bf00 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -22,8 +22,6 @@ import ( "testing" "time" - "go.pinniped.dev/pkg/oidcclient/oidctypes" - coreosoidc "github.com/coreos/go-oidc/v3/oidc" "github.com/creack/pty" "github.com/stretchr/testify/require" @@ -42,6 +40,7 @@ import ( "go.pinniped.dev/internal/testutil" "go.pinniped.dev/pkg/oidcclient" "go.pinniped.dev/pkg/oidcclient/filesession" + "go.pinniped.dev/pkg/oidcclient/oidctypes" "go.pinniped.dev/test/testlib" "go.pinniped.dev/test/testlib/browsertest" ) @@ -290,20 +289,125 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo Resource: "namespaces", }) - var additionalScopes []string - // If we're using dex, we will test that we see a warning when the access token - // lifetime is too short (we have it set to 20 minutes) and it's using access token based refresh. - // To ensure that access token refresh happens rather than refresh token, don't ask for the offline_access scope. - // In other environments, test the refresh token based flow. - if len(env.ToolsNamespace) == 0 { - additionalScopes = env.SupervisorUpstreamOIDC.AdditionalScopes - } else { - for _, additionalScope := range env.SupervisorUpstreamOIDC.AdditionalScopes { - if additionalScope != "offline_access" { - additionalScopes = append(additionalScopes, additionalScope) - } + // Create upstream OIDC provider and wait for it to become ready. + testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ + Issuer: env.SupervisorUpstreamOIDC.Issuer, + TLS: &idpv1alpha1.TLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorUpstreamOIDC.CABundle)), + }, + AuthorizationConfig: idpv1alpha1.OIDCAuthorizationConfig{ + AdditionalScopes: env.SupervisorUpstreamOIDC.AdditionalScopes, + }, + Claims: idpv1alpha1.OIDCClaims{ + Username: env.SupervisorUpstreamOIDC.UsernameClaim, + Groups: env.SupervisorUpstreamOIDC.GroupsClaim, + }, + Client: idpv1alpha1.OIDCClient{ + SecretName: testlib.CreateClientCredsSecret(t, env.SupervisorUpstreamOIDC.ClientID, env.SupervisorUpstreamOIDC.ClientSecret).Name, + }, + }, idpv1alpha1.PhaseReady) + + // Use a specific session cache for this test. + sessionCachePath := tempDir + "/oidc-test-sessions-manual.yaml" + kubeconfigPath := runPinnipedGetKubeconfig(t, env, pinnipedExe, tempDir, []string{ + "get", "kubeconfig", + "--concierge-api-group-suffix", env.APIGroupSuffix, + "--concierge-authenticator-type", "jwt", + "--concierge-authenticator-name", authenticator.Name, + "--oidc-skip-browser", + "--oidc-skip-listen", + "--oidc-ca-bundle", testCABundlePath, + "--oidc-session-cache", sessionCachePath, + }) + + // Run "kubectl get namespaces" which should trigger a browser login via the plugin. + start := time.Now() + kubectlCmd := exec.CommandContext(ctx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath) + kubectlCmd.Env = append(os.Environ(), env.ProxyEnv()...) + + ptyFile, err := pty.Start(kubectlCmd) + require.NoError(t, err) + + // Wait for the subprocess to print the login prompt. + t.Logf("waiting for CLI to output login URL and manual prompt") + output := readFromFileUntilStringIsSeen(t, ptyFile, "Optionally, paste your authorization code: ") + require.Contains(t, output, "Log in by visiting this link:") + require.Contains(t, output, "Optionally, paste your authorization code: ") + + // Find the line with the login URL. + var loginURL string + for _, line := range strings.Split(output, "\n") { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "https://") { + loginURL = trimmed } } + require.NotEmptyf(t, loginURL, "didn't find login URL in output: %s", output) + + t.Logf("navigating to login page") + require.NoError(t, page.Navigate(loginURL)) + + // Expect to be redirected to the upstream provider and log in. + browsertest.LoginToUpstream(t, page, env.SupervisorUpstreamOIDC) + + // Expect to be redirected to the downstream callback which is serving the form_post HTML. + t.Logf("waiting for response page %s", downstream.Spec.Issuer) + browsertest.WaitForURL(t, page, regexp.MustCompile(regexp.QuoteMeta(downstream.Spec.Issuer))) + + // The response page should have failed to automatically post, and should now be showing the manual instructions. + authCode := formpostExpectManualState(t, page) + + // Enter the auth code in the waiting prompt, followed by a newline. + t.Logf("'manually' pasting authorization code %q to waiting prompt", authCode) + _, err = ptyFile.WriteString(authCode + "\n") + require.NoError(t, err) + + // Read all of the remaining output from the subprocess until EOF. + t.Logf("waiting for kubectl to output namespace list") + // Read all output from the subprocess until EOF. + // Ignore any errors returned because there is always an error on linux. + kubectlOutputBytes, _ := ioutil.ReadAll(ptyFile) + requireKubectlGetNamespaceOutput(t, env, string(kubectlOutputBytes)) + + t.Logf("first kubectl command took %s", time.Since(start).String()) + + requireUserCanUseKubectlWithoutAuthenticatingAgain(ctx, t, env, + downstream, + kubeconfigPath, + sessionCachePath, + pinnipedExe, + expectedUsername, + expectedGroups, + ) + }) + + t.Run("access token based refresh with Supervisor OIDC upstream IDP and manual authcode copy-paste from browser flow", func(t *testing.T) { + // Start a fresh browser driver because we don't want to share cookies between the various tests in this file. + page := browsertest.Open(t) + + expectedUsername := env.SupervisorUpstreamOIDC.Username + expectedGroups := env.SupervisorUpstreamOIDC.ExpectedGroups + + // Create a ClusterRoleBinding to give our test user from the upstream read-only access to the cluster. + testlib.CreateTestClusterRoleBinding(t, + rbacv1.Subject{Kind: rbacv1.UserKind, APIGroup: rbacv1.GroupName, Name: expectedUsername}, + rbacv1.RoleRef{Kind: "ClusterRole", APIGroup: rbacv1.GroupName, Name: "view"}, + ) + testlib.WaitForUserToHaveAccess(t, expectedUsername, []string{}, &authorizationv1.ResourceAttributes{ + Verb: "get", + Group: "", + Version: "v1", + Resource: "namespaces", + }) + + var additionalScopes []string + // To ensure that access token refresh happens rather than refresh token, don't ask for the offline_access scope. + for _, additionalScope := range env.SupervisorUpstreamOIDC.AdditionalScopes { + if additionalScope != "offline_access" { + additionalScopes = append(additionalScopes, additionalScope) + } + } + // Create upstream OIDC provider and wait for it to become ready. testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ Issuer: env.SupervisorUpstreamOIDC.Issuer, @@ -383,9 +487,7 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo // Ignore any errors returned because there is always an error on linux. kubectlOutputBytes, _ := ioutil.ReadAll(ptyFile) requireKubectlGetNamespaceOutput(t, env, string(kubectlOutputBytes)) - if len(env.ToolsNamespace) > 0 { - require.Contains(t, string(kubectlOutputBytes), "Access token from identity provider has lifetime of less than 3 hours. Expect frequent prompts to log in.") - } + require.Contains(t, string(kubectlOutputBytes), "Access token from identity provider has lifetime of less than 3 hours. Expect frequent prompts to log in.") t.Logf("first kubectl command took %s", time.Since(start).String()) From 842ef388686127ad8b43dfad821d4cc6ab14e1ef Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Thu, 20 Jan 2022 13:43:29 -0800 Subject: [PATCH 4/4] Ensure warning is on stderr and not stdout. --- test/integration/e2e_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index 76c9bf00..4f334541 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -443,7 +443,8 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo start := time.Now() kubectlCmd := exec.CommandContext(ctx, "kubectl", "get", "namespace", "--kubeconfig", kubeconfigPath) kubectlCmd.Env = append(os.Environ(), env.ProxyEnv()...) - + stdoutPipe, err := kubectlCmd.StdoutPipe() + require.NoError(t, err) ptyFile, err := pty.Start(kubectlCmd) require.NoError(t, err) @@ -485,9 +486,10 @@ func TestE2EFullIntegration(t *testing.T) { // nolint:gocyclo t.Logf("waiting for kubectl to output namespace list") // Read all output from the subprocess until EOF. // Ignore any errors returned because there is always an error on linux. - kubectlOutputBytes, _ := ioutil.ReadAll(ptyFile) - requireKubectlGetNamespaceOutput(t, env, string(kubectlOutputBytes)) - require.Contains(t, string(kubectlOutputBytes), "Access token from identity provider has lifetime of less than 3 hours. Expect frequent prompts to log in.") + kubectlStdOutOutputBytes, _ := ioutil.ReadAll(stdoutPipe) + kubectlStdErrOutputBytes, _ := ioutil.ReadAll(ptyFile) + requireKubectlGetNamespaceOutput(t, env, string(kubectlStdOutOutputBytes)) + require.Contains(t, string(kubectlStdErrOutputBytes), "Access token from identity provider has lifetime of less than 3 hours. Expect frequent prompts to log in.") t.Logf("first kubectl command took %s", time.Since(start).String())