graph/sharedWitMe: fix response for shares from project space

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: #8215
Fixes: #8027
This commit is contained in:
Ralf Haferkamp
2024-01-17 15:05:24 +01:00
committed by Ralf Haferkamp
parent 54a7da451f
commit 0e1b5dd989
3 changed files with 73 additions and 35 deletions
@@ -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
+32 -35
View File
@@ -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"
@@ -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
g.logger.Warn().Err(err).Str("userid", userID.String()).Msg("could not get owner of the share")
}
identitySet := libregraph.IdentitySet{
User: &libregraph.Identity{
DisplayName: user.GetDisplayName(),
Id: libregraph.PtrString(user.GetId()),
},
}
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
g.logger.Warn().Err(err).Str("userid", userID.String()).Msg("could not get creator of the share")
}
identitySet := libregraph.IdentitySet{
User: &libregraph.Identity{
DisplayName: user.GetDisplayName(),
Id: libregraph.PtrString(user.GetId()),
},
}
shared.SetSharedBy(identitySet)
shared.SetSharedBy(libregraph.IdentitySet{User: &identity})
}
@@ -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
}
@@ -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))
})
})
})