From ab4282c1a7d73d678d4f75213224280e240a0477 Mon Sep 17 00:00:00 2001 From: "Zyad A. Ali" Date: Fri, 19 Mar 2021 16:51:02 +0200 Subject: [PATCH 1/5] Use UidNumber and GidNumber fields in User objects (#1516) --- changelog/unreleased/use-uid-gid-fields.md | 6 ++ pkg/auth/manager/json/json.go | 4 ++ pkg/auth/manager/ldap/ldap.go | 25 ++++---- pkg/auth/manager/oidc/oidc.go | 24 ++------ pkg/cbox/user/rest/rest.go | 43 ++++---------- pkg/storage/utils/eosfs/eosfs.go | 20 ++----- pkg/user/manager/demo/demo.go | 38 +++--------- pkg/user/manager/demo/demo_test.go | 9 +-- pkg/user/manager/json/json.go | 9 +-- pkg/user/manager/ldap/ldap.go | 68 ++++++++++------------ 10 files changed, 85 insertions(+), 161 deletions(-) create mode 100644 changelog/unreleased/use-uid-gid-fields.md diff --git a/changelog/unreleased/use-uid-gid-fields.md b/changelog/unreleased/use-uid-gid-fields.md new file mode 100644 index 0000000000..2b5fd51907 --- /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 6d8607ca98..7c1e5f6ca6 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 bc2ef623b8..272d327ff6 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, err + } + uidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.UIDNumber), 10, 64) + if err != nil { + return 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 39c4a690e8..b6e5177cf4 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 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{ @@ -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: uid, + GidNumber: gid, } scope, err := scope.GetOwnerScope() diff --git a/pkg/cbox/user/rest/rest.go b/pkg/cbox/user/rest/rest.go index a0a878c6a2..62d32d6f94 100644 --- a/pkg/cbox/user/rest/rest.go +++ b/pkg/cbox/user/rest/rest.go @@ -27,7 +27,6 @@ import ( "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 +168,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, @@ -179,18 +180,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: uidNumber, + GidNumber: gidNumber, } if err := m.cacheUserDetails(u); err != nil { @@ -273,6 +264,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, @@ -283,18 +276,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: uidNumber, + GidNumber: gidNumber, } } @@ -385,12 +368,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 index 85d1830b22..4aaf480d8a 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -1528,23 +1528,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 index 3eadeab718..ed0a2e3efd 100644 --- a/pkg/user/manager/demo/demo.go +++ b/pkg/user/manager/demo/demo.go @@ -21,10 +21,10 @@ package demo import ( "context" "errors" + "fmt" "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 fmt.Sprintf("%v", u.UidNumber), 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 e2c23f71c2..509c69b2ef 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 6a02e6c09e..7ea10cd305 100644 --- 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") diff --git a/pkg/user/manager/ldap/ldap.go b/pkg/user/manager/ldap/ldap.go index ad68105bbd..18ec424b7f 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) } From b5464e13a357acd529fc75dfbc0101875f624c36 Mon Sep 17 00:00:00 2001 From: "Zyad A. Ali" Date: Fri, 19 Mar 2021 21:40:54 +0200 Subject: [PATCH 2/5] eosfs: update getUser to check uid and gid Update getUser to verify that uid and gid are not zero to avoid granting access to users by mistake. --- pkg/storage/utils/eosfs/eosfs.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index 4aaf480d8a..ac08478ac1 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 } From 5e99c22b23cfc5b7d51d4f24f660f55893089469 Mon Sep 17 00:00:00 2001 From: "Zyad A. Ali" Date: Mon, 22 Mar 2021 18:42:12 +0200 Subject: [PATCH 3/5] Update test case for JSON user provider integration test Remove "opaque" map and use the correct value for "uid_number". --- tests/integration/grpc/fixtures/users.demo.json | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/integration/grpc/fixtures/users.demo.json b/tests/integration/grpc/fixtures/users.demo.json index cf3ef4b2c8..62b7442aee 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": { From 468b2b15d7666c328fa4fb3e17c7accfc14fc934 Mon Sep 17 00:00:00 2001 From: "Zyad A. Ali" Date: Tue, 23 Mar 2021 14:29:44 +0200 Subject: [PATCH 4/5] Avoid nil pointer dereference in assertGetUserByClaimResponses Update assertGetUserByClaimResponses integration test to verify that user is not nil. --- tests/integration/grpc/userprovider_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/grpc/userprovider_test.go b/tests/integration/grpc/userprovider_test.go index 21f64d44b0..0a19454e81 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")) } }) From 1db79caddb84b5b64e2076d2b604e766d6c31d6e Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Thu, 10 Jun 2021 15:32:18 +0200 Subject: [PATCH 5/5] Minor fixes --- pkg/auth/manager/ldap/ldap.go | 4 ++-- pkg/auth/manager/oidc/oidc.go | 10 +++++----- pkg/cbox/user/rest/rest.go | 19 ++++++++++--------- pkg/storage/utils/eosfs/eosfs.go | 2 +- pkg/user/manager/demo/demo.go | 4 ++-- pkg/user/manager/json/json.go | 4 ++-- 6 files changed, 22 insertions(+), 21 deletions(-) diff --git a/pkg/auth/manager/ldap/ldap.go b/pkg/auth/manager/ldap/ldap.go index 272d327ff6..b7dbfd306c 100644 --- a/pkg/auth/manager/ldap/ldap.go +++ b/pkg/auth/manager/ldap/ldap.go @@ -186,11 +186,11 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string) } gidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.GIDNumber), 10, 64) if err != nil { - return nil, err + return nil, nil, err } uidNumber, err := strconv.ParseInt(sr.Entries[0].GetEqualFoldAttributeValue(am.c.Schema.UIDNumber), 10, 64) if err != nil { - return nil, err + return nil, nil, err } u := &user.User{ Id: userID, diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go index b6e5177cf4..6958828ab3 100644 --- a/pkg/auth/manager/oidc/oidc.go +++ b/pkg/auth/manager/oidc/oidc.go @@ -130,12 +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") } - var uid, gid int64 + var uid, gid float64 if am.c.UIDClaim != "" { - uid, _ = claims[am.c.UIDClaim].(int64) + uid, _ = claims[am.c.UIDClaim].(float64) } if am.c.GIDClaim != "" { - gid, _ = claims[am.c.GIDClaim].(int64) + gid, _ = claims[am.c.GIDClaim].(float64) } userID := &user.UserId{ @@ -167,8 +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), - UidNumber: uid, - GidNumber: gid, + 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 62d32d6f94..c73a2797ef 100644 --- a/pkg/cbox/user/rest/rest.go +++ b/pkg/cbox/user/rest/rest.go @@ -24,6 +24,7 @@ import ( "fmt" "net/url" "regexp" + "strconv" "strings" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -168,8 +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"].(int64) - gidNumber, _ := userData["gid"].(int64) + uidNumber, _ := userData["uid"].(float64) + gidNumber, _ := userData["gid"].(float64) userID := &userpb.UserId{ OpaqueId: upn, @@ -180,8 +181,8 @@ func (m *manager) parseAndCacheUser(ctx context.Context, userData map[string]int Username: upn, Mail: mail, DisplayName: name, - UidNumber: uidNumber, - GidNumber: gidNumber, + UidNumber: int64(uidNumber), + GidNumber: int64(gidNumber), } if err := m.cacheUserDetails(u); err != nil { @@ -264,8 +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"].(int64) - gidNumber, _ := usrInfo["gid"].(int64) + uidNumber, _ := usrInfo["uid"].(float64) + gidNumber, _ := usrInfo["gid"].(float64) uid := &userpb.UserId{ OpaqueId: upn, @@ -276,8 +277,8 @@ func (m *manager) findUsersByFilter(ctx context.Context, url string, users map[s Username: upn, Mail: mail, DisplayName: name, - UidNumber: uidNumber, - GidNumber: gidNumber, + UidNumber: int64(uidNumber), + GidNumber: int64(gidNumber), } } @@ -371,5 +372,5 @@ func extractUID(u *userpb.User) (string, error) { if u.UidNumber == 0 { return "", errors.New("rest: could not retrieve UID from user") } - return fmt.Sprintf("%v", u.UidNumber), nil + return strconv.FormatInt(u.UidNumber, 10), nil } diff --git a/pkg/storage/utils/eosfs/eosfs.go b/pkg/storage/utils/eosfs/eosfs.go index ac08478ac1..0629a879a7 100644 --- a/pkg/storage/utils/eosfs/eosfs.go +++ b/pkg/storage/utils/eosfs/eosfs.go @@ -1540,7 +1540,7 @@ func (fs *eosfs) extractUIDAndGID(u *userpb.User) (string, string, error) { if u.GidNumber == 0 { return "", "", errors.New("eos: gid missing for user") } - return fmt.Sprintf("%v", u.UidNumber), fmt.Sprintf("%v", u.GidNumber), 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 ed0a2e3efd..75e3deec3d 100644 --- a/pkg/user/manager/demo/demo.go +++ b/pkg/user/manager/demo/demo.go @@ -21,7 +21,7 @@ package demo import ( "context" "errors" - "fmt" + "strconv" "strings" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -70,7 +70,7 @@ func extractClaim(u *userpb.User, claim string) (string, error) { return u.Username, nil case "uid": if u.UidNumber != 0 { - return fmt.Sprintf("%v", u.UidNumber), nil + return strconv.FormatInt(u.UidNumber, 10), nil } } return "", errors.New("demo: invalid field") diff --git a/pkg/user/manager/json/json.go b/pkg/user/manager/json/json.go index 7ea10cd305..3e7db910f1 100644 --- a/pkg/user/manager/json/json.go +++ b/pkg/user/manager/json/json.go @@ -21,8 +21,8 @@ package json import ( "context" "encoding/json" - "fmt" "io/ioutil" + "strconv" "strings" "github.com/cs3org/reva/pkg/user" @@ -113,7 +113,7 @@ func extractClaim(u *userpb.User, claim string) (string, error) { return u.Username, nil case "uid": if u.UidNumber != 0 { - return fmt.Sprintf("%v", u.UidNumber), nil + return strconv.FormatInt(u.UidNumber, 10), nil } } return "", errors.New("json: invalid field")