diff --git a/changelog/unreleased/sharing-ng-empty-owner.md b/changelog/unreleased/sharing-ng-empty-owner.md new file mode 100644 index 0000000000..51769ead4f --- /dev/null +++ b/changelog/unreleased/sharing-ng-empty-owner.md @@ -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 diff --git a/services/graph/pkg/service/v0/sharedwithme.go b/services/graph/pkg/service/v0/sharedwithme.go index 14c56caee2..73558f324f 100644 --- a/services/graph/pkg/service/v0/sharedwithme.go +++ b/services/graph/pkg/service/v0/sharedwithme.go @@ -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 +} diff --git a/services/graph/pkg/service/v0/sharedwithme_test.go b/services/graph/pkg/service/v0/sharedwithme_test.go index 00df68ac1e..3426fe93f1 100644 --- a/services/graph/pkg/service/v0/sharedwithme_test.go +++ b/services/graph/pkg/service/v0/sharedwithme_test.go @@ -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)) + }) }) })