fix(graph): avoid superfluous GetPublicShare call when deleting space permissions

changelog/unreleased/fix-graph-delete-permission-superfluous-call.md

@@ -0,0 +1,9 @@
+Bugfix: Avoid superfluous GetPublicShare call when deleting space permissions
+
+We fixed `DeletePermission` to recognise space permission IDs (prefixed with
+`u:` or `g:`) by their format before making any gateway calls. Previously,
+deleting a space member always triggered a `GetPublicShare` lookup that was
+guaranteed to fail, producing a confusing error log.
+
+https://github.com/owncloud/ocis/pull/12122
+https://github.com/owncloud/ocis/issues/12012

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

@@ -556,40 +556,13 @@ func (s DriveItemPermissionsService) DeletePermission(ctx context.Context, itemI
 	ctx, span := s.tp.Tracer("graph").Start(ctx, "DeletePermission")
 	defer span.End()
 
-	var permissionType permissionType
-
-	sharedResourceID, err := s.getLinkPermissionResourceID(ctx, permissionID)
-	switch {
-	// Check if the ID is referring to a public share
-	case err == nil:
-		permissionType = Public
-	// If the item id is referring to a space root and this is not a public share
-	// we have to deal with space permissions
-	case IsSpaceRoot(itemID):
-		permissionType = Space
-		sharedResourceID = itemID
-		err = nil
-	// If this is neither a public share nor a space permission, check if this is a
-	// user share
-	default:
-		sharedResourceID, err = s.getUserPermissionResourceID(ctx, permissionID)
-		if err == nil {
-			permissionType = User
-		}
-	}
-
-	if sharedResourceID == nil && s.config.IncludeOCMSharees {
-		sharedResourceID, err = s.getOCMPermissionResourceID(ctx, permissionID)
-		if err == nil {
-			permissionType = OCM
-		}
-	}
+	permType, sharedResourceID, err := s.resolveDeletePermission(ctx, itemID, permissionID)
 
 	switch {
 	case err != nil:
 		s.logger.Error().Err(err).Msg("permission error")
 		return err
-	case permissionType == Unknown:
+	case permType == Unknown:
 		s.logger.Error().Err(err).Msg("permission not found")
 		return errorcode.New(errorcode.ItemNotFound, "permission not found")
 	case sharedResourceID == nil:
@@ -605,7 +578,7 @@ func (s DriveItemPermissionsService) DeletePermission(ctx context.Context, itemI
 		return err
 	}
 
-	switch permissionType {
+	switch permType {
 	case User:
 		return s.removeUserShare(ctx, permissionID)
 	case Public:
@@ -622,6 +595,54 @@ func (s DriveItemPermissionsService) DeletePermission(ctx context.Context, itemI
 	}
 }
 
+// resolveDeletePermission determines the type and shared resource ID for a permission
+// that is about to be deleted.
+func (s DriveItemPermissionsService) resolveDeletePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string) (permissionType, *storageprovider.ResourceId, error) {
+	if IsSpaceRoot(itemID) {
+		return s.resolveSpaceRootPermission(ctx, itemID, permissionID)
+	}
+	return s.resolveItemPermission(ctx, permissionID)
+}
+
+// resolveSpaceRootPermission resolves permissions on a space root. Space roots
+// carry space permissions (member/group grants with "u:"/"g:" prefixed IDs) and
+// public links. Space permissions are recognised by their ID format to avoid a
+// superfluous gateway call to GetPublicShare.
+func (s DriveItemPermissionsService) resolveSpaceRootPermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string) (permissionType, *storageprovider.ResourceId, error) {
+	if _, parseErr := spacePermissionIdToCS3Grantee(permissionID); parseErr == nil {
+		return Space, itemID, nil
+	}
+
+	sharedResourceID, err := s.getLinkPermissionResourceID(ctx, permissionID)
+	if err == nil {
+		return Public, sharedResourceID, nil
+	}
+	return Unknown, nil, err
+}
+
+// resolveItemPermission resolves permissions on a non-space-root item by trying
+// public link, user share, then OCM share lookups in order.
+func (s DriveItemPermissionsService) resolveItemPermission(ctx context.Context, permissionID string) (permissionType, *storageprovider.ResourceId, error) {
+	sharedResourceID, err := s.getLinkPermissionResourceID(ctx, permissionID)
+	if err == nil {
+		return Public, sharedResourceID, nil
+	}
+
+	sharedResourceID, err = s.getUserPermissionResourceID(ctx, permissionID)
+	if err == nil {
+		return User, sharedResourceID, nil
+	}
+
+	if s.config.IncludeOCMSharees {
+		sharedResourceID, err = s.getOCMPermissionResourceID(ctx, permissionID)
+		if err == nil {
+			return OCM, sharedResourceID, nil
+		}
+	}
+
+	return Unknown, nil, err
+}
+
 // DeleteSpaceRootPermission deletes a permission on the root item of a project space
 func (s DriveItemPermissionsService) DeleteSpaceRootPermission(ctx context.Context, driveID *storageprovider.ResourceId, permissionID string) error {
 

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

@@ -822,9 +822,8 @@ var _ = Describe("DriveItemPermissionsService", func() {
 			Expect(err).ToNot(HaveOccurred())
 		})
 		It("deletes a space permission as expected", func() {
-			getPublicShareResponse.Status = status.NewNotFound(context.Background(), "")
-			gatewayClient.On("GetPublicShare", mock.Anything, mock.Anything).Return(&getPublicShareResponse, nil)
-
+			// No GetPublicShare mock needed — space permissions (u:/g: prefixed IDs)
+			// are recognised by their format and skip the public share lookup.
 			gatewayClient.On("RemoveShare",
 				mock.Anything,
 				mock.Anything,