look up users and sites by id or name (#2973)

Adds a lookup step to graph connector to find
an owner's id and name given some identifier.
The identifier, for either sites or users, can be a
well formed id or name.

---

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

- [x] ⛔ No

#### Type of change

- [x] 🌻 Feature

#### Issue(s)

* #2825

#### Test Plan

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

src/cli/backup/backup.go

@@ -253,12 +253,12 @@ func runBackups(
 // genericDeleteCommand is a helper function that all services can use
 // for the removal of an entry from the repository
 func genericDeleteCommand(cmd *cobra.Command, bID, designation string, args []string) error {
-	ctx := clues.Add(cmd.Context(), "delete_backup_id", bID)
-
 	if utils.HasNoFlagsAndShownHelp(cmd) {
 		return nil
 	}
 
+	ctx := clues.Add(cmd.Context(), "delete_backup_id", bID)
+
 	r, _, err := getAccountAndConnect(ctx)
 	if err != nil {
 		return Only(ctx, err)

src/cli/backup/exchange_e2e_test.go

@@ -235,7 +235,7 @@ func (suite *BackupExchangeE2ESuite) TestExchangeBackupCmd_UserNotInTenant() {
 			assert.Contains(
 				t,
 				err.Error(),
-				"not found within tenant", "error missing user not found")
+				"not found in tenant", "error missing user not found")
 			assert.NotContains(t, err.Error(), "runtime error", "panic happened")
 
 			t.Logf("backup error message: %s", err.Error())

src/cli/backup/onedrive_e2e_test.go

@@ -140,7 +140,7 @@ func (suite *NoBackupOneDriveE2ESuite) TestOneDriveBackupCmd_UserNotInTenant() {
 	assert.Contains(
 		t,
 		err.Error(),
-		"not found within tenant", "error missing user not found")
+		"not found in tenant", "error missing user not found")
 	assert.NotContains(t, err.Error(), "runtime error", "panic happened")
 
 	t.Logf("backup error message: %s", err.Error())

src/internal/connector/data_collections.go

@@ -53,7 +53,7 @@ func (gc *GraphConnector) ProduceBackupCollections(
 
 	serviceEnabled, err := checkServiceEnabled(
 		ctx,
-		gc.Owners.Users(),
+		gc.Discovery.Users(),
 		path.ServiceType(sels.Service),
 		sels.DiscreteOwner)
 	if err != nil {
@@ -162,7 +162,7 @@ func verifyBackupInputs(sels selectors.Selector, siteIDs []string) error {
 	}
 
 	if !found {
-		return clues.New("resource owner not found within tenant").With("missing_resource_owner", sels.DiscreteOwner)
+		return clues.Stack(graph.ErrResourceOwnerNotFound).With("missing_resource_owner", sels.DiscreteOwner)
 	}
 
 	return nil

src/internal/connector/data_collections_test.go

@@ -25,27 +25,21 @@ import (
 // DataCollection tests
 // ---------------------------------------------------------------------------
 
-type ConnectorDataCollectionIntegrationSuite struct {
+type DataCollectionIntgSuite struct {
 	tester.Suite
-	connector *GraphConnector
+	user string
-	user      string
+	site string
-	site      string
 }
 
-func TestConnectorDataCollectionIntegrationSuite(t *testing.T) {
+func TestDataCollectionIntgSuite(t *testing.T) {
-	suite.Run(t, &ConnectorDataCollectionIntegrationSuite{
+	suite.Run(t, &DataCollectionIntgSuite{
 		Suite: tester.NewIntegrationSuite(
 			t,
-			[][]string{tester.M365AcctCredEnvs},
+			[][]string{tester.M365AcctCredEnvs}),
-		),
 	})
 }
 
-func (suite *ConnectorDataCollectionIntegrationSuite) SetupSuite() {
+func (suite *DataCollectionIntgSuite) SetupSuite() {
-	ctx, flush := tester.NewContext()
-	defer flush()
-
-	suite.connector = loadConnector(ctx, suite.T(), graph.HTTPClient(graph.NoTimeout()), AllResources)
 	suite.user = tester.M365UserID(suite.T())
 	suite.site = tester.M365SiteID(suite.T())
 
@@ -58,7 +52,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) SetupSuite() {
 // - mail
 // - contacts
 // - events
-func (suite *ConnectorDataCollectionIntegrationSuite) TestExchangeDataCollection() {
+func (suite *DataCollectionIntgSuite) TestExchangeDataCollection() {
 	ctx, flush := tester.NewContext()
 	defer flush()
 
@@ -138,7 +132,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestExchangeDataCollection
 }
 
 // TestInvalidUserForDataCollections ensures verification process for users
-func (suite *ConnectorDataCollectionIntegrationSuite) TestDataCollections_invalidResourceOwner() {
+func (suite *DataCollectionIntgSuite) TestDataCollections_invalidResourceOwner() {
 	ctx, flush := tester.NewContext()
 	defer flush()
 
@@ -223,7 +217,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestDataCollections_invali
 // TestSharePointDataCollection verifies interface between operation and
 // GraphConnector remains stable to receive a non-zero amount of Collections
 // for the SharePoint Package.
-func (suite *ConnectorDataCollectionIntegrationSuite) TestSharePointDataCollection() {
+func (suite *DataCollectionIntgSuite) TestSharePointDataCollection() {
 	ctx, flush := tester.NewContext()
 	defer flush()
 
@@ -299,14 +293,14 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestSharePointDataCollecti
 // CreateSharePointCollection tests
 // ---------------------------------------------------------------------------
 
-type ConnectorCreateSharePointCollectionIntegrationSuite struct {
+type SPCollectionIntgSuite struct {
 	tester.Suite
 	connector *GraphConnector
 	user      string
 }
 
-func TestConnectorCreateSharePointCollectionIntegrationSuite(t *testing.T) {
+func TestSPCollectionIntgSuite(t *testing.T) {
-	suite.Run(t, &ConnectorCreateSharePointCollectionIntegrationSuite{
+	suite.Run(t, &SPCollectionIntgSuite{
 		Suite: tester.NewIntegrationSuite(
 			t,
 			[][]string{tester.M365AcctCredEnvs},
@@ -314,7 +308,7 @@ func TestConnectorCreateSharePointCollectionIntegrationSuite(t *testing.T) {
 	})
 }
 
-func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) SetupSuite() {
+func (suite *SPCollectionIntgSuite) SetupSuite() {
 	ctx, flush := tester.NewContext()
 	defer flush()
 
@@ -324,7 +318,7 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) SetupSuite() {
 	tester.LogTimeOfTest(suite.T())
 }
 
-func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateSharePointCollection_Libraries() {
+func (suite *SPCollectionIntgSuite) TestCreateSharePointCollection_Libraries() {
 	ctx, flush := tester.NewContext()
 	defer flush()
 
@@ -335,7 +329,7 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateShar
 		siteIDs = []string{siteID}
 	)
 
-	id, name, err := gc.PopulateOwnerIDAndNamesFrom(siteID, nil)
+	id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, siteID, nil)
 	require.NoError(t, err, clues.ToCore(err))
 
 	sel := selectors.NewSharePointBackup(siteIDs)
@@ -368,7 +362,7 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateShar
 		cols[1].FullPath().Service().String())
 }
 
-func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateSharePointCollection_Lists() {
+func (suite *SPCollectionIntgSuite) TestCreateSharePointCollection_Lists() {
 	ctx, flush := tester.NewContext()
 	defer flush()
 
@@ -379,7 +373,7 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateShar
 		siteIDs = []string{siteID}
 	)
 
-	id, name, err := gc.PopulateOwnerIDAndNamesFrom(siteID, nil)
+	id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, siteID, nil)
 	require.NoError(t, err, clues.ToCore(err))
 
 	sel := selectors.NewSharePointBackup(siteIDs)

src/internal/connector/discovery/api/sites.go

@@ -3,6 +3,8 @@ package api
 import (
 	"context"
 	"fmt"
+	"net/url"
+	"regexp"
 	"strings"
 
 	"github.com/alcionai/clues"
@@ -12,6 +14,7 @@ import (
 
 	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
+	"github.com/alcionai/corso/src/internal/connector/graph/betasdk/sites"
 	"github.com/alcionai/corso/src/pkg/fault"
 )
 
@@ -85,15 +88,74 @@ func (c Sites) GetAll(ctx context.Context, errs *fault.Bus) ([]models.Siteable,
 	return us, el.Failure()
 }
 
-func (c Sites) GetByID(ctx context.Context, id string) (models.Siteable, error) {
+const uuidRE = "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}"
-	resp, err := c.stable.Client().SitesById(id).Get(ctx, nil)
+
+// matches a site ID, with or without a doman name.  Ex, either one of:
+// 10rqc2.sharepoint.com,deadbeef-0000-0000-0000-000000000000,beefdead-0000-0000-0000-000000000000
+// deadbeef-0000-0000-0000-000000000000,beefdead-0000-0000-0000-000000000000
+var siteIDRE = regexp.MustCompile(`(.+,)?` + uuidRE + "," + uuidRE)
+
+const webURLGetTemplate = "https://graph.microsoft.com/v1.0/sites/%s:/%s"
+
+// GetByID looks up the site matching the given identifier.  The identifier can be either a
+// canonical site id or a webURL.  Assumes the webURL is complete and well formed;
+// eg: https://10rqc2.sharepoint.com/sites/Example
+func (c Sites) GetByID(ctx context.Context, identifier string) (models.Siteable, error) {
+	var (
+		resp models.Siteable
+		err  error
+	)
+
+	ctx = clues.Add(ctx, "given_site_id", identifier)
+
+	if siteIDRE.MatchString(identifier) {
+		resp, err = c.stable.Client().SitesById(identifier).Get(ctx, nil)
+		if err != nil {
+			return nil, graph.Wrap(ctx, err, "getting site by id")
+		}
+
+		return resp, err
+	}
+
+	// if the id is not a standard sharepoint ID, assume it's a url.
+	// if it has a leading slash, assume it's only a path.  If it doesn't,
+	// ensure it has a prefix https://
+	if !strings.HasPrefix(identifier, "/") {
+		identifier = strings.TrimPrefix(identifier, "https://")
+		identifier = "https://" + identifier
+	}
+
+	u, err := url.Parse(identifier)
 	if err != nil {
-		return nil, graph.Wrap(ctx, err, "getting site")
+		return nil, clues.Wrap(err, "site is not parseable as a url")
+	}
+
+	// don't construct a path with double leading slashes
+	path := strings.TrimPrefix(u.Path, "/")
+	rawURL := fmt.Sprintf(webURLGetTemplate, u.Host, path)
+
+	resp, err = sites.
+		NewItemSitesSiteItemRequestBuilder(rawURL, c.stable.Adapter()).
+		Get(ctx, nil)
+	if err != nil {
+		return nil, graph.Wrap(ctx, err, "getting site by weburl")
 	}
 
 	return resp, err
 }
 
+// GetIDAndName looks up the site matching the given ID, and returns
+// its canonical ID and the webURL as the name.  Accepts an ID or a
+// WebURL as an ID.
+func (c Sites) GetIDAndName(ctx context.Context, siteID string) (string, string, error) {
+	s, err := c.GetByID(ctx, siteID)
+	if err != nil {
+		return "", "", err
+	}
+
+	return ptr.Val(s.GetId()), ptr.Val(s.GetWebUrl()), nil
+}
+
 // ---------------------------------------------------------------------------
 // helpers
 // ---------------------------------------------------------------------------
@@ -116,23 +178,23 @@ func validateSite(item any) (models.Siteable, error) {
 		return nil, clues.New("missing ID")
 	}
 
-	url := ptr.Val(m.GetWebUrl())
+	wURL := ptr.Val(m.GetWebUrl())
-	if len(url) == 0 {
+	if len(wURL) == 0 {
 		return nil, clues.New("missing webURL").With("site_id", id) // TODO: pii
 	}
 
 	// personal (ie: oneDrive) sites have to be filtered out server-side.
-	if strings.Contains(url, personalSitePath) {
+	if strings.Contains(wURL, personalSitePath) {
 		return nil, clues.Stack(errKnownSkippableCase).
-			With("site_id", id, "site_url", url) // TODO: pii
+			With("site_id", id, "site_web_url", wURL) // TODO: pii
 	}
 
 	name := ptr.Val(m.GetDisplayName())
 	if len(name) == 0 {
 		// the built-in site at "https://{tenant-domain}/search" never has a name.
-		if strings.HasSuffix(url, "/search") {
+		if strings.HasSuffix(wURL, "/search") {
 			return nil, clues.Stack(errKnownSkippableCase).
-				With("site_id", id, "site_url", url) // TODO: pii
+				With("site_id", id, "site_web_url", wURL) // TODO: pii
 		}
 
 		return nil, clues.New("missing site display name").With("site_id", id)

src/internal/connector/discovery/api/sites_test.go

@@ -1,9 +1,11 @@
 package api
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/alcionai/clues"
+	"github.com/google/uuid"
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -153,3 +155,48 @@ func (suite *SitesIntgSuite) TestGetAll() {
 		assert.NotContains(t, ptr.Val(site.GetWebUrl()), personalSitePath, "must not return onedrive sites")
 	}
 }
+
+func (suite *SitesIntgSuite) TestSites_GetByID() {
+	var (
+		t       = suite.T()
+		siteID  = tester.M365SiteID(t)
+		host    = strings.Split(siteID, ",")[0]
+		shortID = strings.TrimPrefix(siteID, host+",")
+		siteURL = tester.M365SiteURL(t)
+		acct    = tester.NewM365Account(t)
+	)
+
+	creds, err := acct.M365Config()
+	require.NoError(t, err, clues.ToCore(err))
+
+	client, err := NewClient(creds)
+	require.NoError(t, err, clues.ToCore(err))
+
+	sitesAPI := client.Sites()
+
+	table := []struct {
+		name      string
+		id        string
+		expectErr assert.ErrorAssertionFunc
+	}{
+		{"3 part id", siteID, assert.NoError},
+		{"2 part id", shortID, assert.NoError},
+		{"malformed id", uuid.NewString(), assert.Error},
+		{"random id", uuid.NewString() + "," + uuid.NewString(), assert.Error},
+		{"url", siteURL, assert.NoError},
+		{"host only", host, assert.NoError},
+		{"malformed url", "barunihlda", assert.Error},
+		{"non-matching url", "https://test/sites/testing", assert.Error},
+	}
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			ctx, flush := tester.NewContext()
+			defer flush()
+
+			t := suite.T()
+
+			_, err := sitesAPI.GetByID(ctx, test.id)
+			test.expectErr(t, err, clues.ToCore(err))
+		})
+	}
+}

src/internal/connector/discovery/api/users.go

@@ -10,6 +10,7 @@ import (
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 	"github.com/microsoftgraph/msgraph-sdk-go/users"
 
+	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/path"
@@ -147,13 +148,15 @@ func (c Users) GetAll(ctx context.Context, errs *fault.Bus) ([]models.Userable,
 	return us, el.Failure()
 }
 
-func (c Users) GetByID(ctx context.Context, userID string) (models.Userable, error) {
+// GetByID looks up the user matching the given identifier.  The identifier can be either a
+// canonical user id or a princpalName.
+func (c Users) GetByID(ctx context.Context, identifier string) (models.Userable, error) {
 	var (
 		resp models.Userable
 		err  error
 	)
 
-	resp, err = c.stable.Client().UsersById(userID).Get(ctx, nil)
+	resp, err = c.stable.Client().UsersById(identifier).Get(ctx, nil)
 
 	if err != nil {
 		return nil, graph.Wrap(ctx, err, "getting user")
@@ -162,6 +165,17 @@ func (c Users) GetByID(ctx context.Context, userID string) (models.Userable, err
 	return resp, err
 }
 
+// GetIDAndName looks up the user matching the given ID, and returns
+// its canonical ID and the PrincipalName as the name.
+func (c Users) GetIDAndName(ctx context.Context, userID string) (string, string, error) {
+	u, err := c.GetByID(ctx, userID)
+	if err != nil {
+		return "", "", err
+	}
+
+	return ptr.Val(u.GetId()), ptr.Val(u.GetUserPrincipalName()), nil
+}
+
 func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
 	// Assume all services are enabled
 	// then filter down to only services the user has enabled

src/internal/connector/discovery/discovery.go

@@ -77,7 +77,7 @@ func User(
 	u, err := gwi.GetByID(ctx, userID)
 	if err != nil {
 		if graph.IsErrUserNotFound(err) {
-			return nil, nil, clues.New("resource owner not found within tenant").With("user_id", userID)
+			return nil, nil, clues.Stack(graph.ErrResourceOwnerNotFound).With("user_id", userID)
 		}
 
 		return nil, nil, clues.Wrap(err, "getting user")

src/internal/connector/graph/errors.go

@@ -70,6 +70,8 @@ var (
 	// graph client's built-in retries.
 	// https://github.com/microsoftgraph/msgraph-sdk-go/issues/302
 	ErrTimeout = clues.New("communication timeout")
+
+	ErrResourceOwnerNotFound = clues.New("resource owner not found in tenant")
 )
 
 func IsErrDeletedInFlight(err error) bool {
@@ -191,7 +193,11 @@ func Wrap(ctx context.Context, e error, msg string) *clues.Err {
 		return clues.Wrap(e, msg).WithClues(ctx)
 	}
 
-	data, innerMsg := errData(odErr)
+	mainMsg, data, innerMsg := errData(odErr)
+
+	if len(mainMsg) > 0 {
+		e = clues.Stack(e, clues.New(mainMsg))
+	}
 
 	return setLabels(clues.Wrap(e, msg).WithClues(ctx).With(data...), innerMsg)
 }
@@ -208,7 +214,11 @@ func Stack(ctx context.Context, e error) *clues.Err {
 		return clues.Stack(e).WithClues(ctx)
 	}
 
-	data, innerMsg := errData(odErr)
+	mainMsg, data, innerMsg := errData(odErr)
+
+	if len(mainMsg) > 0 {
+		e = clues.Stack(e, clues.New(mainMsg))
+	}
 
 	return setLabels(clues.Stack(e).WithClues(ctx).With(data...), innerMsg)
 }
@@ -226,11 +236,12 @@ func setLabels(err *clues.Err, msg string) *clues.Err {
 	return err
 }
 
-func errData(err odataerrors.ODataErrorable) ([]any, string) {
+func errData(err odataerrors.ODataErrorable) (string, []any, string) {
 	data := make([]any, 0)
 
 	// Get MainError
 	mainErr := err.GetError()
+	mainMsg := ptr.Val(mainErr.GetMessage())
 
 	data = appendIf(data, "odataerror_code", mainErr.GetCode())
 	data = appendIf(data, "odataerror_message", mainErr.GetMessage())
@@ -251,7 +262,7 @@ func errData(err odataerrors.ODataErrorable) ([]any, string) {
 		data = appendIf(data, "odataerror_inner_req_id", inner.GetRequestId())
 	}
 
-	return data, strings.ToLower(msgConcat)
+	return mainMsg, data, strings.ToLower(msgConcat)
 }
 
 func appendIf(a []any, k string, v *string) []any {

src/internal/connector/graph_connector.go

@@ -9,7 +9,6 @@ import (
 	"sync"
 
 	"github.com/alcionai/clues"
-	"github.com/pkg/errors"
 
 	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/internal/connector/discovery/api"
@@ -36,12 +35,13 @@ var (
 // bookkeeping and interfacing with other component.
 type GraphConnector struct {
 	Service    graph.Servicer
-	Owners     api.Client
+	Discovery  api.Client
 	itemClient *http.Client // configured to handle large item downloads
 
 	tenant      string
 	credentials account.M365Config
 
+	ownerLookup getOwnerIDAndNamer
 	// maps of resource owner ids to names, and names to ids.
 	// not guaranteed to be populated, only here as a post-population
 	// reference for processes that choose to populate the values.
@@ -56,15 +56,6 @@ type GraphConnector struct {
 	status support.ConnectorOperationStatus // contains the status of the last run status
 }
 
-type resource int
-
-const (
-	UnknownResource resource = iota
-	AllResources
-	Users
-	Sites
-)
-
 func NewGraphConnector(
 	ctx context.Context,
 	itemClient *http.Client,
@@ -72,95 +63,51 @@ func NewGraphConnector(
 	r resource,
 	errs *fault.Bus,
 ) (*GraphConnector, error) {
-	m365, err := acct.M365Config()
+	creds, err := acct.M365Config()
 	if err != nil {
 		return nil, clues.Wrap(err, "retrieving m365 account configuration").WithClues(ctx)
 	}
 
-	gc := GraphConnector{
+	service, err := createService(creds)
-		itemClient:   itemClient,
-		tenant:       m365.AzureTenantID,
-		wg:           &sync.WaitGroup{},
-		credentials:  m365,
-		IDNameLookup: common.IDsNames{},
-	}
-
-	gc.Service, err = gc.createService()
 	if err != nil {
 		return nil, clues.Wrap(err, "creating service connection").WithClues(ctx)
 	}
 
-	gc.Owners, err = api.NewClient(m365)
+	discovery, err := api.NewClient(creds)
 	if err != nil {
 		return nil, clues.Wrap(err, "creating api client").WithClues(ctx)
 	}
 
+	rc, err := r.resourceClient(discovery)
+	if err != nil {
+		return nil, clues.Wrap(err, "creating resource client").WithClues(ctx)
+	}
+
+	gc := GraphConnector{
+		Discovery:    discovery,
+		IDNameLookup: common.IDsNames{},
+		Service:      service,
+
+		credentials: creds,
+		itemClient:  itemClient,
+		ownerLookup: rc,
+		tenant:      acct.ID(),
+		wg:          &sync.WaitGroup{},
+	}
+
 	return &gc, nil
 }
 
-// PopulateOwnerIDAndNamesFrom takes the provided owner identifier and produces
+// ---------------------------------------------------------------------------
-// the owner's name and ID from that value.  Returns an error if the owner is
+// Service Client
-// not recognized by the current tenant.
+// ---------------------------------------------------------------------------
-//
-// The id-name swapper is optional.  Some processes will look up all owners in
-// the tenant before reaching this step.  In that case, the data gets handed
-// down for this func to consume instead of performing further queries.  The
-// maps get stored inside the gc instance for later re-use.
-//
-// TODO: If the maps are nil or empty, this func will perform a lookup on the given
-// owner, and populate each map with that owner's id and name for downstream
-// guarantees about that data being present.  Optional performance enhancement
-// idea: downstream from here, we should _only_ need the given user's id and name,
-// and could store minimal map copies with that info instead of the whole tenant.
-func (gc *GraphConnector) PopulateOwnerIDAndNamesFrom(
-	owner string, // input value, can be either id or name
-	ins common.IDNameSwapper,
-) (string, string, error) {
-	// move this to GC method
-	id, name, err := getOwnerIDAndNameFrom(owner, ins)
-	if err != nil {
-		return "", "", errors.Wrap(err, "resolving resource owner details")
-	}
-
-	gc.IDNameLookup = ins
-
-	if ins == nil || (len(ins.IDs()) == 0 && len(ins.Names()) == 0) {
-		gc.IDNameLookup = common.IDsNames{
-			IDToName: map[string]string{id: name},
-			NameToID: map[string]string{name: id},
-		}
-	}
-
-	return id, name, nil
-}
-
-func getOwnerIDAndNameFrom(
-	owner string,
-	ins common.IDNameSwapper,
-) (string, string, error) {
-	if ins == nil {
-		return owner, owner, nil
-	}
-
-	if n, ok := ins.NameOf(owner); ok {
-		return owner, n, nil
-	} else if i, ok := ins.IDOf(owner); ok {
-		return i, owner, nil
-	}
-
-	// TODO: look-up user by owner, either id or name,
-	// and populate with maps as a result.  Only
-	// return owner, owner as a very last resort.
-
-	return "", "", clues.New("not found within tenant")
-}
 
 // createService constructor for graphService component
-func (gc *GraphConnector) createService() (*graph.Service, error) {
+func createService(creds account.M365Config) (*graph.Service, error) {
 	adapter, err := graph.CreateAdapter(
-		gc.credentials.AzureTenantID,
+		creds.AzureTenantID,
-		gc.credentials.AzureClientID,
+		creds.AzureClientID,
-		gc.credentials.AzureClientSecret)
+		creds.AzureClientSecret)
 	if err != nil {
 		return &graph.Service{}, err
 	}
@@ -168,6 +115,10 @@ func (gc *GraphConnector) createService() (*graph.Service, error) {
 	return graph.NewService(adapter), nil
 }
 
+// ---------------------------------------------------------------------------
+// Processing Status
+// ---------------------------------------------------------------------------
+
 // AwaitStatus waits for all gc tasks to complete and then returns status
 func (gc *GraphConnector) Wait() *data.CollectionStats {
 	defer func() {
@@ -223,3 +174,129 @@ func (gc *GraphConnector) incrementAwaitingMessages() {
 func (gc *GraphConnector) incrementMessagesBy(num int) {
 	gc.wg.Add(num)
 }
+
+// ---------------------------------------------------------------------------
+// Resource Lookup Handling
+// ---------------------------------------------------------------------------
+
+type resource int
+
+const (
+	UnknownResource resource = iota
+	AllResources             // unused
+	Users
+	Sites
+)
+
+func (r resource) resourceClient(discovery api.Client) (*resourceClient, error) {
+	switch r {
+	case Users:
+		return &resourceClient{enum: r, getter: discovery.Users()}, nil
+	case Sites:
+		return &resourceClient{enum: r, getter: discovery.Sites()}, nil
+	default:
+		return nil, clues.New("unrecognized owner resource enum").With("resource_enum", r)
+	}
+}
+
+type resourceClient struct {
+	enum   resource
+	getter getIDAndNamer
+}
+
+type getIDAndNamer interface {
+	GetIDAndName(ctx context.Context, owner string) (
+		ownerID string,
+		ownerName string,
+		err error,
+	)
+}
+
+var _ getOwnerIDAndNamer = &resourceClient{}
+
+type getOwnerIDAndNamer interface {
+	getOwnerIDAndNameFrom(
+		ctx context.Context,
+		discovery api.Client,
+		owner string,
+		ins common.IDNameSwapper,
+	) (
+		ownerID string,
+		ownerName string,
+		err error,
+	)
+}
+
+// getOwnerIDAndNameFrom looks up the owner's canonical id and display name.
+// If the owner is present in the idNameSwapper, then that interface's id and
+// name values are returned.  As a fallback, the resource calls the discovery
+// api to fetch the user or site using the owner value. This fallback assumes
+// that the owner is a well formed ID or display name of appropriate design
+// (PrincipalName for users, WebURL for sites).
+func (r resourceClient) getOwnerIDAndNameFrom(
+	ctx context.Context,
+	discovery api.Client,
+	owner string,
+	ins common.IDNameSwapper,
+) (string, string, error) {
+	if ins != nil {
+		if n, ok := ins.NameOf(owner); ok {
+			return owner, n, nil
+		} else if i, ok := ins.IDOf(owner); ok {
+			return i, owner, nil
+		}
+	}
+
+	ctx = clues.Add(ctx, "owner_identifier", owner)
+
+	var (
+		id, name string
+		err      error
+	)
+
+	// if r.enum == Sites {
+	// TODO: check all suffixes in nameToID
+	// }
+
+	id, name, err = r.getter.GetIDAndName(ctx, owner)
+	if err != nil {
+		if graph.IsErrUserNotFound(err) {
+			return "", "", clues.Stack(graph.ErrResourceOwnerNotFound, err)
+		}
+
+		return "", "", err
+	}
+
+	if len(id) == 0 || len(name) == 0 {
+		return "", "", clues.Stack(graph.ErrResourceOwnerNotFound)
+	}
+
+	return id, name, nil
+}
+
+// PopulateOwnerIDAndNamesFrom takes the provided owner identifier and produces
+// the owner's name and ID from that value.  Returns an error if the owner is
+// not recognized by the current tenant.
+//
+// The id-name swapper is optional.  Some processes will look up all owners in
+// the tenant before reaching this step.  In that case, the data gets handed
+// down for this func to consume instead of performing further queries.  The
+// data gets stored inside the gc instance for later re-use.
+func (gc *GraphConnector) PopulateOwnerIDAndNamesFrom(
+	ctx context.Context,
+	owner string, // input value, can be either id or name
+	ins common.IDNameSwapper,
+) (string, string, error) {
+	// move this to GC method
+	id, name, err := gc.ownerLookup.getOwnerIDAndNameFrom(ctx, gc.Discovery, owner, ins)
+	if err != nil {
+		return "", "", clues.Wrap(err, "identifying resource owner")
+	}
+
+	gc.IDNameLookup = common.IDsNames{
+		IDToName: map[string]string{id: name},
+		NameToID: map[string]string{name: id},
+	}
+
+	return id, name, nil
+}

src/internal/connector/graph_connector_onedrive_test.go

@@ -452,11 +452,11 @@ func (suite *GraphConnectorSharePointIntegrationSuite) SetupSuite() {
 
 	si.resourceOwner = tester.M365SiteID(suite.T())
 
-	user, err := si.connector.Owners.Users().GetByID(ctx, si.user)
+	user, err := si.connector.Discovery.Users().GetByID(ctx, si.user)
 	require.NoError(suite.T(), err, "fetching user", si.user, clues.ToCore(err))
 	si.userID = ptr.Val(user.GetId())
 
-	secondaryUser, err := si.connector.Owners.Users().GetByID(ctx, si.secondaryUser)
+	secondaryUser, err := si.connector.Discovery.Users().GetByID(ctx, si.secondaryUser)
 	require.NoError(suite.T(), err, "fetching user", si.secondaryUser, clues.ToCore(err))
 	si.secondaryUserID = ptr.Val(secondaryUser.GetId())
 
@@ -499,11 +499,11 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) SetupSuite() {
 
 	si.resourceOwner = si.user
 
-	user, err := si.connector.Owners.Users().GetByID(ctx, si.user)
+	user, err := si.connector.Discovery.Users().GetByID(ctx, si.user)
 	require.NoError(suite.T(), err, "fetching user", si.user, clues.ToCore(err))
 	si.userID = ptr.Val(user.GetId())
 
-	secondaryUser, err := si.connector.Owners.Users().GetByID(ctx, si.secondaryUser)
+	secondaryUser, err := si.connector.Discovery.Users().GetByID(ctx, si.secondaryUser)
 	require.NoError(suite.T(), err, "fetching user", si.secondaryUser, clues.ToCore(err))
 	si.secondaryUserID = ptr.Val(secondaryUser.GetId())
 
@@ -558,11 +558,11 @@ func (suite *GraphConnectorOneDriveNightlySuite) SetupSuite() {
 
 	si.resourceOwner = si.user
 
-	user, err := si.connector.Owners.Users().GetByID(ctx, si.user)
+	user, err := si.connector.Discovery.Users().GetByID(ctx, si.user)
 	require.NoError(suite.T(), err, "fetching user", si.user, clues.ToCore(err))
 	si.userID = ptr.Val(user.GetId())
 
-	secondaryUser, err := si.connector.Owners.Users().GetByID(ctx, si.secondaryUser)
+	secondaryUser, err := si.connector.Discovery.Users().GetByID(ctx, si.secondaryUser)
 	require.NoError(suite.T(), err, "fetching user", si.secondaryUser, clues.ToCore(err))
 	si.secondaryUserID = ptr.Val(secondaryUser.GetId())
 

src/internal/connector/graph_connector_test.go

@@ -39,132 +39,231 @@ func TestGraphConnectorUnitSuite(t *testing.T) {
 	suite.Run(t, &GraphConnectorUnitSuite{Suite: tester.NewUnitSuite(t)})
 }
 
+var _ getIDAndNamer = &mockNameIDGetter{}
+
+type mockNameIDGetter struct {
+	id, name string
+}
+
+func (mnig mockNameIDGetter) GetIDAndName(
+	_ context.Context,
+	_ string,
+) (string, string, error) {
+	return mnig.id, mnig.name, nil
+}
+
 func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() {
 	const (
-		ownerID   = "owner-id"
+		id   = "owner-id"
-		ownerName = "owner-name"
+		name = "owner-name"
 	)
 
 	var (
-		itn = map[string]string{ownerID: ownerName}
+		itn    = map[string]string{id: name}
-		nti = map[string]string{ownerName: ownerID}
+		nti    = map[string]string{name: id}
+		lookup = &resourceClient{
+			enum:   Users,
+			getter: &mockNameIDGetter{id: id, name: name},
+		}
+		noLookup = &resourceClient{enum: Users, getter: &mockNameIDGetter{}}
 	)
 
 	table := []struct {
 		name       string
 		owner      string
 		ins        common.IDsNames
+		rc         *resourceClient
 		expectID   string
 		expectName string
 		expectErr  require.ErrorAssertionFunc
 	}{
 		{
 			name:       "nil ins",
-			owner:      ownerID,
+			owner:      id,
+			rc:         lookup,
+			expectID:   id,
+			expectName: name,
+			expectErr:  require.NoError,
+		},
+		{
+			name:       "nil ins no lookup",
+			owner:      id,
+			rc:         noLookup,
 			expectID:   "",
 			expectName: "",
 			expectErr:  require.Error,
 		},
 		{
 			name:  "only id map with owner id",
-			owner: ownerID,
+			owner: id,
 			ins: common.IDsNames{
 				IDToName: itn,
 				NameToID: nil,
 			},
-			expectID:   ownerID,
+			rc:         noLookup,
-			expectName: ownerName,
+			expectID:   id,
+			expectName: name,
 			expectErr:  require.NoError,
 		},
 		{
 			name:  "only name map with owner id",
-			owner: ownerID,
+			owner: id,
 			ins: common.IDsNames{
 				IDToName: nil,
 				NameToID: nti,
 			},
+			rc:         noLookup,
 			expectID:   "",
 			expectName: "",
 			expectErr:  require.Error,
 		},
+		{
+			name:  "only name map with owner id and lookup",
+			owner: id,
+			ins: common.IDsNames{
+				IDToName: nil,
+				NameToID: nti,
+			},
+			rc:         lookup,
+			expectID:   id,
+			expectName: name,
+			expectErr:  require.NoError,
+		},
 		{
 			name:  "only id map with owner name",
-			owner: ownerName,
+			owner: name,
 			ins: common.IDsNames{
 				IDToName: itn,
 				NameToID: nil,
 			},
+			rc:         lookup,
+			expectID:   id,
+			expectName: name,
+			expectErr:  require.NoError,
+		},
+		{
+			name:  "only name map with owner name",
+			owner: name,
+			ins: common.IDsNames{
+				IDToName: nil,
+				NameToID: nti,
+			},
+			rc:         noLookup,
+			expectID:   id,
+			expectName: name,
+			expectErr:  require.NoError,
+		},
+		{
+			name:  "only id map with owner name",
+			owner: name,
+			ins: common.IDsNames{
+				IDToName: itn,
+				NameToID: nil,
+			},
+			rc:         noLookup,
 			expectID:   "",
 			expectName: "",
 			expectErr:  require.Error,
 		},
 		{
-			name:  "only name map with owner name",
+			name:  "only id map with owner name and lookup",
-			owner: ownerName,
+			owner: name,
 			ins: common.IDsNames{
-				IDToName: nil,
+				IDToName: itn,
-				NameToID: nti,
+				NameToID: nil,
 			},
-			expectID:   ownerID,
+			rc:         lookup,
-			expectName: ownerName,
+			expectID:   id,
+			expectName: name,
 			expectErr:  require.NoError,
 		},
 		{
 			name:  "both maps with owner id",
-			owner: ownerID,
+			owner: id,
 			ins: common.IDsNames{
 				IDToName: itn,
 				NameToID: nti,
 			},
-			expectID:   ownerID,
+			rc:         noLookup,
-			expectName: ownerName,
+			expectID:   id,
+			expectName: name,
 			expectErr:  require.NoError,
 		},
 		{
 			name:  "both maps with owner name",
-			owner: ownerName,
+			owner: name,
 			ins: common.IDsNames{
 				IDToName: itn,
 				NameToID: nti,
 			},
-			expectID:   ownerID,
+			rc:         noLookup,
-			expectName: ownerName,
+			expectID:   id,
+			expectName: name,
 			expectErr:  require.NoError,
 		},
 		{
 			name:  "non-matching maps with owner id",
-			owner: ownerID,
+			owner: id,
 			ins: common.IDsNames{
 				IDToName: map[string]string{"foo": "bar"},
 				NameToID: map[string]string{"fnords": "smarf"},
 			},
+			rc:         noLookup,
 			expectID:   "",
 			expectName: "",
 			expectErr:  require.Error,
 		},
 		{
 			name:  "non-matching with owner name",
-			owner: ownerName,
+			owner: name,
 			ins: common.IDsNames{
 				IDToName: map[string]string{"foo": "bar"},
 				NameToID: map[string]string{"fnords": "smarf"},
 			},
+			rc:         noLookup,
 			expectID:   "",
 			expectName: "",
 			expectErr:  require.Error,
 		},
+		{
+			name:  "non-matching maps with owner id and lookup",
+			owner: id,
+			ins: common.IDsNames{
+				IDToName: map[string]string{"foo": "bar"},
+				NameToID: map[string]string{"fnords": "smarf"},
+			},
+			rc:         lookup,
+			expectID:   id,
+			expectName: name,
+			expectErr:  require.NoError,
+		},
+		{
+			name:  "non-matching with owner name and lookup",
+			owner: name,
+			ins: common.IDsNames{
+				IDToName: map[string]string{"foo": "bar"},
+				NameToID: map[string]string{"fnords": "smarf"},
+			},
+			rc:         lookup,
+			expectID:   id,
+			expectName: name,
+			expectErr:  require.NoError,
+		},
 	}
 	for _, test := range table {
 		suite.Run(test.name, func() {
+			ctx, flush := tester.NewContext()
+			defer flush()
+
 			var (
 				t  = suite.T()
-				gc = &GraphConnector{}
+				gc = &GraphConnector{ownerLookup: test.rc}
 			)
 
-			id, name, err := gc.PopulateOwnerIDAndNamesFrom(test.owner, test.ins)
+			rID, rName, err := gc.PopulateOwnerIDAndNamesFrom(ctx, test.owner, test.ins)
 			test.expectErr(t, err, clues.ToCore(err))
-			assert.Equal(t, test.expectID, id)
+			assert.Equal(t, test.expectID, rID, "id")
-			assert.Equal(t, test.expectName, name)
+			assert.Equal(t, test.expectName, rName, "name")
 		})
 	}
 }
@@ -1165,7 +1264,7 @@ func (suite *GraphConnectorIntegrationSuite) TestBackup_CreatesPrefixCollections
 				start     = time.Now()
 			)
 
-			id, name, err := backupGC.PopulateOwnerIDAndNamesFrom(backupSel.DiscreteOwner, nil)
+			id, name, err := backupGC.PopulateOwnerIDAndNamesFrom(ctx, backupSel.DiscreteOwner, nil)
 			require.NoError(t, err, clues.ToCore(err))
 
 			backupSel.SetDiscreteOwnerIDName(id, name)

src/internal/operations/backup_integration_test.go

@@ -123,7 +123,7 @@ func prepNewTestBackupOp(
 		t.FailNow()
 	}
 
-	id, name, err := gc.PopulateOwnerIDAndNamesFrom(sel.DiscreteOwner, nil)
+	id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, sel.DiscreteOwner, nil)
 	require.NoError(t, err, clues.ToCore(err))
 
 	sel.SetDiscreteOwnerIDName(id, name)

src/internal/operations/restore_test.go

@@ -278,7 +278,7 @@ func setupExchangeBackup(
 		fault.New(true))
 	require.NoError(t, err, clues.ToCore(err))
 
-	id, name, err := gc.PopulateOwnerIDAndNamesFrom(owner, nil)
+	id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, owner, nil)
 	require.NoError(t, err, clues.ToCore(err))
 
 	bsel.DiscreteOwner = owner
@@ -340,7 +340,7 @@ func setupSharePointBackup(
 		fault.New(true))
 	require.NoError(t, err, clues.ToCore(err))
 
-	id, name, err := gc.PopulateOwnerIDAndNamesFrom(owner, nil)
+	id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, owner, nil)
 	require.NoError(t, err, clues.ToCore(err))
 
 	spsel.DiscreteOwner = owner

src/internal/tester/config.go

@@ -24,6 +24,7 @@ const (
 	// M365 config
 	TestCfgAzureTenantID    = "azure_tenantid"
 	TestCfgSiteID           = "m365siteid"
+	TestCfgSiteURL          = "m365siteurl"
 	TestCfgUserID           = "m365userid"
 	TestCfgSecondaryUserID  = "secondarym365userid"
 	TestCfgLoadTestUserID   = "loadtestm365userid"
@@ -34,6 +35,7 @@ const (
 // test specific env vars
 const (
 	EnvCorsoM365TestSiteID          = "CORSO_M365_TEST_SITE_ID"
+	EnvCorsoM365TestSiteURL         = "CORSO_M365_TEST_SITE_URL"
 	EnvCorsoM365TestUserID          = "CORSO_M365_TEST_USER_ID"
 	EnvCorsoSecondaryM365TestUserID = "CORSO_SECONDARY_M365_TEST_USER_ID"
 	EnvCorsoM365LoadTestUserID      = "CORSO_M365_LOAD_TEST_USER_ID"
@@ -136,6 +138,12 @@ func readTestConfig() (map[string]string, error) {
 		os.Getenv(EnvCorsoM365TestSiteID),
 		vpr.GetString(TestCfgSiteID),
 		"10rqc2.sharepoint.com,4892edf5-2ebf-46be-a6e5-a40b2cbf1c1a,38ab6d06-fc82-4417-af93-22d8733c22be")
+	fallbackTo(
+		testEnv,
+		TestCfgSiteURL,
+		os.Getenv(EnvCorsoM365TestSiteURL),
+		vpr.GetString(TestCfgSiteURL),
+		"https://10rqc2.sharepoint.com/sites/CorsoCI")
 
 	testEnv[EnvCorsoTestConfigFilePath] = os.Getenv(EnvCorsoTestConfigFilePath)
 	testConfig = testEnv

src/internal/tester/resource_owners.go

@@ -81,7 +81,6 @@ func LoadTestM365SiteID(t *testing.T) string {
 	cfg, err := readTestConfig()
 	require.NoError(t, err, "retrieving load test m365 site id from test configuration", clues.ToCore(err))
 
-	// TODO: load test site id, not standard test site id
 	return cfg[TestCfgSiteID]
 }
 
@@ -162,3 +161,14 @@ func M365SiteID(t *testing.T) string {
 
 	return cfg[TestCfgSiteID]
 }
+
+// M365SiteURL returns a site webURL string representing the m365SiteURL described
+// by either the env var CORSO_M365_TEST_SITE_URL, the corso_test.toml config
+// file or the default value (in that order of priority).  The default is a
+// last-attempt fallback that will only work on alcion's testing org.
+func M365SiteURL(t *testing.T) string {
+	cfg, err := readTestConfig()
+	require.NoError(t, err, "retrieving m365 site url from test configuration", clues.ToCore(err))
+
+	return cfg[TestCfgSiteURL]
+}

src/pkg/repository/repository.go

@@ -314,7 +314,7 @@ func (r repository) NewBackupWithLookup(
 		return operations.BackupOperation{}, errors.Wrap(err, "connecting to m365")
 	}
 
-	ownerID, ownerName, err := gc.PopulateOwnerIDAndNamesFrom(sel.DiscreteOwner, ins)
+	ownerID, ownerName, err := gc.PopulateOwnerIDAndNamesFrom(ctx, sel.DiscreteOwner, ins)
 	if err != nil {
 		return operations.BackupOperation{}, errors.Wrap(err, "resolving resource owner details")
 	}

src/pkg/repository/repository_test.go

@@ -198,7 +198,9 @@ func (suite *RepositoryIntegrationSuite) TestNewBackup() {
 	r, err := repository.Initialize(ctx, acct, st, control.Options{})
 	require.NoError(t, err, clues.ToCore(err))
 
-	bo, err := r.NewBackup(ctx, selectors.Selector{DiscreteOwner: "test"})
+	userID := tester.M365UserID(t)
+
+	bo, err := r.NewBackup(ctx, selectors.Selector{DiscreteOwner: userID})
 	require.NoError(t, err, clues.ToCore(err))
 	require.NotNil(t, bo)
 }