diff --git a/protogen/gen/ocis/messages/settings/v0/settings.pb.go b/protogen/gen/ocis/messages/settings/v0/settings.pb.go index 70600cb9f19..ede907dba32 100644 --- a/protogen/gen/ocis/messages/settings/v0/settings.pb.go +++ b/protogen/gen/ocis/messages/settings/v0/settings.pb.go @@ -589,7 +589,6 @@ type Setting struct { DisplayName string `protobuf:"bytes,3,opt,name=display_name,json=displayName,proto3" json:"display_name,omitempty"` Description string `protobuf:"bytes,4,opt,name=description,proto3" json:"description,omitempty"` // Types that are assignable to Value: - // // *Setting_IntValue // *Setting_StringValue // *Setting_BoolValue @@ -1194,7 +1193,6 @@ type Value struct { AccountUuid string `protobuf:"bytes,4,opt,name=account_uuid,json=accountUuid,proto3" json:"account_uuid,omitempty"` Resource *Resource `protobuf:"bytes,5,opt,name=resource,proto3" json:"resource,omitempty"` // Types that are assignable to Value: - // // *Value_BoolValue // *Value_IntValue // *Value_StringValue @@ -1385,7 +1383,6 @@ type ListOptionValue struct { unknownFields protoimpl.UnknownFields // Types that are assignable to Option: - // // *ListOptionValue_StringValue // *ListOptionValue_IntValue Option isListOptionValue_Option `protobuf_oneof:"option"` diff --git a/protogen/gen/ocis/services/thumbnails/v0/thumbnails.pb.go b/protogen/gen/ocis/services/thumbnails/v0/thumbnails.pb.go index 25633f26e52..0329e98191c 100644 --- a/protogen/gen/ocis/services/thumbnails/v0/thumbnails.pb.go +++ b/protogen/gen/ocis/services/thumbnails/v0/thumbnails.pb.go @@ -37,7 +37,6 @@ type GetThumbnailRequest struct { // The height of the thumbnail Height int32 `protobuf:"varint,4,opt,name=height,proto3" json:"height,omitempty"` // Types that are assignable to Source: - // // *GetThumbnailRequest_WebdavSource // *GetThumbnailRequest_Cs3Source Source isGetThumbnailRequest_Source `protobuf_oneof:"source"` diff --git a/services/graph/Makefile b/services/graph/Makefile index d17a086a335..d239f9c17be 100644 --- a/services/graph/Makefile +++ b/services/graph/Makefile @@ -28,6 +28,7 @@ ci-go-generate: $(MOCKERY) # CI runs ci-node-generate automatically before this $(MOCKERY) --dir pkg/service/v0 --case underscore --name GatewayClient $(MOCKERY) --dir pkg/service/v0 --case underscore --name HTTPClient $(MOCKERY) --dir pkg/service/v0 --case underscore --name Publisher + $(MOCKERY) --dir pkg/service/v0 --case underscore --name Permissions $(MOCKERY) --srcpkg github.com/go-ldap/ldap/v3 --case underscore --filename ldapclient.go --name Client diff --git a/services/graph/mocks/permissions.go b/services/graph/mocks/permissions.go new file mode 100644 index 00000000000..78b47047059 --- /dev/null +++ b/services/graph/mocks/permissions.go @@ -0,0 +1,78 @@ +// Code generated by mockery v2.10.4. DO NOT EDIT. + +package mocks + +import ( + context "context" + + client "go-micro.dev/v4/client" + + mock "github.com/stretchr/testify/mock" + + v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" +) + +// Permissions is an autogenerated mock type for the Permissions type +type Permissions struct { + mock.Mock +} + +// GetPermissionByID provides a mock function with given fields: ctx, request, opts +func (_m *Permissions) GetPermissionByID(ctx context.Context, request *v0.GetPermissionByIDRequest, opts ...client.CallOption) (*v0.GetPermissionByIDResponse, error) { + _va := make([]interface{}, len(opts)) + for _i := range opts { + _va[_i] = opts[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, request) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 *v0.GetPermissionByIDResponse + if rf, ok := ret.Get(0).(func(context.Context, *v0.GetPermissionByIDRequest, ...client.CallOption) *v0.GetPermissionByIDResponse); ok { + r0 = rf(ctx, request, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v0.GetPermissionByIDResponse) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *v0.GetPermissionByIDRequest, ...client.CallOption) error); ok { + r1 = rf(ctx, request, opts...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// ListPermissionsByResource provides a mock function with given fields: ctx, in, opts +func (_m *Permissions) ListPermissionsByResource(ctx context.Context, in *v0.ListPermissionsByResourceRequest, opts ...client.CallOption) (*v0.ListPermissionsByResourceResponse, error) { + _va := make([]interface{}, len(opts)) + for _i := range opts { + _va[_i] = opts[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, in) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 *v0.ListPermissionsByResourceResponse + if rf, ok := ret.Get(0).(func(context.Context, *v0.ListPermissionsByResourceRequest, ...client.CallOption) *v0.ListPermissionsByResourceResponse); ok { + r0 = rf(ctx, in, opts...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v0.ListPermissionsByResourceResponse) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, *v0.ListPermissionsByResourceRequest, ...client.CallOption) error); ok { + r1 = rf(ctx, in, opts...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index e75b09a86bf..f7974bae3c0 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -200,10 +200,8 @@ func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { } } -func canCreateSpace(ctx context.Context, ownPersonalHome bool) bool { - s := settingssvc.NewPermissionService("com.owncloud.api.settings", grpc.DefaultClient()) - - pr, err := s.GetPermissionByID(ctx, &settingssvc.GetPermissionByIDRequest{ +func (g Graph) canCreateSpace(ctx context.Context, ownPersonalHome bool) bool { + pr, err := g.permissionsService.GetPermissionByID(ctx, &settingssvc.GetPermissionByIDRequest{ PermissionId: settingsServiceExt.CreateSpacePermissionID, }) if err != nil || pr.Permission == nil { @@ -228,7 +226,7 @@ func (g Graph) CreateDrive(w http.ResponseWriter, r *http.Request) { } // TODO determine if the user tries to create his own personal space and pass that as a boolean - canCreateSpace := canCreateSpace(r.Context(), false) + canCreateSpace := g.canCreateSpace(r.Context(), false) if !canCreateSpace { logger.Debug().Bool("cancreatespace", canCreateSpace).Msg("could not create drive: insufficient permissions") // if the permission is not existing for the user in context we can assume we don't have it. Return 401. @@ -708,7 +706,7 @@ func (g Graph) getDriveQuota(ctx context.Context, space *storageprovider.Storage return nil, nil case res.GetStatus().GetCode() != cs3rpc.Code_CODE_OK: logger.Debug().Str("grpc", res.GetStatus().GetMessage()).Msg("error sending get quota grpc request") - return nil, err + return nil, errors.New(res.GetStatus().GetMessage()) } var remaining int64 diff --git a/services/graph/pkg/service/v0/drives_test.go b/services/graph/pkg/service/v0/drives_test.go index ac18991b473..2c13866e30a 100644 --- a/services/graph/pkg/service/v0/drives_test.go +++ b/services/graph/pkg/service/v0/drives_test.go @@ -153,4 +153,7 @@ func TestSpaceNameValidation(t *testing.T) { err := validateSpaceName(tc.SpaceName) require.Equal(t, tc.ExpectedError, err, tc.Alias) } + + // set max length back to protect other tests + _maxSpaceNameLength = 255 } diff --git a/services/graph/pkg/service/v0/graph.go b/services/graph/pkg/service/v0/graph.go index 45f6ab68a78..af58f61d3e9 100644 --- a/services/graph/pkg/service/v0/graph.go +++ b/services/graph/pkg/service/v0/graph.go @@ -15,6 +15,7 @@ import ( settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" "github.com/owncloud/ocis/v2/services/graph/pkg/config" "github.com/owncloud/ocis/v2/services/graph/pkg/identity" + "go-micro.dev/v4/client" mevents "go-micro.dev/v4/events" "google.golang.org/grpc" ) @@ -62,6 +63,12 @@ type Publisher interface { Publish(string, interface{}, ...mevents.PublishOption) error } +// Permissions is the interface used to access the permissions service +type Permissions interface { + GetPermissionByID(ctx context.Context, request *settingssvc.GetPermissionByIDRequest, opts ...client.CallOption) (*settingssvc.GetPermissionByIDResponse, error) + ListPermissionsByResource(ctx context.Context, in *settingssvc.ListPermissionsByResourceRequest, opts ...client.CallOption) (*settingssvc.ListPermissionsByResourceResponse, error) +} + // HTTPClient is the subset of the http.Client that is being used to interact with the download gateway type HTTPClient interface { Do(req *http.Request) (*http.Response, error) @@ -78,6 +85,7 @@ type Graph struct { identityBackend identity.Backend gatewayClient GatewayClient roleService settingssvc.RoleService + permissionsService Permissions spacePropertiesCache *ttlcache.Cache eventsPublisher events.Publisher } diff --git a/services/graph/pkg/service/v0/graph_test.go b/services/graph/pkg/service/v0/graph_test.go index 6eff1f2da10..201ea998e39 100644 --- a/services/graph/pkg/service/v0/graph_test.go +++ b/services/graph/pkg/service/v0/graph_test.go @@ -1,43 +1,49 @@ package svc_test import ( + "bytes" "context" "encoding/json" "fmt" "io" "net/http" "net/http/httptest" - "net/url" "time" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + userprovider "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/rgrpc/status" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" libregraph "github.com/owncloud/libre-graph-api-go" ogrpc "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc" "github.com/owncloud/ocis/v2/ocis-pkg/shared" + v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/settings/v0" + settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" "github.com/owncloud/ocis/v2/services/graph/mocks" "github.com/owncloud/ocis/v2/services/graph/pkg/config" "github.com/owncloud/ocis/v2/services/graph/pkg/config/defaults" service "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0" "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode" + "github.com/pkg/errors" "github.com/stretchr/testify/mock" ) var _ = Describe("Graph", func() { var ( - svc service.Service - gatewayClient *mocks.GatewayClient - eventsPublisher mocks.Publisher - ctx context.Context - cfg *config.Config + svc service.Service + gatewayClient *mocks.GatewayClient + eventsPublisher mocks.Publisher + permissionService mocks.Permissions + ctx context.Context + cfg *config.Config ) JustBeforeEach(func() { - ctx = context.Background() + ctx = ctxpkg.ContextSetUser(context.Background(), &userprovider.User{Id: &userprovider.UserId{Type: userprovider.UserType_USER_TYPE_PRIMARY, OpaqueId: "testuser"}, Username: "testuser"}) cfg = defaults.FullDefaultConfig() cfg.Identity.LDAP.CACert = "" // skip the startup checks, we don't use LDAP at all in this tests cfg.TokenManager.JWTSecret = "loremipsum" @@ -47,10 +53,12 @@ var _ = Describe("Graph", func() { _ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...) gatewayClient = &mocks.GatewayClient{} eventsPublisher = mocks.Publisher{} + permissionService = mocks.Permissions{} svc = service.NewService( service.Config(cfg), service.WithGatewayClient(gatewayClient), service.EventsPublisher(&eventsPublisher), + service.PermissionService(&permissionService), ) }) @@ -60,7 +68,7 @@ var _ = Describe("Graph", func() { }) }) - Describe("drive", func() { + Describe("List drives", func() { It("can list an empty list of spaces", func() { gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ Status: status.NewOK(ctx), @@ -72,9 +80,20 @@ var _ = Describe("Graph", func() { svc.GetDrives(rr, r) Expect(rr.Code).To(Equal(http.StatusOK)) }) + It("can list an empty list of all spaces", func() { + gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Times(1).Return(&provider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: []*provider.StorageSpace{}, + }, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/drives", nil) + rr := httptest.NewRecorder() + svc.GetAllDrives(rr, r) + Expect(rr.Code).To(Equal(http.StatusOK)) + }) It("can list a space without owner", func() { - gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ + gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Times(1).Return(&provider.ListStorageSpacesResponse{ Status: status.NewOK(ctx), StorageSpaces: []*provider.StorageSpace{ { @@ -256,12 +275,11 @@ var _ = Describe("Graph", func() { Expect(err).ToNot(HaveOccurred()) Expect(len(response["value"])).To(Equal(1)) value := response["value"][0] - webdavURL, _ := url.PathUnescape(*value.Root.WebDavUrl) Expect(*value.DriveAlias).To(Equal("mountpoint/new-folder")) Expect(*value.DriveType).To(Equal("mountpoint")) Expect(*value.Id).To(Equal("prID$aID!differentID")) Expect(*value.Name).To(Equal("New Folder")) - Expect(webdavURL).To(Equal("https://localhost:9200/dav/spaces/prID$aID!differentID")) + Expect(*value.Root.WebDavUrl).To(Equal("https://localhost:9200/dav/spaces/prID$aID%21differentID")) Expect(*value.Root.ETag).To(Equal("101112131415")) Expect(*value.Root.Id).To(Equal("prID$aID!differentID")) Expect(*value.Root.RemoteItem.ETag).To(Equal("123456789")) @@ -296,27 +314,391 @@ var _ = Describe("Graph", func() { Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String())) }) It("can list a spaces with invalid query parameter", func() { + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives?§orderby=owner%20asc", nil) + rr := httptest.NewRecorder() + svc.GetDrives(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("Query parameter '§orderby' is not supported. Cause: Query parameter '§orderby' is not supported")) + Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String())) + }) + It("can list a spaces with an unsupported operand", func() { + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives?$filter=contains(driveType,personal)", nil) + rr := httptest.NewRecorder() + svc.GetDrives(rr, r) + Expect(rr.Code).To(Equal(http.StatusNotImplemented)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("unsupported filter operand: contains")) + Expect(libreError.Error.Code).To(Equal(errorcode.NotSupported.String())) + }) + It("transport error", func() { + gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(nil, errors.New("transport error")) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives)", nil) + rr := httptest.NewRecorder() + svc.GetDrives(rr, r) + Expect(rr.Code).To(Equal(http.StatusInternalServerError)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("transport error")) + Expect(libreError.Error.Code).To(Equal(errorcode.GeneralException.String())) + }) + It("grpc error", func() { gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ - Status: status.NewOK(ctx), + Status: status.NewInternal(ctx, "internal error"), + StorageSpaces: []*provider.StorageSpace{}}, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives)", nil) + rr := httptest.NewRecorder() + svc.GetDrives(rr, r) + Expect(rr.Code).To(Equal(http.StatusInternalServerError)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("internal error")) + Expect(libreError.Error.Code).To(Equal(errorcode.GeneralException.String())) + }) + It("grpc not found", func() { + gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ + Status: status.NewNotFound(ctx, "no spaces found"), StorageSpaces: []*provider.StorageSpace{}}, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives)", nil) + rr := httptest.NewRecorder() + svc.GetDrives(rr, r) + Expect(rr.Code).To(Equal(http.StatusOK)) + + body, _ := io.ReadAll(rr.Body) + + var response map[string][]libregraph.Drive + err := json.Unmarshal(body, &response) + Expect(err).ToNot(HaveOccurred()) + Expect(len(response)).To(Equal(0)) + }) + It("quota error", func() { + gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: []*provider.StorageSpace{ + { + Id: &provider.StorageSpaceId{OpaqueId: "sameID"}, + SpaceType: "aspacetype", + Root: &provider.ResourceId{ + StorageId: "pro-1", + SpaceId: "sameID", + OpaqueId: "sameID", + }, + Name: "aspacename", + }, + }, + }, nil) gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Return(&gateway.InitiateFileDownloadResponse{ Status: status.NewNotFound(ctx, "not found"), }, nil) gatewayClient.On("GetQuota", mock.Anything, mock.Anything).Return(&provider.GetQuotaResponse{ - Status: status.NewUnimplemented(ctx, fmt.Errorf("not supported"), "not supported"), + Status: status.NewInternal(ctx, "internal quota error"), }, nil) - r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives?§orderby=owner%20asc", nil) + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives", nil) rr := httptest.NewRecorder() svc.GetDrives(rr, r) + + Expect(rr.Code).To(Equal(http.StatusInternalServerError)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("internal quota error")) + Expect(libreError.Error.Code).To(Equal(errorcode.GeneralException.String())) + }) + }) + Describe("Create Drive", func() { + It("cannot create a space without valid user in context", func() { + jsonBody := []byte(`{"Name": "Test Space"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusUnauthorized)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("invalid user")) + Expect(libreError.Error.Code).To(Equal(errorcode.NotAllowed.String())) + }) + It("cannot create a space without permission", func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_UNKNOWN, + Constraint: v0.Permission_CONSTRAINT_OWN, + }, + }, nil) + jsonBody := []byte(`{"Name": "Test Space"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusUnauthorized)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("insufficient permissions to create a space.")) + Expect(libreError.Error.Code).To(Equal(errorcode.NotAllowed.String())) + }) + It("cannot create a space with wrong request body", func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + jsonBody := []byte(`{"Name": "Test Space"`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) Expect(rr.Code).To(Equal(http.StatusBadRequest)) body, _ := io.ReadAll(rr.Body) var libreError libregraph.OdataError err := json.Unmarshal(body, &libreError) Expect(err).To(Not(HaveOccurred())) - Expect(libreError.Error.Message).To(Equal("Query parameter '§orderby' is not supported. Cause: Query parameter '§orderby' is not supported")) + Expect(libreError.Error.Message).To(Equal("invalid body schema definition")) + Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String())) + }) + It("transport error", func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + gatewayClient.On("CreateStorageSpace", mock.Anything, mock.Anything).Return(&provider.CreateStorageSpaceResponse{}, errors.New("transport error")) + jsonBody := []byte(`{"Name": "Test Space"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusInternalServerError)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("transport error")) + Expect(libreError.Error.Code).To(Equal(errorcode.GeneralException.String())) + }) + It("grpc permission denied error", func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + gatewayClient.On("CreateStorageSpace", mock.Anything, mock.Anything).Return(&provider.CreateStorageSpaceResponse{ + Status: status.NewPermissionDenied(ctx, nil, "grpc permission denied"), + }, nil) + + jsonBody := []byte(`{"Name": "Test Space"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusForbidden)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("permission denied")) + Expect(libreError.Error.Code).To(Equal(errorcode.NotAllowed.String())) + }) + It("grpc general error", func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + gatewayClient.On("CreateStorageSpace", mock.Anything, mock.Anything).Return(&provider.CreateStorageSpaceResponse{ + Status: status.NewInternal(ctx, "grpc error"), + }, nil) + + jsonBody := []byte(`{"Name": "Test Space"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusInternalServerError)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("grpc error")) + Expect(libreError.Error.Code).To(Equal(errorcode.GeneralException.String())) + }) + It("cannot create a space with empty name", func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + jsonBody := []byte(`{"Name": ""}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("invalid spacename: spacename must not be empty")) + Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String())) + }) + It("cannot create a space with a name that exceeds 255 chars", func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + jsonBody := []byte(`{"Name": "uufZ2MEUjUMJa84RkPsjJ1zf4XXRTdVMxRsJGfevwHuUBojo5JEdNU22O1FGgzXXTi9tl5ZKWaluIef8pPmEAxn9lHGIjyDVYeRQPiX5PCAZ7rVszrpLJryY5x1p6fFGQ6WQsPpNaqnKnfMliJDsbkAwMf7rCpzo0GUuadgHY9s2mfoXHDnpxqEmDsheucqVAFcNlFZNbNHoZAebHfv78KYc8C0WnhWvqvSPGBkNPQbZUkFCOAIlqpQ2Q3MubgI2"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("invalid spacename: spacename must be smaller than 255")) + Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String())) + }) + It("cannot create a space with a wrong type", func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + jsonBody := []byte(`{"Name": "Test", "DriveType": "media"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("drives of this type cannot be created via this api")) Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String())) }) + It("cannot create a space with a name that contains invalid chars", func() { + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + jsonBody := []byte(`{"Name": "Space / Name"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusBadRequest)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("invalid spacename: spacenames must not contain [/ \\ . : ? * \" > < |]")) + Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String())) + }) + It("can create a project space", func() { + gatewayClient.On("CreateStorageSpace", mock.Anything, mock.Anything).Return(&provider.CreateStorageSpaceResponse{ + Status: status.NewOK(ctx), + StorageSpace: &provider.StorageSpace{ + Id: &provider.StorageSpaceId{OpaqueId: "newID"}, + Name: "Test Space", + SpaceType: "project", + Root: &provider.ResourceId{ + StorageId: "pro-1", + SpaceId: "newID", + OpaqueId: "newID", + }, + Opaque: &typesv1beta1.Opaque{ + Map: map[string]*typesv1beta1.OpaqueEntry{ + "description": {Decoder: "plain", Value: []byte("This space is for testing")}, + "spaceAlias": {Decoder: "plain", Value: []byte("project/testspace")}, + }, + }, + }, + }, nil) + + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + jsonBody := []byte(`{"Name": "Test Space", "DriveType": "project", "Description": "This space is for testing", "DriveAlias": "project/testspace"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusCreated)) + + body, _ := io.ReadAll(rr.Body) + var response libregraph.Drive + err := json.Unmarshal(body, &response) + Expect(err).ToNot(HaveOccurred()) + Expect(*response.Name).To(Equal("Test Space")) + Expect(*response.DriveType).To(Equal("project")) + Expect(*response.DriveAlias).To(Equal("project/testspace")) + Expect(*response.Description).To(Equal("This space is for testing")) + }) + It("Incomplete space", func() { + gatewayClient.On("CreateStorageSpace", mock.Anything, mock.Anything).Return(&provider.CreateStorageSpaceResponse{ + Status: status.NewOK(ctx), + StorageSpace: &provider.StorageSpace{ + Id: &provider.StorageSpaceId{OpaqueId: "newID"}, + Name: "Test Space", + SpaceType: "project", + }, + }, nil) + + permissionService.On("GetPermissionByID", mock.Anything, mock.Anything).Return(&settingssvc.GetPermissionByIDResponse{ + Permission: &v0.Permission{ + Operation: v0.Permission_OPERATION_READWRITE, + Constraint: v0.Permission_CONSTRAINT_ALL, + }, + }, nil) + jsonBody := []byte(`{"Name": "Test Space"}`) + r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx) + rr := httptest.NewRecorder() + svc.CreateDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusInternalServerError)) + + body, _ := io.ReadAll(rr.Body) + var libreError libregraph.OdataError + err := json.Unmarshal(body, &libreError) + Expect(err).To(Not(HaveOccurred())) + Expect(libreError.Error.Message).To(Equal("space has no root")) + Expect(libreError.Error.Code).To(Equal(errorcode.GeneralException.String())) + }) }) }) diff --git a/services/graph/pkg/service/v0/instrument.go b/services/graph/pkg/service/v0/instrument.go index 80503fe9be2..0b20788c26c 100644 --- a/services/graph/pkg/service/v0/instrument.go +++ b/services/graph/pkg/service/v0/instrument.go @@ -103,3 +103,13 @@ func (i instrument) DeleteGroupMember(w http.ResponseWriter, r *http.Request) { func (i instrument) GetDrives(w http.ResponseWriter, r *http.Request) { i.next.GetDrives(w, r) } + +// GetAllDrives implements the Service interface. +func (i instrument) GetAllDrives(w http.ResponseWriter, r *http.Request) { + i.next.GetAllDrives(w, r) +} + +// CreateDrive implements the Service interface. +func (i instrument) CreateDrive(w http.ResponseWriter, r *http.Request) { + i.next.CreateDrive(w, r) +} diff --git a/services/graph/pkg/service/v0/logging.go b/services/graph/pkg/service/v0/logging.go index 51213475d7e..e9bd3aa12e1 100644 --- a/services/graph/pkg/service/v0/logging.go +++ b/services/graph/pkg/service/v0/logging.go @@ -103,3 +103,13 @@ func (l logging) DeleteGroupMember(w http.ResponseWriter, r *http.Request) { func (l logging) GetDrives(w http.ResponseWriter, r *http.Request) { l.next.GetDrives(w, r) } + +// GetAllDrives implements the Service interface. +func (l logging) GetAllDrives(w http.ResponseWriter, r *http.Request) { + l.next.GetAllDrives(w, r) +} + +// CreateDrive implements the Service interface. +func (l logging) CreateDrive(w http.ResponseWriter, r *http.Request) { + l.next.CreateDrive(w, r) +} diff --git a/services/graph/pkg/service/v0/option.go b/services/graph/pkg/service/v0/option.go index 752c7de3c75..dcaa01a4228 100644 --- a/services/graph/pkg/service/v0/option.go +++ b/services/graph/pkg/service/v0/option.go @@ -16,14 +16,15 @@ type Option func(o *Options) // Options defines the available options for this package. type Options struct { - Logger log.Logger - Config *config.Config - Middleware []func(http.Handler) http.Handler - GatewayClient GatewayClient - IdentityBackend identity.Backend - RoleService settingssvc.RoleService - RoleManager *roles.Manager - EventsPublisher events.Publisher + Logger log.Logger + Config *config.Config + Middleware []func(http.Handler) http.Handler + GatewayClient GatewayClient + IdentityBackend identity.Backend + RoleService settingssvc.RoleService + PermissionService Permissions + RoleManager *roles.Manager + EventsPublisher events.Publisher } // newOptions initializes the available default options. @@ -79,6 +80,13 @@ func RoleService(val settingssvc.RoleService) Option { } } +// PermissionService provides a function to set the PermissionService option. +func PermissionService(val settingssvc.PermissionService) Option { + return func(o *Options) { + o.PermissionService = val + } +} + // RoleManager provides a function to set the RoleManager option. func RoleManager(val *roles.Manager) Option { return func(o *Options) { diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 94be2c07b11..e3e6d76a09c 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -48,6 +48,8 @@ type Service interface { DeleteGroupMember(http.ResponseWriter, *http.Request) GetDrives(w http.ResponseWriter, r *http.Request) + GetAllDrives(w http.ResponseWriter, r *http.Request) + CreateDrive(w http.ResponseWriter, r *http.Request) } // NewService returns a service implementation for Service. @@ -145,6 +147,12 @@ func NewService(opts ...Option) Service { svc.roleService = options.RoleService } + if options.PermissionService == nil { + svc.permissionsService = settingssvc.NewPermissionService("com.owncloud.api.settings", grpc.DefaultClient()) + } else { + svc.permissionsService = options.PermissionService + } + roleManager := options.RoleManager if roleManager == nil { storeOptions := store.OcisStoreOptions{ diff --git a/services/graph/pkg/service/v0/tracing.go b/services/graph/pkg/service/v0/tracing.go index d3a0167b463..0721dd63eb9 100644 --- a/services/graph/pkg/service/v0/tracing.go +++ b/services/graph/pkg/service/v0/tracing.go @@ -99,3 +99,13 @@ func (t tracing) DeleteGroupMember(w http.ResponseWriter, r *http.Request) { func (t tracing) GetDrives(w http.ResponseWriter, r *http.Request) { t.next.GetDrives(w, r) } + +// GetAllDrives implements the Service interface. +func (t tracing) GetAllDrives(w http.ResponseWriter, r *http.Request) { + t.next.GetAllDrives(w, r) +} + +// CreateDrive implements the Service interface. +func (t tracing) CreateDrive(w http.ResponseWriter, r *http.Request) { + t.next.CreateDrive(w, r) +} diff --git a/sonar-project.properties b/sonar-project.properties index 9ae382cd61a..4382ec4e63b 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -37,5 +37,5 @@ sonar.go.golangci-lint.reportPaths=cache/checkstyle/app-provider_checkstyle.xml, # Exclude files sonar.exclusions=**/third_party,docs/**,changelog/**,*/pkg/assets/embed.go,idp/assets/identifier/**,**/package.json,**/rollup.config.js,CHANGELOG.md,**/pkg/proto/**/*.pb.*,deployments/**,tests/**,vendor-bin/**,README.md,**/mocks/ -sonar.coverage.exclusions=**/*_test.go +sonar.coverage.exclusions=**/*_test.go,**mocks/**,/protogen/** sonar.cpd.exclusions=**/*_test.go