From 3e9adea537c66c61793fd69dcdd858f38b445b63 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 3 Nov 2021 16:31:09 +0100 Subject: [PATCH 1/3] Populate owner data in the ocs and ocdav services --- changelog/unreleased/ocs-user-data.md | 3 + .../http/services/owncloud/ocdav/ocdav.go | 17 ++++-- .../http/services/owncloud/ocdav/propfind.go | 55 ++++++++++++++----- .../ocs/handlers/cloud/users/users.go | 17 +++++- 4 files changed, 71 insertions(+), 21 deletions(-) create mode 100644 changelog/unreleased/ocs-user-data.md diff --git a/changelog/unreleased/ocs-user-data.md b/changelog/unreleased/ocs-user-data.md new file mode 100644 index 0000000000..7892d4c4ea --- /dev/null +++ b/changelog/unreleased/ocs-user-data.md @@ -0,0 +1,3 @@ +Enhancement: Populate owner data in the ocs and ocdav services + +https://github.com/cs3org/reva/pull/2233 \ No newline at end of file diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index e53daa7ccf..034bc8c5fa 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -28,6 +28,7 @@ import ( "strings" "time" + "github.com/ReneKroon/ttlcache/v2" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" "github.com/cs3org/reva/pkg/appctx" @@ -114,11 +115,12 @@ func (c *Config) init() { } type svc struct { - c *Config - webDavHandler *WebDavHandler - davHandler *DavHandler - favoritesManager favorite.Manager - client *http.Client + c *Config + webDavHandler *WebDavHandler + davHandler *DavHandler + favoritesManager favorite.Manager + client *http.Client + userIdentifierCache *ttlcache.Cache } func getFavoritesManager(c *Config) (favorite.Manager, error) { @@ -150,8 +152,11 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error) rhttp.Timeout(time.Duration(conf.Timeout*int64(time.Second))), rhttp.Insecure(conf.Insecure), ), - favoritesManager: fm, + favoritesManager: fm, + userIdentifierCache: ttlcache.NewCache(), } + _ = s.userIdentifierCache.SetTTL(60 * time.Second) + // initialize handlers and set default configs if err := s.webDavHandler.init(conf.WebdavNamespace, true); err != nil { return nil, err diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index a9adf2848d..501a74b332 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -537,6 +537,11 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide ) sublog.Debug().Interface("role", role).Str("dav-permissions", wdp).Msg("converted PermissionSet") } + owner, err := s.getOwnerInfo(ctx, md.Owner) + if err != nil { + sublog.Err(err).Interface("owner", owner).Msg("error getting owner info") + return nil, err + } propstatOK := propstatXML{ Status: "HTTP/1.1 200 OK", @@ -732,13 +737,7 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide } case "owner-id": // phoenix only if md.Owner != nil { - if isCurrentUserOwner(ctx, md.Owner) { - u := ctxpkg.ContextMustGetUser(ctx) - propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:owner-id", u.Username)) - } else { - sublog.Debug().Msg("TODO fetch user username") - propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:owner-id", "")) - } + propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:owner-id", owner.Username)) } else { propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:owner-id", "")) } @@ -817,13 +816,7 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide } case "owner-display-name": // phoenix only if md.Owner != nil { - if isCurrentUserOwner(ctx, md.Owner) { - u := ctxpkg.ContextMustGetUser(ctx) - propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:owner-display-name", u.DisplayName)) - } else { - sublog.Debug().Msg("TODO fetch user displayname") - propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:owner-display-name", "")) - } + propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:owner-display-name", owner.DisplayName)) } else { propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:owner-display-name", "")) } @@ -997,6 +990,40 @@ func quoteEtag(etag string) string { return `"` + strings.Trim(etag, `"`) + `"` } +func (s *svc) getOwnerInfo(ctx context.Context, owner *userv1beta1.UserId) (*userv1beta1.User, error) { + if isCurrentUserOwner(ctx, owner) { + return ctxpkg.ContextMustGetUser(ctx), nil + } + + log := appctx.GetLogger(ctx) + if idIf, err := s.userIdentifierCache.Get(owner.OpaqueId); err == nil { + log.Debug().Msg("cache hit") + return idIf.(*userv1beta1.User), nil + } + + client, err := s.getClient() + if err != nil { + log.Error().Err(err).Msg("error getting grpc client") + return nil, err + } + + res, err := client.GetUser(ctx, &userv1beta1.GetUserRequest{UserId: owner}) + if err != nil { + log.Err(err).Msg("could not look up user") + return nil, err + } + if res.GetStatus().GetCode() != rpc.Code_CODE_OK { + log.Err(err).Msg("get user call failed") + return nil, err + } + if res.User == nil { + log.Debug().Msg("user not found") + return nil, err + } + _ = s.userIdentifierCache.Set(owner.OpaqueId, res.User) + return res.User, nil +} + // a file is only yours if you are the owner func isCurrentUserOwner(ctx context.Context, owner *userv1beta1.UserId) bool { contextUser, ok := ctxpkg.ContextGetUser(ctx) diff --git a/internal/http/services/owncloud/ocs/handlers/cloud/users/users.go b/internal/http/services/owncloud/ocs/handlers/cloud/users/users.go index 38436d20a9..ce97004bc0 100644 --- a/internal/http/services/owncloud/ocs/handlers/cloud/users/users.go +++ b/internal/http/services/owncloud/ocs/handlers/cloud/users/users.go @@ -49,7 +49,22 @@ func (h *Handler) Init(c *config.Config) { // GetGroups handles GET requests on /cloud/users/groups // TODO: implement func (h *Handler) GetGroups(w http.ResponseWriter, r *http.Request) { - response.WriteOCSSuccess(w, r, &Groups{}) + ctx := r.Context() + + user := chi.URLParam(r, "userid") + // FIXME use ldap to fetch user info + u, ok := ctxpkg.ContextGetUser(ctx) + if !ok { + response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "missing user in context", fmt.Errorf("missing user in context")) + return + } + if user != u.Username { + // FIXME allow fetching other users info? only for admins + response.WriteOCSError(w, r, http.StatusForbidden, "user id mismatch", fmt.Errorf("%s tried to access %s user info endpoint", u.Id.OpaqueId, user)) + return + } + + response.WriteOCSSuccess(w, r, &Groups{Groups: u.Groups}) } // Quota holds quota information From 0cb25a0d2024c66c732dc2f7c25604e5084d90f7 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Wed, 3 Nov 2021 17:09:37 +0100 Subject: [PATCH 2/3] Handle the case when owner is nil --- .../http/services/owncloud/ocdav/propfind.go | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index 501a74b332..57562feaad 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -41,6 +41,7 @@ import ( "github.com/cs3org/reva/internal/http/services/owncloud/ocs/conversions" "github.com/cs3org/reva/pkg/appctx" ctxpkg "github.com/cs3org/reva/pkg/ctx" + "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/publicshare" rtrace "github.com/cs3org/reva/pkg/trace" "github.com/cs3org/reva/pkg/utils" @@ -537,10 +538,12 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide ) sublog.Debug().Interface("role", role).Str("dav-permissions", wdp).Msg("converted PermissionSet") } + + var ownerUsername, ownerDisplayName string owner, err := s.getOwnerInfo(ctx, md.Owner) - if err != nil { - sublog.Err(err).Interface("owner", owner).Msg("error getting owner info") - return nil, err + if err == nil { + ownerUsername = owner.Username + ownerDisplayName = owner.DisplayName } propstatOK := propstatXML{ @@ -736,8 +739,8 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:size", "")) } case "owner-id": // phoenix only - if md.Owner != nil { - propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:owner-id", owner.Username)) + if ownerUsername != "" { + propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:owner-id", ownerUsername)) } else { propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:owner-id", "")) } @@ -815,8 +818,8 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:"+pf.Prop[i].Local, "")) } case "owner-display-name": // phoenix only - if md.Owner != nil { - propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:owner-display-name", owner.DisplayName)) + if ownerDisplayName != "" { + propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:owner-display-name", ownerDisplayName)) } else { propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:owner-display-name", "")) } @@ -991,6 +994,10 @@ func quoteEtag(etag string) string { } func (s *svc) getOwnerInfo(ctx context.Context, owner *userv1beta1.UserId) (*userv1beta1.User, error) { + if owner == nil { + return nil, errtypes.NotFound("owner is nil") + } + if isCurrentUserOwner(ctx, owner) { return ctxpkg.ContextMustGetUser(ctx), nil } From 1449da5613c09289cdec7ecdf08939d3a3500098 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Thu, 4 Nov 2021 14:32:54 +0100 Subject: [PATCH 3/3] Only fetch owner info when needed --- internal/http/services/owncloud/ocdav/propfind.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index 57562feaad..7fd60c19cc 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -539,13 +539,6 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide sublog.Debug().Interface("role", role).Str("dav-permissions", wdp).Msg("converted PermissionSet") } - var ownerUsername, ownerDisplayName string - owner, err := s.getOwnerInfo(ctx, md.Owner) - if err == nil { - ownerUsername = owner.Username - ownerDisplayName = owner.DisplayName - } - propstatOK := propstatXML{ Status: "HTTP/1.1 200 OK", Prop: []*propertyXML{}, @@ -650,6 +643,13 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide // TODO return other properties ... but how do we put them in a namespace? } else { // otherwise return only the requested properties + var ownerUsername, ownerDisplayName string + owner, err := s.getOwnerInfo(ctx, md.Owner) + if err == nil { + ownerUsername = owner.Username + ownerDisplayName = owner.DisplayName + } + for i := range pf.Prop { switch pf.Prop[i].Space { case _nsOwncloud: