Skip to content

Commit

Permalink
do not swallow permissions errors in ocdav (#1207)
Browse files Browse the repository at this point in the history 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
  • Loading branch information
@butonic
butonic authored Sep 30, 2020
1 parent 640fcbe commit 7ccd528
Show file tree
Hide file tree
Showing 15 changed files with 428 additions and 141 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/ocdav-permissions-errors.md
View file
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package ocdav

import (
"context"
"fmt"
"net/http"
"path"
"strings"
Expand Down Expand Up @@ -167,14 +166,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 @@ -187,26 +202,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
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
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
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
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

0 comments on commit 7ccd528

Please sign in to comment.