Skip to content

Commit

Permalink
Modify group and user managers to skip fetching specified metadata (c…
Browse files Browse the repository at this point in the history
…s3org#2205)

(cherry picked from commit 9e040c9)
  • Loading branch information
ishank011 authored and rhafer committed Mar 21, 2022
1 parent 25cedab commit abb78f3
Show file tree
Hide file tree
Showing 28 changed files with 337 additions and 167 deletions.
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) {
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 @@ -119,7 +119,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 @@ -137,7 +137,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 {
return &grouppb.FindGroupsResponse{
Status: status.NewInternal(ctx, "error finding groups"),
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 @@ -144,7 +144,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 @@ -163,7 +163,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 {
res := &userpb.FindUsersResponse{
Status: status.NewInternal(ctx, "error finding users"),
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 @@ -143,7 +143,7 @@ func (h *sharesHandler) createShare(w http.ResponseWriter, r *http.Request) {

shareWithParts := strings.Split(shareWith, "@")
userRes, err := gatewayClient.GetUser(ctx, &userpb.GetUserRequest{
UserId: &userpb.UserId{OpaqueId: shareWithParts[0]},
UserId: &userpb.UserId{OpaqueId: shareWithParts[0]}, 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 @@ -51,8 +51,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 {
return nil, &ocsError{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1134,6 +1134,7 @@ func (h *Handler) mustGetIdentifiers(ctx context.Context, client GatewayClient,
GroupId: &grouppb.GroupId{
OpaqueId: id,
},
SkipFetchingMembers: true,
})
if err != nil {
sublog.Err(err).Msg("could not look up group")
Expand Down Expand Up @@ -1163,6 +1164,7 @@ func (h *Handler) mustGetIdentifiers(ctx context.Context, client GatewayClient,
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 @@ -53,8 +53,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 {
return nil, &ocsError{
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

0 comments on commit abb78f3

Please sign in to comment.