feat: [OCISDEV-1437] show suggested expiry date of 30 days

Author: mklos-kw

changelog/unreleased/change-default-share-expiration.md

@@ -0,0 +1,10 @@
+Change: Default 30-day expiration for personalized shares
+
+When a user or group share of a file or folder is created without an
+expiration date, the server now preconfigures a default expiration of 30 days
+to encourage data minimization. The sharer can override or shorten this value.
+
+Space memberships are exempt and stay unbounded, as they represent a permanent
+organizational role.
+
+https://github.com/owncloud/ocis/pull/12988

services/graph/pkg/service/v0/api_driveitem_permissions.go

@@ -7,6 +7,7 @@ import (
 	"net/url"
 	"slices"
 	"strings"
+	"time"
 
 	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	grouppb "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
@@ -46,6 +47,9 @@ import (
 const (
 	invalidIdMsg       = "invalid driveID or itemID"
 	parseDriveIDErrMsg = "could not parse driveID"
+
+	// default expiration for user/group shares created without one; space memberships are exempt
+	defaultShareExpirationDays = 30
 )
 
 // DriveItemPermissionsProvider contains the methods related to handling permissions on drive items
@@ -177,6 +181,16 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto
 	var shareid string
 	var expiration *types.Timestamp
 	var cTime *types.Timestamp
+
+	// use the client-supplied expiration, else default non-space shares to defaultShareExpirationDays
+	var shareExpiration *types.Timestamp
+	switch {
+	case invite.ExpirationDateTime != nil:
+		shareExpiration = utils.TimeToTS(*invite.ExpirationDateTime)
+	case !IsSpaceRoot(statResponse.GetInfo().GetId()):
+		shareExpiration = utils.TimeToTS(time.Now().UTC().AddDate(0, 0, defaultShareExpirationDays))
+	}
+
 	switch driveRecipient.GetLibreGraphRecipientType() {
 	case "group":
 		group, err := s.identityCache.GetGroup(ctx, objectID)
@@ -192,8 +206,8 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto
 			},
 		}
 		createShareRequest := createShareRequestToGroup(group, statResponse.GetInfo(), cs3ResourcePermissions)
-		if invite.ExpirationDateTime != nil {
-			createShareRequest.GetGrant().Expiration = utils.TimeToTS(*invite.ExpirationDateTime)
+		if shareExpiration != nil {
+			createShareRequest.GetGrant().Expiration = shareExpiration
 		}
 		createShareResponse, err := gatewayClient.CreateShare(ctx, createShareRequest)
 		if err := errorcode.FromCS3Status(createShareResponse.GetStatus(), err); err != nil {
@@ -259,8 +273,8 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto
 			expiration = createShareResponse.GetShare().GetExpiration()
 		} else {
 			createShareRequest := createShareRequestToUser(user, statResponse.GetInfo(), cs3ResourcePermissions)
-			if invite.ExpirationDateTime != nil {
-				createShareRequest.GetGrant().Expiration = utils.TimeToTS(*invite.ExpirationDateTime)
+			if shareExpiration != nil {
+				createShareRequest.GetGrant().Expiration = shareExpiration
 			}
 			createShareResponse, err := gatewayClient.CreateShare(ctx, createShareRequest)
 			if err := errorcode.FromCS3Status(createShareResponse.GetStatus(), err); err != nil {

services/graph/pkg/service/v0/api_driveitem_permissions_test.go

@@ -174,6 +174,71 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			Expect(permission.GrantedToV2.Group.GetId()).To(Equal("2"))
 		})
 
+		It("applies a default 30-day expiration to a user share created without one", func() {
+			var capturedReq *collaboration.CreateShareRequest
+			gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
+			gatewayClient.On("CreateShare", mock.Anything, mock.Anything).
+				Run(func(args mock.Arguments) {
+					capturedReq = args.Get(1).(*collaboration.CreateShareRequest)
+				}).Return(createShareResponse, nil)
+			driveItemInvite.Recipients = []libregraph.DriveRecipient{
+				{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
+			}
+			driveItemInvite.ExpirationDateTime = nil
+			createShareResponse.Share = &collaboration.Share{
+				Id: &collaboration.ShareId{OpaqueId: "123"},
+			}
+
+			expectedDefault := time.Now().UTC().AddDate(0, 0, 30)
+			_, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(capturedReq.GetGrant().GetExpiration()).ToNot(BeNil())
+			Expect(utils.TSToTime(capturedReq.GetGrant().GetExpiration())).To(BeTemporally("~", expectedDefault, time.Minute))
+		})
+
+		It("applies a default 30-day expiration to a group share created without one", func() {
+			var capturedReq *collaboration.CreateShareRequest
+			gatewayClient.On("GetGroup", mock.Anything, mock.Anything).Return(getGroupResponse, nil)
+			gatewayClient.On("CreateShare", mock.Anything, mock.Anything).
+				Run(func(args mock.Arguments) {
+					capturedReq = args.Get(1).(*collaboration.CreateShareRequest)
+				}).Return(createShareResponse, nil)
+			driveItemInvite.Recipients = []libregraph.DriveRecipient{
+				{ObjectId: libregraph.PtrString("2"), LibreGraphRecipientType: libregraph.PtrString("group")},
+			}
+			driveItemInvite.ExpirationDateTime = nil
+			createShareResponse.Share = &collaboration.Share{
+				Id: &collaboration.ShareId{OpaqueId: "123"},
+			}
+
+			expectedDefault := time.Now().UTC().AddDate(0, 0, 30)
+			_, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(capturedReq.GetGrant().GetExpiration()).ToNot(BeNil())
+			Expect(utils.TSToTime(capturedReq.GetGrant().GetExpiration())).To(BeTemporally("~", expectedDefault, time.Minute))
+		})
+
+		It("keeps a client-provided expiration instead of the default", func() {
+			var capturedReq *collaboration.CreateShareRequest
+			gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
+			gatewayClient.On("CreateShare", mock.Anything, mock.Anything).
+				Run(func(args mock.Arguments) {
+					capturedReq = args.Get(1).(*collaboration.CreateShareRequest)
+				}).Return(createShareResponse, nil)
+			driveItemInvite.Recipients = []libregraph.DriveRecipient{
+				{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
+			}
+			explicit := time.Now().Add(time.Hour)
+			driveItemInvite.ExpirationDateTime = libregraph.PtrTime(explicit)
+			createShareResponse.Share = &collaboration.Share{
+				Id: &collaboration.ShareId{OpaqueId: "123"},
+			}
+
+			_, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(utils.TSToTime(capturedReq.GetGrant().GetExpiration())).To(BeTemporally("~", explicit, time.Second))
+		})
+
 		It("succeeds with file roles (happy path)", func() {
 			gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
 			gatewayClient.On("CreateShare", mock.Anything, mock.Anything).Return(createShareResponse, nil)
@@ -349,6 +414,37 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			Expect(permission.GrantedToV2.User.GetDisplayName()).To(Equal(getUserResponse.User.DisplayName))
 			Expect(permission.GrantedToV2.User.GetId()).To(Equal("1"))
 		})
+		It("does not apply a default expiration to a space membership created without one", func() {
+			root := &provider.ResourceId{
+				StorageId: "1",
+				SpaceId:   "2",
+				OpaqueId:  "2", // space root: OpaqueId == SpaceId
+			}
+			listSpacesResponse.StorageSpaces[0].SpaceType = "project"
+			listSpacesResponse.StorageSpaces[0].Root = root
+			statResponse.Info.Id = root
+			statResponse.Info.Space = &provider.StorageSpace{Root: root}
+
+			var capturedReq *collaboration.CreateShareRequest
+			gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(listSpacesResponse, nil)
+			gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(getUserResponse, nil)
+			gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(statResponse, nil)
+			gatewayClient.On("CreateShare", mock.Anything, mock.Anything).
+				Run(func(args mock.Arguments) {
+					capturedReq = args.Get(1).(*collaboration.CreateShareRequest)
+				}).Return(createShareResponse, nil)
+			driveItemInvite.Recipients = []libregraph.DriveRecipient{
+				{ObjectId: libregraph.PtrString("1"), LibreGraphRecipientType: libregraph.PtrString("user")},
+			}
+			driveItemInvite.ExpirationDateTime = nil
+			createShareResponse.Share = &collaboration.Share{
+				Id: &collaboration.ShareId{OpaqueId: "123"},
+			}
+
+			_, err := driveItemPermissionsService.SpaceRootInvite(context.Background(), driveId, driveItemInvite)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(capturedReq.GetGrant().GetExpiration()).To(BeNil())
+		})
 		It("rejects to add a user to a personal space", func() {
 			gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(listSpacesResponse, nil)
 			driveItemInvite.Recipients = []libregraph.DriveRecipient{

services/proxy/pkg/middleware/deferral_regression_test.go

@@ -0,0 +1,49 @@
+package middleware
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/owncloud/ocis/v2/ocis-pkg/oidc"
+	"github.com/owncloud/ocis/v2/services/proxy/pkg/router"
+)
+
+type stubAuth struct {
+	name  string
+	err   error // non-nil => this authenticator fails with this error
+	calls *[]string
+}
+
+func (s stubAuth) Authenticate(r *http.Request) (*http.Request, error) {
+	*s.calls = append(*s.calls, s.name)
+	if s.err != nil {
+		return nil, s.err
+	}
+	return r, nil
+}
+
+// A transient OIDC failure followed by a succeeding authenticator must serve 200,
+// not 503 — the deferral flag exists so authenticator order does not matter.
+func TestTransientThenSuccessServes200(t *testing.T) {
+	var calls []string
+	auths := []Authenticator{
+		stubAuth{name: "oidc-transient", err: oidc.ErrTemporarilyUnavailable, calls: &calls},
+		stubAuth{name: "public-share-ok", err: nil, calls: &calls},
+	}
+
+	served := false
+	handler := Authentication(auths, EnableBasicAuth(false))(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { served = true }),
+	)
+
+	req := httptest.NewRequest(http.MethodGet, "http://example.com/dav/public-files/x", http.NoBody)
+	req = req.WithContext(router.SetRoutingInfo(req.Context(), router.RoutingInfo{}))
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+
+	t.Logf("authenticators called: %v, status: %d", calls, rr.Code)
+	if !served || rr.Code != http.StatusOK {
+		t.Fatalf("want 200 served by later authenticator, got status=%d served=%v (calls=%v)", rr.Code, served, calls)
+	}
+}

tests/acceptance/features/coreApiShareUpdateToShares/updateShare.feature

@@ -184,7 +184,6 @@ Feature: sharing
       | permissions                | all                  |
       | stime                      | A_NUMBER             |
       | parent                     |                      |
-      | expiration                 |                      |
       | token                      |                      |
       | uid_file_owner             | %username%           |
       | displayname_file_owner     | %displayname%        |