Enhancement: No longer returns shares whose expiration date has passed. by pmedinar01 · Pull Request #5571 · cs3org/reva

pmedinar01 · 2026-04-08T09:36:22Z

No description provided.

Copilot

Pull request overview

Adds expiration handling for user/group shares by filtering expired shares from received listings, providing a DB query to enumerate expired shares, and introducing a gateway-side cleanup routine to remove expired shares and associated storage grants.

Changes:

Filter expired shares out of ListReceivedShares results in the SQL share manager.
Add ListExpiredShares to fetch all expired (non-orphan, non-deleted) shares from the SQL backend.
Add ExpireShares in the gateway to remove storage grants and delete expired shares; add an unreleased changelog entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
pkg/share/manager/sql/share.go	Filters expired shares from received listings; adds DB-level listing of expired shares.
internal/grpc/services/gateway/usershareprovider.go	Adds an expiration cleanup method that removes grants and deletes expired shares.
changelog/unreleased/expire-shares.md	Documents the new expiration behavior and APIs (needs template alignment).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T09:42:17Z

+		if _, err := c.RemoveShare(ctx, &collaboration.RemoveShareRequest{Ref: ref}); err != nil {
+			return errors.Wrap(err, "gateway: error removing expired share "+share.Id.OpaqueId)
+		}


ExpireShares ignores the RemoveShare response status: it only checks the gRPC error, so a non-OK status could be silently treated as success and leave the expired share in the DB. Capture the response and verify Status.Code == CODE_OK (and include Status.Message in the returned error) similarly to the ListShares and removeGrant checks above.

Suggested change

if _, err := c.RemoveShare(ctx, &collaboration.RemoveShareRequest{Ref: ref}); err != nil {

return errors.Wrap(err, "gateway: error removing expired share "+share.Id.OpaqueId)

}

removeShareRes, err := c.RemoveShare(ctx, &collaboration.RemoveShareRequest{Ref: ref})

if err != nil {

return errors.Wrap(err, "gateway: error removing expired share "+share.Id.OpaqueId)

}

if removeShareRes.Status.Code != rpc.Code_CODE_OK {

return fmt.Errorf("gateway: RemoveShare failed for share %s: %s", share.Id.OpaqueId, removeShareRes.Status.Message)

}

Copilot · 2026-04-08T09:42:18Z

+// ListExpiredShares returns all non-deleted, non-orphan shares whose expiration date is in the past.
+func (m *ShareMgr) ListExpiredShares(ctx context.Context) ([]*collaboration.Share, error) {
+	var modelShares []model.Share
+	res := m.db.Model(&model.Share{}).
+		Where("orphan = ?", false).
+		Where("deleted_at IS NULL").
+		Where("expiration IS NOT NULL AND expiration < ?", time.Now()).
+		Find(&modelShares)
+	if res.Error != nil {
+		return nil, res.Error
+	}
+
+	shares := make([]*collaboration.Share, 0, len(modelShares))
+	for _, s := range modelShares {
+		granteeType, _ := m.getUserType(ctx, s.ShareWith)
+		shares = append(shares, s.AsCS3Share(granteeType))
+	}
+	return shares, nil


ListExpiredShares adds new behavior (querying and returning all expired shares) but there is no test coverage for it in the existing SQL share manager test suite (share_test.go). Please add tests that create shares with past and future expirations and verify only the past-expired ones are returned.

Copilot · 2026-04-08T09:42:19Z

+- `ListReceivedShares` no longer returns shares whose expiration date has passed
+- New `ListExpiredShares` method on the SQL share manager returns all shares past their expiry
+- New `ExpireShares` method on the gateway removes storage grants and deletes all expired shares


This changelog entry does not follow changelog/TEMPLATE: it uses a bullet list and is missing the trailing section with issue/PR URLs that the changelog tooling expects. Please adjust the body to match the template format (paragraphs in past tense) and add the relevant PR/issue link(s) at the end.

Suggested change

- `ListReceivedShares` no longer returns shares whose expiration date has passed

- New `ListExpiredShares` method on the SQL share manager returns all shares past their expiry

- New `ExpireShares` method on the gateway removes storage grants and deletes all expired shares

`ListReceivedShares` no longer returned shares whose expiration date had passed. A new `ListExpiredShares` method on the SQL share manager returned all shares past their expiry. A new `ExpireShares` method on the gateway removed storage grants and deleted all expired shares.

Issues:

- https://github.com/<owner>/<repo>/issues/<number>

- https://github.com/<owner>/<repo>/pull/<number>

Copilot · 2026-04-08T09:42:19Z

@@ -236,7 +236,8 @@ func (m *ShareMgr) ListReceivedShares(ctx context.Context, filters []*collaborat
 		Select("share_states.*, shares.*").
 		Joins("RIGHT OUTER JOIN shares ON shares.id = share_states.share_id and share_states.user = ?", user.Username).
 		Where("shares.orphan = ?", false).
-		Where("shares.deleted_at IS NULL")
+		Where("shares.deleted_at IS NULL").
+		Where("shares.expiration IS NULL OR shares.expiration > ?", time.Now())


Expiration is only enforced in ListReceivedShares (via the SQL shares.expiration predicate). Other read paths like GetReceivedShare/GetShare ultimately call getShareByID/getShareByKey, which do not check expiration, so an expired share can still be fetched by reference until a cleanup job removes it. If expiration is meant to revoke access immediately, consider applying the same expiration constraint in the lower-level getShare... queries (or returning NotFound for expired shares).

pmedinar01 requested review from diocas, glpatcern, jessegeens and rawe0 as code owners April 8, 2026 09:36

Copilot AI review requested due to automatic review settings April 8, 2026 09:36

Copilot started reviewing on behalf of pmedinar01 April 8, 2026 09:37 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

pmedinar01 force-pushed the expire_shares branch from 522e906 to 88afb6c Compare April 8, 2026 09:46

Enhancement: Add support for expiring user/group shares

be1e421

pmedinar01 force-pushed the expire_shares branch from 3a226c7 to be1e421 Compare April 8, 2026 12:09

pablomr0603 added 2 commits April 9, 2026 10:05

Do not show Expired shares from ListReceivedShares

43caa5f

Do not show Expired shares from ListReceivedShares

d14b139

pmedinar01 changed the title ~~Enhancement: Add support for expiring user/group shares~~ Enhancement: No longer returns shares whose expiration date has passed. Apr 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancement: No longer returns shares whose expiration date has passed.#5571

Enhancement: No longer returns shares whose expiration date has passed.#5571
pmedinar01 wants to merge 3 commits into
masterfrom
expire_shares

pmedinar01 commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-		if _, err := c.RemoveShare(ctx, &collaboration.RemoveShareRequest{Ref: ref}); err != nil {
-			return errors.Wrap(err, "gateway: error removing expired share "+share.Id.OpaqueId)
-		}
+		removeShareRes, err := c.RemoveShare(ctx, &collaboration.RemoveShareRequest{Ref: ref})
+		if err != nil {
+			return errors.Wrap(err, "gateway: error removing expired share "+share.Id.OpaqueId)
+		}
+		if removeShareRes.Status.Code != rpc.Code_CODE_OK {
+			return fmt.Errorf("gateway: RemoveShare failed for share %s: %s", share.Id.OpaqueId, removeShareRes.Status.Message)
+		}

Conversation

pmedinar01 commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants