From 8c826354ae060f15363924f18cd25428a3347dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 28 Sep 2020 21:01:57 +0200 Subject: [PATCH] do not swallow permissions errors in ocdav MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/ocdav-permissions-errors.md | 6 + internal/http/services/owncloud/ocdav/copy.go | 37 +++++- internal/http/services/owncloud/ocdav/dav.go | 42 +++---- .../http/services/owncloud/ocdav/delete.go | 19 +-- internal/http/services/owncloud/ocdav/get.go | 27 ++++- internal/http/services/owncloud/ocdav/head.go | 13 +- .../http/services/owncloud/ocdav/mkcol.go | 32 +++-- internal/http/services/owncloud/ocdav/move.go | 78 +++++++++--- .../http/services/owncloud/ocdav/propfind.go | 39 ++++-- .../http/services/owncloud/ocdav/proppatch.go | 40 +++++-- internal/http/services/owncloud/ocdav/put.go | 36 +++++- .../services/owncloud/ocdav/putchunked.go | 11 +- .../http/services/owncloud/ocdav/trashbin.go | 112 ++++++++++++++---- internal/http/services/owncloud/ocdav/tus.go | 34 ++++-- .../http/services/owncloud/ocdav/versions.go | 43 +++++-- 15 files changed, 428 insertions(+), 141 deletions(-) create mode 100644 changelog/unreleased/ocdav-permissions-errors.md diff --git a/changelog/unreleased/ocdav-permissions-errors.md b/changelog/unreleased/ocdav-permissions-errors.md new file mode 100644 index 0000000000..e36943150b --- /dev/null +++ b/changelog/unreleased/ocdav-permissions-errors.md @@ -0,0 +1,6 @@ +Bugfix: No longer swallow permissions errors in ocdav + +The ocdav api is no longer ignoring permissions errors. +It will now check the status for `rpc.Code_CODE_PERMISSION_DENIED` codes and report them properly using `http.StatusForbidden` instead of `http.StatusInternalServerError` + +https://github.com/cs3org/reva/pull/1207 \ No newline at end of file diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index 517afe93c0..7f14957d37 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -98,11 +98,17 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { } if srcStatRes.Status.Code != rpc.Code_CODE_OK { - if srcStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND { + switch srcStatRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("src", src).Interface("status", srcStatRes.Status).Msg("resource not found") w.WriteHeader(http.StatusNotFound) - return + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("src", src).Interface("status", srcStatRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("src", src).Interface("status", srcStatRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) } - w.WriteHeader(http.StatusInternalServerError) return } @@ -117,6 +123,17 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { w.WriteHeader(http.StatusInternalServerError) return } + if dstStatRes.Status.Code != rpc.Code_CODE_OK && dstStatRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + switch dstStatRes.Status.Code { + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } + return + } var successCode int if dstStatRes.Status.Code == rpc.Code_CODE_OK { @@ -143,8 +160,18 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { w.WriteHeader(http.StatusInternalServerError) return } - if intStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND { - w.WriteHeader(http.StatusConflict) // 409 if intermediate dir is missing, see https://tools.ietf.org/html/rfc4918#section-9.8.5 + if intStatRes.Status.Code != rpc.Code_CODE_OK { + switch intStatRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + // 409 if intermediate dir is missing, see https://tools.ietf.org/html/rfc4918#section-9.8.5 + w.WriteHeader(http.StatusConflict) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("dst", dst).Interface("status", intStatRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("dst", dst).Interface("status", intStatRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } // TODO what if intermediate is a file? diff --git a/internal/http/services/owncloud/ocdav/dav.go b/internal/http/services/owncloud/ocdav/dav.go index ed9238a8c6..197abc3fdc 100644 --- a/internal/http/services/owncloud/ocdav/dav.go +++ b/internal/http/services/owncloud/ocdav/dav.go @@ -20,7 +20,6 @@ package ocdav import ( "context" - "fmt" "net/http" "path" @@ -161,14 +160,30 @@ func (h *DavHandler) Handler(s *svc) http.Handler { r = r.WithContext(ctx) - statInfo, err := getTokenStatInfo(ctx, c, token) + sRes, err := getTokenStatInfo(ctx, c, token) if err != nil { + log.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } - log.Debug().Interface("statInfo", statInfo).Msg("Stat info from public link token path") - if statInfo.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER { - ctx := context.WithValue(ctx, tokenStatInfoKey{}, statInfo) + if sRes.Status.Code != rpc.Code_CODE_OK { + switch sRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("token", token).Interface("status", res.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("token", token).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("token", token).Interface("status", res.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } + return + } + log.Debug().Interface("statInfo", sRes.Info).Msg("Stat info from public link token path") + + if sRes.Info.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER { + ctx := context.WithValue(ctx, tokenStatInfoKey{}, sRes.Info) r = r.WithContext(ctx) h.PublicFileHandler.Handler(s).ServeHTTP(w, r) } else { @@ -181,7 +196,7 @@ func (h *DavHandler) Handler(s *svc) http.Handler { }) } -func getTokenStatInfo(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, token string) (*provider.ResourceInfo, error) { +func getTokenStatInfo(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, token string) (*provider.StatResponse, error) { ns := "/public" fn := path.Join(ns, token) @@ -189,18 +204,5 @@ func getTokenStatInfo(ctx context.Context, client gatewayv1beta1.GatewayAPIClien Spec: &provider.Reference_Path{Path: fn}, } req := &provider.StatRequest{Ref: ref} - res, err := client.Stat(ctx, req) - if err != nil { - return nil, err - } - - if res.Status.Code != rpc.Code_CODE_OK { - return nil, fmt.Errorf("Failed to stat, status code %d: %s", res.Status.Code, res.Status.Message) - } - - if res.Info == nil { - return nil, fmt.Errorf("Failed to stat, info is nil") - } - - return res.Info, nil + return client.Stat(ctx, req) } diff --git a/internal/http/services/owncloud/ocdav/delete.go b/internal/http/services/owncloud/ocdav/delete.go index 9816e9a364..4cbeb94cb1 100644 --- a/internal/http/services/owncloud/ocdav/delete.go +++ b/internal/http/services/owncloud/ocdav/delete.go @@ -53,16 +53,17 @@ func (s *svc) handleDelete(w http.ResponseWriter, r *http.Request, ns string) { return } - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - log.Warn().Str("code", string(res.Status.Code)).Msg("resource not found") + switch res.Status.Code { + case rpc.Code_CODE_OK: + w.WriteHeader(http.StatusNoContent) + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") w.WriteHeader(http.StatusNotFound) - return - } - - if res.Status.Code != rpc.Code_CODE_OK { - log.Warn().Str("code", string(res.Status.Code)).Msg("grpc request failed") + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc delete request failed") w.WriteHeader(http.StatusInternalServerError) - return } - w.WriteHeader(http.StatusNoContent) } diff --git a/internal/http/services/owncloud/ocdav/get.go b/internal/http/services/owncloud/ocdav/get.go index 449c12ac7c..2d12ece45c 100644 --- a/internal/http/services/owncloud/ocdav/get.go +++ b/internal/http/services/owncloud/ocdav/get.go @@ -62,12 +62,17 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) { } if sRes.Status.Code != rpc.Code_CODE_OK { - log.Warn().Str("code", string(sRes.Status.Code)).Msg("grpc request failed") - statusCode := http.StatusInternalServerError - if sRes.Status.Code == rpc.Code_CODE_NOT_FOUND { - statusCode = http.StatusNotFound + switch sRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) } - w.WriteHeader(statusCode) return } @@ -92,7 +97,17 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) { } if dRes.Status.Code != rpc.Code_CODE_OK { - w.WriteHeader(http.StatusInternalServerError) + switch dRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", dRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", dRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", dRes.Status).Msg("grpc initiate file download request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } diff --git a/internal/http/services/owncloud/ocdav/head.go b/internal/http/services/owncloud/ocdav/head.go index 13052da85a..8760d37d39 100644 --- a/internal/http/services/owncloud/ocdav/head.go +++ b/internal/http/services/owncloud/ocdav/head.go @@ -57,8 +57,17 @@ func (s *svc) handleHead(w http.ResponseWriter, r *http.Request, ns string) { } if res.Status.Code != rpc.Code_CODE_OK { - log.Error().Msgf("error calling grpc: %s", res.Status.String()) - w.WriteHeader(http.StatusInternalServerError) + switch res.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } diff --git a/internal/http/services/owncloud/ocdav/mkcol.go b/internal/http/services/owncloud/ocdav/mkcol.go index 6c9e26207f..f167ea8822 100644 --- a/internal/http/services/owncloud/ocdav/mkcol.go +++ b/internal/http/services/owncloud/ocdav/mkcol.go @@ -63,9 +63,17 @@ func (s *svc) handleMkcol(w http.ResponseWriter, r *http.Request, ns string) { return } - if statRes.Status.Code == rpc.Code_CODE_OK { - log.Warn().Msg("resource already exists") - w.WriteHeader(http.StatusMethodNotAllowed) // 405 if it already exists + if statRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + switch statRes.Status.Code { + case rpc.Code_CODE_OK: + w.WriteHeader(http.StatusMethodNotAllowed) // 405 if it already exists + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", statRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } @@ -77,15 +85,17 @@ func (s *svc) handleMkcol(w http.ResponseWriter, r *http.Request, ns string) { return } - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { + switch res.Status.Code { + case rpc.Code_CODE_OK: + w.WriteHeader(http.StatusCreated) + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("resource not found") w.WriteHeader(http.StatusConflict) - return - } - - if res.Status.Code != rpc.Code_CODE_OK { + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", statRes.Status).Msg("grpc create container request failed") w.WriteHeader(http.StatusInternalServerError) - return } - - w.WriteHeader(http.StatusCreated) } diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index 9aa26979cb..4c98d8e625 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -76,13 +76,18 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { w.WriteHeader(http.StatusInternalServerError) return } - if srcStatRes.Status.Code != rpc.Code_CODE_OK { - if srcStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND { + switch srcStatRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("src", src).Interface("status", srcStatRes.Status).Msg("resource not found") w.WriteHeader(http.StatusNotFound) - return + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("src", src).Interface("status", srcStatRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("src", src).Interface("status", srcStatRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) } - w.WriteHeader(http.StatusInternalServerError) return } @@ -97,6 +102,17 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { w.WriteHeader(http.StatusInternalServerError) return } + if dstStatRes.Status.Code != rpc.Code_CODE_OK && dstStatRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + switch dstStatRes.Status.Code { + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } + return + } var successCode int if dstStatRes.Status.Code == rpc.Code_CODE_OK { @@ -117,10 +133,15 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { return } - // TODO return a forbidden status if read only? - if delRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Err(err).Str("move_request", delRes.Status.Code.String()).Msg("error handling delete request") - w.WriteHeader(http.StatusInternalServerError) + if delRes.Status.Code != rpc.Code_CODE_OK && delRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + switch delRes.Status.Code { + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("dst", dst).Interface("status", delRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("dst", dst).Interface("status", delRes.Status).Msg("grpc delete request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } } else { @@ -138,8 +159,18 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { w.WriteHeader(http.StatusInternalServerError) return } - if intStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND { - w.WriteHeader(http.StatusConflict) // 409 if intermediate dir is missing, see https://tools.ietf.org/html/rfc4918#section-9.9.4 + if intStatRes.Status.Code != rpc.Code_CODE_OK { + switch intStatRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + // 409 if intermediate dir is missing, see https://tools.ietf.org/html/rfc4918#section-9.8.5 + w.WriteHeader(http.StatusConflict) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("parent", intermediateDir).Interface("status", intStatRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("parent", intermediateDir).Interface("status", intStatRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } // TODO what if intermediate is a file? @@ -160,11 +191,19 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { } if mRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Err(err).Str("move_request", mRes.Status.Code.String()).Msg("error handling move request") - w.WriteHeader(http.StatusInternalServerError) + switch mRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("src", src).Str("dst", dst).Interface("status", mRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("src", src).Str("dst", dst).Interface("status", mRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("src", src).Str("dst", dst).Interface("status", mRes.Status).Msg("grpc move request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } - log.Info().Str("move", mRes.Status.Code.String()).Msg("move request status") dstStatRes, err = client.Stat(ctx, dstStatReq) if err != nil { @@ -174,8 +213,17 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { } if dstStatRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Err(err).Str("status", dstStatRes.Status.Code.String()).Msg("error doing stat") - w.WriteHeader(http.StatusInternalServerError) + switch dstStatRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index 8f2490dfb2..530031107c 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -88,12 +88,17 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) } if res.Status.Code != rpc.Code_CODE_OK { - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - log.Warn().Str("path", fn).Msg("resource not found") + switch res.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") w.WriteHeader(http.StatusNotFound) - return + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) } - w.WriteHeader(http.StatusInternalServerError) return } @@ -111,8 +116,17 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) } if res.Status.Code != rpc.Code_CODE_OK { - log.Err(err).Msg("error calling grpc list container") - w.WriteHeader(http.StatusInternalServerError) + switch res.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc list container request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } infos = append(infos, res.Infos...) @@ -136,8 +150,17 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) return } if res.Status.Code != rpc.Code_CODE_OK { - log.Err(err).Str("path", path).Msg("error calling grpc list container") - w.WriteHeader(http.StatusInternalServerError) + switch res.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc list container request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } diff --git a/internal/http/services/owncloud/ocdav/proppatch.go b/internal/http/services/owncloud/ocdav/proppatch.go index 46cbd53959..60e2492e77 100644 --- a/internal/http/services/owncloud/ocdav/proppatch.go +++ b/internal/http/services/owncloud/ocdav/proppatch.go @@ -77,12 +77,17 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) } if statRes.Status.Code != rpc.Code_CODE_OK { - if statRes.Status.Code == rpc.Code_CODE_NOT_FOUND { - log.Warn().Str("path", fn).Msg("resource not found") + switch statRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("resource not found") w.WriteHeader(http.StatusNotFound) - return + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", statRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) } - w.WriteHeader(http.StatusInternalServerError) return } @@ -132,12 +137,17 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) } if res.Status.Code != rpc.Code_CODE_OK { - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - log.Warn().Str("path", fn).Msg("resource not found") + switch res.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") w.WriteHeader(http.StatusNotFound) - return + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc unset arbitrary metadata request failed") + w.WriteHeader(http.StatusInternalServerError) } - w.WriteHeader(http.StatusInternalServerError) return } removedProps = append(removedProps, propNameXML) @@ -151,14 +161,20 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) } if res.Status.Code != rpc.Code_CODE_OK { - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - log.Warn().Str("path", fn).Msg("resource not found") + switch res.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") w.WriteHeader(http.StatusNotFound) - return + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc set arbitrary metadata request failed") + w.WriteHeader(http.StatusInternalServerError) } - w.WriteHeader(http.StatusInternalServerError) return } + acceptedProps = append(acceptedProps, propNameXML) delete(sreq.ArbitraryMetadata.Metadata, key) } diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index 718e896bad..51fae4d9a7 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -176,11 +176,16 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { return } - if sRes.Status.Code != rpc.Code_CODE_OK { - if sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + if sRes.Status.Code != rpc.Code_CODE_OK && sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + switch sRes.Status.Code { + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") w.WriteHeader(http.StatusInternalServerError) - return } + return } info := sRes.Info @@ -248,7 +253,17 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { } if uRes.Status.Code != rpc.Code_CODE_OK { - w.WriteHeader(http.StatusInternalServerError) + switch uRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", uRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", uRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", uRes.Status).Msg("grpc initiate file upload request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } @@ -314,8 +329,17 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { } if sRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Err(err).Msgf("error status %d when sending grpc stat request", sRes.Status.Code) - w.WriteHeader(http.StatusInternalServerError) + switch sRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } diff --git a/internal/http/services/owncloud/ocdav/putchunked.go b/internal/http/services/owncloud/ocdav/putchunked.go index bdba6488c3..80ec093edf 100644 --- a/internal/http/services/owncloud/ocdav/putchunked.go +++ b/internal/http/services/owncloud/ocdav/putchunked.go @@ -270,11 +270,16 @@ func (s *svc) handlePutChunked(w http.ResponseWriter, r *http.Request) { return } - if res.Status.Code != rpc.Code_CODE_OK { - if res.Status.Code != rpc.Code_CODE_NOT_FOUND { + if res.Status.Code != rpc.Code_CODE_OK && res.Status.Code != rpc.Code_CODE_NOT_FOUND { + switch res.Status.Code { + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc stat request failed") w.WriteHeader(http.StatusInternalServerError) - return } + return } info := res.Info diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go index 61ae0f2256..4a43afa3d5 100644 --- a/internal/http/services/owncloud/ocdav/trashbin.go +++ b/internal/http/services/owncloud/ocdav/trashbin.go @@ -148,13 +148,23 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s getHomeRes, err := gc.GetHome(ctx, &provider.GetHomeRequest{}) if err != nil { - log.Error().Err(err).Msg("error calling GetHomeProvider") + log.Error().Err(err).Msg("error calling GetHome") w.WriteHeader(http.StatusInternalServerError) return } if getHomeRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Int32("code", int32(getHomeRes.Status.Code)).Str("trace", getHomeRes.Status.Trace).Msg(getHomeRes.Status.Message) - w.WriteHeader(http.StatusInternalServerError) + switch getHomeRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("grpc get home request failed") + w.WriteHeader(http.StatusInternalServerError) + } + return } // ask gateway for recycle items @@ -172,9 +182,20 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s w.WriteHeader(http.StatusInternalServerError) return } + if getRecycleRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Int32("code", int32(getRecycleRes.Status.Code)).Str("trace", getRecycleRes.Status.Trace).Msg(getRecycleRes.Status.Message) - w.WriteHeader(http.StatusInternalServerError) + switch getRecycleRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", getHomeRes.Path).Interface("status", getRecycleRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", getHomeRes.Path).Interface("status", getRecycleRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", getHomeRes.Path).Interface("status", getRecycleRes.Status).Msg("grpc list recycle request failed") + w.WriteHeader(http.StatusInternalServerError) + } + return } propRes, err := h.formatTrashPropfind(ctx, s, u, &pf, getRecycleRes.RecycleItems) @@ -358,13 +379,23 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc getHomeRes, err := client.GetHome(ctx, &provider.GetHomeRequest{}) if err != nil { - log.Error().Err(err).Msg("error calling GetHomeProvider") + log.Error().Err(err).Msg("error calling GetHome") w.WriteHeader(http.StatusInternalServerError) return } if getHomeRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Int32("code", int32(getHomeRes.Status.Code)).Str("trace", getHomeRes.Status.Trace).Msg(getHomeRes.Status.Message) - w.WriteHeader(http.StatusInternalServerError) + switch getHomeRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("grpc get home request failed") + w.WriteHeader(http.StatusInternalServerError) + } + return } req := &provider.RestoreRecycleItemRequest{ @@ -387,15 +418,20 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc w.WriteHeader(http.StatusInternalServerError) return } - if res.Status.Code != rpc.Code_CODE_OK { - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - w.WriteHeader(http.StatusNotFound) - return - } + + switch res.Status.Code { + case rpc.Code_CODE_OK: + w.WriteHeader(http.StatusNoContent) + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("key", key).Str("dst", dst).Interface("status", res.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("key", key).Str("dst", dst).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("key", key).Str("dst", dst).Interface("status", res.Status).Msg("grpc restore recycle item request failed") w.WriteHeader(http.StatusInternalServerError) - return } - w.WriteHeader(http.StatusNoContent) } // delete has only a key @@ -418,8 +454,18 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, return } if getHomeRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Int32("code", int32(getHomeRes.Status.Code)).Str("trace", getHomeRes.Status.Trace).Msg(getHomeRes.Status.Message) - w.WriteHeader(http.StatusInternalServerError) + switch getHomeRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("grpc get home request failed") + w.WriteHeader(http.StatusInternalServerError) + } + return } sRes, err := client.Stat(ctx, &provider.StatRequest{ Ref: &provider.Reference{ @@ -434,8 +480,18 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, return } if sRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Int32("code", int32(sRes.Status.Code)).Str("trace", sRes.Status.Trace).Msg(sRes.Status.Message) - w.WriteHeader(http.StatusInternalServerError) + switch sRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", getHomeRes.Path).Interface("status", sRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", getHomeRes.Path).Interface("status", sRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", getHomeRes.Path).Interface("status", sRes.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) + } + return } // set key as opaque id, the storageprovider will use it as the key for the @@ -458,13 +514,17 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, w.WriteHeader(http.StatusInternalServerError) return } - if res.Status.Code != rpc.Code_CODE_OK { - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - w.WriteHeader(http.StatusNotFound) - return - } + switch res.Status.Code { + case rpc.Code_CODE_OK: + w.WriteHeader(http.StatusNoContent) + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("storageid", sRes.Info.Id.StorageId).Str("key", key).Interface("status", res.Status).Msg("resource not found") + w.WriteHeader(http.StatusConflict) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("storageid", sRes.Info.Id.StorageId).Str("key", key).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("storageid", sRes.Info.Id.StorageId).Str("key", key).Interface("status", res.Status).Msg("grpc purge recycle request failed") w.WriteHeader(http.StatusInternalServerError) - return } - w.WriteHeader(http.StatusNoContent) } diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index b4d6d69cf8..42f6fa29a5 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -89,11 +89,16 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { return } - if sRes.Status.Code != rpc.Code_CODE_OK { - if sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + if sRes.Status.Code != rpc.Code_CODE_OK && sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + switch sRes.Status.Code { + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") w.WriteHeader(http.StatusInternalServerError) - return } + return } info := sRes.Info @@ -148,7 +153,17 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { } if uRes.Status.Code != rpc.Code_CODE_OK { - w.WriteHeader(http.StatusInternalServerError) + switch uRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Str("path", fn).Interface("status", uRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", uRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", uRes.Status).Msg("grpc initiate file upload request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } @@ -213,11 +228,16 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { return } - if sRes.Status.Code != rpc.Code_CODE_OK { - if sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + if sRes.Status.Code != rpc.Code_CODE_OK && sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + switch sRes.Status.Code { + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") w.WriteHeader(http.StatusInternalServerError) - return } + return } info := sRes.Info diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index ae3716e0c0..ca9f1432e0 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -101,11 +101,17 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, return } if res.Status.Code != rpc.Code_CODE_OK { - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { + switch res.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Interface("resourceid", rid).Interface("status", res.Status).Msg("resource not found") w.WriteHeader(http.StatusNotFound) - return + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Interface("resourceid", rid).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Interface("resourceid", rid).Interface("status", res.Status).Msg("grpc stat request failed") + w.WriteHeader(http.StatusInternalServerError) } - w.WriteHeader(http.StatusInternalServerError) return } @@ -121,9 +127,20 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, return } if lvRes.Status.Code != rpc.Code_CODE_OK { - w.WriteHeader(http.StatusInternalServerError) + switch lvRes.Status.Code { + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Interface("resourceid", rid).Interface("status", lvRes.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Interface("resourceid", rid).Interface("status", lvRes.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Interface("resourceid", rid).Interface("status", lvRes.Status).Msg("grpc list file revisions request failed") + w.WriteHeader(http.StatusInternalServerError) + } return } + versions := lvRes.GetVersions() infos := make([]*provider.ResourceInfo, 0, len(versions)+1) // add version dir . entry, derived from file info @@ -207,13 +224,17 @@ func (h *VersionsHandler) doRestore(w http.ResponseWriter, r *http.Request, s *s w.WriteHeader(http.StatusInternalServerError) return } - if res.Status.Code != rpc.Code_CODE_OK { - if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - w.WriteHeader(http.StatusNotFound) - return - } + switch res.Status.Code { + case rpc.Code_CODE_OK: + w.WriteHeader(http.StatusNoContent) + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Interface("resourceid", rid).Str("key", key).Interface("status", res.Status).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Interface("resourceid", rid).Str("key", key).Interface("status", res.Status).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + default: + log.Error().Interface("resourceid", rid).Str("key", key).Interface("status", res.Status).Msg("grpc restore file revision request failed") w.WriteHeader(http.StatusInternalServerError) - return } - w.WriteHeader(http.StatusNoContent) }