Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify group and user managers to skip fetching specified metadata #2205

Merged
merged 4 commits into from
Feb 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/unreleased/fetch-groups-members-config.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Enhancement: Modify group and user managers to skip fetching specified metadata

https://github.com/cs3org/reva/pull/2205
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) {
ishank011 marked this conversation as resolved.
Show resolved Hide resolved
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
2 changes: 1 addition & 1 deletion internal/grpc/interceptors/auth/scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func checkIfNestedResource(ctx context.Context, ref *provider.Reference, parent
// We mint a token as the owner of the public share and try to stat the reference
// TODO(ishank011): We need to find a better alternative to this

userResp, err := client.GetUser(ctx, &userpb.GetUserRequest{UserId: statResponse.Info.Owner})
userResp, err := client.GetUser(ctx, &userpb.GetUserRequest{UserId: statResponse.Info.Owner, SkipFetchingUserGroups: true})
if err != nil || userResp.Status.Code != rpc.Code_CODE_OK {
return false, err
}
Expand Down
6 changes: 3 additions & 3 deletions internal/grpc/services/groupprovider/groupprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (s *service) Register(ss *grpc.Server) {
}

func (s *service) GetGroup(ctx context.Context, req *grouppb.GetGroupRequest) (*grouppb.GetGroupResponse, error) {
group, err := s.groupmgr.GetGroup(ctx, req.GroupId)
group, err := s.groupmgr.GetGroup(ctx, req.GroupId, req.SkipFetchingMembers)
if err != nil {
res := &grouppb.GetGroupResponse{}
if _, ok := err.(errtypes.NotFound); ok {
Expand All @@ -120,7 +120,7 @@ func (s *service) GetGroup(ctx context.Context, req *grouppb.GetGroupRequest) (*
}

func (s *service) GetGroupByClaim(ctx context.Context, req *grouppb.GetGroupByClaimRequest) (*grouppb.GetGroupByClaimResponse, error) {
group, err := s.groupmgr.GetGroupByClaim(ctx, req.Claim, req.Value)
group, err := s.groupmgr.GetGroupByClaim(ctx, req.Claim, req.Value, req.SkipFetchingMembers)
if err != nil {
res := &grouppb.GetGroupByClaimResponse{}
if _, ok := err.(errtypes.NotFound); ok {
Expand All @@ -139,7 +139,7 @@ func (s *service) GetGroupByClaim(ctx context.Context, req *grouppb.GetGroupByCl
}

func (s *service) FindGroups(ctx context.Context, req *grouppb.FindGroupsRequest) (*grouppb.FindGroupsResponse, error) {
groups, err := s.groupmgr.FindGroups(ctx, req.Filter)
groups, err := s.groupmgr.FindGroups(ctx, req.Filter, req.SkipFetchingMembers)
if err != nil {
err = errors.Wrap(err, "groupprovidersvc: error finding groups")
return &grouppb.FindGroupsResponse{
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 {
res := &userpb.GetUserResponse{}
if _, ok := err.(errtypes.NotFound); ok {
Expand All @@ -145,7 +145,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 {
res := &userpb.GetUserByClaimResponse{}
if _, ok := err.(errtypes.NotFound); ok {
Expand All @@ -165,7 +165,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
2 changes: 1 addition & 1 deletion internal/http/services/ocmd/shares.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (h *sharesHandler) createShare(w http.ResponseWriter, r *http.Request) {
}

userRes, err := gatewayClient.GetUser(ctx, &userpb.GetUserRequest{
UserId: &userpb.UserId{OpaqueId: shareWith},
UserId: &userpb.UserId{OpaqueId: shareWith}, SkipFetchingUserGroups: true,
})
if err != nil {
WriteError(w, r, APIErrorServerError, "error searching recipient", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (h *Handler) FindSharees(w http.ResponseWriter, r *http.Request) {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting gateway grpc client", err)
return
}
usersRes, err := gwc.FindUsers(r.Context(), &userpb.FindUsersRequest{Filter: term})
usersRes, err := gwc.FindUsers(r.Context(), &userpb.FindUsersRequest{Filter: term, SkipFetchingUserGroups: true})
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error searching users", err)
return
Expand All @@ -73,7 +73,7 @@ func (h *Handler) FindSharees(w http.ResponseWriter, r *http.Request) {
userMatches = append(userMatches, match)
}

groupsRes, err := gwc.FindGroups(r.Context(), &grouppb.FindGroupsRequest{Filter: term})
groupsRes, err := gwc.FindGroups(r.Context(), &grouppb.FindGroupsRequest{Filter: term, SkipFetchingMembers: true})
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error searching groups", err)
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ func (h *Handler) createGroupShare(w http.ResponseWriter, r *http.Request, statI
}

groupRes, err := c.GetGroupByClaim(ctx, &grouppb.GetGroupByClaimRequest{
Claim: "group_name",
Value: shareWith,
Claim: "group_name",
Value: shareWith,
SkipFetchingMembers: true,
})
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error searching recipient", err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,7 @@ func (h *Handler) mustGetIdentifiers(ctx context.Context, client gateway.Gateway
GroupId: &grouppb.GroupId{
OpaqueId: id,
},
SkipFetchingMembers: true,
})
if err != nil {
sublog.Err(err).Msg("could not look up group")
Expand Down Expand Up @@ -947,6 +948,7 @@ func (h *Handler) mustGetIdentifiers(ctx context.Context, client gateway.Gateway
UserId: &userpb.UserId{
OpaqueId: id,
},
SkipFetchingUserGroups: true,
})
if err != nil {
sublog.Err(err).Msg("could not look up user")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ func (h *Handler) createUserShare(w http.ResponseWriter, r *http.Request, statIn
}

userRes, err := c.GetUserByClaim(ctx, &userpb.GetUserByClaimRequest{
Claim: "username",
Value: shareWith,
Claim: "username",
Value: shareWith,
SkipFetchingUserGroups: true,
})
if err != nil {
response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error searching recipient", err)
Expand Down
42 changes: 28 additions & 14 deletions pkg/cbox/group/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func (m *manager) parseAndCacheGroup(ctx context.Context, groupData map[string]i

}

func (m *manager) GetGroup(ctx context.Context, gid *grouppb.GroupId) (*grouppb.Group, error) {
func (m *manager) GetGroup(ctx context.Context, gid *grouppb.GroupId, skipFetchingMembers bool) (*grouppb.Group, error) {
g, err := m.fetchCachedGroupDetails(gid)
if err != nil {
groupData, err := m.getGroupByParam(ctx, "groupIdentifier", gid.OpaqueId)
Expand All @@ -202,20 +202,22 @@ func (m *manager) GetGroup(ctx context.Context, gid *grouppb.GroupId) (*grouppb.
g = m.parseAndCacheGroup(ctx, groupData)
}

groupMembers, err := m.GetMembers(ctx, gid)
if err != nil {
return nil, err
if !skipFetchingMembers {
groupMembers, err := m.GetMembers(ctx, gid)
if err != nil {
return nil, err
}
g.Members = groupMembers
}
g.Members = groupMembers

return g, nil
}

func (m *manager) GetGroupByClaim(ctx context.Context, claim, value string) (*grouppb.Group, error) {
func (m *manager) GetGroupByClaim(ctx context.Context, claim, value string, skipFetchingMembers bool) (*grouppb.Group, error) {
value = url.QueryEscape(value)
opaqueID, err := m.fetchCachedParam(claim, value)
if err == nil {
return m.GetGroup(ctx, &grouppb.GroupId{OpaqueId: opaqueID})
return m.GetGroup(ctx, &grouppb.GroupId{OpaqueId: opaqueID}, skipFetchingMembers)
}

switch claim {
Expand All @@ -236,17 +238,19 @@ func (m *manager) GetGroupByClaim(ctx context.Context, claim, value string) (*gr
}
g := m.parseAndCacheGroup(ctx, groupData)

groupMembers, err := m.GetMembers(ctx, g.Id)
if err != nil {
return nil, err
if !skipFetchingMembers {
groupMembers, err := m.GetMembers(ctx, g.Id)
if err != nil {
return nil, err
}
g.Members = groupMembers
}
g.Members = groupMembers

return g, nil

}

func (m *manager) findGroupsByFilter(ctx context.Context, url string, groups map[string]*grouppb.Group) error {
func (m *manager) findGroupsByFilter(ctx context.Context, url string, groups map[string]*grouppb.Group, skipFetchingMembers bool) error {

groupData, err := m.apiTokenManager.SendAPIGetRequest(ctx, url, false)
if err != nil {
Expand All @@ -265,23 +269,33 @@ func (m *manager) findGroupsByFilter(ctx context.Context, url string, groups map
OpaqueId: id,
Idp: m.conf.IDProvider,
}

var groupMembers []*userpb.UserId
if !skipFetchingMembers {
groupMembers, err = m.GetMembers(ctx, groupID)
if err != nil {
return err
}
}
gid, ok := grpInfo["gid"].(int64)
if !ok {
gid = 0
}

groups[groupID.OpaqueId] = &grouppb.Group{
Id: groupID,
GroupName: id,
Mail: id + "@cern.ch",
DisplayName: name,
GidNumber: gid,
Members: groupMembers,
}
}

return nil
}

func (m *manager) FindGroups(ctx context.Context, query string) ([]*grouppb.Group, error) {
func (m *manager) FindGroups(ctx context.Context, query string, skipFetchingMembers bool) ([]*grouppb.Group, error) {

// Look at namespaces filters. If the query starts with:
// "a" or none => get egroups
Expand All @@ -308,7 +322,7 @@ func (m *manager) FindGroups(ctx context.Context, query string) ([]*grouppb.Grou
for _, f := range filters {
url := fmt.Sprintf("%s/Group/?filter=%s:contains:%s&field=groupIdentifier&field=displayName&field=gid",
m.conf.APIBaseURL, f, url.QueryEscape(query))
err := m.findGroupsByFilter(ctx, url, groups)
err := m.findGroupsByFilter(ctx, url, groups, skipFetchingMembers)
if err != nil {
return nil, err
}
Expand Down
Loading