diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index 5bfc50db2..34f51706e 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -328,6 +328,9 @@ func flowOptions( flowSource, requestedIDPType, requestedFlow, strings.Join([]string{idpdiscoveryv1alpha1.IDPFlowCLIPassword.String(), idpdiscoveryv1alpha1.IDPFlowBrowserAuthcode.String()}, ", ")) } + // TODO: implement this + case idpdiscoveryv1alpha1.IDPTypeGitHub: + panic("GitHub has not been implemented yet.") default: // Surprisingly cobra does not support this kind of flag validation. See https://github.com/spf13/pflag/issues/236 return nil, fmt.Errorf( diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go index e6897e323..bfcbba5bb 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher.go @@ -238,7 +238,7 @@ func validateOrganizationsPolicy(organizationsSpec *v1alpha1.GitHubOrganizations } func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx controllerlib.Context, upstream *v1alpha1.GitHubIdentityProvider) ( - *upstreamgithub.ProviderConfig, // If validated, returns the config + *upstreamgithub.Provider, // If validated, returns the config error, // This error will only refer to programmatic errors such as inability to perform a Dial or dereference a pointer, not configuration errors ) { conditions := make([]*metav1.Condition, 0) @@ -291,22 +291,24 @@ func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx contro return nil, k8sutilerrors.NewAggregate(applicationErrors) } - providerConfig := &upstreamgithub.ProviderConfig{ - Name: upstream.Name, - ResourceUID: upstream.UID, - Host: hostURL, - GroupNameAttribute: groupNameAttribute, - UsernameAttribute: usernameAttribute, - OAuth2Config: &oauth2.Config{ - ClientID: clientID, - ClientSecret: clientSecret, + provider := upstreamgithub.New( + upstreamgithub.ProviderConfig{ + Name: upstream.Name, + ResourceUID: upstream.UID, + Host: hostURL, + GroupNameAttribute: groupNameAttribute, + UsernameAttribute: usernameAttribute, + OAuth2Config: &oauth2.Config{ + ClientID: clientID, + ClientSecret: clientSecret, + }, + AllowedOrganizations: upstream.Spec.AllowAuthentication.Organizations.Allowed, + OrganizationLoginPolicy: policy, + AuthorizationURL: fmt.Sprintf("%s/login/oauth/authorize", hostURL), + HttpClient: httpClient, }, - AllowedOrganizations: upstream.Spec.AllowAuthentication.Organizations.Allowed, - OrganizationLoginPolicy: policy, - AuthorizationURL: fmt.Sprintf("%s/login/oauth/authorize", hostURL), - HttpClient: httpClient, - } - return providerConfig, k8sutilerrors.NewAggregate(applicationErrors) + ) + return provider, k8sutilerrors.NewAggregate(applicationErrors) } func validateHost(gitHubAPIConfig v1alpha1.GitHubAPIConfig) (*metav1.Condition, *endpointaddr.HostPort) { diff --git a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go index d8b72110a..92562b641 100644 --- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go @@ -371,7 +371,8 @@ func TestController(t *testing.T) { wantErr string wantLogs []string wantResultingCache []*upstreamgithub.ProviderConfig - wantResultingUpstreams []v1alpha1.GitHubIdentityProvider + // wantResultingCache []*oidctestutil.TestUpstreamGitHubIdentityProvider + wantResultingUpstreams []v1alpha1.GitHubIdentityProvider }{ { name: "no GitHubIdentityProviders", @@ -1717,7 +1718,9 @@ func TestController(t *testing.T) { cache := dynamicupstreamprovider.NewDynamicUpstreamIDPProvider() cache.SetGitHubIdentityProviders([]upstreamprovider.UpstreamGithubIdentityProviderI{ - &upstreamgithub.ProviderConfig{Name: "initial-entry-to-remove"}, + upstreamgithub.New( + upstreamgithub.ProviderConfig{Name: "initial-entry-to-remove"}, + ), }) var log bytes.Buffer @@ -1757,12 +1760,12 @@ func TestController(t *testing.T) { require.Equal(t, len(tt.wantResultingCache), len(actualIDPList)) for i := 0; i < len(tt.wantResultingCache); i++ { // Do not expect any particular order in the cache - var actualIDP *upstreamgithub.ProviderConfig + var actualIDP *upstreamgithub.Provider for _, possibleIDP := range actualIDPList { if possibleIDP.GetName() == tt.wantResultingCache[i].Name { // For this check, we know that the actual IDPs are going to have type upstreamgithub.ProviderConfig var ok bool - actualIDP, ok = possibleIDP.(*upstreamgithub.ProviderConfig) + actualIDP, ok = possibleIDP.(*upstreamgithub.Provider) require.True(t, ok) break } @@ -1785,7 +1788,8 @@ func TestController(t *testing.T) { require.NoError(t, err) compareTLSClientConfigWithinHttpClients(t, phttp.Default(certPool), actualIDP.GetHttpClient()) - require.Equal(t, tt.wantResultingCache[i].OAuth2Config, actualIDP.OAuth2Config) + require.Equal(t, tt.wantResultingCache[i].OAuth2Config, actualIDP.GetOAuth2Config()) + } // Verify the status conditions as reported in Kubernetes diff --git a/internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder.go b/internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder.go index 1c0e398aa..2e4aa887b 100644 --- a/internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder.go +++ b/internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder.go @@ -189,7 +189,10 @@ func (u *FederationDomainIdentityProvidersListerFinder) GetIdentityProviders() [ for _, p := range cachedGitHubProviders { if idp.UID == p.GetResourceUID() { providers = append(providers, &resolvedgithub.FederationDomainResolvedGitHubIdentityProvider{ - // TODO: fill this out. + DisplayName: idp.DisplayName, + Provider: p, + SessionProviderType: psession.ProviderTypeGitHub, + Transforms: idp.Transforms, }) } } diff --git a/internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder_test.go b/internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder_test.go index 7e4d7a5b9..53ae52045 100644 --- a/internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder_test.go +++ b/internal/federationdomain/federationdomainproviders/federation_domain_identity_providers_lister_finder_test.go @@ -10,6 +10,7 @@ import ( "go.pinniped.dev/internal/federationdomain/idplister" "go.pinniped.dev/internal/federationdomain/resolvedprovider" + "go.pinniped.dev/internal/federationdomain/resolvedprovider/resolvedgithub" "go.pinniped.dev/internal/federationdomain/resolvedprovider/resolvedldap" "go.pinniped.dev/internal/federationdomain/resolvedprovider/resolvedoidc" "go.pinniped.dev/internal/testutil/oidctestutil" @@ -52,6 +53,14 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { WithName("my-ad-idp2"). WithResourceUID("my-ad-uid-idp2"). Build() + myDefaultGitHubIDP := oidctestutil.NewTestUpstreamGitHubIdentityProviderBuilder(). + WithName("my-default-github-idp"). + WithResourceUID("my-default-github-uid-idp"). + Build() + myGitHubIDP1 := oidctestutil.NewTestUpstreamGitHubIdentityProviderBuilder(). + WithName("my-github-idp1"). + WithResourceUID("my-github-uid-idp1"). + Build() // FederationDomainIssuers fakeIssuerURL := "https://www.fakeissuerurl.com" @@ -77,13 +86,20 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { }) require.NoError(t, err) - fdIssuerWithOIDCAndLDAPAndADIDPs, err := NewFederationDomainIssuer(fakeIssuerURL, []*FederationDomainIdentityProvider{ + fdIssuerWithDefaultGitHubIDP, err := NewFederationDomainIssuerWithDefaultIDP(fakeIssuerURL, &FederationDomainIdentityProvider{ + DisplayName: "my-default-github-idp", + UID: "my-default-github-uid-idp", + }) + require.NoError(t, err) + + fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, err := NewFederationDomainIssuer(fakeIssuerURL, []*FederationDomainIdentityProvider{ {DisplayName: "my-oidc-idp1", UID: "my-oidc-uid-idp1"}, {DisplayName: "my-oidc-idp2", UID: "my-oidc-uid-idp2"}, {DisplayName: "my-ldap-idp1", UID: "my-ldap-uid-idp1"}, {DisplayName: "my-ldap-idp2", UID: "my-ldap-uid-idp2"}, {DisplayName: "my-ad-idp1", UID: "my-ad-uid-idp1"}, {DisplayName: "my-ad-idp2", UID: "my-ad-uid-idp2"}, + {DisplayName: "my-github-idp1", UID: "my-github-uid-idp1"}, }) require.NoError(t, err) @@ -99,6 +115,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { {DisplayName: "my-ldap-idp4", UID: "my-ldap-uid-idp4"}, {DisplayName: "my-ad-idp2", UID: "my-ad-uid-idp2"}, {DisplayName: "my-ad-idp3", UID: "my-ad-uid-idp3"}, + {DisplayName: "my-github-idp1", UID: "my-github-uid-idp1"}, }) require.NoError(t, err) @@ -133,6 +150,11 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { Provider: myADIDP1, SessionProviderType: "activedirectory", } + myGitHub1Resolved := &resolvedgithub.FederationDomainResolvedGitHubIdentityProvider{ + DisplayName: "my-github-idp1", + Provider: myGitHubIDP1, + SessionProviderType: "github", + } myDefaultOIDCIDPResolved := &resolvedoidc.FederationDomainResolvedOIDCIdentityProvider{ DisplayName: "my-default-oidc-idp", @@ -144,15 +166,21 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { Provider: myDefaultLDAPIDP, SessionProviderType: "ldap", } + myDefaultGitHubIDPResolved := &resolvedgithub.FederationDomainResolvedGitHubIdentityProvider{ + DisplayName: "my-default-github-idp", + Provider: myDefaultGitHubIDP, + SessionProviderType: "github", + } testFindUpstreamIDPByDisplayName := []struct { - name string - wrappedLister idplister.UpstreamIdentityProvidersLister - federationDomainIssuer *FederationDomainIssuer - findIDPByDisplayName string - wantOIDCIDPByDisplayName *resolvedoidc.FederationDomainResolvedOIDCIdentityProvider - wantLDAPIDPByDisplayName *resolvedldap.FederationDomainResolvedLDAPIdentityProvider - wantError string + name string + wrappedLister idplister.UpstreamIdentityProvidersLister + federationDomainIssuer *FederationDomainIssuer + findIDPByDisplayName string + wantOIDCIDPByDisplayName *resolvedoidc.FederationDomainResolvedOIDCIdentityProvider + wantLDAPIDPByDisplayName *resolvedldap.FederationDomainResolvedLDAPIdentityProvider + wantGitHubIDPByDisplayName *resolvedgithub.FederationDomainResolvedGitHubIdentityProvider + wantError string }{ { name: "FindUpstreamIDPByDisplayName will find an upstream IdP by display name with one IDP configured", @@ -182,8 +210,9 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { WithOIDC(myOIDCIDP2). WithLDAP(myLDAPIDP1). WithLDAP(myLDAPIDP2). + WithGitHub(myGitHubIDP1). BuildDynamicUpstreamIDPProvider(), - federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs, + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, wantOIDCIDPByDisplayName: myOIDCIDP1Resolved, }, { @@ -195,6 +224,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { WithLDAP(myLDAPIDP1). WithLDAP(myLDAPIDP2). WithActiveDirectory(myADIDP1). + WithGitHub(myGitHubIDP1). BuildDynamicUpstreamIDPProvider(), federationDomainIssuer: fdIssuerWithOIDCIDP1, wantOIDCIDPByDisplayName: myOIDCIDP1Resolved, @@ -208,11 +238,13 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { WithLDAP(myLDAPIDP1). WithLDAP(myLDAPIDP2). WithActiveDirectory(myADIDP1). + WithGitHub(myGitHubIDP1). BuildDynamicUpstreamIDPProvider(), - federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs, + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, + wantLDAPIDPByDisplayName: myLDAPIDP1Resolved, }, { - name: "FindUpstreamIDPByDisplayName will find an upstream IDP of type AD (LDAP) by display name", + name: "FindUpstreamIDPByDisplayName will find an upstream IDP of type AD (LDAP) by display name", findIDPByDisplayName: "my-ad-idp1", wrappedLister: testidplister.NewUpstreamIDPListerBuilder(). WithOIDC(myOIDCIDP1). @@ -220,10 +252,25 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { WithLDAP(myLDAPIDP1). WithLDAP(myLDAPIDP2). WithActiveDirectory(myADIDP1). + WithGitHub(myGitHubIDP1). BuildDynamicUpstreamIDPProvider(), - federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs, + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, wantLDAPIDPByDisplayName: myADIDP1Resolved, }, + { + name: "FindUpstreamIDPByDisplayName will find an upstream IDP of type GitHub by display name", + findIDPByDisplayName: "my-github-idp1", + wrappedLister: testidplister.NewUpstreamIDPListerBuilder(). + WithOIDC(myOIDCIDP1). + WithOIDC(myOIDCIDP2). + WithLDAP(myLDAPIDP1). + WithLDAP(myLDAPIDP2). + WithActiveDirectory(myADIDP1). + WithGitHub(myGitHubIDP1). + BuildDynamicUpstreamIDPProvider(), + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, + wantGitHubIDPByDisplayName: myGitHub1Resolved, + }, { name: "FindUpstreamIDPByDisplayName will error if IDP by display name is not found - no such display name", findIDPByDisplayName: "i-cant-find-my-idp", @@ -233,8 +280,9 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { WithLDAP(myLDAPIDP1). WithLDAP(myLDAPIDP2). WithActiveDirectory(myADIDP1). + WithGitHub(myGitHubIDP1). BuildDynamicUpstreamIDPProvider(), - federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs, + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, wantError: `identity provider not found: "i-cant-find-my-idp"`, }, { @@ -266,6 +314,9 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { if tt.wantLDAPIDPByDisplayName != nil { require.Equal(t, tt.wantLDAPIDPByDisplayName, foundIDP) } + if tt.wantGitHubIDPByDisplayName != nil { + require.Equal(t, tt.wantGitHubIDPByDisplayName, foundIDP) + } }) } @@ -275,6 +326,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { federationDomainIssuer *FederationDomainIssuer wantDefaultOIDCIDP *resolvedoidc.FederationDomainResolvedOIDCIdentityProvider wantDefaultLDAPIDP *resolvedldap.FederationDomainResolvedLDAPIdentityProvider + wantDefaultGitHubIDP *resolvedgithub.FederationDomainResolvedGitHubIdentityProvider wantError string }{ { @@ -293,6 +345,14 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { federationDomainIssuer: fdIssuerWithDefaultLDAPIDP, wantDefaultLDAPIDP: myDefaultLDAPIDPResolved, }, + { + name: "FindDefaultIDP resturns a GitHubIdentityProvider if there is a GitHubIdentityProvider defined as the default IDP", + wrappedLister: testidplister.NewUpstreamIDPListerBuilder(). + WithGitHub(myDefaultGitHubIDP). + BuildDynamicUpstreamIDPProvider(), + federationDomainIssuer: fdIssuerWithDefaultGitHubIDP, + wantDefaultGitHubIDP: myDefaultGitHubIDPResolved, + }, { name: "FindDefaultIDP returns an error if there is no default IDP to return", wrappedLister: testidplister.NewUpstreamIDPListerBuilder(). @@ -342,6 +402,9 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { if tt.wantDefaultLDAPIDP != nil { require.Equal(t, tt.wantDefaultLDAPIDP, foundIDP) } + if tt.wantDefaultGitHubIDP != nil { + require.Equal(t, tt.wantDefaultGitHubIDP, foundIDP) + } }) } @@ -359,14 +422,16 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { WithLDAP(myLDAPIDP1). WithLDAP(myLDAPIDP2). WithActiveDirectory(myADIDP1). + WithGitHub(myGitHubIDP1). BuildDynamicUpstreamIDPProvider(), - federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs, + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, wantIDPs: []resolvedprovider.FederationDomainResolvedIdentityProvider{ myOIDCIDP1Resolved, myOIDCIDP2Resolved, myLDAPIDP1Resolved, myLDAPIDP2Resolved, myADIDP1Resolved, + myGitHub1Resolved, }, }, { @@ -380,6 +445,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { Build()). WithLDAP(myLDAPIDP1). WithActiveDirectory(myADIDP1). + WithGitHub(myGitHubIDP1). BuildDynamicUpstreamIDPProvider(), federationDomainIssuer: fdIssuerWithLotsOfIDPs, wantIDPs: []resolvedprovider.FederationDomainResolvedIdentityProvider{ @@ -387,13 +453,14 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { myOIDCIDP2Resolved, myLDAPIDP1Resolved, myADIDP1Resolved, + myGitHub1Resolved, }, }, { name: "GetIdentityProviders will return empty list if no IDPs are found", wrappedLister: testidplister.NewUpstreamIDPListerBuilder(). BuildDynamicUpstreamIDPProvider(), - federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs, + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, wantIDPs: []resolvedprovider.FederationDomainResolvedIdentityProvider{}, }, } @@ -420,7 +487,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { name: "IDPCount when there are none to be found", wrappedLister: testidplister.NewUpstreamIDPListerBuilder(). BuildDynamicUpstreamIDPProvider(), - federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs, + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, wantCount: 0, }, { @@ -443,9 +510,14 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { WithName("my-ad-idp-that-isnt-in-fd-issuer"). WithResourceUID("my-ad-idp-that-isnt-in-fd-issuer"). Build()). + WithGitHub(myGitHubIDP1). + WithGitHub(oidctestutil.NewTestUpstreamGitHubIdentityProviderBuilder(). + WithName("my-github-idp-that-isnt-in-fd-issuer"). + WithResourceUID("my-github-idp-that-isnt-in-fd-issuer"). + Build()). BuildDynamicUpstreamIDPProvider(), - federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs, - wantCount: 5, + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, + wantCount: 6, }, } @@ -482,6 +554,14 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { federationDomainIssuer: fdIssuerWithDefaultLDAPIDP, wantHasDefaultIDP: true, }, + { + name: "HasDefaultIDP when there is a GitHub provider set as default", + wrappedLister: testidplister.NewUpstreamIDPListerBuilder(). + WithGitHub(myDefaultGitHubIDP). + BuildDynamicUpstreamIDPProvider(), + federationDomainIssuer: fdIssuerWithDefaultGitHubIDP, + wantHasDefaultIDP: true, + }, { name: "HasDefaultIDP when there is one set even if it cannot be found", wrappedLister: testidplister.NewUpstreamIDPListerBuilder(). @@ -497,7 +577,7 @@ func TestFederationDomainIdentityProvidersListerFinder(t *testing.T) { name: "HasDefaultIDP when there is none set", wrappedLister: testidplister.NewUpstreamIDPListerBuilder(). BuildDynamicUpstreamIDPProvider(), - federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADIDPs, + federationDomainIssuer: fdIssuerWithOIDCAndLDAPAndADAndGitHubIDPs, wantHasDefaultIDP: false, }, } diff --git a/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider.go b/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider.go index 673af6578..e84e92d9d 100644 --- a/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider.go +++ b/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider.go @@ -1,3 +1,6 @@ +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + package resolvedgithub import ( @@ -60,34 +63,37 @@ func (p *FederationDomainResolvedGitHubIdentityProvider) ApplyIDPSpecificSession session.GitHub = idpSpecificSessionData.(*psession.GitHubSessionData) } -func (p *FederationDomainResolvedGitHubIdentityProvider) UpstreamAuthorizeRedirectURL(state *resolvedprovider.UpstreamAuthorizeRequestState, downstreamIssuerURL string) (string, error) { +func (p *FederationDomainResolvedGitHubIdentityProvider) UpstreamAuthorizeRedirectURL( + state *resolvedprovider.UpstreamAuthorizeRequestState, //nolint:all + downstreamIssuerURL string, //nolint:all +) (string, error) { // TODO: implement return "", nil } func (p *FederationDomainResolvedGitHubIdentityProvider) Login( - ctx context.Context, - submittedUsername string, - submittedPassword string, + ctx context.Context, //nolint:all + submittedUsername string, //nolint:all + submittedPassword string, //nolint:all ) (*resolvedprovider.Identity, *resolvedprovider.IdentityLoginExtras, error) { // TODO: implement return nil, nil, nil } func (p *FederationDomainResolvedGitHubIdentityProvider) LoginFromCallback( - ctx context.Context, - authCode string, - pkce pkce.Code, - nonce nonce.Nonce, - redirectURI string, + ctx context.Context, //nolint:all + authCode string, //nolint:all + pkce pkce.Code, //nolint:all + nonce nonce.Nonce, //nolint:all + redirectURI string, //nolint:all ) (*resolvedprovider.Identity, *resolvedprovider.IdentityLoginExtras, error) { // TODO: implement return nil, nil, nil } func (p *FederationDomainResolvedGitHubIdentityProvider) UpstreamRefresh( - ctx context.Context, - identity *resolvedprovider.Identity, + ctx context.Context, //nolint:all + identity *resolvedprovider.Identity, //nolint:all ) (refreshedIdentity *resolvedprovider.RefreshedIdentity, err error) { // TODO: implement return nil, nil diff --git a/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider_test.go b/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider_test.go index c36c1bea7..274144d56 100644 --- a/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider_test.go +++ b/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider_test.go @@ -1 +1,4 @@ +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + package resolvedgithub diff --git a/internal/federationdomain/resolvedprovider/resolvedldap/resolved_ldap_provider.go b/internal/federationdomain/resolvedprovider/resolvedldap/resolved_ldap_provider.go index c8fea641c..d993ec12f 100644 --- a/internal/federationdomain/resolvedprovider/resolvedldap/resolved_ldap_provider.go +++ b/internal/federationdomain/resolvedprovider/resolvedldap/resolved_ldap_provider.go @@ -78,6 +78,8 @@ func (p *FederationDomainResolvedLDAPIdentityProvider) CloneIDPSpecificSessionDa return session.ActiveDirectory.Clone() case psession.ProviderTypeOIDC: // this is just here to avoid a lint error about not handling all cases fallthrough + case psession.ProviderTypeGitHub: // this is just here to avoid a lint error about not handling all cases + fallthrough default: return nil } @@ -153,6 +155,8 @@ func (p *FederationDomainResolvedLDAPIdentityProvider) Login( } case psession.ProviderTypeOIDC: // this is just here to avoid a lint error about not handling all cases fallthrough + case psession.ProviderTypeGitHub: // this is just here to avoid a lint error about not handling all cases + fallthrough default: return nil, nil, ErrUnexpectedUpstreamLDAPError.WithWrap(fmt.Errorf("unexpected provider type %q", p.GetSessionProviderType())) } @@ -207,6 +211,8 @@ func (p *FederationDomainResolvedLDAPIdentityProvider) UpstreamRefresh( additionalAttributes = sessionData.ExtraRefreshAttributes case psession.ProviderTypeOIDC: // this is just here to avoid a lint error about not handling all cases fallthrough + case psession.ProviderTypeGitHub: // this is just here to avoid a lint error about not handling all cases + fallthrough default: // This shouldn't really happen. return nil, resolvedprovider.ErrUpstreamRefreshError().WithHintf( diff --git a/internal/federationdomain/upstreamprovider/upsteam_provider.go b/internal/federationdomain/upstreamprovider/upsteam_provider.go index ab5920bdf..4717fb01c 100644 --- a/internal/federationdomain/upstreamprovider/upsteam_provider.go +++ b/internal/federationdomain/upstreamprovider/upsteam_provider.go @@ -128,6 +128,7 @@ type UpstreamLDAPIdentityProviderI interface { PerformRefresh(ctx context.Context, storedRefreshAttributes RefreshAttributes, idpDisplayName string) (groups []string, err error) } +// TODO: Impl this interface thoroughly to support GitHub login. type UpstreamGithubIdentityProviderI interface { UpstreamIdentityProviderI diff --git a/internal/fositestorage/authorizationcode/authorizationcode.go b/internal/fositestorage/authorizationcode/authorizationcode.go index 41635a7ad..dad061800 100644 --- a/internal/fositestorage/authorizationcode/authorizationcode.go +++ b/internal/fositestorage/authorizationcode/authorizationcode.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package authorizationcode @@ -190,6 +190,7 @@ func (e *errSerializationFailureWithCause) Error() string { return fmt.Sprintf("%s: %s", fosite.ErrSerializationFailure, e.cause) } +// TODO: need to revisit this, there is a unit test now failing. // ExpectedAuthorizeCodeSessionJSONFromFuzzing is used for round tripping tests. // It is exported to allow integration tests to use it. const ExpectedAuthorizeCodeSessionJSONFromFuzzing = `{ diff --git a/internal/psession/pinniped_session.go b/internal/psession/pinniped_session.go index e27b489b0..76fccc3a0 100644 --- a/internal/psession/pinniped_session.go +++ b/internal/psession/pinniped_session.go @@ -85,6 +85,7 @@ const ( ProviderTypeOIDC ProviderType = "oidc" ProviderTypeLDAP ProviderType = "ldap" ProviderTypeActiveDirectory ProviderType = "activedirectory" + ProviderTypeGitHub ProviderType = "github" ) // OIDCSessionData is the additional data needed by Pinniped when the upstream IDP is an OIDC provider. @@ -143,8 +144,9 @@ func (s *ActiveDirectorySessionData) Clone() *ActiveDirectorySessionData { } } +// TODO: flesh this out, GitHub will need additional data. type GitHubSessionData struct { - // TODO: flesh this out + // TODO: flesh this out. } func (s *GitHubSessionData) Clone() *GitHubSessionData { diff --git a/internal/testutil/githubtestutil/testgithubprovider.go b/internal/testutil/githubtestutil/testgithubprovider.go deleted file mode 100644 index 532b9b8a0..000000000 --- a/internal/testutil/githubtestutil/testgithubprovider.go +++ /dev/null @@ -1,8 +0,0 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package githubtestutil - -type TestUpstreamGithubIdentityProvider struct { - Name string -} diff --git a/internal/testutil/oidctestutil/testgithubprovider.go b/internal/testutil/oidctestutil/testgithubprovider.go new file mode 100644 index 000000000..361ae30df --- /dev/null +++ b/internal/testutil/oidctestutil/testgithubprovider.go @@ -0,0 +1,77 @@ +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package oidctestutil + +import ( + "k8s.io/apimachinery/pkg/types" + + "go.pinniped.dev/internal/federationdomain/upstreamprovider" + "go.pinniped.dev/internal/idtransform" +) + +// TODO: flesh this out. +type TestUpstreamGitHubIdentityProviderBuilder struct { + name string + clientID string + resourceUID types.UID + displayNameForFederationDomain string + transformsForFederationDomain *idtransform.TransformationPipeline +} + +func (u *TestUpstreamGitHubIdentityProviderBuilder) WithName(value string) *TestUpstreamGitHubIdentityProviderBuilder { + u.name = value + return u +} + +func (u *TestUpstreamGitHubIdentityProviderBuilder) WithResourceUID(value types.UID) *TestUpstreamGitHubIdentityProviderBuilder { + u.resourceUID = value + return u +} + +func (u *TestUpstreamGitHubIdentityProviderBuilder) WithClientID(value string) *TestUpstreamGitHubIdentityProviderBuilder { + u.clientID = value + return u +} + +func (u *TestUpstreamGitHubIdentityProviderBuilder) Build() *TestUpstreamGitHubIdentityProvider { + if u.displayNameForFederationDomain == "" { + // default it to the CR name + u.displayNameForFederationDomain = u.name + } + if u.transformsForFederationDomain == nil { + // default to an empty pipeline + u.transformsForFederationDomain = idtransform.NewTransformationPipeline() + } + // TODO: flesh this out. + return &TestUpstreamGitHubIdentityProvider{ + Name: u.name, + ResourceUID: u.resourceUID, + ClientID: u.clientID, + DisplayNameForFederationDomain: u.displayNameForFederationDomain, + TransformsForFederationDomain: u.transformsForFederationDomain, + } +} + +func NewTestUpstreamGitHubIdentityProviderBuilder() *TestUpstreamGitHubIdentityProviderBuilder { + return &TestUpstreamGitHubIdentityProviderBuilder{} +} + +// TODO: flesh this out. +type TestUpstreamGitHubIdentityProvider struct { + Name string + ClientID string + ResourceUID types.UID + DisplayNameForFederationDomain string + TransformsForFederationDomain *idtransform.TransformationPipeline +} + +var _ upstreamprovider.UpstreamGithubIdentityProviderI = &TestUpstreamGitHubIdentityProvider{} + +func (u *TestUpstreamGitHubIdentityProvider) GetResourceUID() types.UID { + return u.ResourceUID +} + +func (u *TestUpstreamGitHubIdentityProvider) GetName() string { + return u.Name +} diff --git a/internal/testutil/testidplister/testidplister.go b/internal/testutil/testidplister/testidplister.go index ee0f9ae82..c753d9af4 100644 --- a/internal/testutil/testidplister/testidplister.go +++ b/internal/testutil/testidplister/testidplister.go @@ -11,6 +11,7 @@ import ( "go.pinniped.dev/internal/federationdomain/dynamicupstreamprovider" "go.pinniped.dev/internal/federationdomain/resolvedprovider" + "go.pinniped.dev/internal/federationdomain/resolvedprovider/resolvedgithub" "go.pinniped.dev/internal/federationdomain/resolvedprovider/resolvedldap" "go.pinniped.dev/internal/federationdomain/resolvedprovider/resolvedoidc" "go.pinniped.dev/internal/federationdomain/upstreamprovider" @@ -24,6 +25,7 @@ type TestFederationDomainIdentityProvidersListerFinder struct { upstreamOIDCIdentityProviders []*oidctestutil.TestUpstreamOIDCIdentityProvider upstreamLDAPIdentityProviders []*oidctestutil.TestUpstreamLDAPIdentityProvider upstreamActiveDirectoryIdentityProviders []*oidctestutil.TestUpstreamLDAPIdentityProvider + upstreamGitHubIdentityProviders []*oidctestutil.TestUpstreamGitHubIdentityProvider defaultIDPDisplayName string } @@ -69,6 +71,16 @@ func (t *TestFederationDomainIdentityProvidersListerFinder) GetIdentityProviders fdIDPs[i] = fdIDP i++ } + for _, testIDP := range t.upstreamGitHubIdentityProviders { + fdIDP := &resolvedgithub.FederationDomainResolvedGitHubIdentityProvider{ + DisplayName: testIDP.DisplayNameForFederationDomain, + Provider: testIDP, + SessionProviderType: psession.ProviderTypeGitHub, + Transforms: testIDP.TransformsForFederationDomain, + } + fdIDPs[i] = fdIDP + i++ + } return fdIDPs } @@ -110,6 +122,16 @@ func (t *TestFederationDomainIdentityProvidersListerFinder) FindUpstreamIDPByDis }, nil } } + for _, testIDP := range t.upstreamGitHubIdentityProviders { + if upstreamIDPDisplayName == testIDP.DisplayNameForFederationDomain { + return &resolvedgithub.FederationDomainResolvedGitHubIdentityProvider{ + DisplayName: testIDP.DisplayNameForFederationDomain, + Provider: testIDP, + SessionProviderType: psession.ProviderTypeGitHub, + Transforms: testIDP.TransformsForFederationDomain, + }, nil + } + } return nil, fmt.Errorf("did not find IDP with name %q", upstreamIDPDisplayName) } @@ -125,12 +147,17 @@ func (t *TestFederationDomainIdentityProvidersListerFinder) SetActiveDirectoryId t.upstreamActiveDirectoryIdentityProviders = providers } +func (t *TestFederationDomainIdentityProvidersListerFinder) SetGitHubIdentityProviders(providers []*oidctestutil.TestUpstreamGitHubIdentityProvider) { + t.upstreamGitHubIdentityProviders = providers +} + // UpstreamIDPListerBuilder can be used to build either a dynamicupstreamprovider.DynamicUpstreamIDPProvider // or a FederationDomainIdentityProvidersListerFinderI for testing. type UpstreamIDPListerBuilder struct { upstreamOIDCIdentityProviders []*oidctestutil.TestUpstreamOIDCIdentityProvider upstreamLDAPIdentityProviders []*oidctestutil.TestUpstreamLDAPIdentityProvider upstreamActiveDirectoryIdentityProviders []*oidctestutil.TestUpstreamLDAPIdentityProvider + upstreamGitHubIdentityProviders []*oidctestutil.TestUpstreamGitHubIdentityProvider defaultIDPDisplayName string } @@ -149,6 +176,11 @@ func (b *UpstreamIDPListerBuilder) WithActiveDirectory(upstreamActiveDirectoryId return b } +func (b *UpstreamIDPListerBuilder) WithGitHub(upstreamGithubIdentityProviders ...*oidctestutil.TestUpstreamGitHubIdentityProvider) *UpstreamIDPListerBuilder { + b.upstreamGitHubIdentityProviders = append(b.upstreamGitHubIdentityProviders, upstreamGithubIdentityProviders...) + return b +} + func (b *UpstreamIDPListerBuilder) WithDefaultIDPDisplayName(defaultIDPDisplayName string) *UpstreamIDPListerBuilder { b.defaultIDPDisplayName = defaultIDPDisplayName return b @@ -159,6 +191,7 @@ func (b *UpstreamIDPListerBuilder) BuildFederationDomainIdentityProvidersListerF upstreamOIDCIdentityProviders: b.upstreamOIDCIdentityProviders, upstreamLDAPIdentityProviders: b.upstreamLDAPIdentityProviders, upstreamActiveDirectoryIdentityProviders: b.upstreamActiveDirectoryIdentityProviders, + upstreamGitHubIdentityProviders: b.upstreamGitHubIdentityProviders, defaultIDPDisplayName: b.defaultIDPDisplayName, } } @@ -184,6 +217,12 @@ func (b *UpstreamIDPListerBuilder) BuildDynamicUpstreamIDPProvider() dynamicupst } idpProvider.SetActiveDirectoryIdentityProviders(adUpstreams) + githubUpstreams := make([]upstreamprovider.UpstreamGithubIdentityProviderI, len(b.upstreamGitHubIdentityProviders)) + for i := range b.upstreamGitHubIdentityProviders { + githubUpstreams[i] = upstreamprovider.UpstreamGithubIdentityProviderI(b.upstreamGitHubIdentityProviders[i]) + } + idpProvider.SetGitHubIdentityProviders(githubUpstreams) + return idpProvider } @@ -204,6 +243,8 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyOneCallToPasswordCredentialsGra actualArgs = upstreamOIDC.PasswordCredentialsGrantAndValidateTokensArgs(0) } } + // TODO: probably add GitHub loop once we flesh out the structs + require.Equal(t, 1, actualCallCountAcrossAllOIDCUpstreams, "should have been exactly one call to PasswordCredentialsGrantAndValidateTokens() by all OIDC upstreams", ) @@ -219,6 +260,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyZeroCallsToPasswordCredentialsG for _, upstreamOIDC := range b.upstreamOIDCIdentityProviders { actualCallCountAcrossAllOIDCUpstreams += upstreamOIDC.PasswordCredentialsGrantAndValidateTokensCallCount() } + // TODO: probably add GitHub loop once we flesh out the structs require.Equal(t, 0, actualCallCountAcrossAllOIDCUpstreams, "expected exactly zero calls to PasswordCredentialsGrantAndValidateTokens()", ) @@ -241,6 +283,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyOneCallToExchangeAuthcodeAndVal actualArgs = upstreamOIDC.ExchangeAuthcodeAndValidateTokensArgs(0) } } + // TODO: probably add GitHub loop once we flesh out the structs require.Equal(t, 1, actualCallCountAcrossAllOIDCUpstreams, "should have been exactly one call to ExchangeAuthcodeAndValidateTokens() by all OIDC upstreams", ) @@ -256,6 +299,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyZeroCallsToExchangeAuthcodeAndV for _, upstreamOIDC := range b.upstreamOIDCIdentityProviders { actualCallCountAcrossAllOIDCUpstreams += upstreamOIDC.ExchangeAuthcodeAndValidateTokensCallCount() } + // TODO: probably add GitHub loop once we flesh out the structs require.Equal(t, 0, actualCallCountAcrossAllOIDCUpstreams, "expected exactly zero calls to ExchangeAuthcodeAndValidateTokens()", ) @@ -294,6 +338,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyOneCallToPerformRefresh( actualArgs = upstreamAD.PerformRefreshArgs(0) } } + // TODO: probably add GitHub loop once we flesh out the structs require.Equal(t, 1, actualCallCountAcrossAllUpstreams, "should have been exactly one call to PerformRefresh() by all upstreams", ) @@ -315,6 +360,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyZeroCallsToPerformRefresh(t *te for _, upstreamActiveDirectory := range b.upstreamActiveDirectoryIdentityProviders { actualCallCountAcrossAllUpstreams += upstreamActiveDirectory.PerformRefreshCallCount() } + // TODO: probably add GitHub loop once we flesh out the structs require.Equal(t, 0, actualCallCountAcrossAllUpstreams, "expected exactly zero calls to PerformRefresh()", @@ -338,6 +384,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyOneCallToValidateToken( actualArgs = upstreamOIDC.ValidateTokenAndMergeWithUserInfoArgs(0) } } + // TODO: probably add GitHub loop once we flesh out the structs require.Equal(t, 1, actualCallCountAcrossAllOIDCUpstreams, "should have been exactly one call to ValidateTokenAndMergeWithUserInfo() by all OIDC upstreams", ) @@ -353,6 +400,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyZeroCallsToValidateToken(t *tes for _, upstreamOIDC := range b.upstreamOIDCIdentityProviders { actualCallCountAcrossAllOIDCUpstreams += upstreamOIDC.ValidateTokenAndMergeWithUserInfoCallCount() } + // TODO: probably add GitHub loop once we flesh out structs require.Equal(t, 0, actualCallCountAcrossAllOIDCUpstreams, "expected exactly zero calls to ValidateTokenAndMergeWithUserInfo()", ) @@ -375,6 +423,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyOneCallToRevokeToken( actualArgs = upstreamOIDC.RevokeTokenArgs(0) } } + // TODO: probably add GitHub loop once we flesh out structs require.Equal(t, 1, actualCallCountAcrossAllOIDCUpstreams, "should have been exactly one call to RevokeToken() by all OIDC upstreams", ) @@ -390,6 +439,7 @@ func (b *UpstreamIDPListerBuilder) RequireExactlyZeroCallsToRevokeToken(t *testi for _, upstreamOIDC := range b.upstreamOIDCIdentityProviders { actualCallCountAcrossAllOIDCUpstreams += upstreamOIDC.RevokeTokenCallCount() } + // TODO: probably add GitHub loop once we flesh out structs require.Equal(t, 0, actualCallCountAcrossAllOIDCUpstreams, "expected exactly zero calls to RevokeToken()", ) diff --git a/internal/upstreamgithub/upstreamgithub.go b/internal/upstreamgithub/upstreamgithub.go index fa9bf1f39..cd2c15875 100644 --- a/internal/upstreamgithub/upstreamgithub.go +++ b/internal/upstreamgithub/upstreamgithub.go @@ -5,12 +5,15 @@ package upstreamgithub import ( + "context" "net/http" "golang.org/x/oauth2" + "k8s.io/apimachinery/pkg/types" "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" + "go.pinniped.dev/internal/authenticators" "go.pinniped.dev/internal/federationdomain/upstreamprovider" ) @@ -28,44 +31,74 @@ type ProviderConfig struct { HttpClient *http.Client } -var _ upstreamprovider.UpstreamGithubIdentityProviderI = (*ProviderConfig)(nil) - -func (p *ProviderConfig) GetResourceUID() types.UID { - return p.ResourceUID +type Provider struct { + c ProviderConfig } -func (p *ProviderConfig) GetName() string { - return p.Name +var _ upstreamprovider.UpstreamGithubIdentityProviderI = &Provider{} +var _ authenticators.UserAuthenticator = &Provider{} + +// New creates a Provider. The config is not a pointer to ensure that a copy of the config is created, +// making the resulting Provider use an effectively read-only configuration. +func New(config ProviderConfig) *Provider { + return &Provider{c: config} } -func (p *ProviderConfig) GetClientID() string { - return p.OAuth2Config.ClientID +// GetConfig is a reader for the config. Returns a copy of the config to keep the underlying config read-only. +func (p *Provider) GetConfig() ProviderConfig { + return p.c } -func (p *ProviderConfig) GetHost() string { - return p.Host +// GetName returns a name for this upstream provider. +func (p *Provider) GetName() string { + return p.c.Name } -func (p *ProviderConfig) GetUsernameAttribute() v1alpha1.GitHubUsernameAttribute { - return p.UsernameAttribute +func (p *Provider) GetResourceUID() types.UID { + return p.c.ResourceUID } -func (p *ProviderConfig) GetGroupNameAttribute() v1alpha1.GitHubGroupNameAttribute { - return p.GroupNameAttribute +func (p *Provider) GetClientID() string { + return p.c.OAuth2Config.ClientID } -func (p *ProviderConfig) GetAllowedOrganizations() []string { - return p.AllowedOrganizations +func (p *Provider) GetOAuth2Config() *oauth2.Config { + return p.c.OAuth2Config } -func (p *ProviderConfig) GetOrganizationLoginPolicy() v1alpha1.GitHubAllowedAuthOrganizationsPolicy { - return p.OrganizationLoginPolicy +func (p *Provider) GetHost() string { + return p.c.Host } -func (p *ProviderConfig) GetAuthorizationURL() string { - return p.AuthorizationURL +func (p *Provider) GetUsernameAttribute() v1alpha1.GitHubUsernameAttribute { + return p.c.UsernameAttribute } -func (p *ProviderConfig) GetHttpClient() *http.Client { - return p.HttpClient +func (p *Provider) GetGroupNameAttribute() v1alpha1.GitHubGroupNameAttribute { + return p.c.GroupNameAttribute +} + +func (p *Provider) GetAllowedOrganizations() []string { + return p.c.AllowedOrganizations +} + +func (p *Provider) GetOrganizationLoginPolicy() v1alpha1.GitHubAllowedAuthOrganizationsPolicy { + return p.c.OrganizationLoginPolicy +} + +func (p *Provider) GetAuthorizationURL() string { + return p.c.AuthorizationURL +} + +func (p *Provider) GetHttpClient() *http.Client { + return p.c.HttpClient +} + +// AuthenticateUser authenticates an end user and returns their mapped username, groups, and UID. Implements authenticators.UserAuthenticator. +func (p *Provider) AuthenticateUser( + ctx context.Context, //nolint:all + username, password string, //nolint:all +) (*authenticators.Response, bool, error) { + // TODO: implement this, currently just placeholder to satisfy UserAuthenticator interface above + return nil, false, nil } diff --git a/internal/upstreamgithub/upstreamgithub_test.go b/internal/upstreamgithub/upstreamgithub_test.go index 37e13266f..c5a1cda99 100644 --- a/internal/upstreamgithub/upstreamgithub_test.go +++ b/internal/upstreamgithub/upstreamgithub_test.go @@ -2,3 +2,5 @@ // SPDX-License-Identifier: Apache-2.0 package upstreamgithub + +// TODO: as we flesh out the Provider & ProviderConfig add tests