Use UidNumber and GidNumber fields in User objects (#1516)

cs3org · Mar 19, 2021 · de34f48 · de34f48
1 parent 1628c9c
commit de34f48
Show file tree

Hide file tree

Showing 9 changed files with 91 additions and 137 deletions.
diff --git a/changelog/unreleased/use-uid-gid-fields.md b/changelog/unreleased/use-uid-gid-fields.md
@@ -0,0 +1,6 @@
+Enhancement: use UidNumber and GidNumber fields in User objects
+
+Update instances where CS3API's `User` objects are created and used to use `GidNumber`,
+and `UidNumber` fields instead of storing them in `Opaque` map.
+
+https://github.com/cs3org/reva/issues/1516
diff --git a/pkg/auth/manager/ldap/ldap.go b/pkg/auth/manager/ldap/ldap.go
@@ -22,6 +22,7 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
+	"strconv"
 	"strings"
 
 	user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
@@ -182,7 +183,14 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 	if getGroupsResp.Status.Code != rpc.Code_CODE_OK {
 		return nil, errors.Wrap(err, "ldap: grpc getting user groups failed")
 	}
-
+	gidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.GIDNumber), 10, 0)
+	if err != nil {
+		return nil, err
+	}
+	uidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.UIDNumber), 10, 0)
+	if err != nil {
+		return nil, err
+	}
 	u := &user.User{
 		Id: userID,
 		// TODO add more claims from the StandardClaims, eg EmailVerified
@@ -191,17 +199,10 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 		Groups:      getGroupsResp.Groups,
 		Mail:        sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.Mail),
 		DisplayName: sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.DisplayName),
+		UidNumber:   uidNumber,
+		GidNumber:   gidNumber,
 		Opaque: &types.Opaque{
-			Map: map[string]*types.OpaqueEntry{
-				"uid": {
-					Decoder: "plain",
-					Value:   []byte(sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.UIDNumber)),
-				},
-				"gid": {
-					Decoder: "plain",
-					Value:   []byte(sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.GIDNumber)),
-				},
-			},
+			Map: map[string]*types.OpaqueEntry{},
 		},
 	}
 	log.Debug().Interface("entry", sr.Entries[0]).Interface("user", u).Msg("authenticated user")

diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go
@@ -129,26 +129,12 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 		return nil, fmt.Errorf("no \"preferred_username\" or \"name\" attribute found in userinfo: maybe the client did not request the oidc \"profile\"-scope")
 	}
 
-	opaqueObj := &types.Opaque{
-		Map: map[string]*types.OpaqueEntry{},
-	}
+	var uid, gid int64
 	if am.c.UIDClaim != "" {
-		uid, ok := claims[am.c.UIDClaim]
-		if ok {
-			opaqueObj.Map["uid"] = &types.OpaqueEntry{
-				Decoder: "plain",
-				Value:   []byte(fmt.Sprintf("%0.f", uid)),
-			}
-		}
+		uid, _ = claims[am.c.UIDClaim].(int64)
 	}
 	if am.c.GIDClaim != "" {
-		gid, ok := claims[am.c.GIDClaim]
-		if ok {
-			opaqueObj.Map["gid"] = &types.OpaqueEntry{
-				Decoder: "plain",
-				Value:   []byte(fmt.Sprintf("%0.f", gid)),
-			}
-		}
+		gid, _ = claims[am.c.GIDClaim].(int64)
 	}
 
 	userID := &user.UserId{
@@ -180,7 +166,9 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 		Mail:         claims["email"].(string),
 		MailVerified: claims["email_verified"].(bool),
 		DisplayName:  claims["name"].(string),
-		Opaque:       opaqueObj,
+		UidNumber:    uid,
+		GidNumber:    gid,
+		Opaque:       &types.Opaque{Map: map[string]*types.OpaqueEntry{}},
 	}
 
 	return u, nil

