Merge pull request #4256 from dragonchaser/rework-hidden-shares

Rework hidden shares

changelog/unreleased/rename-hidden-share-vaue.md

@@ -0,0 +1,6 @@
+Enhancement: Rename hidden share variable name
+
+We have renamed the hidden flag on shares from Hide -> Hidden to align to the cs3api
+
+https://github.com/cs3org/reva/pull/4256
+https://github.com/cs3org/cs3apis/pull/214

go.mod

@@ -18,7 +18,7 @@ require (
 	github.com/cheggaaa/pb v1.0.29
 	github.com/coreos/go-oidc v2.2.1+incompatible
 	github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e
-	github.com/cs3org/go-cs3apis v0.0.0-20230727093620-0f4399be4543
+	github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781
 	github.com/dgraph-io/ristretto v0.1.1
 	github.com/emvi/iso-639-1 v1.0.1
 	github.com/eventials/go-tus v0.0.0-20220610120217-05d0564bb571
@@ -227,6 +227,3 @@ require (
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
-
-// the replacement build is based on https://github.com/dragonchaser/cs3apis/tree/master
-replace github.com/cs3org/go-cs3apis => github.com/aduffeck/go-cs3apis v0.0.0-20231009082215-ad45e19edac0

go.sum

@@ -424,8 +424,6 @@ github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbt
 github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
 github.com/acomagu/bufpipe v1.0.3 h1:fxAGrHZTgQ9w5QqVItgzwj235/uYZYgbXitB+dLupOk=
 github.com/acomagu/bufpipe v1.0.3/go.mod h1:mxdxdup/WdsKVreO5GpW4+M/1CE2sMG4jeGJ2sYmHc4=
-github.com/aduffeck/go-cs3apis v0.0.0-20231009082215-ad45e19edac0 h1:y9gvblZJHMENTzgolQ9sChKONSILkJ4U/j7OqMIF4QM=
-github.com/aduffeck/go-cs3apis v0.0.0-20231009082215-ad45e19edac0/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
@@ -521,6 +519,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46t
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJffz4pz0o1WuQxJ28+5x5JgaHD8=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4=
+github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 h1:BUdwkIlf8IS2FasrrPg8gGPHQPOrQ18MS1Oew2tmGtY=
+github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go

@@ -122,9 +122,20 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) {
 			// TODO we could delete shares here if the stat returns code NOT FOUND ... but listening for file deletes would be better
 		}
 	}
