Skip to content

Commit

Permalink
fix: allow one invite at a time only and implement related validation…
Browse files Browse the repository at this point in the history 
…s and http status code handling
  • Loading branch information
@fschade
fschade committed Dec 22, 2023
1 parent dee53ce commit 4c42b1e
Show file tree
Hide file tree
Showing 4 changed files with 238 additions and 189 deletions.
194 changes: 71 additions & 123 deletions services/graph/pkg/service/v0/driveitems.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"reflect"
"strconv"
"strings"
"sync"
"time"

gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
}
createShareRequest := &collaboration.CreateShareRequest{
ResourceInfo: statResponse.GetInfo(),
Grant: &collaboration.ShareGrant{
Permissions: &collaboration.SharePermissions{
Permissions: cs3ResourcePermissions,
},
},
}

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
}
permission := &libregraph.Permission{}
if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*cs3ResourcePermissions, unifiedrole.UnifiedRoleConditionGrantee, g.config.FilesSharing.EnableResharing); role != nil {
permission.Roles = []string{role.GetId()}
}

if id := createShareResponse.GetShare().GetId().GetOpaqueId(); id != "" {
permission.Id = conversions.ToPointer(id)
}
if len(permission.GetRoles()) == 0 {
permission.LibreGraphPermissionsActions = unifiedrole.CS3ResourcePermissionsToLibregraphActions(*cs3ResourcePermissions)
}

if expiration := createShareResponse.GetShare().GetExpiration(); expiration != nil {
permission.SetExpirationDateTime(utils.TSToTime(expiration))
}
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
}

createShareSuccesses.Store(objectId, permission)
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()),
},
}
}

return nil
})
if driveItemInvite.ExpirationDateTime != nil {
createShareRequest.GetGrant().Expiration = utils.TimeToTS(*driveItemInvite.ExpirationDateTime)
}

if err := shareCreateGroup.Wait(); err != nil {
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
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
})
if id := createShareResponse.GetShare().GetId().GetOpaqueId(); id != "" {
permission.Id = conversions.ToPointer(id)
}

switch {
case hasErrors && hasSuccesses:
render.Status(r, http.StatusMultiStatus)
case hasSuccesses:
render.Status(r, http.StatusOK)
default:
render.Status(r, http.StatusInternalServerError)
if expiration := createShareResponse.GetShare().GetExpiration(); expiration != nil {
permission.SetExpirationDateTime(utils.TSToTime(expiration))
}

render.JSON(w, r, &ListResponse{Value: value})
render.Status(r, http.StatusOK)
render.JSON(w, r, &ListResponse{Value: []interface{}{permission}})
}

// UpdatePermission updates a Permission of a Drive item
Expand Down
56 changes: 38 additions & 18 deletions services/graph/pkg/service/v0/driveitems_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()},
}
Expand Down Expand Up @@ -936,10 +938,9 @@ 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")},
{ObjectId: libregraph.PtrString("2"), LibreGraphRecipientType: libregraph.PtrString("group")},
{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
}
driveItemInvite.ExpirationDateTime = libregraph.PtrTime(time.Now().Add(time.Hour))
createShareResponse.Share = &collaboration.Share{
Expand All @@ -956,17 +957,36 @@ 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"))
})

It("creates group shares as expected (happy path)", func() {
driveItemInvite.Recipients = []libregraph.DriveRecipient{
{ObjectId: libregraph.PtrString("2"), LibreGraphRecipientType: libregraph.PtrString("group")},
}
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.group.displayName").Array()[0].Str).To(Equal(getGroupResponse.Group.GroupName))
Expect(jsonData.Get("#.grantedToV2.group.id").Array()[0].Str).To(Equal("2"))
})
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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),
Expand Down
Loading

0 comments on commit 4c42b1e

Please sign in to comment.