Remove GraphConnector user population (#2582)

## Description

Don't populate users when GraphConnector instance is created

## Does this PR need a docs update or release note?

- [x] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ] ⛔ No 

## Type of change

- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

## Issue(s)

* #1994

## Test Plan

- [x] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

CHANGELOG.md

@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Resolved an issue where progress bar displays could fail to exit, causing unbounded CPU consumption.
 - Fix Corso panic within Docker images
 - Debugging with the CORSO_URL_LOGGING env variable no longer causes accidental request failures.
+- Don't discover all users when backing up each user in a multi-user backup
 
 ### Changed
 - When using Restore and Details on Exchange Calendars, the `--event-calendar` flag can now identify calendars by either a Display Name or a Microsoft 365 ID.

src/cmd/factory/impl/common.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/alcionai/clues"
 	"github.com/google/uuid"
 	"github.com/pkg/errors"
 
@@ -23,6 +24,7 @@ import (
 	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/path"
 	"github.com/alcionai/corso/src/pkg/selectors"
+	"github.com/alcionai/corso/src/pkg/services/m365"
 )
 
 var (
@@ -118,15 +120,14 @@ func getGCAndVerifyUser(ctx context.Context, userID string) (*connector.GraphCon
 	// build a graph connector
 	// TODO: log/print recoverable errors
 	errs := fault.New(false)
-
-	gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), acct, connector.Users, errs)
-	if err != nil {
-		return nil, account.Account{}, errors.Wrap(err, "connecting to graph api")
-	}
-
 	normUsers := map[string]struct{}{}
 
-	for k := range gc.Users {
+	users, err := m365.UserPNs(ctx, acct, errs)
+	if err != nil {
+		return nil, account.Account{}, clues.Wrap(err, "getting tenant users")
+	}
+
+	for _, k := range users {
 		normUsers[strings.ToLower(k)] = struct{}{}
 	}
 
@@ -134,6 +135,16 @@ func getGCAndVerifyUser(ctx context.Context, userID string) (*connector.GraphCon
 		return nil, account.Account{}, errors.New("user not found within tenant")
 	}
 
+	gc, err := connector.NewGraphConnector(
+		ctx,
+		graph.HTTPClient(graph.NoTimeout()),
+		acct,
+		connector.Users,
+		errs)
+	if err != nil {
+		return nil, account.Account{}, errors.Wrap(err, "connecting to graph api")
+	}
+
 	return gc, acct, nil
 }
 

src/cmd/purge/purge.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"time"
 
