fix: Fixed shared drive owner notification on revocation

Author: shepilov

model/sharing/invitation.go

@@ -336,7 +336,6 @@ func (s *Sharing) CreateDriveShortcut(inst *instance.Instance, seen bool) error
 		return err
 	}
 
-	s.Active = true
 	s.ShortcutID = fileDoc.DocID
 	if err := couchdb.UpdateDoc(inst, s); err != nil {
 		inst.Logger().Warnf("Cannot save shortcut id %s: %s", s.ShortcutID, err)

model/sharing/member.go

@@ -1044,14 +1044,23 @@ func (s *Sharing) NotifyMemberRevocation(inst *instance.Instance, m *Member, c *
 	}
 	var token string
 	if s.Drive {
-		interact, err := permission.GetForShareInteract(inst, s.ID())
-		if err != nil {
-			return err
-		}
-		if code, ok := interact.Codes[m.Instance]; ok {
-			token = code
-		} else if code, ok := interact.Codes[m.Email]; ok {
-			token = code
+		if m.Status == MemberStatusOwner {
+			// Recipient notifying owner - use DriveToken from credentials
+			if c == nil || c.DriveToken == "" {
+				return ErrNoOAuthClient
+			}
+			token = c.DriveToken
+		} else {
+			// Owner notifying recipient - use interact codes
+			interact, err := permission.GetForShareInteract(inst, s.ID())
+			if err != nil {
+				return err
+			}
+			if code, ok := interact.Codes[m.Instance]; ok {
+				token = code
+			} else if code, ok := interact.Codes[m.Email]; ok {
+				token = code
+			}
 		}
 	} else {
 		if c.Client == nil || c.AccessToken == nil {

model/sharing/sharing.go

@@ -780,7 +780,7 @@ func (s *Sharing) RevokeRecipientByNotification(inst *instance.Instance, m *Memb
 // NoMoreRecipient cleans up the sharing if there is no more active recipient
 func (s *Sharing) NoMoreRecipient(inst *instance.Instance) error {
 	if s.Drive {
-		return nil
+		return couchdb.UpdateDoc(inst, s)
 	}
 	for _, m := range s.Members {
 		if m.Status == MemberStatusReady {

web/sharings/drives_test.go

@@ -2037,6 +2037,54 @@ func TestSharedDriveRevocation(t *testing.T) {
 	})
 }
 
+// TestSharedDriveRecipientSelfRevocation tests that a recipient can revoke themselves
+// from a shared drive and the owner is properly notified.
+func TestSharedDriveRecipientSelfRevocation(t *testing.T) {
+	if testing.Short() {
+		t.Skip("an instance is required for this test: test skipped due to the use of --short flag")
+	}
+
+	env := setupSharedDrivesEnv(t)
+	eA, eB, _ := env.createClients(t)
+
+	// Use the existing shared drive from setupSharedDrivesEnv where Betty is already a member
+	sharingID := env.firstSharingID
+
+	t.Run("VerifyBettyIsMember", func(t *testing.T) {
+		obj := eA.GET("/sharings/"+sharingID).
+			WithHeader("Authorization", "Bearer "+env.acmeToken).
+			Expect().Status(200).
+			JSON(httpexpect.ContentOpts{MediaType: "application/vnd.api+json"}).
+			Object()
+
+		// Betty should be a ready member (member index 1, after owner)
+		members := obj.Path("$.data.attributes.members").Array()
+		members.Value(1).Object().HasValue("status", "ready")
+		members.Value(1).Object().HasValue("name", "Betty")
+	})
+
+	t.Run("BettyRevokesHerself", func(t *testing.T) {
+		// Betty revokes herself from the sharing
+		eB.DELETE("/sharings/"+sharingID+"/recipients/self").
+			WithHeader("Authorization", "Bearer "+env.bettyToken).
+			Expect().Status(204)
+	})
+
+	t.Run("VerifyBettyIsRevokedOnOwnerSide", func(t *testing.T) {
+		// Wait for the revocation notification to be processed on owner's side
+		require.Eventually(t, func() bool {
+			obj := eA.GET("/sharings/"+sharingID).
+				WithHeader("Authorization", "Bearer "+env.acmeToken).
+				Expect().Status(200).
+				JSON(httpexpect.ContentOpts{MediaType: "application/vnd.api+json"}).
+				Object()
+
+			status := obj.Path("$.data.attributes.members[1].status").String().Raw()
+			return status == "revoked"
+		}, 5*time.Second, 100*time.Millisecond, "Betty's status should be revoked on owner's side")
+	})
+}
+
 // TestSharedDriveGroupDynamicMembership tests that when a contact is added to a group
 // that is part of a shared drive, the new member automatically gets access to the drive
 // without needing to accept a specific invitation.

web/sharings/revoke.go

@@ -82,6 +82,25 @@ func RevokeGroup(c echo.Context) error {
 	return c.NoContent(http.StatusNoContent)
 }
 
+// getBearerToken extracts the Bearer token from the Authorization header
+func getBearerToken(c echo.Context) string {
+	token := c.Request().Header.Get(echo.HeaderAuthorization)
+	return strings.TrimPrefix(token, "Bearer ")
+}
+
+// checkDriveSharingWritePermissions checks if the request has valid permissions for a Drive sharing.
+// For non-Drive sharings, it falls back to OAuth-based permission checking.
+func checkDriveSharingWritePermissions(c echo.Context, s *sharing.Sharing) error {
+	if s.Drive {
+		token := getBearerToken(c)
+		if token == "" || len(s.Credentials) == 0 || token != s.Credentials[0].DriveToken {
+			return echo.NewHTTPError(http.StatusForbidden)
+		}
+		return nil
+	}
+	return hasSharingWritePermissions(c)
+}
+
 // RevocationRecipientNotif is used to inform a recipient that the sharing is revoked
 func RevocationRecipientNotif(c echo.Context) error {
 	inst := middlewares.GetInstance(c)
@@ -90,16 +109,8 @@ func RevocationRecipientNotif(c echo.Context) error {
 	if err != nil {
 		return wrapErrors(err)
 	}
-	if s.Drive {
-		token := c.Request().Header.Get(echo.HeaderAuthorization)
-		token = strings.TrimPrefix(token, "Bearer ")
-		if token == "" || len(s.Credentials) == 0 || token != s.Credentials[0].DriveToken {
-			return echo.NewHTTPError(http.StatusForbidden)
-		}
-	} else {
-		if err := hasSharingWritePermissions(c); err != nil {
-			return err
-		}
+	if err := checkDriveSharingWritePermissions(c, s); err != nil {
+		return err
 	}
 	if err = s.RevokeByNotification(inst); err != nil {
 		return wrapErrors(err)
@@ -116,10 +127,26 @@ func RevocationOwnerNotif(c echo.Context) error {
 	if err != nil {
 		return wrapErrors(err)
 	}
-	member, err := requestMember(c, s)
+
+	// For Drive sharings, FindMemberByInteractCode validates the token and finds the member.
+	// For non-Drive sharings, we check permissions first, then get the member via OAuth.
+	var member *sharing.Member
+	if s.Drive {
+		token := getBearerToken(c)
+		if token == "" {
+			return echo.NewHTTPError(http.StatusForbidden)
+		}
+		member, err = s.FindMemberByInteractCode(inst, token)
+	} else {
+		if err := hasSharingWritePermissions(c); err != nil {
+			return err
+		}
+		member, err = requestMember(c, s)
+	}
 	if err != nil {
 		return wrapErrors(err)
 	}
+
 	if err = s.RevokeRecipientByNotification(inst, member); err != nil {
 		return wrapErrors(err)
 	}

web/sharings/sharings.go

@@ -1019,7 +1019,7 @@ func Routes(router *echo.Group) {
 	router.DELETE("/:sharing-id/recipients/self/readonly", UpgradeToReadWrite, checkSharingWritePermissions) // On the recipient
 	router.DELETE("/:sharing-id", RevocationRecipientNotif)                                                  // On the recipient
 	router.DELETE("/:sharing-id/recipients/self", RevokeRecipientBySelf)                                     // On the recipient
-	router.DELETE("/:sharing-id/answer", RevocationOwnerNotif, checkSharingWritePermissions)                 // On the sharer
+	router.DELETE("/:sharing-id/answer", RevocationOwnerNotif)
 	router.POST("/:sharing-id/public-key", ReceivePublicKey)
 
 	// Delegated routes for open sharing