From 79d0e7405677ff808bd699a0345c6505de2fef20 Mon Sep 17 00:00:00 2001 From: "Benjamin A. Petersen" Date: Thu, 11 Apr 2024 13:19:24 -0400 Subject: [PATCH] Fix github_upstream_watcher so GitHub is listed in Supervisor idp discovery doc --- cmd/pinniped/cmd/login_oidc.go | 2 +- .../supervisorconfig/federation_domain_watcher.go | 5 +++-- .../resolvedgithub/resolved_github_provider.go | 14 ++++++++++++-- internal/psession/pinniped_session.go | 1 - internal/upstreamgithub/upstreamgithub.go | 2 +- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index edda536a5..7d73ab9ee 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/cmd/pinniped/cmd/login_oidc.go @@ -304,7 +304,7 @@ func flowOptions( } switch requestedIDPType { - // TODO(BEN): previously i had put a panic("GitHub NOPE") here. Now its time to work out the details of doing an actual GitHub Login.... + // TODO: Decide if we can bundle GitHub here long term or if it needs its own case case idpdiscoveryv1alpha1.IDPTypeOIDC, idpdiscoveryv1alpha1.IDPTypeGitHub: switch requestedFlow { case idpdiscoveryv1alpha1.IDPFlowCLIPassword: diff --git a/internal/controller/supervisorconfig/federation_domain_watcher.go b/internal/controller/supervisorconfig/federation_domain_watcher.go index d40f7f85f..1edcc474f 100644 --- a/internal/controller/supervisorconfig/federation_domain_watcher.go +++ b/internal/controller/supervisorconfig/federation_domain_watcher.go @@ -90,8 +90,9 @@ type federationDomainWatcherController struct { ldapIdentityProviderInformer idpinformers.LDAPIdentityProviderInformer activeDirectoryIdentityProviderInformer idpinformers.ActiveDirectoryIdentityProviderInformer githubIdentityProviderInformer idpinformers.GitHubIdentityProviderInformer - celTransformer *celtransformer.CELTransformer - allowedKinds sets.Set[string] + + celTransformer *celtransformer.CELTransformer + allowedKinds sets.Set[string] } // NewFederationDomainWatcherController creates a controllerlib.Controller that watches diff --git a/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider.go b/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider.go index e84e92d9d..3574d4e06 100644 --- a/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider.go +++ b/internal/federationdomain/resolvedprovider/resolvedgithub/resolved_github_provider.go @@ -5,6 +5,7 @@ package resolvedgithub import ( "context" + "fmt" "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1" "go.pinniped.dev/internal/federationdomain/resolvedprovider" @@ -44,8 +45,13 @@ func (p *FederationDomainResolvedGitHubIdentityProvider) GetIDPDiscoveryType() v } func (p *FederationDomainResolvedGitHubIdentityProvider) GetIDPDiscoveryFlows() []v1alpha1.IDPFlow { - // TODO: implement - return []v1alpha1.IDPFlow{} + // TODO: review and see if this is actually true to follow the OIDC model + flows := []v1alpha1.IDPFlow{v1alpha1.IDPFlowBrowserAuthcode} + // TODO: coming as a later feature? The UpstreamGithubIdentityProviderI does not currently impl this func + // if p.Provider.AllowsPasswordGrant() { + // flows = append(flows, v1alpha1.IDPFlowCLIPassword) + // } + return flows } func (p *FederationDomainResolvedGitHubIdentityProvider) GetTransforms() *idtransform.TransformationPipeline { @@ -68,6 +74,7 @@ func (p *FederationDomainResolvedGitHubIdentityProvider) UpstreamAuthorizeRedire downstreamIssuerURL string, //nolint:all ) (string, error) { // TODO: implement + fmt.Printf("GithubResolvedIdentityProvider ~ UpstreamAuthorizeRedirectURL() called with state: %#v, downstreamIssuerURL %s", state, downstreamIssuerURL) return "", nil } @@ -77,6 +84,7 @@ func (p *FederationDomainResolvedGitHubIdentityProvider) Login( submittedPassword string, //nolint:all ) (*resolvedprovider.Identity, *resolvedprovider.IdentityLoginExtras, error) { // TODO: implement + fmt.Printf("GithubResolvedIdentityProvider ~ Login() called with submittedUserName %s, submittedPassword %s", submittedUsername, submittedPassword) return nil, nil, nil } @@ -88,6 +96,7 @@ func (p *FederationDomainResolvedGitHubIdentityProvider) LoginFromCallback( redirectURI string, //nolint:all ) (*resolvedprovider.Identity, *resolvedprovider.IdentityLoginExtras, error) { // TODO: implement + fmt.Printf("GithubResolvedIdentityProvider ~ LoginFromCallback() called wtih authCode: %s, pkce: %#v, nonce: %#v, redirectURI: %s", authCode, pkce, nonce, redirectURI) return nil, nil, nil } @@ -96,5 +105,6 @@ func (p *FederationDomainResolvedGitHubIdentityProvider) UpstreamRefresh( identity *resolvedprovider.Identity, //nolint:all ) (refreshedIdentity *resolvedprovider.RefreshedIdentity, err error) { // TODO: implement + fmt.Printf("GithubResolvedIdentityProvider ~ UpstreamRefresh() called with identity %#v", identity) return nil, nil } diff --git a/internal/psession/pinniped_session.go b/internal/psession/pinniped_session.go index 76fccc3a0..a1f34a11b 100644 --- a/internal/psession/pinniped_session.go +++ b/internal/psession/pinniped_session.go @@ -146,7 +146,6 @@ func (s *ActiveDirectorySessionData) Clone() *ActiveDirectorySessionData { // TODO: flesh this out, GitHub will need additional data. type GitHubSessionData struct { - // TODO: flesh this out. } func (s *GitHubSessionData) Clone() *GitHubSessionData { diff --git a/internal/upstreamgithub/upstreamgithub.go b/internal/upstreamgithub/upstreamgithub.go index cd2c15875..678ab0ae1 100644 --- a/internal/upstreamgithub/upstreamgithub.go +++ b/internal/upstreamgithub/upstreamgithub.go @@ -96,7 +96,7 @@ func (p *Provider) GetHttpClient() *http.Client { // 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 + 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