From 47119beffae215190124e98e063f1734b9e75938 Mon Sep 17 00:00:00 2001 From: Ishank Arora Date: Fri, 15 Jan 2021 15:05:35 +0100 Subject: [PATCH] Use the user in request for deciding the layout for non-home DAV requests (#1401) --- changelog/unreleased/fix-dav-namespace.md | 16 +++++++++ internal/http/services/owncloud/ocdav/copy.go | 2 -- internal/http/services/owncloud/ocdav/dav.go | 6 ++-- .../http/services/owncloud/ocdav/delete.go | 2 -- internal/http/services/owncloud/ocdav/get.go | 2 -- internal/http/services/owncloud/ocdav/head.go | 2 -- .../http/services/owncloud/ocdav/mkcol.go | 2 -- internal/http/services/owncloud/ocdav/move.go | 2 -- .../http/services/owncloud/ocdav/ocdav.go | 19 ++++++++-- .../http/services/owncloud/ocdav/propfind.go | 2 -- .../http/services/owncloud/ocdav/proppatch.go | 2 -- internal/http/services/owncloud/ocdav/put.go | 2 -- internal/http/services/owncloud/ocdav/tus.go | 2 -- .../http/services/owncloud/ocdav/webdav.go | 35 ++++++++++--------- 14 files changed, 54 insertions(+), 42 deletions(-) create mode 100644 changelog/unreleased/fix-dav-namespace.md diff --git a/changelog/unreleased/fix-dav-namespace.md b/changelog/unreleased/fix-dav-namespace.md new file mode 100644 index 0000000000..e2d565529a --- /dev/null +++ b/changelog/unreleased/fix-dav-namespace.md @@ -0,0 +1,16 @@ +Bugfix: Use the user in request for deciding the layout for non-home DAV requests + +For the incoming /dav/files/userID requests, we have different namespaces +depending on whether the request is for the logged-in user's namespace or not. +Since in the storage drivers, we specify the layout depending only on the user +whose resources are to be accessed, this fails when a user wants to access +another user's namespace when the storage provider depends on the logged in +user's namespace. This PR fixes that. + +For example, consider the following case. The owncloud fs uses a layout +{{substr 0 1 .Id.OpaqueId}}/{{.Id.OpaqueId}}. The user einstein sends a request +to access a resource shared with him, say /dav/files/marie/abcd, which should be +allowed. However, based on the way we applied the layout, there's no way in +which this can be translated to /m/marie/. + +https://github.com/cs3org/reva/pull/1401 diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index 819ae73ae3..1a7e29a704 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -40,8 +40,6 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { ctx, span := trace.StartSpan(ctx, "head") defer span.End() - ns = applyLayout(ctx, ns) - src := path.Join(ns, r.URL.Path) dstHeader := r.Header.Get("Destination") overwrite := r.Header.Get("Overwrite") diff --git a/internal/http/services/owncloud/ocdav/dav.go b/internal/http/services/owncloud/ocdav/dav.go index cfbc05830c..e3d17a7caf 100644 --- a/internal/http/services/owncloud/ocdav/dav.go +++ b/internal/http/services/owncloud/ocdav/dav.go @@ -57,11 +57,11 @@ func (h *DavHandler) init(c *Config) error { return err } h.FilesHandler = new(WebDavHandler) - if err := h.FilesHandler.init(c.FilesNamespace); err != nil { + if err := h.FilesHandler.init(c.FilesNamespace, false); err != nil { return err } h.FilesHomeHandler = new(WebDavHandler) - if err := h.FilesHomeHandler.init(c.WebdavNamespace); err != nil { + if err := h.FilesHomeHandler.init(c.WebdavNamespace, true); err != nil { return err } h.MetaHandler = new(MetaHandler) @@ -71,7 +71,7 @@ func (h *DavHandler) init(c *Config) error { h.TrashbinHandler = new(TrashbinHandler) h.PublicFolderHandler = new(WebDavHandler) - if err := h.PublicFolderHandler.init("public"); err != nil { // jail public file requests to /public/ prefix + if err := h.PublicFolderHandler.init("public", true); err != nil { // jail public file requests to /public/ prefix return err } diff --git a/internal/http/services/owncloud/ocdav/delete.go b/internal/http/services/owncloud/ocdav/delete.go index c18ca503e0..0bdf55561a 100644 --- a/internal/http/services/owncloud/ocdav/delete.go +++ b/internal/http/services/owncloud/ocdav/delete.go @@ -33,8 +33,6 @@ func (s *svc) handleDelete(w http.ResponseWriter, r *http.Request, ns string) { ctx, span := trace.StartSpan(ctx, "head") defer span.End() - ns = applyLayout(ctx, ns) - fn := path.Join(ns, r.URL.Path) sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() diff --git a/internal/http/services/owncloud/ocdav/get.go b/internal/http/services/owncloud/ocdav/get.go index 94239be64e..5e892e8337 100644 --- a/internal/http/services/owncloud/ocdav/get.go +++ b/internal/http/services/owncloud/ocdav/get.go @@ -40,8 +40,6 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) { ctx, span := trace.StartSpan(ctx, "get") defer span.End() - ns = applyLayout(ctx, ns) - fn := path.Join(ns, r.URL.Path) sublog := appctx.GetLogger(ctx).With().Str("path", fn).Str("svc", "ocdav").Str("handler", "get").Logger() diff --git a/internal/http/services/owncloud/ocdav/head.go b/internal/http/services/owncloud/ocdav/head.go index 6b4849b701..13d4f1fb02 100644 --- a/internal/http/services/owncloud/ocdav/head.go +++ b/internal/http/services/owncloud/ocdav/head.go @@ -36,8 +36,6 @@ func (s *svc) handleHead(w http.ResponseWriter, r *http.Request, ns string) { ctx, span := trace.StartSpan(ctx, "head") defer span.End() - ns = applyLayout(ctx, ns) - fn := path.Join(ns, r.URL.Path) sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() diff --git a/internal/http/services/owncloud/ocdav/mkcol.go b/internal/http/services/owncloud/ocdav/mkcol.go index f2b8345d64..c93a8f9f67 100644 --- a/internal/http/services/owncloud/ocdav/mkcol.go +++ b/internal/http/services/owncloud/ocdav/mkcol.go @@ -34,8 +34,6 @@ func (s *svc) handleMkcol(w http.ResponseWriter, r *http.Request, ns string) { ctx, span := trace.StartSpan(ctx, "mkcol") defer span.End() - ns = applyLayout(ctx, ns) - fn := path.Join(ns, r.URL.Path) sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index 3c9f2a4b30..77debda500 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -34,8 +34,6 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { ctx, span := trace.StartSpan(ctx, "move") defer span.End() - ns = applyLayout(ctx, ns) - src := path.Join(ns, r.URL.Path) dstHeader := r.Header.Get("Destination") overwrite := r.Header.Get("Overwrite") diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index 7718cd3a80..ba82880388 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -29,6 +29,7 @@ import ( "time" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" @@ -102,7 +103,7 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error) ), } // initialize handlers and set default configs - if err := s.webDavHandler.init(conf.WebdavNamespace); err != nil { + if err := s.webDavHandler.init(conf.WebdavNamespace, true); err != nil { return nil, err } if err := s.davHandler.init(conf); err != nil { @@ -186,8 +187,20 @@ func (s *svc) getClient() (gateway.GatewayAPIClient, error) { return pool.GetGatewayServiceClient(s.c.GatewaySvc) } -func applyLayout(ctx context.Context, ns string) string { - return templates.WithUser(ctxuser.ContextMustGetUser(ctx), ns) +func applyLayout(ctx context.Context, ns string, useLoggedInUserNS bool, requestPath string) string { + // If useLoggedInUserNS is false, that implies that the request is coming from + // the FilesHandler method invoked by a /dav/files/fileOwner where fileOwner + // is not the same as the logged in user. In that case, we'll treat fileOwner + // as the username whose files are to be accessed and use that in the + // namespace template. + u, ok := ctxuser.ContextGetUser(ctx) + if !ok || !useLoggedInUserNS { + requestUserID, _ := router.ShiftPath(requestPath) + u = &userpb.User{ + Username: requestUserID, + } + } + return templates.WithUser(u, ns) } func wrapResourceID(r *provider.ResourceId) string { diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index 4722357b7b..c56bb11bfb 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -59,8 +59,6 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) ctx, span := trace.StartSpan(ctx, "propfind") defer span.End() - ns = applyLayout(ctx, ns) - fn := path.Join(ns, r.URL.Path) depth := r.Header.Get("Depth") if depth == "" { diff --git a/internal/http/services/owncloud/ocdav/proppatch.go b/internal/http/services/owncloud/ocdav/proppatch.go index 6e007b5f8a..fd83cd762a 100644 --- a/internal/http/services/owncloud/ocdav/proppatch.go +++ b/internal/http/services/owncloud/ocdav/proppatch.go @@ -44,8 +44,6 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) acceptedProps := []xml.Name{} removedProps := []xml.Name{} - ns = applyLayout(ctx, ns) - fn := path.Join(ns, r.URL.Path) sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index b884a0e43e..77ac6e8859 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -103,8 +103,6 @@ func isContentRange(r *http.Request) bool { func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() - - ns = applyLayout(ctx, ns) fn := path.Join(ns, r.URL.Path) sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index 4d7b81cb8e..d173cb752a 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -65,8 +65,6 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { return } - ns = applyLayout(ctx, ns) - // append filename to current dir fn := path.Join(ns, r.URL.Path, meta["filename"]) diff --git a/internal/http/services/owncloud/ocdav/webdav.go b/internal/http/services/owncloud/ocdav/webdav.go index ad1594c2c3..b8693ef97f 100644 --- a/internal/http/services/owncloud/ocdav/webdav.go +++ b/internal/http/services/owncloud/ocdav/webdav.go @@ -25,46 +25,49 @@ import ( // WebDavHandler implements a dav endpoint type WebDavHandler struct { - namespace string + namespace string + useLoggedInUserNS bool } -func (h *WebDavHandler) init(ns string) error { +func (h *WebDavHandler) init(ns string, useLoggedInUserNS bool) error { h.namespace = path.Join("/", ns) + h.useLoggedInUserNS = useLoggedInUserNS return nil } // Handler handles requests func (h *WebDavHandler) Handler(s *svc) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ns := applyLayout(r.Context(), h.namespace, h.useLoggedInUserNS, r.URL.Path) switch r.Method { case "PROPFIND": - s.handlePropfind(w, r, h.namespace) + s.handlePropfind(w, r, ns) case "LOCK": - s.handleLock(w, r, h.namespace) + s.handleLock(w, r, ns) case "UNLOCK": - s.handleUnlock(w, r, h.namespace) + s.handleUnlock(w, r, ns) case "PROPPATCH": - s.handleProppatch(w, r, h.namespace) + s.handleProppatch(w, r, ns) case "MKCOL": - s.handleMkcol(w, r, h.namespace) + s.handleMkcol(w, r, ns) case "MOVE": - s.handleMove(w, r, h.namespace) + s.handleMove(w, r, ns) case "COPY": - s.handleCopy(w, r, h.namespace) + s.handleCopy(w, r, ns) case "REPORT": - s.handleReport(w, r, h.namespace) + s.handleReport(w, r, ns) case http.MethodGet: - s.handleGet(w, r, h.namespace) + s.handleGet(w, r, ns) case http.MethodPut: - s.handlePut(w, r, h.namespace) + s.handlePut(w, r, ns) case http.MethodPost: - s.handleTusPost(w, r, h.namespace) + s.handleTusPost(w, r, ns) case http.MethodOptions: - s.handleOptions(w, r, h.namespace) + s.handleOptions(w, r, ns) case http.MethodHead: - s.handleHead(w, r, h.namespace) + s.handleHead(w, r, ns) case http.MethodDelete: - s.handleDelete(w, r, h.namespace) + s.handleDelete(w, r, ns) default: w.WriteHeader(http.StatusNotFound) }