Skip to content

Commit

Permalink
fixup! Refactor code to be more idiomatic
Browse files Browse the repository at this point in the history
  • Loading branch information
evrardjp-cagip committed Jan 13, 2025
1 parent 2fcde3d commit 8e5f74a
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 5 deletions.
4 changes: 2 additions & 2 deletions internal/ldap/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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")
Expand Down
6 changes: 4 additions & 2 deletions internal/ldap/membership.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/services/token-provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down

0 comments on commit 8e5f74a

Please sign in to comment.