diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index b1ad92cb2..e125e9125 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -17,7 +17,6 @@ import ( "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/repository" "github.com/alcionai/corso/src/pkg/selectors" - "github.com/alcionai/corso/src/pkg/services/m365" ) // ------------------------------------------------------------------------------------------------ @@ -162,10 +161,7 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error { sel := exchangeBackupCreateSelectors(utils.UserFV, utils.CategoryDataFV) - // TODO: log/print recoverable errors - errs := fault.New(false) - - ins, err := m365.UsersMap(ctx, *acct, errs) + ins, err := utils.UsersMap(ctx, *acct, fault.New(true)) if err != nil { return Only(ctx, clues.Wrap(err, "Failed to retrieve M365 users")) } diff --git a/src/cli/backup/onedrive.go b/src/cli/backup/onedrive.go index 27cfeb244..ec5c192f8 100644 --- a/src/cli/backup/onedrive.go +++ b/src/cli/backup/onedrive.go @@ -17,7 +17,6 @@ import ( "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/repository" "github.com/alcionai/corso/src/pkg/selectors" - "github.com/alcionai/corso/src/pkg/services/m365" ) // ------------------------------------------------------------------------------------------------ @@ -144,10 +143,7 @@ func createOneDriveCmd(cmd *cobra.Command, args []string) error { sel := oneDriveBackupCreateSelectors(utils.UserFV) - // TODO: log/print recoverable errors - errs := fault.New(false) - - ins, err := m365.UsersMap(ctx, *acct, errs) + ins, err := utils.UsersMap(ctx, *acct, fault.New(true)) if err != nil { return Only(ctx, clues.Wrap(err, "Failed to retrieve M365 users")) } diff --git a/src/cli/utils/users.go b/src/cli/utils/users.go new file mode 100644 index 000000000..610f0e2c6 --- /dev/null +++ b/src/cli/utils/users.go @@ -0,0 +1,40 @@ +package utils + +import ( + "context" + + "github.com/alcionai/clues" + + "github.com/alcionai/corso/src/internal/common/idname" + "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/services/m365/api" +) + +// UsersMap retrieves all users in the tenant and returns them in an idname.Cacher +func UsersMap( + ctx context.Context, + acct account.Account, + errs *fault.Bus, +) (idname.Cacher, error) { + au, err := makeUserAPI(acct) + if err != nil { + return nil, clues.Wrap(err, "constructing a graph client") + } + + return au.GetAllIDsAndNames(ctx, errs) +} + +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/cmd/factory/impl/common.go b/src/cmd/factory/impl/common.go index 4cb0e013d..25c437305 100644 --- a/src/cmd/factory/impl/common.go +++ b/src/cmd/factory/impl/common.go @@ -3,7 +3,6 @@ package impl import ( "context" "os" - "strings" "time" "github.com/alcionai/clues" @@ -22,7 +21,6 @@ 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 ( @@ -119,21 +117,6 @@ func getGCAndVerifyUser(ctx context.Context, userID string) (*connector.GraphCon return nil, account.Account{}, clues.Wrap(err, "finding m365 account details") } - // TODO: log/print recoverable errors - errs := fault.New(false) - - ins, err := m365.UsersMap(ctx, acct, errs) - if err != nil { - return nil, account.Account{}, clues.Wrap(err, "getting tenant users") - } - - _, idOK := ins.NameOf(strings.ToLower(userID)) - _, nameOK := ins.IDOf(strings.ToLower(userID)) - - if !idOK && !nameOK { - return nil, account.Account{}, clues.New("user not found within tenant") - } - gc, err := connector.NewGraphConnector( ctx, acct, @@ -142,6 +125,10 @@ func getGCAndVerifyUser(ctx context.Context, userID string) (*connector.GraphCon return nil, account.Account{}, clues.Wrap(err, "connecting to graph api") } + if _, _, err := gc.PopulateOwnerIDAndNamesFrom(ctx, userID, nil); err != nil { + return nil, account.Account{}, clues.Wrap(err, "verifying user") + } + return gc, acct, nil } diff --git a/src/internal/connector/discovery/discovery_test.go b/src/internal/connector/discovery/discovery_test.go index 9c889d28a..198e9e653 100644 --- a/src/internal/connector/discovery/discovery_test.go +++ b/src/internal/connector/discovery/discovery_test.go @@ -193,7 +193,7 @@ func (suite *DiscoveryIntgSuite) TestUserInfo() { name: "standard test user", user: tester.M365UserID(t), expect: &api.UserInfo{ - DiscoveredServices: map[path.ServiceType]struct{}{ + ServicesEnabled: map[path.ServiceType]struct{}{ path.ExchangeService: {}, path.OneDriveService: {}, }, @@ -208,8 +208,8 @@ func (suite *DiscoveryIntgSuite) TestUserInfo() { name: "user does not exist", user: uuid.NewString(), expect: &api.UserInfo{ - DiscoveredServices: map[path.ServiceType]struct{}{}, - Mailbox: api.MailboxInfo{}, + ServicesEnabled: map[path.ServiceType]struct{}{}, + Mailbox: api.MailboxInfo{}, }, expectErr: require.NoError, }, @@ -228,7 +228,7 @@ func (suite *DiscoveryIntgSuite) TestUserInfo() { return } - assert.Equal(t, test.expect.DiscoveredServices, result.DiscoveredServices) + assert.Equal(t, test.expect.ServicesEnabled, result.ServicesEnabled) }) } } @@ -247,7 +247,7 @@ func (suite *DiscoveryIntgSuite) TestUserWithoutDrive() { name: "user without drive and exchange", user: "a53c26f7-5100-4acb-a910-4d20960b2c19", // User: testevents@10rqc2.onmicrosoft.com expect: &api.UserInfo{ - DiscoveredServices: map[path.ServiceType]struct{}{}, + ServicesEnabled: map[path.ServiceType]struct{}{}, Mailbox: api.MailboxInfo{ ErrGetMailBoxSetting: []error{api.ErrMailBoxSettingsNotFound}, }, @@ -257,7 +257,7 @@ func (suite *DiscoveryIntgSuite) TestUserWithoutDrive() { name: "user with drive and exchange", user: userID, expect: &api.UserInfo{ - DiscoveredServices: map[path.ServiceType]struct{}{ + ServicesEnabled: map[path.ServiceType]struct{}{ path.ExchangeService: {}, path.OneDriveService: {}, }, @@ -277,7 +277,7 @@ func (suite *DiscoveryIntgSuite) TestUserWithoutDrive() { result, err := discovery.GetUserInfo(ctx, acct, test.user, fault.New(true)) require.NoError(t, err, clues.ToCore(err)) - assert.Equal(t, test.expect.DiscoveredServices, result.DiscoveredServices) + assert.Equal(t, test.expect.ServicesEnabled, result.ServicesEnabled) assert.Equal(t, test.expect.Mailbox.ErrGetMailBoxSetting, result.Mailbox.ErrGetMailBoxSetting) assert.Equal(t, test.expect.Mailbox.Purpose, result.Mailbox.Purpose) }) diff --git a/src/pkg/services/m365/api/users.go b/src/pkg/services/m365/api/users.go index 5d21a0333..32d2c2aba 100644 --- a/src/pkg/services/m365/api/users.go +++ b/src/pkg/services/m365/api/users.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "strings" "github.com/alcionai/clues" abstractions "github.com/microsoft/kiota-abstractions-go" @@ -11,6 +12,7 @@ import ( "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/microsoftgraph/msgraph-sdk-go/users" + "github.com/alcionai/corso/src/internal/common/idname" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/pkg/fault" @@ -41,8 +43,8 @@ type Users struct { // --------------------------------------------------------------------------- type UserInfo struct { - DiscoveredServices map[path.ServiceType]struct{} - Mailbox MailboxInfo + ServicesEnabled map[path.ServiceType]struct{} + Mailbox MailboxInfo } type MailboxInfo struct { @@ -88,7 +90,7 @@ type WorkingHours struct { func newUserInfo() *UserInfo { return &UserInfo{ - DiscoveredServices: map[path.ServiceType]struct{}{ + ServicesEnabled: map[path.ServiceType]struct{}{ path.ExchangeService: {}, path.OneDriveService: {}, }, @@ -98,11 +100,11 @@ 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 { + if ui == nil || len(ui.ServicesEnabled) == 0 { return false } - _, ok := ui.DiscoveredServices[service] + _, ok := ui.ServicesEnabled[service] return ok } @@ -225,6 +227,25 @@ func (c Users) GetIDAndName(ctx context.Context, userID string) (string, string, return ptr.Val(u.GetId()), ptr.Val(u.GetUserPrincipalName()), nil } +// GetAllIDsAndNames retrieves all users in the tenant and returns them in an idname.Cacher +func (c Users) GetAllIDsAndNames(ctx context.Context, errs *fault.Bus) (idname.Cacher, error) { + all, err := c.GetAll(ctx, errs) + if err != nil { + return nil, clues.Wrap(err, "getting all users") + } + + idToName := make(map[string]string, len(all)) + + for _, u := range all { + id := strings.ToLower(ptr.Val(u.GetId())) + name := strings.ToLower(ptr.Val(u.GetUserPrincipalName())) + + idToName[id] = name + } + + return idname.NewCache(idToName), 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 @@ -252,7 +273,7 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) { } logger.Ctx(ctx).Info("resource owner does not have a mailbox enabled") - delete(userInfo.DiscoveredServices, path.ExchangeService) + delete(userInfo.ServicesEnabled, path.ExchangeService) } if _, err := c.GetDrives(ctx, userID); err != nil { @@ -264,7 +285,7 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) { logger.Ctx(ctx).Info("resource owner does not have a drive") - delete(userInfo.DiscoveredServices, path.OneDriveService) + delete(userInfo.ServicesEnabled, path.OneDriveService) } mbxInfo, err := c.getMailboxSettings(ctx, userID) diff --git a/src/pkg/services/m365/m365.go b/src/pkg/services/m365/m365.go index a8ee56b76..ca2c48fa2 100644 --- a/src/pkg/services/m365/m365.go +++ b/src/pkg/services/m365/m365.go @@ -2,7 +2,6 @@ package m365 import ( "context" - "strings" "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" @@ -12,7 +11,6 @@ import ( "github.com/alcionai/corso/src/internal/connector/discovery" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/fault" - "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" ) @@ -35,10 +33,6 @@ type User struct { Info api.UserInfo } -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. @@ -102,40 +96,12 @@ func parseUser(item models.Userable) (*User, error) { return u, nil } -// UsersMap retrieves all users in the tenant, and returns two maps: one id-to-principalName, -// and one principalName-to-id. -func UsersMap( - ctx context.Context, - acct account.Account, - errs *fault.Bus, -) (idname.Cacher, error) { - users, err := Users(ctx, acct, errs) - if err != nil { - return idname.NewCache(nil), err - } - - var ( - idToName = make(map[string]string, len(users)) - nameToID = make(map[string]string, len(users)) - ) - - for _, u := range users { - id, name := strings.ToLower(u.ID), strings.ToLower(u.PrincipalName) - idToName[id] = name - nameToID[name] = id - } - - ins := idname.NewCache(idToName) - - return ins, nil -} - // UserInfo returns the corso-specific set of user metadata. func GetUserInfo( ctx context.Context, acct account.Account, userID string, -) (*UserInfo, error) { +) (*api.UserInfo, error) { uapi, err := makeUserAPI(acct) if err != nil { return nil, clues.Wrap(err, "getting user info").WithClues(ctx) @@ -146,13 +112,7 @@ func GetUserInfo( return nil, err } - info := UserInfo{ - ServicesEnabled: ServiceAccess{ - Exchange: ui.ServiceEnabled(path.ExchangeService), - }, - } - - return &info, nil + return ui, nil } // --------------------------------------------------------------------------- diff --git a/src/pkg/services/m365/m365_test.go b/src/pkg/services/m365/m365_test.go index 94bdfdd34..0353d4e36 100644 --- a/src/pkg/services/m365/m365_test.go +++ b/src/pkg/services/m365/m365_test.go @@ -10,6 +10,7 @@ import ( "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365" ) @@ -66,13 +67,15 @@ func (suite *M365IntegrationSuite) TestGetUserInfo() { require.NotNil(t, info) require.NotEmpty(t, info) - expect := &m365.UserInfo{ - ServicesEnabled: m365.ServiceAccess{ - Exchange: true, - }, + expectEnabled := map[path.ServiceType]struct{}{ + path.ExchangeService: {}, + path.OneDriveService: {}, } - assert.Equal(t, expect, info) + assert.NotEmpty(t, info.ServicesEnabled) + assert.NotEmpty(t, info.Mailbox) + assert.Equal(t, expectEnabled, info.ServicesEnabled) + assert.Equal(t, "user", info.Mailbox.Purpose) } func (suite *M365IntegrationSuite) TestSites() {