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.

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

Dav unit tests #3441

Merged
merged 3 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/dav-unit-tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Enhancement: Cover ocdav with more unit tests

We added unit tests to cover more ocdav handlers:
- delete

https://github.com/cs3org/reva/pull/3441
4 changes: 2 additions & 2 deletions internal/http/services/owncloud/ocdav/dav.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
"github.com/cs3org/reva/v2/pkg/appctx"
ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
"github.com/cs3org/reva/v2/pkg/rhttp/router"
"github.com/cs3org/reva/v2/pkg/utils"
"google.golang.org/grpc/metadata"
Expand Down Expand Up @@ -182,9 +181,10 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
case "public-files":
base := path.Join(ctx.Value(net.CtxKeyBaseURI).(string), "public-files")
ctx = context.WithValue(ctx, net.CtxKeyBaseURI, base)
c, err := pool.GetGatewayServiceClient(s.c.GatewaySvc)
c, err := s.getClient()
if err != nil {
w.WriteHeader(http.StatusNotFound)
return
}

var res *gatewayv1beta1.AuthenticateResponse
Expand Down
22 changes: 15 additions & 7 deletions internal/http/services/owncloud/ocdav/ocdav.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"time"

gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
Expand Down Expand Up @@ -156,6 +155,7 @@ type svc struct {
davHandler *DavHandler
favoritesManager favorite.Manager
client *http.Client
gwClient gateway.GatewayAPIClient
// LockSystem is the lock management system.
LockSystem LockSystem
userIdentifierCache *ttlcache.Cache
Expand Down Expand Up @@ -199,11 +199,11 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error)
return nil, err
}

return NewWith(conf, fm, ls, log, rtrace.DefaultProvider())
return NewWith(conf, fm, ls, log, rtrace.DefaultProvider(), nil)
}

