Add tests for Github in FederationDomain ListerFinder

This commit is contained in:
Benjamin A. Petersen
2024-04-25 17:01:57 -04:00
parent 0c7e95539f
commit 44edba6f75
16 changed files with 349 additions and 84 deletions
+3
View File
@@ -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(
@@ -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) {
@@ -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
@@ -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,
})
}
}
@@ -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,
},
}
@@ -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
@@ -1 +1,4 @@
// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package resolvedgithub
@@ -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(
@@ -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
@@ -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 = `{
+3 -1
View File
@@ -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 {
@@ -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
}
@@ -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
}
@@ -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()",
)
+55 -22
View File
@@ -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
}
@@ -2,3 +2,5 @@
// SPDX-License-Identifier: Apache-2.0
package upstreamgithub
// TODO: as we flesh out the Provider & ProviderConfig add tests