+	"github.com/alcionai/clues"
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -19,6 +20,7 @@ import (
 	"github.com/alcionai/corso/src/pkg/credentials"
 	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/logger"
+	"github.com/alcionai/corso/src/pkg/services/m365"
 )
 
 var purgeCmd = &cobra.Command{
@@ -75,13 +77,16 @@ func handleAllFolderPurge(cmd *cobra.Command, args []string) error {
 		return nil
 	}
 
-	gc, t, err := getGCAndBoundaryTime(ctx)
+	acct, gc, t, err := getGCAndBoundaryTime(ctx)
 	if err != nil {
 		return err
 	}
 
 	err = runPurgeForEachUser(
-		ctx, gc, t,
+		ctx,
+		acct,
+		gc,
+		t,
 		purgeOneDriveFolders,
 	)
 	if err != nil {
@@ -98,12 +103,12 @@ func handleOneDriveFolderPurge(cmd *cobra.Command, args []string) error {
 		return nil
 	}
 
-	gc, t, err := getGCAndBoundaryTime(ctx)
+	acct, gc, t, err := getGCAndBoundaryTime(ctx)
 	if err != nil {
 		return err
 	}
 
-	if err := runPurgeForEachUser(ctx, gc, t, purgeOneDriveFolders); err != nil {
+	if err := runPurgeForEachUser(ctx, acct, gc, t, purgeOneDriveFolders); err != nil {
 		logger.Ctx(ctx).Error(err)
 		return Only(ctx, errors.Wrap(ErrPurging, "OneDrive folders"))
 	}
@@ -124,17 +129,30 @@ type purger func(context.Context, *connector.GraphConnector, time.Time, string)
 
 func runPurgeForEachUser(
 	ctx context.Context,
+	acct account.Account,
 	gc *connector.GraphConnector,
 	boundary time.Time,
 	ps ...purger,
 ) error {
-	var errs error
+	var (
+		errs  error
+		ferrs = fault.New(false)
+	)
 
-	for pn, uid := range userOrUsers(user, gc.Users) {
+	users, err := m365.Users(ctx, acct, ferrs)
-		Infof(ctx, "\nUser: %s - %s", pn, uid)
+	if err != nil {
+		return clues.Wrap(err, "getting users")
+	}
+
+	if len(ferrs.Errors().Recovered) > 0 {
+		errs = multierror.Append(errs, ferrs.Errors().Recovered...)
+	}
+
+	for _, u := range userOrUsers(user, users) {
+		Infof(ctx, "\nUser: %s - %s", u.PrincipalName, u.ID)
 
 		for _, p := range ps {
-			if err := p(ctx, gc, boundary, pn); err != nil {
+			if err := p(ctx, gc, boundary, u.PrincipalName); err != nil {
 				errs = multierror.Append(errs, err)
 			}
 		}
@@ -248,7 +266,7 @@ func purgeFolders(
 // Helpers
 // ------------------------------------------------------------------------------------------
 
-func getGC(ctx context.Context) (*connector.GraphConnector, error) {
+func getGC(ctx context.Context) (account.Account, *connector.GraphConnector, error) {
 	// get account info
 	m365Cfg := account.M365Config{
 		M365:          credentials.GetM365(),
@@ -257,7 +275,7 @@ func getGC(ctx context.Context) (*connector.GraphConnector, error) {
 
 	acct, err := account.NewAccount(account.ProviderM365, m365Cfg)
 	if err != nil {
-		return nil, Only(ctx, errors.Wrap(err, "finding m365 account details"))
+		return account.Account{}, nil, Only(ctx, errors.Wrap(err, "finding m365 account details"))
 	}
 
 	// build a graph connector
@@ -266,10 +284,10 @@ func getGC(ctx context.Context) (*connector.GraphConnector, error) {
 
 	gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), acct, connector.Users, errs)
 	if err != nil {
-		return nil, Only(ctx, errors.Wrap(err, "connecting to graph api"))
+		return account.Account{}, nil, Only(ctx, errors.Wrap(err, "connecting to graph api"))
 	}
 
-	return gc, nil
+	return acct, gc, nil
 }
 
 func getBoundaryTime(ctx context.Context) (time.Time, error) {
@@ -289,21 +307,23 @@ func getBoundaryTime(ctx context.Context) (time.Time, error) {
 	return boundaryTime, nil
 }
 
-func getGCAndBoundaryTime(ctx context.Context) (*connector.GraphConnector, time.Time, error) {
+func getGCAndBoundaryTime(
-	gc, err := getGC(ctx)
+	ctx context.Context,
+) (account.Account, *connector.GraphConnector, time.Time, error) {
+	acct, gc, err := getGC(ctx)
 	if err != nil {
-		return nil, time.Time{}, err
+		return account.Account{}, nil, time.Time{}, err
 	}
 
 	t, err := getBoundaryTime(ctx)
 	if err != nil {
-		return nil, time.Time{}, err
+		return account.Account{}, nil, time.Time{}, err
 	}
 
-	return gc, t, nil
+	return acct, gc, t, nil
 }
 
-func userOrUsers(u string, us map[string]string) map[string]string {
+func userOrUsers(u string, us []*m365.User) []*m365.User {
 	if len(u) == 0 {
 		return nil
 	}
@@ -312,5 +332,5 @@ func userOrUsers(u string, us map[string]string) map[string]string {
 		return us
 	}
 
-	return map[string]string{u: u}
+	return []*m365.User{{PrincipalName: u}}
 }

src/internal/connector/discovery/discovery.go

@@ -4,11 +4,13 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/alcionai/clues"
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 	"github.com/pkg/errors"
 
 	"github.com/alcionai/corso/src/internal/connector/discovery/api"
 	"github.com/alcionai/corso/src/internal/connector/graph"
+	"github.com/alcionai/corso/src/pkg/account"
 	"github.com/alcionai/corso/src/pkg/fault"
 )
 
@@ -16,10 +18,6 @@ import (
 // interfaces
 // ---------------------------------------------------------------------------
 
-type getAller interface {
-	GetAll(context.Context, *fault.Bus) ([]models.Userable, error)
-}
-
 type getter interface {
 	GetByID(context.Context, string) (models.Userable, error)
 }
@@ -38,8 +36,22 @@ type getWithInfoer interface {
 // ---------------------------------------------------------------------------
 
 // Users fetches all users in the tenant.
-func Users(ctx context.Context, ga getAller, errs *fault.Bus) ([]models.Userable, error) {
+func Users(
-	return ga.GetAll(ctx, errs)
+	ctx context.Context,
+	acct account.Account,
+	errs *fault.Bus,
+) ([]models.Userable, error) {
+	m365, err := acct.M365Config()
+	if err != nil {
+		return nil, clues.Wrap(err, "retrieving m365 account configuration").WithClues(ctx)
+	}
+
+	client, err := api.NewClient(m365)
+	if err != nil {
+		return nil, clues.Wrap(err, "creating api client").WithClues(ctx)
+	}
+
+	return client.Users().GetAll(ctx, errs)
 }
 
 func User(ctx context.Context, gwi getWithInfoer, userID string) (models.Userable, *api.UserInfo, error) {

src/internal/connector/discovery/discovery_test.go

@@ -0,0 +1,98 @@
+package discovery_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
+
+	"github.com/alcionai/corso/src/internal/connector/discovery"
+	"github.com/alcionai/corso/src/internal/tester"
+	"github.com/alcionai/corso/src/pkg/account"
+	"github.com/alcionai/corso/src/pkg/credentials"
+	"github.com/alcionai/corso/src/pkg/fault"
+)
+
+type DiscoveryIntegrationSuite struct {
+	tester.Suite
+}
+
+func TestDiscoveryIntegrationSuite(t *testing.T) {
+	suite.Run(t, &DiscoveryIntegrationSuite{Suite: tester.NewIntegrationSuite(
+		t,
+		[][]string{tester.M365AcctCredEnvs},
+		tester.CorsoGraphConnectorTests,
+	)})
+}
+
+func (suite *DiscoveryIntegrationSuite) TestUsers() {
+	ctx, flush := tester.NewContext()
+	defer flush()
+
+	t := suite.T()
+
+	acct := tester.NewM365Account(t)
+	errs := fault.New(true)
+
+	users, err := discovery.Users(ctx, acct, errs)
+	assert.NoError(t, err)
+
+	ferrs := errs.Errors()
+	assert.NoError(t, ferrs.Failure)
+	assert.Empty(t, ferrs.Recovered)
+
+	assert.Less(t, 0, len(users))
+}
+
+func (suite *DiscoveryIntegrationSuite) TestUsers_InvalidCredentials() {
+	ctx, flush := tester.NewContext()
+	defer flush()
+
+	table := []struct {
+		name string
+		acct func(t *testing.T) account.Account
+	}{
+		{
+			name: "Invalid Credentials",
+			acct: func(t *testing.T) account.Account {
+				a, err := account.NewAccount(
+					account.ProviderM365,
+					account.M365Config{
+						M365: credentials.M365{
+							AzureClientID:     "Test",
+							AzureClientSecret: "without",
+						},
+						AzureTenantID: "data",
+					},
+				)
+				require.NoError(t, err)
+
+				return a
+			},
+		},
+		{
+			name: "Empty Credentials",
+			acct: func(t *testing.T) account.Account {
+				// intentionally swallowing the error here
+				a, _ := account.NewAccount(account.ProviderM365)
+				return a
+			},
+		},
+	}
+
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			t := suite.T()
+
+			a := test.acct(t)
+			errs := fault.New(true)
+			users, err := discovery.Users(ctx, a, errs)
+
+			assert.Empty(t, users, "returned some users")
+			assert.NotNil(t, err)
+			// TODO(ashmrtn): Uncomment when fault package is used in discovery API.
+			// assert.NotNil(t, errs.Err())
+		})
+	}
+}

src/internal/connector/graph_connector.go

@@ -17,7 +17,6 @@ import (
 	"github.com/pkg/errors"
 	"golang.org/x/exp/maps"
 
-	"github.com/alcionai/corso/src/internal/connector/discovery"
 	"github.com/alcionai/corso/src/internal/connector/discovery/api"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/internal/connector/sharepoint"
@@ -41,7 +40,6 @@ type GraphConnector struct {
 	itemClient *http.Client // configured to handle large item downloads
 
 	tenant      string
-	Users       map[string]string // key<email> value<id>
 	Sites       map[string]string // key<???> value<???>
 	credentials account.M365Config
 
@@ -78,7 +76,6 @@ func NewGraphConnector(
 	gc := GraphConnector{
 		itemClient:  itemClient,
 		tenant:      m365.AzureTenantID,
-		Users:       make(map[string]string, 0),
 		wg:          &sync.WaitGroup{},
 		credentials: m365,
 	}
@@ -93,16 +90,6 @@ func NewGraphConnector(
 		return nil, clues.Wrap(err, "creating api client").WithClues(ctx)
 	}
 
-	// TODO(ashmrtn): When selectors only encapsulate a single resource owner that
-	// is not a wildcard don't populate users or sites when making the connector.
-	// For now this keeps things functioning if callers do pass in a selector like
-	// "*" instead of.
-	if r == AllResources || r == Users {
-		if err = gc.setTenantUsers(ctx, errs); err != nil {
-			return nil, errors.Wrap(err, "retrieving tenant user list")
-		}
-	}
-
 	if r == AllResources || r == Sites {
 		if err = gc.setTenantSites(ctx, errs); err != nil {
 			return nil, errors.Wrap(err, "retrieveing tenant site list")
@@ -126,27 +113,6 @@ func (gc *GraphConnector) createService() (*graph.Service, error) {
 	return graph.NewService(adapter), nil
 }
 
-// setTenantUsers queries the M365 to identify the users in the
-// workspace. The users field is updated during this method
-// iff the returned error is nil
-func (gc *GraphConnector) setTenantUsers(ctx context.Context, errs *fault.Bus) error {
-	ctx, end := D.Span(ctx, "gc:setTenantUsers")
-	defer end()
-
-	users, err := discovery.Users(ctx, gc.Owners.Users(), errs)
-	if err != nil {
-		return err
-	}
-
-	gc.Users = make(map[string]string, len(users))
-
-	for _, u := range users {
-		gc.Users[*u.GetUserPrincipalName()] = *u.GetId()
-	}
-
-	return nil
-}
-
 // setTenantSites queries the M365 to identify the sites in the
 // workspace. The sites field is updated during this method
 // iff the returned error is nil.

src/internal/connector/graph_connector_disconnected_test.go

@@ -75,7 +75,7 @@ func (suite *DisconnectedGraphConnectorSuite) TestBadConnection() {
 				ctx,
 				graph.HTTPClient(graph.NoTimeout()),
 				test.acct(t),
-				Users,
+				Sites,
 				fault.New(true))
 			assert.Nil(t, gc, test.name+" failed")
 			assert.NotNil(t, err, test.name+" failed")

src/internal/connector/graph_connector_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/stretchr/testify/suite"
 	"golang.org/x/exp/maps"
 
-	"github.com/alcionai/corso/src/internal/connector/discovery/api"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/internal/connector/mockconnector"
 	"github.com/alcionai/corso/src/internal/connector/support"
@@ -168,33 +167,7 @@ func (suite *GraphConnectorIntegrationSuite) SetupSuite() {
 	tester.LogTimeOfTest(suite.T())
 }
 
-// TestSetTenantUsers verifies GraphConnector's ability to query
+// TestSetTenantSites verifies GraphConnector's ability to query
-// the users associated with the credentials
-func (suite *GraphConnectorIntegrationSuite) TestSetTenantUsers() {
-	t := suite.T()
-	newConnector := GraphConnector{
-		tenant:      "test_tenant",
-		Users:       make(map[string]string, 0),
-		credentials: suite.connector.credentials,
-	}
-
-	ctx, flush := tester.NewContext()
-	defer flush()
-
-	owners, err := api.NewClient(suite.connector.credentials)
-	require.NoError(t, err)
-
-	newConnector.Owners = owners
-	assert.Empty(t, len(newConnector.Users))
-
-	errs := fault.New(true)
-
-	err = newConnector.setTenantUsers(ctx, errs)
-	assert.NoError(t, err)
-	assert.Less(t, 0, len(newConnector.Users))
-}
-
-// TestSetTenantUsers verifies GraphConnector's ability to query
 // the sites associated with the credentials
 func (suite *GraphConnectorIntegrationSuite) TestSetTenantSites() {
 	newConnector := GraphConnector{

src/pkg/services/m365/m365.go

@@ -37,12 +37,7 @@ func UsersCompat(ctx context.Context, acct account.Account) ([]*User, error) {
 // Users returns a list of users in the specified M365 tenant
 // TODO: Implement paging support
 func Users(ctx context.Context, acct account.Account, errs *fault.Bus) ([]*User, error) {
-	gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), acct, connector.Users, errs)
+	users, err := discovery.Users(ctx, acct, errs)
-	if err != nil {
-		return nil, errors.Wrap(err, "initializing M365 graph connection")
-	}
-
-	users, err := discovery.Users(ctx, gc.Owners.Users(), errs)
 	if err != nil {
 		return nil, err
 	}