From d6522e01ae22bad475a4d1e6e0b8bea18769c928 Mon Sep 17 00:00:00 2001
From: Keepers <ryanfkeepers@gmail.com>
Date: Tue, 22 Nov 2022 15:17:55 -0700
Subject: [PATCH] filter out personal sites (#1554)

## Description

Adds a server-side filter that removes all
personal (aka: oneDrive) sites from the drive
retrieval.

The approach is not my preferred method.  I'd rather
have used a filter in the query parameters.  In fact, I
have a functioning query in postman:
`$filter=NOT contains(webUrl, 'sharepoint.com/personal/')`
But I can't seem to get the graph client query to build
when I add that to the query params.

## Type of change

- [x] :sunflower: Feature

## Issue(s)

* #1506

## Test Plan

- [x] :green_heart: E2E
---
 src/internal/connector/graph_connector.go      | 8 ++++++++
 src/internal/connector/graph_connector_test.go | 6 +++++-
 src/internal/connector/sharepoint/queries.go   | 2 ++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go
index 4f27c6128..4edbafb75 100644
--- a/src/internal/connector/graph_connector.go
+++ b/src/internal/connector/graph_connector.go
@@ -220,6 +220,8 @@ func (gc *GraphConnector) setTenantSites(ctx context.Context) error {
 
 var errKnownSkippableCase = errors.New("case is known and skippable")
 
+const personalSitePath = "sharepoint.com/personal/"
+
 // Transforms an interface{} into a key,value pair representing
 // siteName:siteID.
 func identifySite(item any) (string, string, error) {
@@ -237,6 +239,12 @@ func identifySite(item any) (string, string, error) {
 		return "", "", errors.Errorf("no name for Site: %s", *m.GetId())
 	}
 
+	// personal (ie: oneDrive) sites have to be filtered out server-side.
+	url := m.GetWebUrl()
+	if url != nil && strings.Contains(*url, personalSitePath) {
+		return "", "", errKnownSkippableCase
+	}
+
 	return *m.GetName(), *m.GetId(), nil
 }
 
diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go
index a8cbe5268..51310b78d 100644
--- a/src/internal/connector/graph_connector_test.go
+++ b/src/internal/connector/graph_connector_test.go
@@ -66,7 +66,7 @@ func (suite *GraphConnectorIntegrationSuite) TestSetTenantUsers() {
 
 	newConnector.graphService = *service
 
-	suite.Equal(len(newConnector.Users), 0)
+	suite.Empty(len(newConnector.Users))
 	err = newConnector.setTenantUsers(ctx)
 	suite.NoError(err)
 	suite.Less(0, len(newConnector.Users))
@@ -93,6 +93,10 @@ func (suite *GraphConnectorIntegrationSuite) TestSetTenantSites() {
 	err = newConnector.setTenantSites(ctx)
 	suite.NoError(err)
 	suite.Less(0, len(newConnector.Sites))
+
+	for _, site := range newConnector.Sites {
+		suite.NotContains("sharepoint.com/personal/", site)
+	}
 }
 
 func (suite *GraphConnectorIntegrationSuite) TestEmptyCollections() {
diff --git a/src/internal/connector/sharepoint/queries.go b/src/internal/connector/sharepoint/queries.go
index 0682bd3d1..41a6641cb 100644
--- a/src/internal/connector/sharepoint/queries.go
+++ b/src/internal/connector/sharepoint/queries.go
@@ -10,6 +10,8 @@ import (
 )
 
 // GetAllSitesForTenant makes a GraphQuery request retrieving all sites in the tenant.
+// Due to restrictions in filter capabilities for site queries, the returned iterable
+// will contain all personal sites for all users in the org.
 func GetAllSitesForTenant(ctx context.Context, gs graph.Service) (absser.Parsable, error) {
 	options := &mssite.SitesRequestBuilderGetRequestConfiguration{
 		QueryParameters: &mssite.SitesRequestBuilderGetQueryParameters{