Consolidate logic to see if Exchange/OneDrive user exists (#2508)

## Description

Use the check to see if a user has access to OneDrive/Exchange to also see if the user exists. This does not stop the population of GraphConnector users every time an instance is created, but does move us one step closer to that

Manually tested the following cases:
* backup "*"
* backup single existing user without error
* backup multiple existing users without error
* error when attempting to backup user that doesn't exist in the same domain
* error when attempting to backup user that doesn't exist in a different domain

Sample error output (progress disabled):
```text
No backups available
Error: 1 error occurred:
        * Failed to run Exchange backup for user foo@other.onmicrosoft.com: doing backup: producing backup data collections: resource owner [foo@other.onmicrosoft.com] not found within tenant
```

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

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

## Type of change

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

## Issue(s)

* #1547 

## Test Plan

- [x] 💪 Manual
- [ ]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-02-20 09:50:47 -08:00 committed by GitHub
parent a86453f138
commit cf62c6283c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 19 additions and 77 deletions

View File

@ -44,7 +44,7 @@ func (gc *GraphConnector) DataCollections(
ctx, end := D.Span(ctx, "gc:dataCollections", D.Index("service", sels.Service.String()))
defer end()
err := verifyBackupInputs(sels, gc.GetUsers(), gc.GetSiteIDs())
err := verifyBackupInputs(sels, gc.GetSiteIDs())
if err != nil {
return nil, nil, clues.Stack(err).WithClues(ctx)
}
@ -118,12 +118,13 @@ func (gc *GraphConnector) DataCollections(
}
}
func verifyBackupInputs(sels selectors.Selector, userPNs, siteIDs []string) error {
func verifyBackupInputs(sels selectors.Selector, siteIDs []string) error {
var ids []string
switch sels.Service {
case selectors.ServiceExchange, selectors.ServiceOneDrive:
ids = userPNs
// Exchange and OneDrive user existence now checked in checkServiceEnabled.
return nil
case selectors.ServiceSharePoint:
ids = siteIDs

View File

@ -2,11 +2,13 @@ package discovery
import (
"context"
"fmt"
"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/fault"
)
@ -43,6 +45,10 @@ func Users(ctx context.Context, ga getAller, errs *fault.Errors) ([]models.Usera
func User(ctx context.Context, gwi getWithInfoer, userID string) (models.Userable, *api.UserInfo, error) {
u, err := gwi.GetByID(ctx, userID)
if err != nil {
if graph.IsErrUserNotFound(err) {
return nil, nil, fmt.Errorf("resource owner [%s] not found within tenant", userID)
}
return nil, nil, errors.Wrap(err, "getting user")
}

View File

@ -26,6 +26,7 @@ const (
errCodeSyncFolderNotFound = "ErrorSyncFolderNotFound"
errCodeSyncStateNotFound = "SyncStateNotFound"
errCodeResourceNotFound = "ResourceNotFound"
errCodeRequestResourceNotFound = "Request_ResourceNotFound"
errCodeMailboxNotEnabledForRESTAPI = "MailboxNotEnabledForRESTAPI"
)
@ -83,6 +84,10 @@ func IsErrExchangeMailFolderNotFound(err error) bool {
return hasErrorCode(err, errCodeResourceNotFound, errCodeMailboxNotEnabledForRESTAPI)
}
func IsErrUserNotFound(err error) bool {
return hasErrorCode(err, errCodeRequestResourceNotFound)
}
// Timeout errors are identified for tracking the need to retry calls.
// Other delay errors, like throttling, are already handled by the
// graph client's built-in retries.

View File

@ -147,16 +147,6 @@ func (gc *GraphConnector) setTenantUsers(ctx context.Context, errs *fault.Errors
return nil
}
// GetUsers returns the email address of users within the tenant.
func (gc *GraphConnector) GetUsers() []string {
return maps.Keys(gc.Users)
}
// GetUsersIds returns the M365 id for the user
func (gc *GraphConnector) GetUsersIds() []string {
return maps.Values(gc.Users)
}
// 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.

View File

@ -130,67 +130,7 @@ func (suite *DisconnectedGraphConnectorSuite) TestGraphConnector_Status() {
assert.Equal(t, 2, gc.Status().FolderCount)
}
func (suite *DisconnectedGraphConnectorSuite) TestVerifyBackupInputs() {
users := []string{
"elliotReid@someHospital.org",
"chrisTurk@someHospital.org",
"carlaEspinosa@someHospital.org",
"bobKelso@someHospital.org",
"johnDorian@someHospital.org",
}
tests := []struct {
name string
getSelector func(t *testing.T) selectors.Selector
checkError assert.ErrorAssertionFunc
}{
{
name: "No scopes",
checkError: assert.Error,
getSelector: func(t *testing.T) selectors.Selector {
return selectors.NewExchangeBackup(nil).Selector
},
},
{
name: "Valid Single User",
checkError: assert.NoError,
getSelector: func(t *testing.T) selectors.Selector {
sel := selectors.NewExchangeBackup([]string{"bobKelso@someHospital.org"})
sel.Include(sel.MailFolders(selectors.Any()))
return sel.Selector
},
},
{
name: "Partial invalid user",
checkError: assert.Error,
getSelector: func(t *testing.T) selectors.Selector {
sel := selectors.NewExchangeBackup([]string{"bobkelso@someHospital.org", "janitor@someHospital.org"})
sel.Include(sel.MailFolders(selectors.Any()))
sel.DiscreteOwner = "janitor@someHospital.org"
return sel.Selector
},
},
{
name: "Invalid discrete owner",
checkError: assert.Error,
getSelector: func(t *testing.T) selectors.Selector {
sel := selectors.NewOneDriveBackup([]string{"janitor@someHospital.org"})
sel.Include(sel.AllData())
return sel.Selector
},
},
}
for _, test := range tests {
suite.T().Run(test.name, func(t *testing.T) {
err := verifyBackupInputs(test.getSelector(t), users, nil)
test.checkError(t, err)
})
}
}
func (suite *DisconnectedGraphConnectorSuite) TestVerifyBackupInputs_allServices() {
users := []string{"elliotReid@someHospital.org"}
sites := []string{"abc.site.foo", "bar.site.baz"}
tests := []struct {
@ -224,7 +164,7 @@ func (suite *DisconnectedGraphConnectorSuite) TestVerifyBackupInputs_allServices
},
{
name: "Invalid User",
checkError: assert.Error,
checkError: assert.NoError,
excludes: func(t *testing.T) selectors.Selector {
sel := selectors.NewOneDriveBackup([]string{"foo@SomeCompany.org"})
sel.Exclude(sel.Folders(selectors.Any()))
@ -286,11 +226,11 @@ func (suite *DisconnectedGraphConnectorSuite) TestVerifyBackupInputs_allServices
for _, test := range tests {
suite.T().Run(test.name, func(t *testing.T) {
err := verifyBackupInputs(test.excludes(t), users, sites)
err := verifyBackupInputs(test.excludes(t), sites)
test.checkError(t, err)
err = verifyBackupInputs(test.filters(t), users, sites)
err = verifyBackupInputs(test.filters(t), sites)
test.checkError(t, err)
err = verifyBackupInputs(test.includes(t), users, sites)
err = verifyBackupInputs(test.includes(t), sites)
test.checkError(t, err)
})
}