Made uid/gid claims parsing more robust in OIDC auth provider (#2759)

changelog/unreleased/oidc-fix.md

@@ -0,0 +1,8 @@
+Bugfix: made uid, gid claims parsing more robust in OIDC auth provider
+
+This fix makes sure the uid and gid claims are defined at init time, and that
+the necessary typecasts are performed correctly when authenticating users.
+A comment was added that in case the uid/gid claims are missing AND that no
+mapping takes place, a user entity is returned with uid = gid = 0.
+
+https://github.com/cs3org/reva/pull/2759

pkg/auth/manager/oidc/oidc.go

@@ -82,6 +82,12 @@ func (c *config) init() {
 	if c.GroupClaim == "" {
 		c.GroupClaim = "groups"
 	}
+	if c.UIDClaim == "" {
+		c.UIDClaim = "uid"
+	}
+	if c.GIDClaim == "" {
+		c.GIDClaim = "gid"
+	}
 
 	c.GatewaySvc = sharedconf.GetGatewaySVC(c.GatewaySvc)
 }
@@ -193,6 +199,12 @@ 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")
 	}
 
+	uid, _ := claims[am.c.UIDClaim].(float64)
+	claims[am.c.UIDClaim] = int64(uid) // in case the uid claim is missing and a mapping is to be performed, resolveUser() will populate it
+	// Note that if not, will silently carry a user with 0 uid, potentially problematic with storage providers
+	gid, _ := claims[am.c.GIDClaim].(float64)
+	claims[am.c.GIDClaim] = int64(gid)
+
 	err = am.resolveUser(ctx, claims)
 	if err != nil {
 		return nil, nil, errors.Wrapf(err, "oidc: error resolving username for external user '%v'", claims["email"])
@@ -218,23 +230,15 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 		return nil, nil, status.NewErrorFromCode(getGroupsResp.Status.Code, "oidc")
 	}
 
-	var uid, gid int64
-	if am.c.UIDClaim != "" {
-		uid, _ = claims[am.c.UIDClaim].(int64)
-	}
-	if am.c.GIDClaim != "" {
-		gid, _ = claims[am.c.GIDClaim].(int64)
-	}
-
 	u := &user.User{
 		Id:           userID,
 		Username:     claims["preferred_username"].(string),
 		Groups:       getGroupsResp.Groups,
 		Mail:         claims["email"].(string),
 		MailVerified: claims["email_verified"].(bool),
 		DisplayName:  claims["name"].(string),
-		UidNumber:    uid,
-		GidNumber:    gid,
+		UidNumber:    claims[am.c.UIDClaim].(int64),
+		GidNumber:    claims[am.c.GIDClaim].(int64),
 	}
 
 	var scopes map[string]*authpb.Scope
@@ -338,12 +342,8 @@ func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) e
 		claims["preferred_username"] = username
 		claims[am.c.IDClaim] = getUserByClaimResp.GetUser().GetId().OpaqueId
 		claims["iss"] = getUserByClaimResp.GetUser().GetId().Idp
-		if am.c.UIDClaim != "" {
-			claims[am.c.UIDClaim] = getUserByClaimResp.GetUser().UidNumber
-		}
-		if am.c.GIDClaim != "" {
-			claims[am.c.GIDClaim] = getUserByClaimResp.GetUser().GidNumber
-		}
+		claims[am.c.UIDClaim] = getUserByClaimResp.GetUser().UidNumber
+		claims[am.c.GIDClaim] = getUserByClaimResp.GetUser().GidNumber
 		appctx.GetLogger(ctx).Debug().Str("username", username).Interface("claims", claims).Msg("resolveUser: claims overridden from mapped user")
 	}
 	return nil