+	// we need to add a path to the share
+	receivedShare := &collaboration.ReceivedShare{
+		Share: &collaboration.Share{
+			Id: &collaboration.ShareId{OpaqueId: shareID},
+		},
+		State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
+		MountPoint: &provider.Reference{
+			Path: mount,
+		},
+	}
+	updateMask := &fieldmaskpb.FieldMask{Paths: []string{"state", "hidden", "mount_point"}}
 
 	for id := range sharesToAccept {
-		data := h.updateReceivedShare(w, r, id, false, mount)
+		data := h.updateReceivedShare(w, r, receivedShare, updateMask)
 		// only render the data for the changed share
 		if id == shareID && data != nil {
 			response.WriteOCSSuccess(w, r, []*conversions.ShareData{data})
@@ -135,45 +146,69 @@ func (h *Handler) AcceptReceivedShare(w http.ResponseWriter, r *http.Request) {
 // RejectReceivedShare handles DELETE Requests on /apps/files_sharing/api/v1/shares/{shareid}
 func (h *Handler) RejectReceivedShare(w http.ResponseWriter, r *http.Request) {
 	shareID := chi.URLParam(r, "shareid")
-	data := h.updateReceivedShare(w, r, shareID, true, "")
+	// we need to add a path to the share
+	receivedShare := &collaboration.ReceivedShare{
+		Share: &collaboration.Share{
+			Id: &collaboration.ShareId{OpaqueId: shareID},
+		},
+		State: collaboration.ShareState_SHARE_STATE_REJECTED,
+	}
+	updateMask := &fieldmaskpb.FieldMask{Paths: []string{"state", "hidden"}}
+
+	data := h.updateReceivedShare(w, r, receivedShare, updateMask)
 	if data != nil {
 		response.WriteOCSSuccess(w, r, []*conversions.ShareData{data})
 	}
 }
 
-func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, shareID string, rejectShare bool, mountPoint string) *conversions.ShareData {
-	ctx := r.Context()
-	logger := appctx.GetLogger(ctx)
+func (h *Handler) UpdateReceivedShare(w http.ResponseWriter, r *http.Request) {
+	shareID := chi.URLParam(r, "shareid")
+	hideFlag, _ := strconv.ParseBool(r.URL.Query().Get("hidden"))
 
+	// unfortunately we need to get the share first to read the state
 	client, err := h.getClient()
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err)
-		return nil
+		return
 	}
 
-	hideFlag, _ := strconv.ParseBool(r.URL.Query().Get("hide"))
-
 	// we need to add a path to the share
-	shareRequest := &collaboration.UpdateReceivedShareRequest{
-		Share: &collaboration.ReceivedShare{
-			Share: &collaboration.Share{
-				Id:   &collaboration.ShareId{OpaqueId: shareID},
-				Hide: hideFlag,
-			},
-			MountPoint: &provider.Reference{
-				Path: mountPoint,
-			},
+	receivedShare := &collaboration.ReceivedShare{
+		Share: &collaboration.Share{
+			Id: &collaboration.ShareId{OpaqueId: shareID},
 		},
-		UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state", "hide"}},
+		Hidden: hideFlag,
+	}
+	updateMask := &fieldmaskpb.FieldMask{Paths: []string{"state", "hidden"}}
+
+	rs, _ := getReceivedShareFromID(r.Context(), client, shareID)
+	if rs != nil && rs.Share != nil {
+		receivedShare.State = rs.Share.State
 	}
-	if rejectShare {
-		shareRequest.Share.State = collaboration.ShareState_SHARE_STATE_REJECTED
-	} else {
-		shareRequest.UpdateMask.Paths = append(shareRequest.UpdateMask.Paths, "mount_point")
-		shareRequest.Share.State = collaboration.ShareState_SHARE_STATE_ACCEPTED
+
+	data := h.updateReceivedShare(w, r, receivedShare, updateMask)
+	if data != nil {
+		response.WriteOCSSuccess(w, r, []*conversions.ShareData{data})
+	}
+	// TODO: do we need error handling here?
+}
+
+func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, receivedShare *collaboration.ReceivedShare, fieldMask *fieldmaskpb.FieldMask) *conversions.ShareData {
+	ctx := r.Context()
+	logger := appctx.GetLogger(ctx)
+
+	updateShareRequest := &collaboration.UpdateReceivedShareRequest{
+		Share:      receivedShare,
+		UpdateMask: fieldMask,
+	}
+
+	client, err := h.getClient()
+	if err != nil {
+		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error getting grpc gateway client", err)
+		return nil
 	}
 
-	shareRes, err := client.UpdateReceivedShare(ctx, shareRequest)
+	shareRes, err := client.UpdateReceivedShare(ctx, updateShareRequest)
 	if err != nil {
 		response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc update received share request failed", err)
 		return nil
@@ -203,6 +238,7 @@ func (h *Handler) updateReceivedShare(w http.ResponseWriter, r *http.Request, sh
 	}
 
 	data.State = mapState(rs.GetState())
+	data.Hidden = rs.Hidden
 
 	h.addFileInfo(ctx, data, info)
 	h.mapUserIds(r.Context(), client, data)

internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go

@@ -580,9 +580,9 @@ func (h *Handler) GetShare(w http.ResponseWriter, r *http.Request) {
 			},
 		})
 		if err == nil && uRes.GetShare() != nil {
-			receivedshare = uRes.Share
 			resourceID = uRes.Share.Share.ResourceId
 			share, err = conversions.CS3Share2ShareData(ctx, uRes.Share.Share)
+			share.Hidden = uRes.Share.Hidden
 			if err != nil {
 				response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err)
 				return
@@ -739,7 +739,7 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, share *col
 
 	share.Permissions = &collaboration.SharePermissions{Permissions: role.CS3ResourcePermissions()}
 
-	var fieldMaskPaths = []string{"permissions", "hide"}
+	var fieldMaskPaths = []string{"permissions"}
 
 	expireDate := r.PostFormValue("expireDate")
 	var expirationTs *types.Timestamp
@@ -958,7 +958,7 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) {
 
 	// TODO(refs) filter out "invalid" shares
 	for _, rs := range lrsRes.GetShares() {
-		if rs.Share.Hide && !showHidden {
+		if rs.Hidden && !showHidden {
 			continue
 		}
 		if stateFilter != ocsStateUnknown && rs.GetState() != stateFilter {

internal/http/services/owncloud/ocs/ocs.go

@@ -122,6 +122,7 @@ func (s *svc) routerInit(log *zerolog.Logger) error {
 				r.Route("/pending/{shareid}", func(r chi.Router) {
 					r.Post("/", sharesHandler.AcceptReceivedShare)
 					r.Delete("/", sharesHandler.RejectReceivedShare)
+					r.Put("/", sharesHandler.UpdateReceivedShare)
 				})
 				r.Route("/remote_shares", func(r chi.Router) {
 					r.Get("/", sharesHandler.ListFederatedShares)

pkg/conversions/main.go

@@ -172,7 +172,7 @@ type ShareData struct {
 	Quicklink bool `json:"quicklink,omitempty" xml:"quicklink,omitempty"`
 	// PasswordProtected represents a public share is password protected
 	// PasswordProtected bool `json:"password_protected,omitempty" xml:"password_protected,omitempty"`
-	Hide bool `json:"hide" xml:"hide"`
+	Hidden bool `json:"hidden" xml:"hidden"`
 }
 
 // ShareeData holds share recipient search results
@@ -233,7 +233,6 @@ func CS3Share2ShareData(ctx context.Context, share *collaboration.Share) (*Share
 		// Displaynames are added later
 		UIDOwner:     LocalUserIDToString(share.GetCreator()),
 		UIDFileOwner: LocalUserIDToString(share.GetOwner()),
-		Hide:         share.Hide,
 	}
 
 	if share.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER {

pkg/share/manager/cs3/cs3.go

@@ -647,7 +647,7 @@ func (m *Manager) UpdateReceivedShare(ctx context.Context, rshare *collaboration
 			rs.State = rshare.State
 		case "mount_point":
 			rs.MountPoint = rshare.MountPoint
-		case "hide":
+		case "hidden":
 			continue
 		default:
 			return nil, errtypes.NotSupported("updating " + fieldMask.Paths[i] + " is not supported")

pkg/share/manager/json/json.go

@@ -403,7 +403,7 @@ func (m *mgr) UpdateShare(ctx context.Context, ref *collaboration.ShareReference
 				m.model.Shares[idx].Permissions = updated.Permissions
 			case "expiration":
 				m.model.Shares[idx].Expiration = updated.Expiration
-			case "hide":
+			case "hidden":
 				continue
 			default:
 				return nil, errtypes.NotSupported("updating " + fieldMask.Paths[i] + " is not supported")

pkg/share/manager/jsoncs3/jsoncs3.go

@@ -511,8 +511,6 @@ func (m *Manager) UpdateShare(ctx context.Context, ref *collaboration.ShareRefer
 				toUpdate.Permissions = updated.Permissions
 			case "expiration":
 				toUpdate.Expiration = updated.Expiration
-			case "hide":
-				toUpdate.Hide = updated.Hide
 			default:
 				return nil, errtypes.NotSupported("updating " + fieldMask.Paths[i] + " is not supported")
 			}
@@ -886,11 +884,11 @@ func (m *Manager) ListReceivedShares(ctx context.Context, filters []*collaborati
 
 					if share.IsGrantedToUser(s, user) {
 						if share.MatchesFiltersWithState(s, state.State, filters) {
-							s.Hide = state.Hide
 							rs := &collaboration.ReceivedShare{
 								Share:      s,
 								State:      state.State,
 								MountPoint: state.MountPoint,
+								Hidden:     state.Hidden,
 							}
 							select {
 							case results <- rs:
@@ -942,7 +940,7 @@ func (m *Manager) convert(ctx context.Context, userID string, s *collaboration.S
 	if err == nil && state != nil {
 		rs.State = state.State
 		rs.MountPoint = state.MountPoint
-		rs.Share.Hide = state.Hide
+		rs.Hidden = state.Hidden
 	}
 	return rs
 }
@@ -1007,8 +1005,8 @@ func (m *Manager) UpdateReceivedShare(ctx context.Context, receivedShare *collab
 			rs.State = receivedShare.State
 		case "mount_point":
 			rs.MountPoint = receivedShare.MountPoint
-		case "hide":
-			rs.Share.Hide = receivedShare.Share.Hide
+		case "hidden":
+			rs.Hidden = receivedShare.Hidden
 		default:
 			return nil, errtypes.NotSupported("updating " + fieldMask.Paths[i] + " is not supported")
 		}

pkg/share/manager/jsoncs3/jsoncs3_test.go

@@ -957,7 +957,7 @@ var _ = Describe("Jsoncs3", func() {
 				})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(rs.MountPoint.Path).To(Equal("newMP"))
-				Expect(rs.Share.Hide).To(Equal(false))
+				Expect(rs.Hidden).To(Equal(false))
 			})
 
 			It("hides the share", func() {
@@ -972,12 +972,12 @@ var _ = Describe("Jsoncs3", func() {
 				rs.MountPoint = &providerv1beta1.Reference{
 					Path: "newMP",
 				}
-				rs.Share.Hide = true
+				rs.Hidden = true
 
-				rs, err = m.UpdateReceivedShare(granteeCtx, rs, &fieldmaskpb.FieldMask{Paths: []string{"hide", "mount_point"}}, nil)
+				rs, err = m.UpdateReceivedShare(granteeCtx, rs, &fieldmaskpb.FieldMask{Paths: []string{"hidden", "mount_point"}}, nil)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(rs.MountPoint.Path).To(Equal("newMP"))
-				Expect(rs.Share.Hide).To(Equal(true))
+				Expect(rs.Hidden).To(Equal(true))
 
 				rs, err = m.GetReceivedShare(granteeCtx, &collaboration.ShareReference{
 					Spec: &collaboration.ShareReference_Id{
@@ -986,7 +986,7 @@ var _ = Describe("Jsoncs3", func() {
 				})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(rs.MountPoint.Path).To(Equal("newMP"))
-				Expect(rs.Share.Hide).To(Equal(true))
+				Expect(rs.Hidden).To(Equal(true))
 			})
 
 			It("handles invalid field masks", func() {
@@ -1071,11 +1071,11 @@ var _ = Describe("Jsoncs3", func() {
 					Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_PENDING))
 
 					rs.State = collaboration.ShareState_SHARE_STATE_ACCEPTED
-					rs.Share.Hide = true
-					rs, err = m.UpdateReceivedShare(granteeCtx, rs, &fieldmaskpb.FieldMask{Paths: []string{"state", "hide"}}, nil)
+					rs.Hidden = true
+					rs, err = m.UpdateReceivedShare(granteeCtx, rs, &fieldmaskpb.FieldMask{Paths: []string{"state", "hidden"}}, nil)
 					Expect(err).ToNot(HaveOccurred())
 					Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED))
-					Expect(rs.Share.Hide).To(Equal(true))
+					Expect(rs.Hidden).To(Equal(true))
 
 					m, err := jsoncs3.New(storage, nil, 0, nil, 0) // Reset in-memory cache
 					Expect(err).ToNot(HaveOccurred())
@@ -1085,7 +1085,7 @@ var _ = Describe("Jsoncs3", func() {
 							Id: gshare.Id,
 						},
 					})
-					Expect(rs.Share.Hide).To(Equal(true))
+					Expect(rs.Hidden).To(Equal(true))
 					Expect(err).ToNot(HaveOccurred())
 					Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED))
 				})
@@ -1100,11 +1100,11 @@ var _ = Describe("Jsoncs3", func() {
 					Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_PENDING))
 
 					rs.State = collaboration.ShareState_SHARE_STATE_ACCEPTED
-					rs.Share.Hide = true
-					rs, err = m.UpdateReceivedShare(granteeCtx, rs, &fieldmaskpb.FieldMask{Paths: []string{"state", "hide"}}, nil)
+					rs.Hidden = true
+					rs, err = m.UpdateReceivedShare(granteeCtx, rs, &fieldmaskpb.FieldMask{Paths: []string{"state", "hidden"}}, nil)
 					Expect(err).ToNot(HaveOccurred())
 					Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED))
-					Expect(rs.Share.Hide).To(Equal(true))
+					Expect(rs.Hidden).To(Equal(true))
 
 					m, err := jsoncs3.New(storage, nil, 0, nil, 0) // Reset in-memory cache
 					Expect(err).ToNot(HaveOccurred())
@@ -1114,22 +1114,22 @@ var _ = Describe("Jsoncs3", func() {
 							Id: gshare.Id,
 						},
 					})
-					Expect(rs.Share.Hide).To(Equal(true))
+					Expect(rs.Hidden).To(Equal(true))
 					Expect(err).ToNot(HaveOccurred())
 					Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED))
 
-					rs.Share.Hide = false
-					rs, err = m.UpdateReceivedShare(granteeCtx, rs, &fieldmaskpb.FieldMask{Paths: []string{"state", "hide"}}, nil)
+					rs.Hidden = false
+					rs, err = m.UpdateReceivedShare(granteeCtx, rs, &fieldmaskpb.FieldMask{Paths: []string{"state", "hidden"}}, nil)
 					Expect(err).ToNot(HaveOccurred())
 					Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED))
-					Expect(rs.Share.Hide).To(Equal(false))
+					Expect(rs.Hidden).To(Equal(false))
 
 					rs, err = m.GetReceivedShare(granteeCtx, &collaboration.ShareReference{
 						Spec: &collaboration.ShareReference_Id{
 							Id: gshare.Id,
 						},
 					})
-					Expect(rs.Share.Hide).To(Equal(false))
+					Expect(rs.Hidden).To(Equal(false))
 					Expect(err).ToNot(HaveOccurred())
 					Expect(rs.State).To(Equal(collaboration.ShareState_SHARE_STATE_ACCEPTED))
 				})