fix: allow one invite at a time only and implement related validations and http status code handling

This commit is contained in:
Florian Schade
2023-12-19 17:18:38 +01:00
committed by Ralf Haferkamp
parent 418e304ab9
commit 465c9e3c20
4 changed files with 247 additions and 198 deletions
+77 -129
View File
@@ -11,7 +11,6 @@ import (
"reflect"
"strconv"
"strings"
"sync"
"time"
gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
@@ -26,7 +25,6 @@ import (
"github.com/go-chi/render"
libregraph "github.com/owncloud/libre-graph-api-go"
"golang.org/x/crypto/sha3"
"golang.org/x/sync/errgroup"
"google.golang.org/protobuf/types/known/fieldmaskpb"
"github.com/cs3org/reva/v2/pkg/publicshare"
@@ -472,142 +470,92 @@ func (g Graph) Invite(w http.ResponseWriter, r *http.Request) {
return
}
createShareErrors := sync.Map{}
createShareSuccesses := sync.Map{}
driveRecipient := driveItemInvite.GetRecipients()[0]
shareCreateGroup, ctx := errgroup.WithContext(ctx)
objectId := driveRecipient.GetObjectId()
cs3ResourcePermissions := unifiedrole.PermissionsToCS3ResourcePermissions(unifiedRolePermissions)
for _, driveRecipient := range driveItemInvite.GetRecipients() {
// not needed anymore with go 1.22 and higher
driveRecipient := driveRecipient // https://golang.org/doc/faq#closures_and_goroutines,
shareCreateGroup.Go(func() error {
objectId := driveRecipient.GetObjectId()
if objectId == "" {
return nil
}
cs3ResourcePermissions := unifiedrole.PermissionsToCS3ResourcePermissions(unifiedRolePermissions)
createShareRequest := &collaboration.CreateShareRequest{
ResourceInfo: statResponse.GetInfo(),
Grant: &collaboration.ShareGrant{
Permissions: &collaboration.SharePermissions{
Permissions: cs3ResourcePermissions,
},
},
}
permission := &libregraph.Permission{}
if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*cs3ResourcePermissions, unifiedrole.UnifiedRoleConditionGrantee, g.config.FilesSharing.EnableResharing); role != nil {
permission.Roles = []string{role.GetId()}
}
if len(permission.GetRoles()) == 0 {
permission.LibreGraphPermissionsActions = unifiedrole.CS3ResourcePermissionsToLibregraphActions(*cs3ResourcePermissions)
}
switch driveRecipient.GetLibreGraphRecipientType() {
case "group":
group, err := g.identityCache.GetGroup(ctx, objectId)
if err != nil {
g.logger.Debug().Err(err).Interface("groupId", objectId).Msg("failed group lookup")
createShareErrors.Store(objectId, errorcode.GeneralException.CreateOdataError(r.Context(), http.StatusText(http.StatusInternalServerError)))
return nil
}
createShareRequest.GetGrant().Grantee = &storageprovider.Grantee{
Type: storageprovider.GranteeType_GRANTEE_TYPE_GROUP,
Id: &storageprovider.Grantee_GroupId{GroupId: &grouppb.GroupId{
OpaqueId: group.GetId(),
}},
}
permission.GrantedToV2 = &libregraph.SharePointIdentitySet{
Group: &libregraph.Identity{
DisplayName: group.GetDisplayName(),
Id: conversions.ToPointer(group.GetId()),
},
}
default:
user, err := g.identityCache.GetUser(ctx, objectId)
if err != nil {
g.logger.Debug().Err(err).Interface("userId", objectId).Msg("failed user lookup")
createShareErrors.Store(objectId, errorcode.GeneralException.CreateOdataError(r.Context(), http.StatusText(http.StatusInternalServerError)))
return nil
}
createShareRequest.GetGrant().Grantee = &storageprovider.Grantee{
Type: storageprovider.GranteeType_GRANTEE_TYPE_USER,
Id: &storageprovider.Grantee_UserId{UserId: &userpb.UserId{
OpaqueId: user.GetId(),
}},
}
permission.GrantedToV2 = &libregraph.SharePointIdentitySet{
User: &libregraph.Identity{
DisplayName: user.GetDisplayName(),
Id: conversions.ToPointer(user.GetId()),
},
}
}
if driveItemInvite.ExpirationDateTime != nil {
createShareRequest.GetGrant().Expiration = utils.TimeToTS(*driveItemInvite.ExpirationDateTime)
}
createShareResponse, err := gatewayClient.CreateShare(ctx, createShareRequest)
switch {
case err != nil:
fallthrough
case createShareResponse.GetStatus().GetCode() != cs3rpc.Code_CODE_OK:
g.logger.Debug().Err(err).Msg("share creation failed")
createShareErrors.Store(objectId, errorcode.GeneralException.CreateOdataError(r.Context(), http.StatusText(http.StatusInternalServerError)))
return nil
}
if id := createShareResponse.GetShare().GetId().GetOpaqueId(); id != "" {
permission.Id = conversions.ToPointer(id)
}
if expiration := createShareResponse.GetShare().GetExpiration(); expiration != nil {
permission.SetExpirationDateTime(utils.TSToTime(expiration))
}
createShareSuccesses.Store(objectId, permission)
return nil
})
createShareRequest := &collaboration.CreateShareRequest{
ResourceInfo: statResponse.GetInfo(),
Grant: &collaboration.ShareGrant{
Permissions: &collaboration.SharePermissions{
Permissions: cs3ResourcePermissions,
},
},
}
if err := shareCreateGroup.Wait(); err != nil {
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
permission := &libregraph.Permission{}
if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*cs3ResourcePermissions, unifiedrole.UnifiedRoleConditionGrantee, g.config.FilesSharing.EnableResharing); role != nil {
permission.Roles = []string{role.GetId()}
}
if len(permission.GetRoles()) == 0 {
permission.LibreGraphPermissionsActions = unifiedrole.CS3ResourcePermissionsToLibregraphActions(*cs3ResourcePermissions)
}
switch driveRecipient.GetLibreGraphRecipientType() {
case "group":
group, err := g.identityCache.GetGroup(ctx, objectId)
if err != nil {
g.logger.Debug().Err(err).Interface("groupId", objectId).Msg("failed group lookup")
errorcode.GeneralException.Render(w, r, http.StatusBadRequest, err.Error())
return
}
createShareRequest.GetGrant().Grantee = &storageprovider.Grantee{
Type: storageprovider.GranteeType_GRANTEE_TYPE_GROUP,
Id: &storageprovider.Grantee_GroupId{GroupId: &grouppb.GroupId{
OpaqueId: group.GetId(),
}},
}
permission.GrantedToV2 = &libregraph.SharePointIdentitySet{
Group: &libregraph.Identity{
DisplayName: group.GetDisplayName(),
Id: conversions.ToPointer(group.GetId()),
},
}
default:
user, err := g.identityCache.GetUser(ctx, objectId)
if err != nil {
g.logger.Debug().Err(err).Interface("userId", objectId).Msg("failed user lookup")
errorcode.GeneralException.Render(w, r, http.StatusBadRequest, err.Error())
return
}
createShareRequest.GetGrant().Grantee = &storageprovider.Grantee{
Type: storageprovider.GranteeType_GRANTEE_TYPE_USER,
Id: &storageprovider.Grantee_UserId{UserId: &userpb.UserId{
OpaqueId: user.GetId(),
}},
}
permission.GrantedToV2 = &libregraph.SharePointIdentitySet{
User: &libregraph.Identity{
DisplayName: user.GetDisplayName(),
Id: conversions.ToPointer(user.GetId()),
},
}
}
if driveItemInvite.ExpirationDateTime != nil {
createShareRequest.GetGrant().Expiration = utils.TimeToTS(*driveItemInvite.ExpirationDateTime)
}
createShareResponse, err := gatewayClient.CreateShare(ctx, createShareRequest)
if errCode := errorcode.FromCS3Status(createShareResponse.GetStatus(), err); errCode != nil {
g.logger.Debug().Err(err).Msg("share creation failed")
errCode.Render(w, r)
return
}
value := make([]interface{}, 0, len(driveItemInvite.Recipients))
hasSuccesses := false
createShareSuccesses.Range(func(key, permission interface{}) bool {
value = append(value, permission)
hasSuccesses = true
return true
})
hasErrors := false
createShareErrors.Range(func(key, err interface{}) bool {
value = append(value, err)
hasErrors = true
return true
})
switch {
case hasErrors && hasSuccesses:
render.Status(r, http.StatusMultiStatus)
case hasSuccesses:
render.Status(r, http.StatusOK)
default:
render.Status(r, http.StatusInternalServerError)
if id := createShareResponse.GetShare().GetId().GetOpaqueId(); id != "" {
permission.Id = conversions.ToPointer(id)
}
render.JSON(w, r, &ListResponse{Value: value})
if expiration := createShareResponse.GetShare().GetExpiration(); expiration != nil {
permission.SetExpirationDateTime(utils.TSToTime(expiration))
}
render.Status(r, http.StatusOK)
render.JSON(w, r, &ListResponse{Value: []interface{}{permission}})
}
// UpdatePermission updates a Permission of a Drive item
@@ -17,17 +17,19 @@ import (
collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
roleconversions "github.com/cs3org/reva/v2/pkg/conversions"
"github.com/go-chi/chi/v5"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
libregraph "github.com/owncloud/libre-graph-api-go"
"github.com/owncloud/ocis/v2/services/graph/pkg/errorcode"
"github.com/owncloud/ocis/v2/services/graph/pkg/linktype"
"github.com/stretchr/testify/mock"
"github.com/tidwall/gjson"
"google.golang.org/grpc"
roleconversions "github.com/cs3org/reva/v2/pkg/conversions"
"github.com/owncloud/ocis/v2/services/graph/pkg/errorcode"
"github.com/owncloud/ocis/v2/services/graph/pkg/linktype"
"github.com/cs3org/reva/v2/pkg/storagespace"
revactx "github.com/cs3org/reva/v2/pkg/ctx"
@@ -891,7 +893,7 @@ var _ = Describe("Driveitems", func() {
driveItemInvite = &libregraph.DriveItemInvite{
Recipients: []libregraph.DriveRecipient{
{ObjectId: libregraph.PtrString("1")},
{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
},
Roles: []string{unifiedrole.NewViewerUnifiedRole(true).GetId()},
}
@@ -936,9 +938,35 @@ var _ = Describe("Driveitems", func() {
return strings.NewReader(string(driveItemInviteBytes))
}
It("creates user and group shares as expected (happy path)", func() {
It("creates user shares as expected (happy path)", func() {
driveItemInvite.Recipients = []libregraph.DriveRecipient{
{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
}
driveItemInvite.ExpirationDateTime = libregraph.PtrTime(time.Now().Add(time.Hour))
createShareResponse.Share = &collaboration.Share{
Id: &collaboration.ShareId{OpaqueId: "123"},
Expiration: utils.TimeToTS(*driveItemInvite.ExpirationDateTime),
}
svc.Invite(
rr,
httptest.NewRequest(http.MethodPost, "/", toJSONReader(driveItemInvite)).
WithContext(ctx),
)
jsonData := gjson.Get(rr.Body.String(), "value")
Expect(rr.Code).To(Equal(http.StatusOK))
Expect(jsonData.Get("#").Num).To(Equal(float64(1)))
Expect(jsonData.Get("0.id").Str).To(Equal("123"))
Expect(jsonData.Get("0.expirationDateTime").Str).To(Equal(driveItemInvite.ExpirationDateTime.Format(time.RFC3339Nano)))
Expect(jsonData.Get("#.grantedToV2.user.displayName").Array()[0].Str).To(Equal(getUserResponse.User.DisplayName))
Expect(jsonData.Get("#.grantedToV2.user.id").Array()[0].Str).To(Equal("1"))
})
It("creates group shares as expected (happy path)", func() {
driveItemInvite.Recipients = []libregraph.DriveRecipient{
{ObjectId: libregraph.PtrString("1")},
{ObjectId: libregraph.PtrString("2"), LibreGraphRecipientType: libregraph.PtrString("group")},
}
driveItemInvite.ExpirationDateTime = libregraph.PtrTime(time.Now().Add(time.Hour))
@@ -956,17 +984,9 @@ var _ = Describe("Driveitems", func() {
jsonData := gjson.Get(rr.Body.String(), "value")
Expect(rr.Code).To(Equal(http.StatusOK))
Expect(jsonData.Get("#").Num).To(Equal(float64(2)))
Expect(jsonData.Get("#").Num).To(Equal(float64(1)))
Expect(jsonData.Get("0.id").Str).To(Equal("123"))
Expect(jsonData.Get("1.id").Str).To(Equal("123"))
Expect(jsonData.Get("0.expirationDateTime").Str).To(Equal(driveItemInvite.ExpirationDateTime.Format(time.RFC3339Nano)))
Expect(jsonData.Get("1.expirationDateTime").Str).To(Equal(driveItemInvite.ExpirationDateTime.Format(time.RFC3339Nano)))
Expect(jsonData.Get("#.grantedToV2.user.displayName").Array()[0].Str).To(Equal(getUserResponse.User.DisplayName))
Expect(jsonData.Get("#.grantedToV2.user.id").Array()[0].Str).To(Equal("1"))
Expect(jsonData.Get("#.grantedToV2.group.displayName").Array()[0].Str).To(Equal(getGroupResponse.Group.GroupName))
Expect(jsonData.Get("#.grantedToV2.group.id").Array()[0].Str).To(Equal("2"))
})
@@ -1049,10 +1069,10 @@ var _ = Describe("Driveitems", func() {
},
Entry("fails if not ok", func() {
getGroupResponse.Status = status.NewNotFound(context.Background(), "")
}, http.StatusInternalServerError),
}, http.StatusBadRequest),
Entry("fails if errors", func() {
getGroupMock.Return(nil, errors.New("error"))
}, http.StatusInternalServerError),
}, http.StatusBadRequest),
)
DescribeTable("GetUser",
@@ -1069,11 +1089,11 @@ var _ = Describe("Driveitems", func() {
getUserMock.Parent.AssertNumberOfCalls(GinkgoT(), "GetUser", 1)
},
Entry("fails if not ok", func() {
getUserResponse.Status = status.NewNotFound(context.Background(), "")
}, http.StatusInternalServerError),
getUserResponse.Status = status.NewInvalid(context.Background(), "")
}, http.StatusBadRequest),
Entry("fails if errors", func() {
getUserMock.Return(nil, errors.New("error"))
}, http.StatusInternalServerError),
}, http.StatusBadRequest),
)
DescribeTable("CreateShare",
@@ -1091,7 +1111,7 @@ var _ = Describe("Driveitems", func() {
},
Entry("fails if not ok", func() {
createShareResponse.Status = status.NewNotFound(context.Background(), "")
}, http.StatusInternalServerError),
}, http.StatusNotFound),
Entry("fails if errors", func() {
createShareMock.Return(nil, errors.New("error"))
}, http.StatusInternalServerError),
+20 -8
View File
@@ -11,16 +11,21 @@ import (
// initLibregraph initializes libregraph validation
func initLibregraph(v *validator.Validate) {
driveItemInvite(v)
permission(v)
for _, f := range []func(*validator.Validate){
libregraphDriveItemInvite,
libregraphDriveRecipient,
libregraphPermission,
} {
f(v)
}
}
// driveItemInvite validates libregraph.DriveItemInvite
func driveItemInvite(v *validator.Validate) {
// libregraphDriveItemInvite validates libregraph.DriveItemInvite
func libregraphDriveItemInvite(v *validator.Validate) {
s := libregraph.DriveItemInvite{}
v.RegisterStructValidationMapRules(map[string]string{
"Recipients": "min=1",
"Recipients": "len=1,dive",
"Roles": "max=1",
"ExpirationDateTime": "omitnil,gt",
}, s)
@@ -29,12 +34,19 @@ func driveItemInvite(v *validator.Validate) {
driveItemInvite := sl.Current().Interface().(libregraph.DriveItemInvite)
rolesAndActions(sl, driveItemInvite.Roles, driveItemInvite.LibreGraphPermissionsActions, false)
}, s)
}
// permission validates libregraph.Permission
func permission(v *validator.Validate) {
// libregraphDriveRecipient validates libregraph.DriveRecipient
func libregraphDriveRecipient(v *validator.Validate) {
v.RegisterStructValidationMapRules(map[string]string{
"ObjectId": "ne=",
"LibreGraphRecipientType": "oneof=user group",
}, libregraph.DriveRecipient{})
}
// libregraphPermission validates libregraph.Permission
func libregraphPermission(v *validator.Validate) {
s := libregraph.Permission{}
v.RegisterStructValidationMapRules(map[string]string{
+109 -40
View File
@@ -8,86 +8,155 @@ import (
. "github.com/onsi/gomega"
libregraph "github.com/owncloud/libre-graph-api-go"
"github.com/owncloud/ocis/v2/ocis-pkg/conversions"
"github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole"
"github.com/owncloud/ocis/v2/services/graph/pkg/validate"
)
var _ = Describe("libregraph", func() {
type validatableFactory[T any] func() (T, bool)
var _ = Describe("libregraph", func() {
var driveItemInvite libregraph.DriveItemInvite
var driveRecipient libregraph.DriveRecipient
BeforeEach(func() {
driveRecipient = libregraph.DriveRecipient{
ObjectId: conversions.ToPointer("1"),
LibreGraphRecipientType: conversions.ToPointer("user"),
}
driveItemInvite = libregraph.DriveItemInvite{
Recipients: []libregraph.DriveRecipient{{ObjectId: libregraph.PtrString("1")}},
Recipients: []libregraph.DriveRecipient{driveRecipient},
Roles: []string{unifiedrole.UnifiedRoleEditorID},
LibreGraphPermissionsActions: []string{unifiedrole.DriveItemVersionsUpdate},
ExpirationDateTime: libregraph.PtrTime(time.Now().Add(time.Hour)),
}
})
DescribeTable("DriveItemInvite",
func(factory func() libregraph.DriveItemInvite, expectError bool) {
f := factory()
switch err := validate.StructCtx(context.Background(), f); expectError {
case true:
Expect(err).To(HaveOccurred())
default:
Expect(err).ToNot(HaveOccurred())
func(factories ...validatableFactory[libregraph.DriveItemInvite]) {
for _, factory := range factories {
s, pass := factory()
switch err := validate.StructCtx(context.Background(), s); pass {
case false:
Expect(err).To(HaveOccurred())
default:
Expect(err).ToNot(HaveOccurred())
}
}
},
Entry("succeed: roles", func() libregraph.DriveItemInvite {
Entry("succeed: roles only", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.LibreGraphPermissionsActions = nil
return driveItemInvite
}, false),
Entry("succeed: permission actions", func() libregraph.DriveItemInvite {
return driveItemInvite, true
}),
Entry("succeed: actions only", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = nil
return driveItemInvite
}, false),
Entry("succeed: without ExpirationDateTime", func() libregraph.DriveItemInvite {
return driveItemInvite, true
}),
Entry("succeed: no ExpirationDateTime", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = nil
driveItemInvite.ExpirationDateTime = nil
return driveItemInvite
}, false),
Entry("fail: multiple role assignment", func() libregraph.DriveItemInvite {
return driveItemInvite, true
}),
Entry("fail: multiple role assignment", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = []string{
unifiedrole.UnifiedRoleEditorID,
unifiedrole.UnifiedRoleManagerID,
}
driveItemInvite.LibreGraphPermissionsActions = nil
return driveItemInvite
}, true),
Entry("fail: unknown role", func() libregraph.DriveItemInvite {
return driveItemInvite, false
}),
Entry("fail: unknown role", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = []string{"foo"}
driveItemInvite.LibreGraphPermissionsActions = nil
return driveItemInvite
}, true),
Entry("fail: unknown action", func() libregraph.DriveItemInvite {
return driveItemInvite, false
}),
Entry("fail: unknown action", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = nil
driveItemInvite.LibreGraphPermissionsActions = []string{"foo"}
return driveItemInvite
}, true),
Entry("fail: missing roles or permission actions", func() libregraph.DriveItemInvite {
return driveItemInvite, false
}),
Entry("fail: missing roles or permission actions", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = nil
driveItemInvite.LibreGraphPermissionsActions = nil
return driveItemInvite
}, true),
Entry("fail: different number of roles and actions", func() libregraph.DriveItemInvite {
return driveItemInvite, false
}),
Entry("fail: different number of roles and actions", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.LibreGraphPermissionsActions = []string{
unifiedrole.DriveItemVersionsUpdate,
unifiedrole.DriveItemChildrenCreate,
}
return driveItemInvite
}, true),
Entry("fail: missing recipients", func() libregraph.DriveItemInvite {
return driveItemInvite, false
}),
Entry("fail: missing recipients", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = nil
driveItemInvite.Recipients = nil
return driveItemInvite
}, true),
Entry("fail: expirationDateTime in the past", func() libregraph.DriveItemInvite {
return driveItemInvite, false
}),
Entry("fail: dive recipients", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = nil
driveItemInvite.Recipients = []libregraph.DriveRecipient{
{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("dive")},
}
return driveItemInvite, false
}),
Entry("fail: more than 1 recipient", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = nil
driveItemInvite.Recipients = []libregraph.DriveRecipient{
driveRecipient,
{ObjectId: libregraph.PtrString("2"), LibreGraphRecipientType: libregraph.PtrString("group")},
}
return driveItemInvite, false
}),
Entry("fail: expirationDateTime in the past", func() (libregraph.DriveItemInvite, bool) {
driveItemInvite.Roles = nil
driveItemInvite.ExpirationDateTime = libregraph.PtrTime(time.Now().Add(-time.Hour))
return driveItemInvite
}, true),
return driveItemInvite, false
}),
)
DescribeTable("DriveRecipient",
func(factories ...validatableFactory[libregraph.DriveRecipient]) {
for _, factory := range factories {
s, pass := factory()
switch err := validate.StructCtx(context.Background(), s); pass {
case false:
Expect(err).To(HaveOccurred())
default:
Expect(err).ToNot(HaveOccurred())
}
}
},
Entry("fail: invalid objectId",
func() (libregraph.DriveRecipient, bool) {
driveRecipient.ObjectId = nil
return driveRecipient, false
},
func() (libregraph.DriveRecipient, bool) {
driveRecipient.ObjectId = conversions.ToPointer("")
return driveRecipient, false
},
),
Entry("succeed: valid role",
func() (libregraph.DriveRecipient, bool) {
driveRecipient.LibreGraphRecipientType = conversions.ToPointer("user")
return driveRecipient, true
},
func() (libregraph.DriveRecipient, bool) {
driveRecipient.LibreGraphRecipientType = conversions.ToPointer("group")
return driveRecipient, true
},
),
Entry("fail: invalid role",
func() (libregraph.DriveRecipient, bool) {
driveRecipient.LibreGraphRecipientType = conversions.ToPointer("foo")
return driveRecipient, false
},
func() (libregraph.DriveRecipient, bool) {
driveRecipient.LibreGraphRecipientType = nil
return driveRecipient, false
},
),
)
})