diff --git a/pkg/cbox/user/rest/rest.go b/pkg/cbox/user/rest/rest.go
@@ -290,6 +290,8 @@ func (m *manager) parseAndCacheUser(ctx context.Context, userData map[string]int
 	upn, _ := userData["upn"].(string)
 	mail, _ := userData["primaryAccountEmail"].(string)
 	name, _ := userData["displayName"].(string)
+	uidNumber, _ := userData["uid"].(int64)
+	gidNumber, _ := userData["gid"].(int64)
 
 	userID := &userpb.UserId{
 		OpaqueId: upn,
@@ -300,17 +302,10 @@ func (m *manager) parseAndCacheUser(ctx context.Context, userData map[string]int
 		Username:    upn,
 		Mail:        mail,
 		DisplayName: name,
+		UidNumber:   uidNumber,
+		GidNumber:   gidNumber,
 		Opaque: &types.Opaque{
-			Map: map[string]*types.OpaqueEntry{
-				"uid": &types.OpaqueEntry{
-					Decoder: "plain",
-					Value:   []byte(fmt.Sprintf("%0.f", userData["uid"])),
-				},
-				"gid": &types.OpaqueEntry{
-					Decoder: "plain",
-					Value:   []byte(fmt.Sprintf("%0.f", userData["gid"])),
-				},
-			},
+			Map: map[string]*types.OpaqueEntry{},
 		},
 	}
 
@@ -395,6 +390,8 @@ func (m *manager) findUsersByFilter(ctx context.Context, url string, users map[s
 		upn, _ := usrInfo["upn"].(string)
 		mail, _ := usrInfo["primaryAccountEmail"].(string)
 		name, _ := usrInfo["displayName"].(string)
+		uidNumber, _ := usrInfo["uid"].(int64)
+		gidNumber, _ := usrInfo["gid"].(int64)
 
 		uid := &userpb.UserId{
 			OpaqueId: upn,
@@ -405,17 +402,10 @@ func (m *manager) findUsersByFilter(ctx context.Context, url string, users map[s
 			Username:    upn,
 			Mail:        mail,
 			DisplayName: name,
+			UidNumber:   uidNumber,
+			GidNumber:   gidNumber,
 			Opaque: &types.Opaque{
-				Map: map[string]*types.OpaqueEntry{
-					"uid": &types.OpaqueEntry{
-						Decoder: "plain",
-						Value:   []byte(fmt.Sprintf("%0.f", usrInfo["uid"])),
-					},
-					"gid": &types.OpaqueEntry{
-						Decoder: "plain",
-						Value:   []byte(fmt.Sprintf("%0.f", usrInfo["gid"])),
-					},
-				},
+				Map: map[string]*types.OpaqueEntry{},
 			},
 		}
 	}
@@ -507,12 +497,8 @@ func (m *manager) IsInGroup(ctx context.Context, uid *userpb.UserId, group strin
 }
 
 func extractUID(u *userpb.User) (string, error) {
-	if u.Opaque != nil && u.Opaque.Map != nil {
-		if uidObj, ok := u.Opaque.Map["uid"]; ok {
-			if uidObj.Decoder == "plain" {
-				return string(uidObj.Value), nil
-			}
-		}
+	if u.UidNumber == 0 {
+		return "", errors.New("rest: could not retrieve UID from user")
 	}
-	return "", errors.New("rest: could not retrieve UID from user")
+	return fmt.Sprintf("%v", u.UidNumber), nil
 }
diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go
@@ -1457,23 +1457,13 @@ func getResourceType(isDir bool) provider.ResourceType {
 }
 
 func (fs *eosfs) extractUIDAndGID(u *userpb.User) (string, string, error) {
-	var uid, gid string
-	if u.Opaque != nil && u.Opaque.Map != nil {
-		if uidObj, ok := u.Opaque.Map["uid"]; ok {
-			if uidObj.Decoder == "plain" {
-				uid = string(uidObj.Value)
-			}
-		}
-		if gidObj, ok := u.Opaque.Map["gid"]; ok {
-			if gidObj.Decoder == "plain" {
-				gid = string(gidObj.Value)
-			}
-		}
+	if u.UidNumber == 0 {
+		return "", "", errors.New("eos: uid missing for user")
 	}
-	if uid == "" || gid == "" {
-		return "", "", errors.New("eos: uid or gid missing for user")
+	if u.GidNumber == 0 {
+		return "", "", errors.New("eos: gid missing for user")
 	}
-	return uid, gid, nil
+	return fmt.Sprintf("%v", u.UidNumber), fmt.Sprintf("%v", u.GidNumber), nil
 }
 
 func (fs *eosfs) getUIDGateway(ctx context.Context, u *userpb.UserId) (string, string, error) {

diff --git a/pkg/user/manager/demo/demo.go b/pkg/user/manager/demo/demo.go
@@ -21,6 +21,7 @@ package demo
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 
 	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
@@ -69,12 +70,8 @@ func extractClaim(u *userpb.User, claim string) (string, error) {
 	case "username":
 		return u.Username, nil
 	case "uid":
-		if u.Opaque != nil && u.Opaque.Map != nil {
-			if uidObj, ok := u.Opaque.Map["uid"]; ok {
-				if uidObj.Decoder == "plain" {
-					return string(uidObj.Value), nil
-				}
-			}
+		if u.UidNumber != 0 {
+			return fmt.Sprintf("%v", u.UidNumber), nil
 		}
 	}
 	return "", errors.New("demo: invalid field")
@@ -114,17 +111,10 @@ func getUsers() map[string]*userpb.User {
 			Groups:      []string{"sailing-lovers", "violin-haters", "physics-lovers"},
 			Mail:        "einstein@example.org",
 			DisplayName: "Albert Einstein",
+			UidNumber:   123,
+			GidNumber:   987,
 			Opaque: &types.Opaque{
-				Map: map[string]*types.OpaqueEntry{
-					"uid": &types.OpaqueEntry{
-						Decoder: "plain",
-						Value:   []byte("123"),
-					},
-					"gid": &types.OpaqueEntry{
-						Decoder: "plain",
-						Value:   []byte("987"),
-					},
-				},
+				Map: map[string]*types.OpaqueEntry{},
 			},
 		},
 		"f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c": &userpb.User{
@@ -136,17 +126,10 @@ func getUsers() map[string]*userpb.User {
 			Groups:      []string{"radium-lovers", "polonium-lovers", "physics-lovers"},
 			Mail:        "marie@example.org",
 			DisplayName: "Marie Curie",
+			UidNumber:   456,
+			GidNumber:   987,
 			Opaque: &types.Opaque{
-				Map: map[string]*types.OpaqueEntry{
-					"uid": &types.OpaqueEntry{
-						Decoder: "plain",
-						Value:   []byte("456"),
-					},
-					"gid": &types.OpaqueEntry{
-						Decoder: "plain",
-						Value:   []byte("987"),
-					},
-				},
+				Map: map[string]*types.OpaqueEntry{},
 			},
 		},
 		"932b4540-8d16-481e-8ef4-588e4b6b151c": &userpb.User{

diff --git a/pkg/user/manager/demo/demo_test.go b/pkg/user/manager/demo/demo_test.go
@@ -42,11 +42,10 @@ func TestUserManager(t *testing.T) {
 		Groups:      []string{"sailing-lovers", "violin-haters", "physics-lovers"},
 		Mail:        "einstein@example.org",
 		DisplayName: "Albert Einstein",
+		UidNumber:   123,
+		GidNumber:   987,
 		Opaque: &types.Opaque{
-			Map: map[string]*types.OpaqueEntry{
-				"uid": &types.OpaqueEntry{Decoder: "plain", Value: []byte("123")},
-				"gid": &types.OpaqueEntry{Decoder: "plain", Value: []byte("987")},
-			},
+			Map: map[string]*types.OpaqueEntry{},
 		},
 	}
 	uidFake := &userpb.UserId{Idp: "nonesense", OpaqueId: "fakeUser"}

diff --git a/pkg/user/manager/json/json.go b/pkg/user/manager/json/json.go
@@ -21,6 +21,7 @@ package json
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"io/ioutil"
 	"strings"
 
@@ -111,12 +112,8 @@ func extractClaim(u *userpb.User, claim string) (string, error) {
 	case "username":
 		return u.Username, nil
 	case "uid":
-		if u.Opaque != nil && u.Opaque.Map != nil {
-			if uidObj, ok := u.Opaque.Map["uid"]; ok {
-				if uidObj.Decoder == "plain" {
-					return string(uidObj.Value), nil
-				}
-			}
+		if u.UidNumber != 0 {
+			return fmt.Sprintf("%v", u.UidNumber), nil
 		}
 	}
 	return "", errors.New("json: invalid field")