Resolve users with no uid/gid by their username claim

changelog/unreleased/oidc-resolve-users.md

@@ -0,0 +1,8 @@
+Bugfix: OIDC: resolve users with no uid/gid by username
+
+Previously we resolved such users (so called "lightweight" or "external" accounts in the CERN realm)
+by email, but it turns out that the same email may have multiple accounts associated to it.
+
+Therefore we now resolve them by username, that is the upn, which is unique.
+
+https://github.com/cs3org/reva/pull/3481

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,

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
 }