Skip to content

Commit

Permalink
graph/sharedWitMe: fix response for shares from project space
Browse files Browse the repository at this point in the history
Resources on project space do not have a real owner assigned. A special
of the type USER_TYPE_SPACE_OWNER is returned as the owner. This type of
user can't be looked up via a GetUser request. So we skip that call for
this usertype.

This also fixes the behavior of 'sharedWithMe' for case when the owner
or creator of a share or shared resource can't be looked up in the 'users'
service. Previously cause the complete request to fail with an error message.
So a single share with an unresolvable owner caused 'sharedWithMe' to fail.
Now we log a warning but return all shares. Those where the owner or creator
couldn't be resolved will have the 'displayName' field of the user in the
'remoteItem.shared.owner' or 'remoteItem.shared.sharedBy' property left
empty.

Fixes: owncloud#8215
Fixes: owncloud#8027
  • Loading branch information
rhafer committed Jan 17, 2024
1 parent 9632a2b commit 95d40c3
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 35 deletions.
11 changes: 11 additions & 0 deletions changelog/unreleased/sharing-ng-empty-owner.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Bugfix: graph/sharedWithMe works for shares from project spaces now

We fixed a bug in the 'graph/v1beta1/me/drive/sharedWithMe' endpoint that
caused an error response when the user received shares from project spaces.
Additionally the endpoint now behaves more graceful in cases where the
displayname of the owner or creator of a share or shared resource couldn't be
resolved.

https://github.com/owncloud/ocis/pull/8233
https://github.com/owncloud/ocis/issues/8027
https://github.com/owncloud/ocis/issues/8215
67 changes: 32 additions & 35 deletions services/graph/pkg/service/v0/sharedwithme.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"reflect"
"slices"

cs3User "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
"github.com/cs3org/reva/v2/pkg/utils"
Expand Down Expand Up @@ -183,55 +184,36 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er
}

if userID := shareStat.GetInfo().GetOwner(); userID != nil {
user, err := g.identityCache.GetUser(ctx, userID.GetOpaqueId())
identity, err := g.cs3UserIdToIdentity(ctx, userID)
if err != nil {
g.logger.Error().Err(err).Msg("could not get user")
return err
// TODO: define a proper error behavior here. We don't
// want the whole request to fail just because a single
// resource owner couldn't be resolved. But, should be
// really return the affect share in the response?
// For now we just log a warning. The returned
// identitySet will just contain the userid.
g.logger.Warn().Err(err).Str("userid", userID.String()).Msg("could not get owner of shared resource")
}

identitySet := libregraph.IdentitySet{
User: &libregraph.Identity{
DisplayName: user.GetDisplayName(),
Id: libregraph.PtrString(user.GetId()),
},
}

remoteItem.SetCreatedBy(identitySet)
driveItem.SetCreatedBy(identitySet)
remoteItem.SetCreatedBy(libregraph.IdentitySet{User: &identity})
driveItem.SetCreatedBy(libregraph.IdentitySet{User: &identity})
}

if userID := receivedShare.GetShare().GetOwner(); userID != nil {
user, err := g.identityCache.GetUser(ctx, userID.GetOpaqueId())
identity, err := g.cs3UserIdToIdentity(ctx, userID)
if err != nil {
g.logger.Error().Err(err).Msg("could not get user")
return err
}

identitySet := libregraph.IdentitySet{
User: &libregraph.Identity{
DisplayName: user.GetDisplayName(),
Id: libregraph.PtrString(user.GetId()),
},
g.logger.Warn().Err(err).Str("userid", userID.String()).Msg("could not get owner of the share")
}

shared.SetOwner(identitySet)
shared.SetOwner(libregraph.IdentitySet{User: &identity})
}

if userID := receivedShare.GetShare().GetCreator(); userID != nil {
user, err := g.identityCache.GetUser(ctx, userID.GetOpaqueId())
identity, err := g.cs3UserIdToIdentity(ctx, userID)
if err != nil {
g.logger.Error().Err(err).Msg("could not get user")
return err
}

identitySet := libregraph.IdentitySet{
User: &libregraph.Identity{
DisplayName: user.GetDisplayName(),
Id: libregraph.PtrString(user.GetId()),
},
g.logger.Warn().Err(err).Str("userid", userID.String()).Msg("could not get creator of the share")
}

shared.SetSharedBy(identitySet)
shared.SetSharedBy(libregraph.IdentitySet{User: &identity})

}

Expand Down Expand Up @@ -344,3 +326,18 @@ func (g Graph) cs3ReceivedShareToLibreGraphPermissions(ctx context.Context, rece

return permission, nil
}

func (g Graph) cs3UserIdToIdentity(ctx context.Context, cs3UserID *cs3User.UserId) (libregraph.Identity, error) {
identity := libregraph.Identity{
Id: libregraph.PtrString(cs3UserID.GetOpaqueId()),
}
var err error
if cs3UserID.GetType() != cs3User.UserType_USER_TYPE_SPACE_OWNER {
var user libregraph.User
user, err = g.identityCache.GetUser(ctx, cs3UserID.GetOpaqueId())
if err == nil {
identity.SetDisplayName(user.GetDisplayName())
}
}
return identity, err
}
30 changes: 30 additions & 0 deletions services/graph/pkg/service/v0/sharedwithme_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,5 +393,35 @@ var _ = Describe("SharedWithMe", func() {
Expect(jsonData.Get("user.displayName").String()).To(Equal(shareCreator.DisplayName))
Expect(jsonData.Get("user.id").String()).To(Equal(shareCreator.Id.OpaqueId))
})

It("returns shares created on project space", func() {
shareCreator := getUserResponseDefault.User

ownerID := &userv1beta1.UserId{
OpaqueId: "project-space-id",
Type: userv1beta1.UserType_USER_TYPE_SPACE_OWNER,
}
share := listReceivedSharesResponse.Shares[0].Share
share.Creator = shareCreator.Id
share.Owner = ownerID
resourceInfo := statResponse.Info
resourceInfo.Owner = ownerID

svc.ListSharedWithMe(
tape,
httptest.NewRequest(http.MethodGet, "/graph/v1beta1/me/drive/sharedWithMe", nil),
)

jsonData := gjson.Get(tape.Body.String(), "value.0.createdBy")

Expect(jsonData.Get("user.displayName").String()).To(Equal(""))
Expect(jsonData.Get("user.id").String()).To(Equal(ownerID.OpaqueId))

jsonData = gjson.Get(tape.Body.String(), "value.0.remoteItem.shared")
Expect(jsonData.Get("sharedBy.user.displayName").String()).To(Equal(shareCreator.DisplayName))
Expect(jsonData.Get("sharedBy.user.id").String()).To(Equal(shareCreator.Id.OpaqueId))
Expect(jsonData.Get("owner.user.displayName").String()).To(Equal(""))
Expect(jsonData.Get("owner.user.id").String()).To(Equal(ownerID.OpaqueId))
})
})
})

0 comments on commit 95d40c3

Please sign in to comment.