diff --git a/changelog/unreleased/use-uid-gid-fields.md b/changelog/unreleased/use-uid-gid-fields.md new file mode 100644 index 00000000000..2b5fd51907a --- /dev/null +++ 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/json/json.go b/pkg/auth/manager/json/json.go index 6d8607ca98a..7c1e5f6ca69 100644 --- a/pkg/auth/manager/json/json.go +++ b/pkg/auth/manager/json/json.go @@ -47,6 +47,8 @@ type Credentials struct { DisplayName string `mapstructure:"display_name" json:"display_name"` Secret string `mapstructure:"secret" json:"secret"` Groups []string `mapstructure:"groups" json:"groups"` + UIDNumber int64 `mapstructure:"uid_number" json:"uid_number"` + GIDNumber int64 `mapstructure:"gid_number" json:"gid_number"` Opaque *typespb.Opaque `mapstructure:"opaque" json:"opaque"` } @@ -118,6 +120,8 @@ func (m *manager) Authenticate(ctx context.Context, username string, secret stri MailVerified: c.MailVerified, DisplayName: c.DisplayName, Groups: c.Groups, + UidNumber: c.UIDNumber, + GidNumber: c.GIDNumber, Opaque: c.Opaque, // TODO add arbitrary keys as opaque data }, scope, nil diff --git a/pkg/auth/manager/ldap/ldap.go b/pkg/auth/manager/ldap/ldap.go index bc2ef623b89..b7dbfd306c1 100644 --- a/pkg/auth/manager/ldap/ldap.go +++ b/pkg/auth/manager/ldap/ldap.go @@ -22,12 +22,12 @@ import ( "context" "crypto/tls" "fmt" + "strconv" "strings" authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/auth" "github.com/cs3org/reva/pkg/auth/manager/registry" @@ -184,7 +184,14 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) if getGroupsResp.Status.Code != rpc.Code_CODE_OK { return nil, nil, errors.Wrap(err, "ldap: grpc getting user groups failed") } - + gidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.GIDNumber), 10, 64) + if err != nil { + return nil, nil, err + } + uidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.UIDNumber), 10, 64) + if err != nil { + return nil, nil, err + } u := &user.User{ Id: userID, // TODO add more claims from the StandardClaims, eg EmailVerified @@ -193,18 +200,8 @@ 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), - 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)), - }, - }, - }, + UidNumber: uidNumber, + GidNumber: gidNumber, } scope, err := scope.GetOwnerScope() diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go index 39c4a690e88..6958828ab32 100644 --- a/pkg/auth/manager/oidc/oidc.go +++ b/pkg/auth/manager/oidc/oidc.go @@ -29,7 +29,6 @@ import ( authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1" user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/pkg/auth" "github.com/cs3org/reva/pkg/auth/manager/registry" "github.com/cs3org/reva/pkg/auth/scope" @@ -131,26 +130,12 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) return nil, 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 float64 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].(float64) } 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].(float64) } userID := &user.UserId{ @@ -182,7 +167,8 @@ 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: int64(uid), + GidNumber: int64(gid), } scope, err := scope.GetOwnerScope() diff --git a/pkg/cbox/user/rest/rest.go b/pkg/cbox/user/rest/rest.go index a0a878c6a2f..c73a2797efd 100644 --- a/pkg/cbox/user/rest/rest.go +++ b/pkg/cbox/user/rest/rest.go @@ -24,10 +24,10 @@ import ( "fmt" "net/url" "regexp" + "strconv" "strings" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/pkg/appctx" utils "github.com/cs3org/reva/pkg/cbox/utils" "github.com/cs3org/reva/pkg/user" @@ -169,6 +169,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"].(float64) + gidNumber, _ := userData["gid"].(float64) userID := &userpb.UserId{ OpaqueId: upn, @@ -179,18 +181,8 @@ func (m *manager) parseAndCacheUser(ctx context.Context, userData map[string]int Username: upn, Mail: mail, DisplayName: name, - 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"])), - }, - }, - }, + UidNumber: int64(uidNumber), + GidNumber: int64(gidNumber), } if err := m.cacheUserDetails(u); err != nil { @@ -273,6 +265,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"].(float64) + gidNumber, _ := usrInfo["gid"].(float64) uid := &userpb.UserId{ OpaqueId: upn, @@ -283,18 +277,8 @@ func (m *manager) findUsersByFilter(ctx context.Context, url string, users map[s Username: upn, Mail: mail, DisplayName: name, - 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"])), - }, - }, - }, + UidNumber: int64(uidNumber), + GidNumber: int64(gidNumber), } } @@ -385,12 +369,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 strconv.FormatInt(u.UidNumber, 10), nil } diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index e0117f62007..7fb4e27c749 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -226,6 +226,12 @@ func getUser(ctx context.Context) (*userpb.User, error) { err := errors.Wrap(errtypes.UserRequired(""), "eos: error getting user from ctx") return nil, err } + if u.UidNumber == 0 { + return nil, errors.New("eos: invalid user id") + } + if u.GidNumber == 0 { + return nil, errors.New("eos: invalid group id") + } return u, nil } @@ -1476,23 +1482,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 strconv.FormatInt(u.UidNumber, 10), strconv.FormatInt(u.GidNumber, 10), 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 index 3eadeab7183..75e3deec3d0 100644 --- a/pkg/user/manager/demo/demo.go +++ b/pkg/user/manager/demo/demo.go @@ -21,10 +21,10 @@ package demo import ( "context" "errors" + "strconv" "strings" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/user" "github.com/cs3org/reva/pkg/user/manager/registry" @@ -69,12 +69,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 strconv.FormatInt(u.UidNumber, 10), nil } } return "", errors.New("demo: invalid field") @@ -114,18 +110,8 @@ func getUsers() map[string]*userpb.User { Groups: []string{"sailing-lovers", "violin-haters", "physics-lovers"}, Mail: "einstein@example.org", DisplayName: "Albert Einstein", - Opaque: &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - "uid": &types.OpaqueEntry{ - Decoder: "plain", - Value: []byte("123"), - }, - "gid": &types.OpaqueEntry{ - Decoder: "plain", - Value: []byte("987"), - }, - }, - }, + UidNumber: 123, + GidNumber: 987, }, "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c": &userpb.User{ Id: &userpb.UserId{ @@ -136,18 +122,8 @@ func getUsers() map[string]*userpb.User { Groups: []string{"radium-lovers", "polonium-lovers", "physics-lovers"}, Mail: "marie@example.org", DisplayName: "Marie Curie", - Opaque: &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - "uid": &types.OpaqueEntry{ - Decoder: "plain", - Value: []byte("456"), - }, - "gid": &types.OpaqueEntry{ - Decoder: "plain", - Value: []byte("987"), - }, - }, - }, + UidNumber: 456, + GidNumber: 987, }, "932b4540-8d16-481e-8ef4-588e4b6b151c": &userpb.User{ Id: &userpb.UserId{ diff --git a/pkg/user/manager/demo/demo_test.go b/pkg/user/manager/demo/demo_test.go index e2c23f71c21..509c69b2efc 100644 --- a/pkg/user/manager/demo/demo_test.go +++ b/pkg/user/manager/demo/demo_test.go @@ -24,7 +24,6 @@ import ( "testing" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/pkg/errtypes" ) @@ -42,12 +41,8 @@ func TestUserManager(t *testing.T) { Groups: []string{"sailing-lovers", "violin-haters", "physics-lovers"}, Mail: "einstein@example.org", DisplayName: "Albert Einstein", - Opaque: &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - "uid": &types.OpaqueEntry{Decoder: "plain", Value: []byte("123")}, - "gid": &types.OpaqueEntry{Decoder: "plain", Value: []byte("987")}, - }, - }, + UidNumber: 123, + GidNumber: 987, } uidFake := &userpb.UserId{Idp: "nonesense", OpaqueId: "fakeUser"} groupsEinstein := []string{"sailing-lovers", "violin-haters", "physics-lovers"} diff --git a/pkg/user/manager/json/json.go b/pkg/user/manager/json/json.go index 6a02e6c09e9..3e7db910f1c 100644 --- a/pkg/user/manager/json/json.go +++ b/pkg/user/manager/json/json.go @@ -22,6 +22,7 @@ import ( "context" "encoding/json" "io/ioutil" + "strconv" "strings" "github.com/cs3org/reva/pkg/user" @@ -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 strconv.FormatInt(u.UidNumber, 10), nil } } return "", errors.New("json: invalid field") diff --git a/pkg/user/manager/ldap/ldap.go b/pkg/user/manager/ldap/ldap.go index ad68105bbdd..18ec424b7fd 100644 --- a/pkg/user/manager/ldap/ldap.go +++ b/pkg/user/manager/ldap/ldap.go @@ -23,12 +23,12 @@ import ( "context" "crypto/tls" "fmt" + "strconv" "strings" "text/template" "github.com/Masterminds/sprig" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/user" @@ -176,24 +176,22 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User if err != nil { return nil, err } + gidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.GIDNumber), 10, 64) + if err != nil { + return nil, err + } + uidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.UIDNumber), 10, 64) + if err != nil { + return nil, err + } u := &userpb.User{ Id: id, Username: sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.CN), Groups: groups, Mail: sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.Mail), DisplayName: sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.DisplayName), - Opaque: &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - "uid": { - Decoder: "plain", - Value: []byte(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.UIDNumber)), - }, - "gid": { - Decoder: "plain", - Value: []byte(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.GIDNumber)), - }, - }, - }, + GidNumber: gidNumber, + UidNumber: uidNumber, } return u, nil @@ -257,24 +255,22 @@ func (m *manager) GetUserByClaim(ctx context.Context, claim, value string) (*use if err != nil { return nil, err } + gidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.GIDNumber), 10, 64) + if err != nil { + return nil, err + } + uidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.UIDNumber), 10, 64) + if err != nil { + return nil, err + } u := &userpb.User{ Id: id, Username: sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.CN), Groups: groups, Mail: sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.Mail), DisplayName: sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.DisplayName), - Opaque: &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - "uid": { - Decoder: "plain", - Value: []byte(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.UIDNumber)), - }, - "gid": { - Decoder: "plain", - Value: []byte(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.GIDNumber)), - }, - }, - }, + GidNumber: gidNumber, + UidNumber: uidNumber, } return u, nil @@ -319,24 +315,22 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, if err != nil { return nil, err } + gidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.GIDNumber), 10, 64) + if err != nil { + return nil, err + } + uidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(m.c.Schema.UIDNumber), 10, 64) + if err != nil { + return nil, err + } user := &userpb.User{ Id: id, Username: entry.GetEqualFoldAttributeValue(m.c.Schema.CN), Groups: groups, Mail: entry.GetEqualFoldAttributeValue(m.c.Schema.Mail), DisplayName: entry.GetEqualFoldAttributeValue(m.c.Schema.DisplayName), - Opaque: &types.Opaque{ - Map: map[string]*types.OpaqueEntry{ - "uid": { - Decoder: "plain", - Value: []byte(entry.GetEqualFoldAttributeValue(m.c.Schema.UIDNumber)), - }, - "gid": { - Decoder: "plain", - Value: []byte(entry.GetEqualFoldAttributeValue(m.c.Schema.GIDNumber)), - }, - }, - }, + GidNumber: gidNumber, + UidNumber: uidNumber, } users = append(users, user) } diff --git a/tests/integration/grpc/fixtures/users.demo.json b/tests/integration/grpc/fixtures/users.demo.json index cf3ef4b2c8e..62b7442aee9 100644 --- a/tests/integration/grpc/fixtures/users.demo.json +++ b/tests/integration/grpc/fixtures/users.demo.json @@ -9,14 +9,7 @@ "mail": "einstein@example.org", "display_name": "Albert Einstein", "groups": ["sailing-lovers", "violin-haters", "physics-lovers"], - "opaque": { - "map": { - "uid": { - "decoder": "plain", - "value": "MTIz" - } - } - } + "uid_number": 123 }, { "id": { diff --git a/tests/integration/grpc/userprovider_test.go b/tests/integration/grpc/userprovider_test.go index 21f64d44b07..0a19454e815 100644 --- a/tests/integration/grpc/userprovider_test.go +++ b/tests/integration/grpc/userprovider_test.go @@ -89,6 +89,7 @@ var _ = Describe("user providers", func() { for claim, value := range tests { user, err := serviceClient.GetUserByClaim(ctx, &userpb.GetUserByClaimRequest{Claim: claim, Value: value}) Expect(err).ToNot(HaveOccurred()) + Expect(user.User).ToNot(BeNil()) Expect(user.User.Mail).To(Equal("einstein@example.org")) } })