From 28f3aef69b38b70d63232bd0e4ac916469818680 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 30 Sep 2021 15:59:23 +0100 Subject: [PATCH] Integrate klog into providers package --- providers/azure.go | 18 ++++++++++-------- providers/bitbucket.go | 12 ++++++------ providers/github.go | 13 ++++++------- providers/gitlab.go | 10 +++++----- providers/google.go | 14 +++++++------- providers/internal_util.go | 14 +++++++------- providers/keycloak.go | 4 ++-- providers/logger.go | 12 ++++++++++++ providers/oidc.go | 10 +++++----- providers/provider_data.go | 6 +++--- providers/providers_suite_test.go | 5 ++--- 11 files changed, 65 insertions(+), 53 deletions(-) create mode 100644 providers/logger.go diff --git a/providers/azure.go b/providers/azure.go index 39beb836..cd501756 100644 --- a/providers/azure.go +++ b/providers/azure.go @@ -11,8 +11,8 @@ import ( "github.com/bitly/go-simplejson" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" + "k8s.io/klog/v2" ) // AzureProvider represents an Azure based Identity Provider @@ -89,6 +89,7 @@ func NewAzureProvider(p *ProviderData) *AzureProvider { func (p *AzureProvider) Configure(tenant string) { if tenant == "" || tenant == "common" { // tenant is empty or default, remain on the default "common" tenant + infoLogger.Infof("Azure provider configured for common tenant") return } @@ -96,6 +97,7 @@ func (p *AzureProvider) Configure(tenant string) { p.Tenant = tenant overrideTenantURL(p.LoginURL, azureDefaultLoginURL, tenant, "authorize") overrideTenantURL(p.RedeemURL, azureDefaultRedeemURL, tenant, "token") + infoLogger.Infof("Azure provider configured for tenant: %s", tenant) } func overrideTenantURL(current, defaultURL *url.URL, tenant, path string) { @@ -159,7 +161,7 @@ func (p *AzureProvider) Redeem(ctx context.Context, redirectURL, code string) (* if err == nil && email != "" { session.Email = email } else { - logger.Printf("unable to get email claim from id_token: %v", err) + debugLogger.Infof("Unable to get email claim from id_token: %v", err) } if session.Email == "" { @@ -167,7 +169,7 @@ func (p *AzureProvider) Redeem(ctx context.Context, redirectURL, code string) (* if err == nil && email != "" { session.Email = email } else { - logger.Printf("unable to get email claim from access token: %v", err) + debugLogger.Infof("Unable to get email claim from access token: %v", err) } } @@ -226,10 +228,10 @@ func (p *AzureProvider) verifyTokenAndExtractEmail(ctx context.Context, token st if err == nil { email = claims.Email } else { - logger.Printf("unable to get claims from token: %v", err) + debugLogger.Infof("Unable to get claims from token: %v", err) } } else { - logger.Printf("unable to verify token: %v", err) + debugLogger.Infof("Unable to verify token: %v", err) } } @@ -296,7 +298,7 @@ func (p *AzureProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sess if err == nil && email != "" { s.Email = email } else { - logger.Printf("unable to get email claim from id_token: %v", err) + debugLogger.Infof("Unable to get email claim from id_token: %v", err) } if s.Email == "" { @@ -304,7 +306,7 @@ func (p *AzureProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sess if err == nil && email != "" { s.Email = email } else { - logger.Printf("unable to get email claim from access token: %v", err) + debugLogger.Infof("Unable to get email claim from access token: %v", err) } } @@ -332,7 +334,7 @@ func getEmailFromJSON(json *simplejson.Json) (string, error) { if err != nil || email == "" { email, err = json.Get("userPrincipalName").String() if err != nil { - logger.Errorf("unable to find userPrincipalName: %s", err) + klog.Errorf("Unable to find userPrincipalName: %s", err) return "", err } } diff --git a/providers/bitbucket.go b/providers/bitbucket.go index 2612a4ba..68153c43 100644 --- a/providers/bitbucket.go +++ b/providers/bitbucket.go @@ -6,8 +6,8 @@ import ( "strings" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" + "k8s.io/klog/v2" ) // BitbucketProvider represents an Bitbucket based Identity Provider @@ -107,7 +107,7 @@ func (p *BitbucketProvider) GetEmailAddress(ctx context.Context, s *sessions.Ses Do(). UnmarshalInto(&emails) if err != nil { - logger.Errorf("failed making request: %v", err) + klog.Errorf("Failed making request: %v", err) return "", err } @@ -123,7 +123,7 @@ func (p *BitbucketProvider) GetEmailAddress(ctx context.Context, s *sessions.Ses Do(). UnmarshalInto(&teams) if err != nil { - logger.Errorf("failed requesting teams membership: %v", err) + klog.Errorf("Failed requesting teams membership: %v", err) return "", err } var found = false @@ -134,7 +134,7 @@ func (p *BitbucketProvider) GetEmailAddress(ctx context.Context, s *sessions.Ses } } if !found { - logger.Error("team membership test failed, access denied") + klog.Error("Team membership test failed, access denied") return "", nil } } @@ -153,7 +153,7 @@ func (p *BitbucketProvider) GetEmailAddress(ctx context.Context, s *sessions.Ses Do(). UnmarshalInto(&repositories) if err != nil { - logger.Errorf("failed checking repository access: %v", err) + klog.Errorf("Failed checking repository access: %v", err) return "", err } @@ -165,7 +165,7 @@ func (p *BitbucketProvider) GetEmailAddress(ctx context.Context, s *sessions.Ses } } if !found { - logger.Error("repository access test failed, access denied") + klog.Error("Repository access test failed, access denied") return "", nil } } diff --git a/providers/github.go b/providers/github.go index 064329c4..b21e31ee 100644 --- a/providers/github.go +++ b/providers/github.go @@ -12,7 +12,6 @@ import ( "strings" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" ) @@ -162,13 +161,13 @@ func (p *GitHubProvider) hasOrg(ctx context.Context, accessToken string) (bool, presentOrgs := make([]string, 0, len(orgs)) for _, org := range orgs { if p.Org == org.Login { - logger.Printf("Found Github Organization: %q", org.Login) + debugLogger.Infof("Found Github Organization: %q", org.Login) return true, nil } presentOrgs = append(presentOrgs, org.Login) } - logger.Printf("Missing Organization:%q in %v", p.Org, presentOrgs) + debugLogger.Infof("Missing Organization:%q in %v", p.Org, presentOrgs) return false, nil } @@ -270,7 +269,7 @@ func (p *GitHubProvider) hasOrgAndTeam(ctx context.Context, accessToken string) ts := strings.Split(p.Team, ",") for _, t := range ts { if t == team.Slug { - logger.Printf("Found Github Organization:%q Team:%q (Name:%q)", team.Org.Login, team.Slug, team.Name) + debugLogger.Infof("Found Github Organization:%q Team:%q (Name:%q)", team.Org.Login, team.Slug, team.Name) return true, nil } } @@ -278,13 +277,13 @@ func (p *GitHubProvider) hasOrgAndTeam(ctx context.Context, accessToken string) } } if hasOrg { - logger.Printf("Missing Team:%q from Org:%q in teams: %v", p.Team, p.Org, presentTeams) + debugLogger.Infof("Missing Team:%q from Org:%q in teams: %v", p.Team, p.Org, presentTeams) } else { var allOrgs []string for org := range presentOrgs { allOrgs = append(allOrgs, org) } - logger.Printf("Missing Organization:%q in %#v", p.Org, allOrgs) + debugLogger.Infof("Missing Organization:%q in %#v", p.Org, allOrgs) } return false, nil } @@ -373,7 +372,7 @@ func (p *GitHubProvider) isCollaborator(ctx context.Context, username, accessTok result.StatusCode(), endpoint.String(), result.Body()) } - logger.Printf("got %d from %q %s", result.StatusCode(), endpoint.String(), result.Body()) + traceLogger.Infof("Checking collaborator status: Got %d from %q %s", result.StatusCode(), endpoint.String(), result.Body()) return true, nil } diff --git a/providers/gitlab.go b/providers/gitlab.go index b73e19d3..9295637a 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -8,8 +8,8 @@ import ( "strings" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" + "k8s.io/klog/v2" ) const ( @@ -177,12 +177,12 @@ func (p *GitLabProvider) addProjectsToSession(ctx context.Context, s *sessions.S for _, project := range p.allowedProjects { projectInfo, err := p.getProjectInfo(ctx, s, project.Name) if err != nil { - logger.Errorf("Warning: project info request failed: %v", err) + klog.Warningf("Warning: project info request failed: %v", err) continue } if projectInfo.Archived { - logger.Errorf("Warning: project %s is archived", project.Name) + klog.Warningf("Warning: project %s is archived", project.Name) continue } @@ -192,14 +192,14 @@ func (p *GitLabProvider) addProjectsToSession(ctx context.Context, s *sessions.S perms = projectInfo.Permissions.GroupAccess // group project access is not set for this user then we give up if perms == nil { - logger.Errorf("Warning: user %q has no project level access to %s", + klog.Warningf("Warning: user %q has no project level access to %s", s.Email, project.Name) continue } } if perms.AccessLevel < project.AccessLevel { - logger.Errorf( + klog.Warningf( "Warning: user %q does not have the minimum required access level for project %q", s.Email, project.Name, diff --git a/providers/google.go b/providers/google.go index a467c50f..52606439 100644 --- a/providers/google.go +++ b/providers/google.go @@ -14,12 +14,12 @@ import ( "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" "golang.org/x/oauth2/google" admin "google.golang.org/api/admin/directory/v1" "google.golang.org/api/googleapi" "google.golang.org/api/option" + "k8s.io/klog/v2" ) // GoogleProvider represents an Google based Identity Provider @@ -213,11 +213,11 @@ func (p *GoogleProvider) SetGroupRestriction(groups []string, adminEmail string, func getAdminService(adminEmail string, credentialsReader io.Reader) *admin.Service { data, err := ioutil.ReadAll(credentialsReader) if err != nil { - logger.Fatal("can't read Google credentials file:", err) + klog.Fatalf("can't read Google credentials file:", err) } conf, err := google.JWTConfigFromJSON(data, admin.AdminDirectoryUserReadonlyScope, admin.AdminDirectoryGroupReadonlyScope) if err != nil { - logger.Fatal("can't load Google credentials file:", err) + klog.Fatalf("can't load Google credentials file:", err) } conf.Subject = adminEmail @@ -225,7 +225,7 @@ func getAdminService(adminEmail string, credentialsReader io.Reader) *admin.Serv client := conf.Client(ctx) adminService, err := admin.NewService(ctx, option.WithHTTPClient(client)) if err != nil { - logger.Fatal(err) + klog.Fatalf("cannot create admin service: %v", err) } return adminService } @@ -241,7 +241,7 @@ func userInGroup(service *admin.Service, group string, email string) bool { gerr, ok := err.(*googleapi.Error) switch { case ok && gerr.Code == 404: - logger.Errorf("error checking membership in group %s: group does not exist", group) + klog.Errorf("error checking membership in group %s: group does not exist", group) case ok && gerr.Code == 400: // It is possible for Members.HasMember to return false even if the email is a group member. // One case that can cause this is if the user email is from a different domain than the group, @@ -250,7 +250,7 @@ func userInGroup(service *admin.Service, group string, email string) bool { req := service.Members.Get(group, email) r, err := req.Do() if err != nil { - logger.Errorf("error using get API to check member %s of google group %s: user not in the group", email, group) + klog.Errorf("error using get API to check member %s of google group %s: user not in the group", email, group) return false } @@ -260,7 +260,7 @@ func userInGroup(service *admin.Service, group string, email string) bool { return true } default: - logger.Errorf("error checking group membership: %v", err) + klog.Errorf("error checking group membership: %v", err) } return false } diff --git a/providers/internal_util.go b/providers/internal_util.go index 346ee80d..d7bb0619 100644 --- a/providers/internal_util.go +++ b/providers/internal_util.go @@ -5,8 +5,8 @@ import ( "net/http" "net/url" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" + "k8s.io/klog/v2" ) // stripToken is a helper function to obfuscate "access_token" @@ -24,14 +24,14 @@ func stripToken(endpoint string) string { func stripParam(param, endpoint string) string { u, err := url.Parse(endpoint) if err != nil { - logger.Errorf("error attempting to strip %s: %s", param, err) + klog.Errorf("error attempting to strip %s: %s", param, err) return endpoint } if u.RawQuery != "" { values, err := url.ParseQuery(u.RawQuery) if err != nil { - logger.Errorf("error attempting to strip %s: %s", param, err) + klog.Errorf("error attempting to strip %s: %s", param, err) return u.String() } @@ -61,16 +61,16 @@ func validateToken(ctx context.Context, p Provider, accessToken string, header h WithHeaders(header). Do() if result.Error() != nil { - logger.Errorf("GET %s", stripToken(endpoint)) - logger.Errorf("token validation request failed: %s", result.Error()) + debugLogger.Infof("GET %s", stripToken(endpoint)) + debugLogger.Infof("token validation request failed: %s", result.Error()) return false } - logger.Printf("%d GET %s %s", result.StatusCode(), stripToken(endpoint), result.Body()) + traceLogger.Infof("%d GET %s %s", result.StatusCode(), stripToken(endpoint), result.Body()) if result.StatusCode() == 200 { return true } - logger.Errorf("token validation request failed: status %d - %s", result.StatusCode(), result.Body()) + klog.Errorf("token validation request failed: status %d - %s", result.StatusCode(), result.Body()) return false } diff --git a/providers/keycloak.go b/providers/keycloak.go index a1e4f064..e6041e5c 100644 --- a/providers/keycloak.go +++ b/providers/keycloak.go @@ -6,8 +6,8 @@ import ( "net/url" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" + "k8s.io/klog/v2" ) type KeycloakProvider struct { @@ -75,7 +75,7 @@ func (p *KeycloakProvider) EnrichSession(ctx context.Context, s *sessions.Sessio Do(). UnmarshalJSON() if err != nil { - logger.Errorf("failed making request %v", err) + klog.Errorf("failed making request %v", err) return err } diff --git a/providers/logger.go b/providers/logger.go new file mode 100644 index 00000000..82bbbe4f --- /dev/null +++ b/providers/logger.go @@ -0,0 +1,12 @@ +package providers + +import ( + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" + "k8s.io/klog/v2" +) + +var ( + infoLogger = klog.V(logger.ProviderInfo) + debugLogger = klog.V(logger.ProviderDebug) + traceLogger = klog.V(logger.ProviderTrace) +) diff --git a/providers/oidc.go b/providers/oidc.go index b1711d54..518b3cfe 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -9,9 +9,9 @@ import ( "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" "golang.org/x/oauth2" + "k8s.io/klog/v2" ) // OIDCProvider represents an OIDC based Identity Provider @@ -79,7 +79,7 @@ func (p *OIDCProvider) EnrichSession(ctx context.Context, s *sessions.SessionSta if s.Email == "" || s.Groups == nil { err := p.enrichFromProfileURL(ctx, s) if err != nil { - logger.Errorf("Warning: Profile URL request failed: %v", err) + klog.Warningf("Warning: Profile URL request failed: %v", err) } } @@ -113,7 +113,7 @@ func (p *OIDCProvider) enrichFromProfileURL(ctx context.Context, s *sessions.Ses for _, group := range coerceArray(respJSON, p.GroupsClaim) { formatted, err := formatGroup(group) if err != nil { - logger.Errorf("Warning: unable to format group of type %s with error %s", + klog.Warningf("Warning: unable to format group of type %s with error %s", reflect.TypeOf(group), err) continue } @@ -127,7 +127,7 @@ func (p *OIDCProvider) enrichFromProfileURL(ctx context.Context, s *sessions.Ses func (p *OIDCProvider) ValidateSession(ctx context.Context, s *sessions.SessionState) bool { idToken, err := p.Verifier.Verify(ctx, s.IDToken) if err != nil { - logger.Errorf("id_token verification failed: %v", err) + klog.Errorf("id_token verification failed: %v", err) return false } @@ -136,7 +136,7 @@ func (p *OIDCProvider) ValidateSession(ctx context.Context, s *sessions.SessionS } err = p.checkNonce(s, idToken) if err != nil { - logger.Errorf("nonce verification failed: %v", err) + klog.Errorf("nonce verification failed: %v", err) return false } diff --git a/providers/provider_data.go b/providers/provider_data.go index de2aae3e..5d773e5a 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -11,8 +11,8 @@ import ( "github.com/coreos/go-oidc/v3/oidc" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" "golang.org/x/oauth2" + "k8s.io/klog/v2" ) const ( @@ -62,7 +62,7 @@ func (p *ProviderData) GetClientSecret() (clientSecret string, err error) { // Getting ClientSecret can fail in runtime so we need to report it without returning the file name to the user fileClientSecret, err := ioutil.ReadFile(p.ClientSecretFile) if err != nil { - logger.Errorf("error reading client secret file %s: %s", p.ClientSecretFile, err) + klog.Errorf("error reading client secret file %s: %s", p.ClientSecretFile, err) return "", errors.New("could not read client secret file") } return string(fileClientSecret), nil @@ -240,7 +240,7 @@ func (p *ProviderData) extractGroups(claims map[string]interface{}) []string { for _, rawGroup := range claimGroups { formattedGroup, err := formatGroup(rawGroup) if err != nil { - logger.Errorf("Warning: unable to format group of type %s with error %s", + klog.Warningf("Warning: unable to format group of type %s with error %s", reflect.TypeOf(rawGroup), err) continue } diff --git a/providers/providers_suite_test.go b/providers/providers_suite_test.go index 9194eff4..1de49988 100644 --- a/providers/providers_suite_test.go +++ b/providers/providers_suite_test.go @@ -3,14 +3,13 @@ package providers_test import ( "testing" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "k8s.io/klog/v2" ) func TestProviderSuite(t *testing.T) { - logger.SetOutput(GinkgoWriter) - logger.SetErrOutput(GinkgoWriter) + klog.SetOutput(GinkgoWriter) RegisterFailHandler(Fail) RunSpecs(t, "Providers")