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.

Sign up for GitHub

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

Jump to bottom

do not swallow permissions errors in ocdav #1207

Merged
merged 1 commit into from
Sep 30, 2020
Merged

do not swallow permissions errors in ocdav #1207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions changelog/unreleased/ocdav-permissions-errors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
37 changes: 32 additions & 5 deletions internal/http/services/owncloud/ocdav/copy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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 {
Expand All @@ -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?
Expand Down
42 changes: 22 additions & 20 deletions internal/http/services/owncloud/ocdav/dav.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package ocdav

import (
"context"
"fmt"
"net/http"
"path"

Expand Down Expand Up @@ -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 {
Expand All @@ -181,26 +196,13 @@ 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)
ref := &provider.Reference{
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)
}
19 changes: 10 additions & 9 deletions internal/http/services/owncloud/ocdav/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
27 changes: 21 additions & 6 deletions internal/http/services/owncloud/ocdav/get.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
13 changes: 11 additions & 2 deletions internal/http/services/owncloud/ocdav/head.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
32 changes: 21 additions & 11 deletions internal/http/services/owncloud/ocdav/mkcol.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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)
}
Loading