From 8e5f74aa2b7e7457eec4de2c5637e3de58046c4c Mon Sep 17 00:00:00 2001 From: Jean-Philippe Evrard Date: Mon, 13 Jan 2025 18:00:08 +0100 Subject: [PATCH] fixup! Refactor code to be more idiomatic --- internal/ldap/client.go | 4 ++-- internal/ldap/membership.go | 6 ++++-- internal/services/token-provider.go | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/ldap/client.go b/internal/ldap/client.go index 9c02d869..df25e0b1 100644 --- a/internal/ldap/client.go +++ b/internal/ldap/client.go @@ -66,7 +66,7 @@ func (c *LDAPClient) AuthZ(user *types.User) (*types.User, error) { return &types.User{}, fmt.Errorf("cannot get memberships for user %s in LDAP, %v", user.Username, err) } - slog.Info(fmt.Sprintf("user is member of the following groups: %v", ldapMemberships.toGroupNames())) + slog.Debug("listing user groups", "user", user.Username, "groups", ldapMemberships.toGroupNames()) // We now have all the user details (including special groups). // we can check if the user has the basic right to get a token. @@ -99,7 +99,7 @@ func (c *LDAPClient) AuthZ(user *types.User) (*types.User, error) { func (c *LDAPClient) ListProjects() ([]*types.Project, error) { allClusterGroups, err := c.getProjectGroups() if err != nil { - return nil, err + return nil, fmt.Errorf("get Project groups failed, preventing to List Projects: %v", err) } if len(allClusterGroups) == 0 { return nil, fmt.Errorf("no ldap groups found") diff --git a/internal/ldap/membership.go b/internal/ldap/membership.go index 555daf95..4236ac1b 100644 --- a/internal/ldap/membership.go +++ b/internal/ldap/membership.go @@ -117,8 +117,6 @@ func (m *LDAPMemberships) isUserAllowedOnCluster(regexpPatterns []string) (bool, if len(m.AdminAccess) > 0 || len(m.AppOpsAccess) > 0 || len(m.CustomerOpsAccess) > 0 || len(m.ViewerAccess) > 0 || len(m.ServiceAccess) > 0 || len(m.CloudOpsAccess) > 0 || len(m.ClusterGroupsAccess) > 0 { allowedInCluster = true } else { // else is not mandatory it's just an optimisation: don't browse all groups if we already know the user has the rights to the cluster - slog.Debug("Now browsing for user's extra accesses", "groups", fmt.Sprint(m.NonSpecificGroups)) - for _, groupName := range m.NonSpecificGroups { for _, pattern := range regexpPatterns { matched, err := regexp.MatchString(strings.ToUpper(pattern), strings.ToUpper(groupName.DN)) // we match on full DN rather than CN because nobody prevents the ppl in the different entities to create a CN identical as the one used for adminGroup. This is purely out of precaution. In the future, we might want to change the regexp to match only the cn of the groups if we have the guarantee the users will not create groups that are duplicate. @@ -131,6 +129,10 @@ func (m *LDAPMemberships) isUserAllowedOnCluster(regexpPatterns []string) (bool, break } } + if allowedInCluster { + slog.Info("not evaluating further group patterns, the user has access to the cluster") + break + } } } return allowedInCluster, nil diff --git a/internal/services/token-provider.go b/internal/services/token-provider.go index 03d4a477..a8260fb5 100644 --- a/internal/services/token-provider.go +++ b/internal/services/token-provider.go @@ -292,7 +292,7 @@ func generateKubeConfig(serverURL string, CA string, user types.User, token *str func (issuer *TokenIssuer) VerifyToken(usertoken string) (*types.AuthJWTClaims, error) { // this verifies the token and its signature - token, err := jwt.ParseWithClaims(usertoken, types.AuthJWTClaims{}, func(token *jwt.Token) (interface{}, error) { + token, err := jwt.ParseWithClaims(usertoken, &types.AuthJWTClaims{}, func(token *jwt.Token) (interface{}, error) { if issuer.EcdsaPublic == nil { return nil, fmt.Errorf("the public key is nil") }