From 65682aa60d9ba5f06b5f0ac71122173bf8e08fc3 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Wed, 22 May 2024 23:04:15 -0500 Subject: [PATCH] Add sample unit test for GitHub in token_handler_test.go --- .../endpoints/token/token_handler_test.go | 72 ++++++++++++++++++- .../testutil/testidplister/testidplister.go | 11 ++- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/internal/federationdomain/endpoints/token/token_handler_test.go b/internal/federationdomain/endpoints/token/token_handler_test.go index 799990fdd..2992ccc57 100644 --- a/internal/federationdomain/endpoints/token/token_handler_test.go +++ b/internal/federationdomain/endpoints/token/token_handler_test.go @@ -51,6 +51,7 @@ import ( "go.pinniped.dev/internal/federationdomain/oidc" "go.pinniped.dev/internal/federationdomain/oidcclientvalidator" "go.pinniped.dev/internal/federationdomain/storage" + "go.pinniped.dev/internal/federationdomain/upstreamprovider" "go.pinniped.dev/internal/fositestorage/accesstoken" "go.pinniped.dev/internal/fositestorage/authorizationcode" "go.pinniped.dev/internal/fositestorage/openidconnect" @@ -1828,6 +1829,11 @@ func TestRefreshGrant(t *testing.T) { activeDirectoryUpstreamType = "activedirectory" activeDirectoryUpstreamDN = "some-ad-user-dn" + githubUpstreamName = "some-github-idp" + githubUpstreamResourceUID = "github-resource-uid" + githubUpstreamType = "github" + githubUpstreamAccessToken = "some-opaque-access-token-from-github" + transformationUsernamePrefix = "username_prefix:" transformationGroupsPrefix = "groups_prefix:" ) @@ -1843,6 +1849,18 @@ func TestRefreshGrant(t *testing.T) { WithResourceUID(oidcUpstreamResourceUID) } + upstreamGitHubIdentityProviderBuilder := func() *oidctestutil.TestUpstreamGitHubIdentityProviderBuilder { + goodGitHubUser := &upstreamprovider.GitHubUser{ + Username: goodUsername, + Groups: goodGroups, + DownstreamSubject: goodSubject, + } + return oidctestutil.NewTestUpstreamGitHubIdentityProviderBuilder(). + WithName(githubUpstreamName). + WithResourceUID(githubUpstreamResourceUID). + WithUser(goodGitHubUser) + } + initialUpstreamOIDCRefreshTokenCustomSessionData := func() *psession.CustomSessionData { return &psession.CustomSessionData{ Username: goodUsername, @@ -1859,6 +1877,20 @@ func TestRefreshGrant(t *testing.T) { } } + initialUpstreamGitHubCustomSessionData := func() *psession.CustomSessionData { + return &psession.CustomSessionData{ + Username: goodUsername, + UpstreamUsername: goodUsername, + UpstreamGroups: goodGroups, + ProviderName: githubUpstreamName, + ProviderUID: githubUpstreamResourceUID, + ProviderType: githubUpstreamType, + GitHub: &psession.GitHubSessionData{ + UpstreamAccessToken: githubUpstreamAccessToken, + }, + } + } + initialUpstreamOIDCRefreshTokenCustomSessionDataWithUsername := func(downstreamUsername string) *psession.CustomSessionData { customSessionData := initialUpstreamOIDCRefreshTokenCustomSessionData() customSessionData.Username = downstreamUsername @@ -1903,6 +1935,12 @@ func TestRefreshGrant(t *testing.T) { } } + happyGitHubUpstreamRefreshCall := func() *expectedUpstreamRefresh { + return &expectedUpstreamRefresh{ + performedByUpstreamName: githubUpstreamName, + } + } + happyLDAPUpstreamRefreshCall := func() *expectedUpstreamRefresh { return &expectedUpstreamRefresh{ performedByUpstreamName: ldapUpstreamName, @@ -1995,6 +2033,15 @@ func TestRefreshGrant(t *testing.T) { return want } + happyRefreshTokenResponseForGitHubAndOfflineAccessWithUsernameAndGroups := func(wantCustomSessionDataStored *psession.CustomSessionData, wantDownstreamUsername string, wantDownstreamGroups []string) tokenEndpointResponseExpectedValues { + // Should always have some custom session data stored. The other expectations happens to be the + // same as the same values as the authcode exchange case. + want := happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccessWithUsernameAndGroups(wantCustomSessionDataStored, wantDownstreamUsername, wantDownstreamGroups) + // Should always try to perform an upstream refresh. + want.wantUpstreamRefreshCall = happyGitHubUpstreamRefreshCall() + return want + } + happyRefreshTokenResponseForOpenIDAndOfflineAccessWithAdditionalClaims := func(wantCustomSessionDataStored *psession.CustomSessionData, expectToValidateToken *oauth2.Token, wantAdditionalClaims map[string]interface{}) tokenEndpointResponseExpectedValues { want := happyRefreshTokenResponseForOpenIDAndOfflineAccess(wantCustomSessionDataStored, expectToValidateToken) want.wantAdditionalClaims = wantAdditionalClaims @@ -2151,6 +2198,27 @@ func TestRefreshGrant(t *testing.T) { ), }, }, + { + name: "happy path refresh grant with GitHub upstream", + idps: testidplister.NewUpstreamIDPListerBuilder().WithGitHub( + upstreamGitHubIdentityProviderBuilder().Build()), + authcodeExchange: authcodeExchangeInputs{ + modifyAuthRequest: func(r *http.Request) { r.Form.Set("scope", "openid offline_access username groups") }, + customSessionData: initialUpstreamGitHubCustomSessionData(), + want: happyAuthcodeExchangeTokenResponseForOpenIDAndOfflineAccessWithUsernameAndGroups( + initialUpstreamGitHubCustomSessionData(), + goodUsername, + goodGroups, + ), + }, + refreshRequest: refreshRequestInputs{ + want: happyRefreshTokenResponseForGitHubAndOfflineAccessWithUsernameAndGroups( + initialUpstreamGitHubCustomSessionData(), + goodUsername, + goodGroups, + ), + }, + }, { name: "happy path refresh grant with OIDC upstream with identity transformations which modify the username and group names when the upstream refresh does not return new username or groups then it reruns the transformations on the old upstream username and groups", idps: testidplister.NewUpstreamIDPListerBuilder().WithOIDC( @@ -4568,7 +4636,9 @@ func TestRefreshGrant(t *testing.T) { // Test that we did or did not make a call to the upstream OIDC provider interface to perform a token refresh. if test.refreshRequest.want.wantUpstreamRefreshCall != nil { - test.refreshRequest.want.wantUpstreamRefreshCall.args.Ctx = reqContext + if test.authcodeExchange.customSessionData.ProviderType != "github" { + test.refreshRequest.want.wantUpstreamRefreshCall.args.Ctx = reqContext + } test.idps.RequireExactlyOneCallToPerformRefresh(t, test.refreshRequest.want.wantUpstreamRefreshCall.performedByUpstreamName, test.refreshRequest.want.wantUpstreamRefreshCall.args, diff --git a/internal/testutil/testidplister/testidplister.go b/internal/testutil/testidplister/testidplister.go index d7fed23eb..2dded4809 100644 --- a/internal/testutil/testidplister/testidplister.go +++ b/internal/testutil/testidplister/testidplister.go @@ -363,7 +363,16 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyOneCallToPerformRefresh( actualArgs = upstreamAD.PerformRefreshArgs(0) } } - // TODO: probably add GitHub loop once we flesh out the structs + for _, upstream := range b.upstreamGitHubIdentityProviders { + // Remember that GitHub does not have a traditional PerformRefresh function. + // GitHub calls GetUser during both the original authcode exchange and the refresh. + callCountOnThisUpstream := upstream.GetUserCallCount() + actualCallCountAcrossAllUpstreams += callCountOnThisUpstream + if callCountOnThisUpstream == 1 { + actualNameOfUpstreamWhichMadeCall = upstream.Name + actualArgs = nil + } + } require.Equal(t, 1, actualCallCountAcrossAllUpstreams, "should have been exactly one call to PerformRefresh() by all upstreams", )