// NewWith returns a new ocdav service
func NewWith(conf *Config, fm favorite.Manager, ls LockSystem, _ *zerolog.Logger, tp trace.TracerProvider) (global.Service, error) {
func NewWith(conf *Config, fm favorite.Manager, ls LockSystem, _ *zerolog.Logger, tp trace.TracerProvider, gwc gateway.GatewayAPIClient) (global.Service, error) {
s := &svc{
c: conf,
webDavHandler: new(WebDavHandler),
Expand All @@ -212,6 +212,7 @@ func NewWith(conf *Config, fm favorite.Manager, ls LockSystem, _ *zerolog.Logger
rhttp.Timeout(time.Duration(conf.Timeout*int64(time.Second))),
rhttp.Insecure(conf.Insecure),
),
gwClient: gwc,
favoritesManager: fm,
LockSystem: ls,
userIdentifierCache: ttlcache.NewCache(),
Expand All @@ -226,6 +227,13 @@ func NewWith(conf *Config, fm favorite.Manager, ls LockSystem, _ *zerolog.Logger
if err := s.davHandler.init(conf); err != nil {
return nil, err
}
if gwc == nil {
var err error
s.gwClient, err = pool.GetGatewayServiceClient(s.c.GatewaySvc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about moving this default behavior into New and just erroring out in here if the given client is nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have to duplicate it to pkg/micro/ocdav/service.go as well, otherwise we would change the behavior of the micro ocdav service, requiring a major version bump. So I put the fallback here, to make passing in a gwclient optional.

if err != nil {
return nil, err
}
}
return s, nil
}

Expand Down Expand Up @@ -315,7 +323,7 @@ func (s *svc) Handler() http.Handler {
}

func (s *svc) getClient() (gateway.GatewayAPIClient, error) {
return pool.GetGatewayServiceClient(s.c.GatewaySvc)
return s.gwClient, nil
}

func (s *svc) ApplyLayout(ctx context.Context, ns string, useLoggedInUserNS bool, requestPath string) (string, string, error) {
Expand Down Expand Up @@ -393,7 +401,7 @@ func addAccessHeaders(w http.ResponseWriter, r *http.Request) {
}
}

func authContextForUser(client gatewayv1beta1.GatewayAPIClient, userID *userpb.UserId, machineAuthAPIKey string) (context.Context, error) {
func authContextForUser(client gateway.GatewayAPIClient, userID *userpb.UserId, machineAuthAPIKey string) (context.Context, error) {
if machineAuthAPIKey == "" {
return nil, errtypes.NotSupported("machine auth not configured")
}
Expand All @@ -415,7 +423,7 @@ func authContextForUser(client gatewayv1beta1.GatewayAPIClient, userID *userpb.U
return granteeCtx, nil
}

func (s *svc) sspReferenceIsChildOf(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
func (s *svc) sspReferenceIsChildOf(ctx context.Context, client gateway.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
parentStatRes, err := client.Stat(ctx, &provider.StatRequest{Ref: parent})
if err != nil {
return false, err
Expand Down Expand Up @@ -457,7 +465,7 @@ func (s *svc) sspReferenceIsChildOf(ctx context.Context, client gatewayv1beta1.G
return strings.HasPrefix(cp, pp), nil
}

func (s *svc) referenceIsChildOf(ctx context.Context, client gatewayv1beta1.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
func (s *svc) referenceIsChildOf(ctx context.Context, client gateway.GatewayAPIClient, child, parent *provider.Reference) (bool, error) {
if child.ResourceId.SpaceId != parent.ResourceId.SpaceId {
return false, nil // Not on the same storage -> not a child
}
Expand Down
169 changes: 169 additions & 0 deletions internal/http/services/owncloud/ocdav/ocdav_blackbox_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
// Copyright 2021 CERN
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// In applying this license, CERN does not waive the privileges and immunities
// granted to it by virtue of its status as an Intergovernmental Organization
// or submit itself to any jurisdiction.
package ocdav_test

import (
"context"
"net/http"
"net/http/httptest"
"strings"

cs3gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
cs3user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
cs3storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav"
ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
"github.com/cs3org/reva/v2/pkg/rgrpc/status"
"github.com/cs3org/reva/v2/pkg/rhttp/global"
"github.com/cs3org/reva/v2/pkg/storagespace"
"github.com/cs3org/reva/v2/pkg/utils"
"github.com/cs3org/reva/v2/tests/cs3mocks/mocks"
"github.com/stretchr/testify/mock"
"go.opentelemetry.io/otel/trace"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

// TODO for now we have to test all of ocdav. when this testsuite is complete we can move
// the handlers to dedicated packages to reduce the amount of complexity to get a test environment up
var _ = Describe("ocdav", func() {
var (
handler global.Service
client *mocks.GatewayAPIClient
ctx context.Context

userspace *cs3storageprovider.StorageSpace
user *cs3user.User
)

BeforeEach(func() {
user = &cs3user.User{Id: &cs3user.UserId{OpaqueId: "username"}, Username: "username"}

ctx = ctxpkg.ContextSetUser(context.Background(), user)
client = &mocks.GatewayAPIClient{}

var err error
handler, err = ocdav.NewWith(&ocdav.Config{
FilesNamespace: "/users/{{.Username}}",
WebdavNamespace: "/users/{{.Username}}",
}, nil, ocdav.NewCS3LS(client), nil, trace.NewNoopTracerProvider(), client)
Expect(err).ToNot(HaveOccurred())

userspace = &cs3storageprovider.StorageSpace{
Opaque: &typesv1beta1.Opaque{
Map: map[string]*typesv1beta1.OpaqueEntry{
"path": {
Decoder: "plain",
Value: []byte("/users/username/"),
},
},
},
Id: &cs3storageprovider.StorageSpaceId{OpaqueId: storagespace.FormatResourceID(cs3storageprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "root"})},
Root: &cs3storageprovider.ResourceId{StorageId: "provider-1", SpaceId: "userspace", OpaqueId: "root"},
Name: "username",
}

client.On("GetUser", mock.Anything, mock.Anything).Return(&cs3user.GetUserResponse{
Status: status.NewNotFound(ctx, "not found"),
}, nil)
client.On("GetUserByClaim", mock.Anything, mock.Anything).Return(&cs3user.GetUserByClaimResponse{
Status: status.NewNotFound(ctx, "not found"),
}, nil)

// for public access
client.On("Authenticate", mock.Anything, mock.MatchedBy(func(req *cs3gateway.AuthenticateRequest) bool {
return req.Type == "publicshares" &&
strings.HasPrefix(req.ClientId, "tokenfor") &&
strings.HasPrefix(req.ClientSecret, "signature||")
})).Return(&cs3gateway.AuthenticateResponse{
Status: status.NewOK(ctx),
User: user,
Token: "jwt",
}, nil)
client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool {
return req.Ref.ResourceId.StorageId == utils.PublicStorageProviderID &&
req.Ref.ResourceId.SpaceId == utils.PublicStorageSpaceID &&
req.Ref.ResourceId.OpaqueId == "tokenforfile"
})).Return(&cs3storageprovider.StatResponse{
Status: status.NewOK(ctx),
Info: &cs3storageprovider.ResourceInfo{
Type: cs3storageprovider.ResourceType_RESOURCE_TYPE_FILE,
},
}, nil)
client.On("Stat", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.StatRequest) bool {
return req.Ref.ResourceId.StorageId == utils.PublicStorageProviderID &&
req.Ref.ResourceId.SpaceId == utils.PublicStorageSpaceID &&
req.Ref.ResourceId.OpaqueId == "tokenforfolder"
})).Return(&cs3storageprovider.StatResponse{
Status: status.NewOK(ctx),
Info: &cs3storageprovider.ResourceInfo{
Type: cs3storageprovider.ResourceType_RESOURCE_TYPE_CONTAINER,
},
}, nil)
})

Describe("NewHandler", func() {
It("returns a handler", func() {
Expect(handler).ToNot(BeNil())
})
})

Context("When a default space is used", func() {

DescribeTable("HandleDelete",
func(endpoint string, expectedPathPrefix string, expectedPath string, expectedStatus int) {

client.On("ListStorageSpaces", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.ListStorageSpacesRequest) bool {
p := string(req.Opaque.Map["path"].Value)
return p == "/" || strings.HasPrefix(p, expectedPathPrefix)
})).Return(&cs3storageprovider.ListStorageSpacesResponse{
Status: status.NewOK(ctx),
StorageSpaces: []*cs3storageprovider.StorageSpace{userspace},
}, nil)

ref := cs3storageprovider.Reference{
ResourceId: userspace.Root,
Path: expectedPath,
}

client.On("Delete", mock.Anything, mock.MatchedBy(func(req *cs3storageprovider.DeleteRequest) bool {
return utils.ResourceEqual(req.Ref, &ref)
})).Return(&cs3storageprovider.DeleteResponse{
Status: status.NewOK(ctx),
}, nil)

rr := httptest.NewRecorder()
req, err := http.NewRequest("DELETE", endpoint+"/foo", strings.NewReader(""))
Expect(err).ToNot(HaveOccurred())
req = req.WithContext(ctx)

handler.Handler().ServeHTTP(rr, req)
Expect(rr).To(HaveHTTPStatus(expectedStatus))
Expect(rr).To(HaveHTTPBody(""), "Body must be empty")

},
Entry("at the /webdav endpoint", "/webdav", "/users", "./foo", http.StatusNoContent),
Entry("at the /dav/files endpoint", "/dav/files/username", "/users/username", "./foo", http.StatusNoContent),
Entry("at the /dav/spaces endpoint", "/dav/spaces/provider-1$userspace!root", "/users/username", "./foo", http.StatusNoContent),
Entry("at the /dav/public-files endpoint for a file", "/dav/public-files/tokenforfile", "", "", http.StatusMethodNotAllowed),
Entry("at the /dav/public-files endpoint for a folder", "/dav/public-files/tokenforfolder", "/public/tokenforfolder", ".", http.StatusNoContent),
)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ package ocdav
import (
"testing"

providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
sprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
"github.com/cs3org/reva/v2/pkg/storagespace"
)

func TestWrapResourceID(t *testing.T) {
expected := "storageid" + "$" + "spaceid" + "!" + "opaqueid"
wrapped := storagespace.FormatResourceID(providerv1beta1.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"})
wrapped := storagespace.FormatResourceID(sprovider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"})

if wrapped != expected {
t.Errorf("wrapped id doesn't have the expected format: got %s expected %s", wrapped, expected)
Expand Down Expand Up @@ -83,5 +83,4 @@ func TestNameDoesNotContainRule(t *testing.T) {
}
}
}

}
9 changes: 9 additions & 0 deletions pkg/micro/ocdav/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"context"
"crypto/tls"

gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav"
"github.com/cs3org/reva/v2/pkg/storage/favorite"
"github.com/rs/zerolog"
Expand All @@ -44,6 +45,7 @@ type Options struct {
JWTSecret string

FavoriteManager favorite.Manager
GatewayClient gateway.GatewayAPIClient

TracingEnabled bool
TracingCollector string
Expand Down Expand Up @@ -188,6 +190,13 @@ func FavoriteManager(val favorite.Manager) Option {
}
}

// GatewayClient provides a function to set the GatewayClient option.
func GatewayClient(val gateway.GatewayAPIClient) Option {
return func(o *Options) {
o.GatewayClient = val
}
}

// LockSystem provides a function to set the LockSystem option.
func LockSystem(val ocdav.LockSystem) Option {
return func(o *Options) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/micro/ocdav/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func Service(opts ...Option) (micro.Service, error) {
)

tp := rtrace.GetTracerProvider(sopts.TracingEnabled, sopts.TracingCollector, sopts.TracingEndpoint, sopts.Name)
revaService, err := ocdav.NewWith(&sopts.config, sopts.FavoriteManager, sopts.lockSystem, &sopts.Logger, tp)
revaService, err := ocdav.NewWith(&sopts.config, sopts.FavoriteManager, sopts.lockSystem, &sopts.Logger, tp, sopts.GatewayClient)
if err != nil {
return nil, err
}
Expand Down