From 3f749e8b988208277e90825f5b6ba30306fbc191 Mon Sep 17 00:00:00 2001
From: Giuseppe Lo Presti <giuseppe.lopresti@cern.ch>
Date: Mon, 21 Nov 2022 17:45:52 +0100
Subject: [PATCH] Resolve users with no uid/gid by their username claim

---
 .../services/authprovider/authprovider.go     |  2 +-
 pkg/auth/manager/oidc/oidc.go                 | 20 ++++++++++---------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/internal/grpc/services/authprovider/authprovider.go b/internal/grpc/services/authprovider/authprovider.go
index 2b819c82177..2fdc5ce4ab5 100644
--- a/internal/grpc/services/authprovider/authprovider.go
+++ b/internal/grpc/services/authprovider/authprovider.go
@@ -150,7 +150,7 @@ func (s *service) Authenticate(ctx context.Context, req *provider.AuthenticateRe
 	u, scope, err := s.authmgr.Authenticate(ctx, username, password)
 	switch v := err.(type) {
 	case nil:
-		log.Info().Msgf("user %s authenticated", u.Id)
+		log.Info().Interface("userId", u.Id).Msg("user authenticated")
 		return &provider.AuthenticateResponse{
 			Status:     status.NewOK(ctx),
 			User:       u,
diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go
index d43e0fdfdca..6b4cb70676d 100644
--- a/pkg/auth/manager/oidc/oidc.go
+++ b/pkg/auth/manager/oidc/oidc.go
@@ -199,7 +199,7 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 		return nil, nil, fmt.Errorf("no \"email\" attribute found in userinfo: maybe the client did not request the oidc \"email\"-scope")
 	}
 
-	err = am.resolveUser(ctx, claims)
+	err = am.resolveUser(ctx, claims, clientID)
 	if err != nil {
 		return nil, nil, errors.Wrapf(err, "oidc: error resolving username for external user '%v'", claims["email"])
 	}
@@ -302,9 +302,8 @@ func (am *mgr) getOIDCProvider(ctx context.Context) (*oidc.Provider, error) {
 	return am.provider, nil
 }
 
-func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) error {
+func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}, clientID string) error {
 	var (
-		claim   string
 		value   string
 		resolve bool
 	)
@@ -316,7 +315,6 @@ func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) e
 	}
 
 	if len(am.oidcUsersMapping) > 0 {
-		claim = "username"
 		// map and discover the user's username when a mapping is defined
 		if claims[am.c.GroupClaim] == nil {
 			// we are required to perform a user mapping but the group claim is not available
@@ -342,8 +340,7 @@ func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) e
 		}
 		resolve = true
 	} else if uid == 0 || gid == 0 {
-		claim = "mail"
-		value = claims["email"].(string)
+		value = clientID
 		resolve = true
 	}
 
@@ -356,11 +353,11 @@ func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) e
 		return errors.Wrap(err, "error getting user provider grpc client")
 	}
 	getUserByClaimResp, err := upsc.GetUserByClaim(ctx, &user.GetUserByClaimRequest{
-		Claim: claim,
+		Claim: "username",
 		Value: value,
 	})
 	if err != nil {
-		return errors.Wrapf(err, "error getting user by %s '%v'", claim, value)
+		return errors.Wrapf(err, "error getting user by username '%v'", value)
 	}
 	if getUserByClaimResp.Status.Code != rpc.Code_CODE_OK {
 		return status.NewErrorFromCode(getUserByClaimResp.Status.Code, "oidc")
@@ -372,7 +369,12 @@ func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) e
 	claims["iss"] = getUserByClaimResp.GetUser().GetId().Idp
 	claims[am.c.UIDClaim] = getUserByClaimResp.GetUser().UidNumber
 	claims[am.c.GIDClaim] = getUserByClaimResp.GetUser().GidNumber
-	appctx.GetLogger(ctx).Debug().Str("username", value).Interface("claims", claims).Msg("resolveUser: claims overridden from mapped user")
+	log := appctx.GetLogger(ctx).Debug().Str("username", value).Interface("claims", claims)
+	if uid == 0 || gid == 0 {
+		log.Msgf("resolveUser: claims overridden from '%s'", clientID)
+	} else {
+		log.Msg("resolveUser: claims overridden from mapped user")
+	}
 	return nil
 }