Skip to content

Commit

Permalink
Modify user managers to skip fetching groups when specified
Browse files Browse the repository at this point in the history
  • Loading branch information
ishank011 committed Oct 26, 2021
1 parent fe35bd1 commit 3d4e609
Show file tree
Hide file tree
Showing 15 changed files with 214 additions and 115 deletions.
26 changes: 19 additions & 7 deletions examples/plugin/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,28 @@ func (m *Manager) Configure(ml map[string]interface{}) error {
}

// GetUser returns the user based on the uid.
func (m *Manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User, error) {
func (m *Manager) GetUser(ctx context.Context, uid *userpb.UserId, skipFetchingGroups bool) (*userpb.User, error) {
for _, u := range m.users {
if (u.Id.GetOpaqueId() == uid.OpaqueId || u.Username == uid.OpaqueId) && (uid.Idp == "" || uid.Idp == u.Id.GetIdp()) {
return u, nil
user := *u
if skipFetchingGroups {
user.Groups = nil
}
return &user, nil
}
}
return nil, nil
}

// GetUserByClaim returns user based on the claim
func (m *Manager) GetUserByClaim(ctx context.Context, claim, value string) (*userpb.User, error) {
func (m *Manager) GetUserByClaim(ctx context.Context, claim, value string, skipFetchingGroups bool) (*userpb.User, error) {
for _, u := range m.users {
if userClaim, err := extractClaim(u, claim); err == nil && value == userClaim {
return u, nil
user := *u
if skipFetchingGroups {
user.Groups = nil
}
return &user, nil
}
}
return nil, errtypes.NotFound(value)
Expand Down Expand Up @@ -126,19 +134,23 @@ func userContains(u *userpb.User, query string) bool {
}

// FindUsers returns the user based on the query
func (m *Manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, error) {
func (m *Manager) FindUsers(ctx context.Context, query string, skipFetchingGroups bool) ([]*userpb.User, error) {
users := []*userpb.User{}
for _, u := range m.users {
if userContains(u, query) {
users = append(users, u)
user := *u
if skipFetchingGroups {
user.Groups = nil
}
users = append(users, &user)
}
}
return users, nil
}

// GetUserGroups returns the user groups
func (m *Manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]string, error) {
user, err := m.GetUser(ctx, uid)
user, err := m.GetUser(ctx, uid, false)
if err != nil {
return nil, err
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ require (
go 1.16

replace (
github.com/cs3org/go-cs3apis => github.com/ishank011/go-cs3apis v0.0.0-20211026072245-95303cdc06dc
github.com/eventials/go-tus => github.com/andrewmostello/go-tus v0.0.0-20200314041820-904a9904af9a
github.com/oleiade/reflections => github.com/oleiade/reflections v1.0.1
google.golang.org/grpc => google.golang.org/grpc v1.26.0 // temporary downgrade
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ github.com/coreos/go-systemd/v22 v22.3.2/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSV
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJffz4pz0o1WuQxJ28+5x5JgaHD8=
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4=
github.com/cs3org/go-cs3apis v0.0.0-20211018122138-391b29bd7803 h1:R/6llgTNKxQQ7GaSTgFn6Fp8N50wIlagmdR7WY5LntM=
github.com/cs3org/go-cs3apis v0.0.0-20211018122138-391b29bd7803/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8 h1:Z9lwXumT5ACSmJ7WGnFl+OMLLjpz5uR2fyz7dC255FI=
github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8/go.mod h1:4abs/jPXcmJzYoYGF91JF9Uq9s/KL5n1jvFDix8KcqY=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down Expand Up @@ -380,6 +378,8 @@ github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:
github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/ishank011/go-cs3apis v0.0.0-20211026072245-95303cdc06dc h1:S2KTqUo/trscihgP250SJoXA+JRVNIk/FWXIgFy32D0=
github.com/ishank011/go-cs3apis v0.0.0-20211026072245-95303cdc06dc/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/jedib0t/go-pretty v4.3.0+incompatible h1:CGs8AVhEKg/n9YbUenWmNStRW2PHJzaeDodcfvRAbIo=
github.com/jedib0t/go-pretty v4.3.0+incompatible/go.mod h1:XemHduiw8R651AF9Pt4FwCTKeG3oo7hrHJAoznj9nag=
github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4=
Expand Down
6 changes: 3 additions & 3 deletions internal/grpc/services/userprovider/userprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func (s *service) Register(ss *grpc.Server) {
}

func (s *service) GetUser(ctx context.Context, req *userpb.GetUserRequest) (*userpb.GetUserResponse, error) {
user, err := s.usermgr.GetUser(ctx, req.UserId)
user, err := s.usermgr.GetUser(ctx, req.UserId, req.SkipFetchingUserGroups)
if err != nil {
// TODO(labkode): check for not found.
err = errors.Wrap(err, "userprovidersvc: error getting user")
Expand All @@ -143,7 +143,7 @@ func (s *service) GetUser(ctx context.Context, req *userpb.GetUserRequest) (*use
}

func (s *service) GetUserByClaim(ctx context.Context, req *userpb.GetUserByClaimRequest) (*userpb.GetUserByClaimResponse, error) {
user, err := s.usermgr.GetUserByClaim(ctx, req.Claim, req.Value)
user, err := s.usermgr.GetUserByClaim(ctx, req.Claim, req.Value, req.SkipFetchingUserGroups)
if err != nil {
// TODO(labkode): check for not found.
err = errors.Wrap(err, "userprovidersvc: error getting user by claim")
Expand All @@ -161,7 +161,7 @@ func (s *service) GetUserByClaim(ctx context.Context, req *userpb.GetUserByClaim
}

func (s *service) FindUsers(ctx context.Context, req *userpb.FindUsersRequest) (*userpb.FindUsersResponse, error) {
users, err := s.usermgr.FindUsers(ctx, req.Filter)
users, err := s.usermgr.FindUsers(ctx, req.Filter, req.SkipFetchingUserGroups)
if err != nil {
err = errors.Wrap(err, "userprovidersvc: error finding users")
res := &userpb.FindUsersResponse{
Expand Down
41 changes: 27 additions & 14 deletions pkg/cbox/user/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (m *manager) parseAndCacheUser(ctx context.Context, userData map[string]int

}

func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User, error) {
func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId, skipFetchingGroups bool) (*userpb.User, error) {
u, err := m.fetchCachedUserDetails(uid)
if err != nil {
userData, err := m.getUserByParam(ctx, "upn", uid.OpaqueId)
Expand All @@ -224,19 +224,21 @@ func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User
u = m.parseAndCacheUser(ctx, userData)
}

userGroups, err := m.GetUserGroups(ctx, uid)
if err != nil {
return nil, err
if !skipFetchingGroups {
userGroups, err := m.GetUserGroups(ctx, uid)
if err != nil {
return nil, err
}
u.Groups = userGroups
}
u.Groups = userGroups

return u, nil
}

func (m *manager) GetUserByClaim(ctx context.Context, claim, value string) (*userpb.User, error) {
func (m *manager) GetUserByClaim(ctx context.Context, claim, value string, skipFetchingGroups bool) (*userpb.User, error) {
opaqueID, err := m.fetchCachedParam(claim, value)
if err == nil {
return m.GetUser(ctx, &userpb.UserId{OpaqueId: opaqueID})
return m.GetUser(ctx, &userpb.UserId{OpaqueId: opaqueID}, skipFetchingGroups)
}

switch claim {
Expand All @@ -256,17 +258,19 @@ func (m *manager) GetUserByClaim(ctx context.Context, claim, value string) (*use
}
u := m.parseAndCacheUser(ctx, userData)

userGroups, err := m.GetUserGroups(ctx, u.Id)
if err != nil {
return nil, err
if !skipFetchingGroups {
userGroups, err := m.GetUserGroups(ctx, u.Id)
if err != nil {
return nil, err
}
u.Groups = userGroups
}
u.Groups = userGroups

return u, nil

}

func (m *manager) findUsersByFilter(ctx context.Context, url string, users map[string]*userpb.User) error {
func (m *manager) findUsersByFilter(ctx context.Context, url string, users map[string]*userpb.User, skipFetchingGroups bool) error {

userData, err := m.apiTokenManager.SendAPIGetRequest(ctx, url, false)
if err != nil {
Expand Down Expand Up @@ -296,20 +300,29 @@ func (m *manager) findUsersByFilter(ctx context.Context, url string, users map[s
Idp: m.conf.IDProvider,
Type: userType,
}
var userGroups []string
if !skipFetchingGroups {
userGroups, err = m.GetUserGroups(ctx, uid)
if err != nil {
return err
}
}

users[uid.OpaqueId] = &userpb.User{
Id: uid,
Username: upn,
Mail: mail,
DisplayName: name,
UidNumber: int64(uidNumber),
GidNumber: int64(gidNumber),
Groups: userGroups,
}
}

return nil
}

func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, error) {
func (m *manager) FindUsers(ctx context.Context, query string, skipFetchingGroups bool) ([]*userpb.User, error) {

// Look at namespaces filters. If the query starts with:
// "a" => look into primary/secondary/service accounts
Expand Down Expand Up @@ -339,7 +352,7 @@ func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User,
for _, f := range filters {
url := fmt.Sprintf("%s/Identity/?filter=%s:contains:%s&field=id&field=upn&field=primaryAccountEmail&field=displayName&field=uid&field=gid&field=type",
m.conf.APIBaseURL, f, url.QueryEscape(query))
err := m.findUsersByFilter(ctx, url, users)
err := m.findUsersByFilter(ctx, url, users, skipFetchingGroups)
if err != nil {
return nil, err
}
Expand Down
26 changes: 19 additions & 7 deletions pkg/user/manager/demo/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,27 @@ func (m *manager) Configure(ml map[string]interface{}) error {
return nil
}

func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId) (*userpb.User, error) {
func (m *manager) GetUser(ctx context.Context, uid *userpb.UserId, skipFetchingGroups bool) (*userpb.User, error) {
if user, ok := m.catalog[uid.OpaqueId]; ok {
if uid.Idp == "" || user.Id.Idp == uid.Idp {
return user, nil
u := *user
if skipFetchingGroups {
u.Groups = nil
}
return &u, nil
}
}
return nil, errtypes.NotFound(uid.OpaqueId)
}

func (m *manager) GetUserByClaim(ctx context.Context, claim, value string) (*userpb.User, error) {
func (m *manager) GetUserByClaim(ctx context.Context, claim, value string, skipFetchingGroups bool) (*userpb.User, error) {
for _, u := range m.catalog {
if userClaim, err := extractClaim(u, claim); err == nil && value == userClaim {
return u, nil
user := *u
if skipFetchingGroups {
user.Groups = nil
}
return &user, nil
}
}
return nil, errtypes.NotFound(value)
Expand All @@ -91,18 +99,22 @@ func userContains(u *userpb.User, query string) bool {
return strings.Contains(u.Username, query) || strings.Contains(u.DisplayName, query) || strings.Contains(u.Mail, query) || strings.Contains(u.Id.OpaqueId, query)
}

func (m *manager) FindUsers(ctx context.Context, query string) ([]*userpb.User, error) {
func (m *manager) FindUsers(ctx context.Context, query string, skipFetchingGroups bool) ([]*userpb.User, error) {
users := []*userpb.User{}
for _, u := range m.catalog {
if userContains(u, query) {
users = append(users, u)
user := *u
if skipFetchingGroups {
user.Groups = nil
}
users = append(users, &user)
}
}
return users, nil
}

func (m *manager) GetUserGroups(ctx context.Context, uid *userpb.UserId) ([]string, error) {
user, err := m.GetUser(ctx, uid)
user, err := m.GetUser(ctx, uid, false)
if err != nil {
return nil, err
}
Expand Down
53 changes: 34 additions & 19 deletions pkg/user/manager/demo/demo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,49 +44,64 @@ func TestUserManager(t *testing.T) {
UidNumber: 123,
GidNumber: 987,
}
uidFake := &userpb.UserId{Idp: "nonesense", OpaqueId: "fakeUser"}
groupsEinstein := []string{"sailing-lovers", "violin-haters", "physics-lovers"}

// positive test GetUserGroups
resGroups, _ := manager.GetUserGroups(ctx, uidEinstein)
if !reflect.DeepEqual(resGroups, groupsEinstein) {
t.Fatalf("groups differ: expected=%v got=%v", groupsEinstein, resGroups)
userEinsteinWithoutGroups := &userpb.User{
Id: uidEinstein,
Username: "einstein",
Mail: "einstein@example.org",
DisplayName: "Albert Einstein",
UidNumber: 123,
GidNumber: 987,
}

// negative test GetUserGroups
expectedErr := errtypes.NotFound(uidFake.OpaqueId)
_, err := manager.GetUserGroups(ctx, uidFake)
if !reflect.DeepEqual(err, expectedErr) {
t.Fatalf("user not found error differs: expected='%v' got='%v'", expectedErr, err)
}
uidFake := &userpb.UserId{Idp: "nonesense", OpaqueId: "fakeUser"}
groupsEinstein := []string{"sailing-lovers", "violin-haters", "physics-lovers"}

// positive test GetUserByClaim by uid
resUserByUID, _ := manager.GetUserByClaim(ctx, "uid", "123")
resUserByUID, _ := manager.GetUserByClaim(ctx, "uid", "123", false)
if !reflect.DeepEqual(resUserByUID, userEinstein) {
t.Fatalf("user differs: expected=%v got=%v", userEinstein, resUserByUID)
}

// negative test GetUserByClaim by uid
expectedErr = errtypes.NotFound("789")
_, err = manager.GetUserByClaim(ctx, "uid", "789")
expectedErr := errtypes.NotFound("789")
_, err := manager.GetUserByClaim(ctx, "uid", "789", false)
if !reflect.DeepEqual(err, expectedErr) {
t.Fatalf("user not found error differs: expected='%v' got='%v'", expectedErr, err)
}

// positive test GetUserByClaim by mail
resUserByEmail, _ := manager.GetUserByClaim(ctx, "mail", "einstein@example.org")
resUserByEmail, _ := manager.GetUserByClaim(ctx, "mail", "einstein@example.org", false)
if !reflect.DeepEqual(resUserByEmail, userEinstein) {
t.Fatalf("user differs: expected=%v got=%v", userEinstein, resUserByEmail)
}

// positive test GetUserByClaim by uid without groups
resUserByUIDWithoutGroups, _ := manager.GetUserByClaim(ctx, "uid", "123", true)
if !reflect.DeepEqual(resUserByUIDWithoutGroups, userEinsteinWithoutGroups) {
t.Fatalf("user differs: expected=%v got=%v", userEinsteinWithoutGroups, resUserByUIDWithoutGroups)
}

// positive test GetUserGroups
resGroups, _ := manager.GetUserGroups(ctx, uidEinstein)
if !reflect.DeepEqual(resGroups, groupsEinstein) {
t.Fatalf("groups differ: expected=%v got=%v", groupsEinstein, resGroups)
}

// negative test GetUserGroups
expectedErr = errtypes.NotFound(uidFake.OpaqueId)
_, err = manager.GetUserGroups(ctx, uidFake)
if !reflect.DeepEqual(err, expectedErr) {
t.Fatalf("user not found error differs: expected='%v' got='%v'", expectedErr, err)
}

// test FindUsers
resUser, _ := manager.FindUsers(ctx, "einstein")
resUser, _ := manager.FindUsers(ctx, "einstein", false)
if !reflect.DeepEqual(resUser, []*userpb.User{userEinstein}) {
t.Fatalf("user differs: expected=%v got=%v", []*userpb.User{userEinstein}, resUser)
}

// negative test FindUsers
resUsers, _ := manager.FindUsers(ctx, "notARealUser")
resUsers, _ := manager.FindUsers(ctx, "notARealUser", false)
if len(resUsers) > 0 {
t.Fatalf("user not in group: expected=%v got=%v", []*userpb.User{}, resUsers)
}
Expand Down
Loading

0 comments on commit 3d4e609

Please sign in to comment.