diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index 680d6ed83..e4b8470c7 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -2,6 +2,7 @@ package backup import ( "context" + "fmt" "github.com/alcionai/clues" "github.com/pkg/errors" @@ -169,6 +170,8 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error { return Only(ctx, clues.Wrap(err, "Failed to retrieve M365 users")) } + fmt.Printf("\n-----\nINS %+v\n-----\n", ins) + selectorSet := []selectors.Selector{} for _, discSel := range sel.SplitByResourceOwner(ins.IDs()) { diff --git a/src/cli/backup/exchange_e2e_test.go b/src/cli/backup/exchange_e2e_test.go index 212706c9e..9dba497e7 100644 --- a/src/cli/backup/exchange_e2e_test.go +++ b/src/cli/backup/exchange_e2e_test.go @@ -293,7 +293,7 @@ func (suite *PreparedBackupExchangeE2ESuite) SetupSuite() { defer flush() - suite.m365UserID = tester.M365UserID(t) + suite.m365UserID = strings.ToLower(tester.M365UserID(t)) // init the repo first suite.repo, err = repository.Initialize(ctx, suite.acct, suite.st, control.Options{}) diff --git a/src/cli/backup/onedrive_e2e_test.go b/src/cli/backup/onedrive_e2e_test.go index 9512b2695..104603d20 100644 --- a/src/cli/backup/onedrive_e2e_test.go +++ b/src/cli/backup/onedrive_e2e_test.go @@ -207,7 +207,7 @@ func (suite *BackupDeleteOneDriveE2ESuite) SetupSuite() { require.NoError(t, err, clues.ToCore(err)) var ( - m365UserID = tester.M365UserID(t) + m365UserID = strings.ToLower(tester.M365UserID(t)) users = []string{m365UserID} idToName = map[string]string{m365UserID: m365UserID} nameToID = map[string]string{m365UserID: m365UserID} diff --git a/src/cli/backup/sharepoint_e2e_test.go b/src/cli/backup/sharepoint_e2e_test.go index 76caa6ef3..20b870af9 100644 --- a/src/cli/backup/sharepoint_e2e_test.go +++ b/src/cli/backup/sharepoint_e2e_test.go @@ -159,7 +159,7 @@ func (suite *BackupDeleteSharePointE2ESuite) SetupSuite() { require.NoError(t, err, clues.ToCore(err)) var ( - m365SiteID = tester.M365SiteID(t) + m365SiteID = strings.ToLower(tester.M365SiteID(t)) sites = []string{m365SiteID} idToName = map[string]string{m365SiteID: m365SiteID} nameToID = map[string]string{m365SiteID: m365SiteID} diff --git a/src/cli/restore/exchange_e2e_test.go b/src/cli/restore/exchange_e2e_test.go index 089e09380..2064868e5 100644 --- a/src/cli/restore/exchange_e2e_test.go +++ b/src/cli/restore/exchange_e2e_test.go @@ -2,6 +2,7 @@ package restore_test import ( "context" + "strings" "testing" "github.com/alcionai/clues" @@ -73,7 +74,7 @@ func (suite *RestoreExchangeE2ESuite) SetupSuite() { } suite.vpr, suite.cfgFP = tester.MakeTempTestConfigClone(t, force) - suite.m365UserID = tester.M365UserID(t) + suite.m365UserID = strings.ToLower(tester.M365UserID(t)) var ( users = []string{suite.m365UserID} diff --git a/src/internal/common/idname.go b/src/internal/common/idname.go index efee8493f..e50f30760 100644 --- a/src/internal/common/idname.go +++ b/src/internal/common/idname.go @@ -1,6 +1,10 @@ package common -import "golang.org/x/exp/maps" +import ( + "strings" + + "golang.org/x/exp/maps" +) type IDNamer interface { // the canonical id of the thing, generated and usable @@ -26,13 +30,13 @@ type IDsNames struct { // IDOf returns the id associated with the given name. func (in IDsNames) IDOf(name string) (string, bool) { - id, ok := in.NameToID[name] + id, ok := in.NameToID[strings.ToLower(name)] return id, ok } // NameOf returns the name associated with the given id. func (in IDsNames) NameOf(id string) (string, bool) { - name, ok := in.IDToName[id] + name, ok := in.IDToName[strings.ToLower(id)] return name, ok } diff --git a/src/internal/connector/data_collections.go b/src/internal/connector/data_collections.go index dac0d6f29..a76ad41d7 100644 --- a/src/internal/connector/data_collections.go +++ b/src/internal/connector/data_collections.go @@ -8,8 +8,8 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/connector/discovery" - "github.com/alcionai/corso/src/internal/connector/discovery/api" "github.com/alcionai/corso/src/internal/connector/exchange" + "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/onedrive" "github.com/alcionai/corso/src/internal/connector/sharepoint" "github.com/alcionai/corso/src/internal/connector/support" @@ -19,7 +19,6 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/fault" - "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -171,7 +170,7 @@ func verifyBackupInputs(sels selectors.Selector, siteIDs []string) error { func checkServiceEnabled( ctx context.Context, - au api.Users, + gi discovery.GetInfoer, service path.ServiceType, resource string, ) (bool, error) { @@ -180,14 +179,13 @@ func checkServiceEnabled( return true, nil } - _, info, err := discovery.User(ctx, au, resource) + info, err := gi.GetInfo(ctx, resource) if err != nil { return false, err } - if _, ok := info.DiscoveredServices[service]; !ok { - logger.Ctx(ctx).Error("service not enabled") - return false, nil + if !info.ServiceEnabled(service) { + return false, clues.Wrap(graph.ErrServiceNotEnabled, "checking service access") } return true, nil diff --git a/src/internal/connector/discovery/api/users.go b/src/internal/connector/discovery/api/users.go index f4e28aa96..186a44e90 100644 --- a/src/internal/connector/discovery/api/users.go +++ b/src/internal/connector/discovery/api/users.go @@ -45,6 +45,18 @@ func newUserInfo() *UserInfo { } } +// ServiceEnabled returns true if the UserInfo has an entry for the +// service. If no entry exists, the service is assumed to not be enabled. +func (ui *UserInfo) ServiceEnabled(service path.ServiceType) bool { + if ui == nil || len(ui.DiscoveredServices) == 0 { + return false + } + + _, ok := ui.DiscoveredServices[service] + + return ok +} + // --------------------------------------------------------------------------- // methods // --------------------------------------------------------------------------- @@ -160,7 +172,6 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) { // TODO: OneDrive _, err = c.stable.Client().UsersById(userID).MailFolders().Get(ctx, nil) - if err != nil { if !graph.IsErrExchangeMailFolderNotFound(err) { return nil, graph.Wrap(ctx, err, "getting user's mail folder") diff --git a/src/internal/connector/discovery/discovery.go b/src/internal/connector/discovery/discovery.go index e955803c0..62e30cecc 100644 --- a/src/internal/connector/discovery/discovery.go +++ b/src/internal/connector/discovery/discovery.go @@ -20,13 +20,17 @@ type getter interface { GetByID(context.Context, string) (models.Userable, error) } -type getInfoer interface { +type GetInfoer interface { GetInfo(context.Context, string) (*api.UserInfo, error) } type getWithInfoer interface { getter - getInfoer + GetInfoer +} + +type getAller interface { + GetAll(ctx context.Context, errs *fault.Bus) ([]models.Userable, error) } // --------------------------------------------------------------------------- @@ -48,25 +52,28 @@ func apiClient(ctx context.Context, acct account.Account) (api.Client, error) { } // --------------------------------------------------------------------------- -// api +// users // --------------------------------------------------------------------------- // Users fetches all users in the tenant. func Users( ctx context.Context, - acct account.Account, + ga getAller, errs *fault.Bus, ) ([]models.Userable, error) { - client, err := apiClient(ctx, acct) + users, err := ga.GetAll(ctx, errs) if err != nil { - return nil, err + return nil, clues.Wrap(err, "getting all users") } - return client.Users().GetAll(ctx, errs) + return users, nil } -// User fetches a single user's data. -func User(ctx context.Context, gwi getWithInfoer, userID string) (models.Userable, *api.UserInfo, error) { +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) { @@ -84,6 +91,25 @@ func User(ctx context.Context, gwi getWithInfoer, userID string) (models.Userabl return u, ui, nil } +// UserInfo produces extensible user info: metadata that is relevant +// or identified in Corso, but not in m365. +func UserInfo( + ctx context.Context, + gi GetInfoer, + userID string, +) (*api.UserInfo, error) { + ui, err := gi.GetInfo(ctx, userID) + if err != nil { + return nil, clues.Wrap(err, "getting user info") + } + + return ui, nil +} + +// --------------------------------------------------------------------------- +// sites +// --------------------------------------------------------------------------- + // Sites fetches all sharepoint sites in the tenant func Sites( ctx context.Context, diff --git a/src/internal/connector/discovery/discovery_test.go b/src/internal/connector/discovery/discovery_test.go index 8191b7b3f..4da2c3867 100644 --- a/src/internal/connector/discovery/discovery_test.go +++ b/src/internal/connector/discovery/discovery_test.go @@ -4,15 +4,18 @@ import ( "testing" "github.com/alcionai/clues" + "github.com/google/uuid" "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/connector/discovery/api" "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" + "github.com/alcionai/corso/src/pkg/path" ) type DiscoveryIntegrationSuite struct { @@ -37,7 +40,13 @@ func (suite *DiscoveryIntegrationSuite) TestUsers() { errs = fault.New(true) ) - users, err := discovery.Users(ctx, acct, errs) + creds, err := acct.M365Config() + require.NoError(t, err) + + cli, err := api.NewClient(creds) + require.NoError(t, err) + + users, err := discovery.Users(ctx, cli.Users(), errs) assert.NoError(t, err, clues.ToCore(err)) ferrs := errs.Errors() @@ -47,9 +56,6 @@ func (suite *DiscoveryIntegrationSuite) TestUsers() { } func (suite *DiscoveryIntegrationSuite) TestUsers_InvalidCredentials() { - ctx, flush := tester.NewContext() - defer flush() - table := []struct { name string acct func(t *testing.T) account.Account @@ -72,25 +78,23 @@ func (suite *DiscoveryIntegrationSuite) TestUsers_InvalidCredentials() { 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() { - var ( - t = suite.T() - a = test.acct(t) - errs = fault.New(true) - ) + ctx, flush := tester.NewContext() + defer flush() - users, err := discovery.Users(ctx, a, errs) + t := suite.T() + acct := test.acct(t) + + creds, err := acct.M365Config() + require.NoError(t, err) + + cli, err := api.NewClient(creds) + require.NoError(t, err) + + users, err := discovery.Users(ctx, cli.Users(), fault.New(true)) assert.Empty(t, users, "returned some users") assert.NotNil(t, err) }) @@ -166,3 +170,55 @@ func (suite *DiscoveryIntegrationSuite) TestSites_InvalidCredentials() { }) } } + +func (suite *DiscoveryIntegrationSuite) TestUserInfo() { + t := suite.T() + acct := tester.NewM365Account(t) + userID := tester.M365UserID(t) + + creds, err := acct.M365Config() + require.NoError(t, err) + + cli, err := api.NewClient(creds) + require.NoError(t, err) + + uapi := cli.Users() + + table := []struct { + name string + user string + expect *api.UserInfo + }{ + { + name: "standard test user", + user: userID, + expect: &api.UserInfo{ + DiscoveredServices: map[path.ServiceType]struct{}{ + path.ExchangeService: {}, + path.OneDriveService: {}, + }, + }, + }, + { + name: "user does not exist", + user: uuid.NewString(), + expect: &api.UserInfo{ + DiscoveredServices: map[path.ServiceType]struct{}{ + path.OneDriveService: {}, // currently statically populated + }, + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + result, err := discovery.UserInfo(ctx, uapi, test.user) + require.NoError(t, err, clues.ToCore(err)) + assert.Equal(t, test.expect, result) + }) + } +} diff --git a/src/internal/connector/graph/errors.go b/src/internal/connector/graph/errors.go index 5148e4dda..4d84a94de 100644 --- a/src/internal/connector/graph/errors.go +++ b/src/internal/connector/graph/errors.go @@ -61,6 +61,10 @@ var ( // https://learn.microsoft.com/en-us/graph/errors#code-property ErrInvalidDelta = clues.New("invalid delta token") + // ErrServiceNotEnabled identifies that a resource owner does not have + // access to a given service. + ErrServiceNotEnabled = clues.New("service is not enabled for that resource owner") + // 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. diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index 0180934a9..4e5a411cc 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -152,7 +152,7 @@ func getOwnerIDAndNameFrom( // and populate with maps as a result. Only // return owner, owner as a very last resort. - return owner, owner, nil + return "", "", clues.New("not found within tenant") } // createService constructor for graphService component diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 2b4525bc0..3f12446b3 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -56,12 +56,14 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { ins common.IDsNames expectID string expectName string + expectErr require.ErrorAssertionFunc }{ { name: "nil ins", owner: ownerID, - expectID: ownerID, - expectName: ownerID, + expectID: "", + expectName: "", + expectErr: require.Error, }, { name: "only id map with owner id", @@ -72,6 +74,7 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { }, expectID: ownerID, expectName: ownerName, + expectErr: require.NoError, }, { name: "only name map with owner id", @@ -80,8 +83,9 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { IDToName: nil, NameToID: nti, }, - expectID: ownerID, - expectName: ownerID, + expectID: "", + expectName: "", + expectErr: require.Error, }, { name: "only id map with owner name", @@ -90,8 +94,9 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { IDToName: itn, NameToID: nil, }, - expectID: ownerName, - expectName: ownerName, + expectID: "", + expectName: "", + expectErr: require.Error, }, { name: "only name map with owner name", @@ -102,6 +107,7 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { }, expectID: ownerID, expectName: ownerName, + expectErr: require.NoError, }, { name: "both maps with owner id", @@ -112,6 +118,7 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { }, expectID: ownerID, expectName: ownerName, + expectErr: require.NoError, }, { name: "both maps with owner name", @@ -122,6 +129,7 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { }, expectID: ownerID, expectName: ownerName, + expectErr: require.NoError, }, { name: "non-matching maps with owner id", @@ -130,8 +138,9 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { IDToName: map[string]string{"foo": "bar"}, NameToID: map[string]string{"fnords": "smarf"}, }, - expectID: ownerID, - expectName: ownerID, + expectID: "", + expectName: "", + expectErr: require.Error, }, { name: "non-matching with owner name", @@ -140,8 +149,9 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { IDToName: map[string]string{"foo": "bar"}, NameToID: map[string]string{"fnords": "smarf"}, }, - expectID: ownerName, - expectName: ownerName, + expectID: "", + expectName: "", + expectErr: require.Error, }, } for _, test := range table { @@ -152,7 +162,7 @@ func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { ) id, name, err := gc.PopulateOwnerIDAndNamesFrom(test.owner, test.ins) - require.NoError(t, err, clues.ToCore(err)) + test.expectErr(t, err, clues.ToCore(err)) assert.Equal(t, test.expectID, id) assert.Equal(t, test.expectName, name) }) diff --git a/src/pkg/services/m365/m365.go b/src/pkg/services/m365/m365.go index b2b8371cd..fc1a66034 100644 --- a/src/pkg/services/m365/m365.go +++ b/src/pkg/services/m365/m365.go @@ -10,16 +10,34 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/discovery" + "github.com/alcionai/corso/src/internal/connector/discovery/api" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/path" ) +// ServiceAccess is true if a resource owner is capable of +// accessing or utilizing the specified service. +type ServiceAccess struct { + Exchange bool + // TODO: onedrive, sharepoint +} + +// --------------------------------------------------------------------------- +// Users +// --------------------------------------------------------------------------- + +// User is the minimal information required to identify and display a user. type User struct { PrincipalName string ID string Name string } +type UserInfo struct { + ServicesEnabled ServiceAccess +} + // UsersCompat returns a list of users in the specified M365 tenant. // TODO(ashmrtn): Remove when upstream consumers of the SDK support the fault // package. @@ -35,9 +53,13 @@ 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) { - users, err := discovery.Users(ctx, acct, errs) + uapi, err := makeUserAPI(acct) + if err != nil { + return nil, clues.Wrap(err, "getting users").WithClues(ctx) + } + + users, err := discovery.Users(ctx, uapi, errs) if err != nil { return nil, err } @@ -47,7 +69,7 @@ func Users(ctx context.Context, acct account.Account, errs *fault.Bus) ([]*User, for _, u := range users { pu, err := parseUser(u) if err != nil { - return nil, clues.Wrap(err, "parsing userable") + return nil, clues.Wrap(err, "formatting user data") } ret = append(ret, pu) @@ -103,8 +125,38 @@ func UsersMap( return ins, nil } +// UserInfo returns the corso-specific set of user metadata. +func GetUserInfo( + ctx context.Context, + acct account.Account, + userID string, +) (*UserInfo, error) { + uapi, err := makeUserAPI(acct) + if err != nil { + return nil, clues.Wrap(err, "getting user info").WithClues(ctx) + } + + ui, err := discovery.UserInfo(ctx, uapi, userID) + if err != nil { + return nil, err + } + + info := UserInfo{ + ServicesEnabled: ServiceAccess{ + Exchange: ui.ServiceEnabled(path.ExchangeService), + }, + } + + return &info, nil +} + +// --------------------------------------------------------------------------- +// Sites +// --------------------------------------------------------------------------- + +// Site is the minimal information required to identify and display a SharePoint site. type Site struct { - // WebURL that displays the item in the browser + // WebURL is the url for the site, works as an alias for the user name. WebURL string // ID is of the format: .. @@ -173,3 +225,21 @@ func SitesMap( return ins, nil } + +// --------------------------------------------------------------------------- +// helpers +// --------------------------------------------------------------------------- + +func makeUserAPI(acct account.Account) (api.Users, error) { + creds, err := acct.M365Config() + if err != nil { + return api.Users{}, clues.Wrap(err, "getting m365 account creds") + } + + cli, err := api.NewClient(creds) + if err != nil { + return api.Users{}, clues.Wrap(err, "constructing api client") + } + + return cli.Users(), nil +} diff --git a/src/pkg/services/m365/m365_test.go b/src/pkg/services/m365/m365_test.go index 9137bb066..b62b52206 100644 --- a/src/pkg/services/m365/m365_test.go +++ b/src/pkg/services/m365/m365_test.go @@ -1,14 +1,16 @@ -package m365 +package m365_test import ( "testing" "github.com/alcionai/clues" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/services/m365" ) type M365IntegrationSuite struct { @@ -33,7 +35,7 @@ func (suite *M365IntegrationSuite) TestUsers() { acct = tester.NewM365Account(suite.T()) ) - users, err := Users(ctx, acct, fault.New(true)) + users, err := m365.Users(ctx, acct, fault.New(true)) assert.NoError(t, err, clues.ToCore(err)) assert.NotEmpty(t, users) @@ -48,6 +50,30 @@ func (suite *M365IntegrationSuite) TestUsers() { } } +func (suite *M365IntegrationSuite) TestGetUserInfo() { + ctx, flush := tester.NewContext() + defer flush() + + var ( + t = suite.T() + acct = tester.NewM365Account(t) + uid = tester.M365UserID(t) + ) + + info, err := m365.GetUserInfo(ctx, acct, uid) + require.NoError(t, err, clues.ToCore(err)) + require.NotNil(t, info) + require.NotEmpty(t, info) + + expect := &m365.UserInfo{ + ServicesEnabled: m365.ServiceAccess{ + Exchange: true, + }, + } + + assert.Equal(t, expect, info) +} + func (suite *M365IntegrationSuite) TestSites() { ctx, flush := tester.NewContext() defer flush() @@ -57,7 +83,7 @@ func (suite *M365IntegrationSuite) TestSites() { acct = tester.NewM365Account(suite.T()) ) - sites, err := Sites(ctx, acct, fault.New(true)) + sites, err := m365.Sites(ctx, acct, fault.New(true)) assert.NoError(t, err, clues.ToCore(err)) assert.NotEmpty(t, sites)