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

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/bootstrap/SharingNgContext.php

@@ -2500,6 +2500,35 @@ public function theJsonResponseShouldOrShouldNotContainTheFollowingShares(
 		}
 	}
 
+	/**
+	 * @Then /^the last share invitation should have an expiration date approximately "([^"]*)" days from now$/
+	 *
+	 * @param string $days
+	 *
+	 * @return void
+	 */
+	public function theLastShareInvitationShouldHaveAnExpirationDateApproximatelyDaysFromNow(
+		string $days,
+	): void {
+		$responseBody = $this->featureContext->getJsonDecodedResponseBodyContent();
+		Assert::assertTrue(
+			isset($responseBody->value[0]->expirationDateTime),
+			"Expected the created share to have an 'expirationDateTime' but none was returned:\n"
+			. print_r($responseBody, true),
+		);
+		$actual = new DateTime($responseBody->value[0]->expirationDateTime);
+		$expected = (new DateTime())->modify("+" . (int)$days . " days");
+		$diffSeconds = \abs($actual->getTimestamp() - $expected->getTimestamp());
+		// allow a 1-day tolerance to absorb any end-of-day rounding of the expiration
+		Assert::assertLessThanOrEqual(
+			86400,
+			$diffSeconds,
+			"Expected an expiration date approximately $days days from now ("
+			. $expected->format(DATE_ATOM) . "), but got " . $actual->format(DATE_ATOM)
+			. " (difference of {$diffSeconds}s)",
+		);
+	}
+
 	/**
 	 * @When /^user "([^"]*)" lists permissions with following filters for (?:folder|file) "([^"]*)" of the space "([^"]*)" using the Graph API:$/
 	 *

tests/acceptance/features/apiSharingNgDriveInvitation/driveShareInvitations.feature

@@ -88,6 +88,51 @@ Feature: Send a drive invitations
       | Manager          |
 
 
+  Scenario Outline: space membership does not get a default expiration when created without one
+    Given using spaces DAV path
+    And the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API
+    And user "Alice" has created a space "NewSpace" with the default quota using the Graph API
+    When user "Alice" sends the following space share invitation using permissions endpoint of the Graph API:
+      | space           | NewSpace           |
+      | sharee          | Brian              |
+      | shareType       | user               |
+      | permissionsRole | <permissions-role> |
+    Then the HTTP status code should be "200"
+    And the JSON data of the response should match
+      """
+      {
+        "type": "object",
+        "required": [
+          "value"
+        ],
+        "properties": {
+          "value": {
+            "type": "array",
+            "minItems": 1,
+            "maxItems": 1,
+            "items": {
+              "type": "object",
+              "required": [
+                "grantedToV2",
+                "roles"
+              ],
+              "not": {
+                "required": [
+                  "expirationDateTime"
+                ]
+              }
+            }
+          }
+        }
+      }
+      """
+    Examples:
+      | permissions-role |
+      | Space Viewer     |
+      | Space Editor     |
+      | Manager          |
+
+
   Scenario Outline: send share invitation for disabled project space to user with different roles (permissions endpoint)
     Given using spaces DAV path
     And the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API

tests/acceptance/features/apiSharingNgItemInvitation/itemShareInvitations.feature

@@ -760,6 +760,54 @@ Feature: Send a sharing invitations
       | Editor           | FolderToShare  |
       | Uploader         | FolderToShare  |
 
+
+  Scenario Outline: server sets a default 30-day expiration when a user share is created without one
+    Given user "Alice" has uploaded file with content "to share" to "/textfile1.txt"
+    And user "Alice" has created folder "FolderToShare"
+    When user "Alice" sends the following resource share invitation using the Graph API:
+      | resource        | <resource>         |
+      | space           | Personal           |
+      | sharee          | Brian              |
+      | shareType       | user               |
+      | permissionsRole | <permissions-role> |
+    Then the HTTP status code should be "200"
+    And the JSON data of the response should match
+      """
+      {
+        "type": "object",
+        "required": [
+          "value"
+        ],
+        "properties": {
+          "value": {
+            "type": "array",
+            "minItems": 1,
+            "maxItems": 1,
+            "items": {
+              "type": "object",
+              "required": [
+                "id",
+                "roles",
+                "grantedToV2",
+                "expirationDateTime"
+              ],
+              "properties": {
+                "expirationDateTime": {
+                  "type": "string",
+                  "format": "date-time"
+                }
+              }
+            }
+          }
+        }
+      }
+      """
+    And the last share invitation should have an expiration date approximately "30" days from now
+    Examples:
+      | permissions-role | resource       |
+      | Viewer           | /textfile1.txt |
+      | Viewer           | FolderToShare  |
+
   @issue-7962
   Scenario Outline: send share invitation to disabled user
     Given user "Alice" has uploaded file with content "to share" to "/textfile1.txt"

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%        |