diff --git a/src/pkg/services/m365/api/user_info.go b/src/pkg/services/m365/api/user_info.go index 90c1145db..4dc6575a5 100644 --- a/src/pkg/services/m365/api/user_info.go +++ b/src/pkg/services/m365/api/user_info.go @@ -5,18 +5,8 @@ import ( "github.com/alcionai/corso/src/internal/common/str" "github.com/alcionai/corso/src/internal/common/tform" - "github.com/alcionai/corso/src/pkg/path" ) -// --------------------------------------------------------------------------- -// User Info -// --------------------------------------------------------------------------- - -type UserInfo struct { - ServicesEnabled map[path.ServiceType]struct{} - Mailbox MailboxInfo -} - type MailboxInfo struct { Purpose string ArchiveFolder string @@ -59,15 +49,6 @@ type WorkingHours struct { } } -func newUserInfo() *UserInfo { - return &UserInfo{ - ServicesEnabled: map[path.ServiceType]struct{}{ - path.ExchangeService: {}, - path.OneDriveService: {}, - }, - } -} - func ParseMailboxSettings( settings models.Userable, mi MailboxInfo, diff --git a/src/pkg/services/m365/api/users.go b/src/pkg/services/m365/api/users.go index 18437b8e7..47a476571 100644 --- a/src/pkg/services/m365/api/users.go +++ b/src/pkg/services/m365/api/users.go @@ -15,8 +15,6 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/pkg/fault" - "github.com/alcionai/corso/src/pkg/logger" - "github.com/alcionai/corso/src/pkg/path" ) // Variables @@ -169,86 +167,6 @@ func appendIfErr(errs []error, err error) []error { return append(errs, err) } -// --------------------------------------------------------------------------- -// Info -// --------------------------------------------------------------------------- - -// TODO(pandeyabs): Remove this once the SDK users have migrated to new APIs -func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) { - var ( - // Assume all services are enabled - // then filter down to only services the user has enabled - userInfo = newUserInfo() - mailFolderFound = true - ) - - // check whether the user is able to access their onedrive drive. - // if they cannot, we can assume they are ineligible for onedrive backups. - if _, err := c.GetDefaultDrive(ctx, userID); err != nil { - if !clues.HasLabel(err, graph.LabelsMysiteNotFound) && !clues.HasLabel(err, graph.LabelsNoSharePointLicense) { - logger.CtxErr(ctx, err).Error("getting user's default drive") - return nil, graph.Wrap(ctx, err, "getting user's default drive info") - } - - logger.Ctx(ctx).Info("resource owner does not have a drive") - delete(userInfo.ServicesEnabled, path.OneDriveService) - } - - // check whether the user is able to access their inbox. - // if they cannot, we can assume they are ineligible for exchange backups. - inbx, err := c.GetMailInbox(ctx, userID) - if err != nil { - if err := EvaluateMailboxError(graph.Stack(ctx, err)); err != nil { - logger.CtxErr(ctx, err).Error("getting user's mail folder") - return nil, err - } - - logger.Ctx(ctx).Info("resource owner does not have a mailbox enabled") - delete(userInfo.ServicesEnabled, path.ExchangeService) - - mailFolderFound = false - } - - // check whether the user has accessible mailbox settings. - // if they do, aggregate them in the MailboxInfo - mi := MailboxInfo{ - ErrGetMailBoxSetting: []error{}, - } - - if !mailFolderFound { - mi.ErrGetMailBoxSetting = append(mi.ErrGetMailBoxSetting, ErrMailBoxNotFound) - userInfo.Mailbox = mi - - return userInfo, nil - } - - mboxSettings, err := c.GetMailboxSettings(ctx, userID) - if err != nil { - logger.CtxErr(ctx, err).Info("err getting user's mailbox settings") - - if !graph.IsErrAccessDenied(err) { - return nil, graph.Wrap(ctx, err, "getting user's mailbox settings") - } - - mi.ErrGetMailBoxSetting = append(mi.ErrGetMailBoxSetting, clues.New("access denied")) - } else { - mi = ParseMailboxSettings(mboxSettings, mi) - } - - err = c.GetFirstInboxMessage(ctx, userID, ptr.Val(inbx.GetId())) - if err != nil { - if !graph.IsErrQuotaExceeded(err) { - return nil, clues.Stack(err) - } - - mi.QuotaExceeded = graph.IsErrQuotaExceeded(err) - } - - userInfo.Mailbox = mi - - return userInfo, nil -} - // EvaluateMailboxError checks whether the provided error can be interpreted // as "user does not have a mailbox", or whether it is some other error. If // the former (no mailbox), returns nil, otherwise returns an error. diff --git a/src/pkg/services/m365/api/users_test.go b/src/pkg/services/m365/api/users_test.go index aee817413..007693448 100644 --- a/src/pkg/services/m365/api/users_test.go +++ b/src/pkg/services/m365/api/users_test.go @@ -4,15 +4,12 @@ import ( "testing" "github.com/alcionai/clues" - "github.com/h2non/gock" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/internal/tester" - "github.com/alcionai/corso/src/internal/tester/tconfig" "github.com/alcionai/corso/src/pkg/services/m365/api" ) @@ -153,164 +150,3 @@ func (suite *UsersUnitSuite) TestIsAnyErrMailboxNotFound() { }) } } - -type UsersIntgSuite struct { - tester.Suite - its intgTesterSetup -} - -func TestUsersIntgSuite(t *testing.T) { - suite.Run(t, &UsersIntgSuite{ - Suite: tester.NewIntegrationSuite( - t, - [][]string{tconfig.M365AcctCredEnvs}), - }) -} - -func (suite *UsersIntgSuite) SetupSuite() { - suite.its = newIntegrationTesterSetup(suite.T()) -} - -func (suite *UsersIntgSuite) TestUsers_GetInfo_errors() { - table := []struct { - name string - setGocks func(t *testing.T) - expectErr func(t *testing.T, err error) - }{ - { - name: "default drive err - mysite not found", - setGocks: func(t *testing.T) { - interceptV1Path("users", "user", "drive"). - Reply(400). - JSON(parseableToMap(t, odErrMsg("anycode", string(graph.MysiteNotFound)))) - interceptV1Path("users", "user", "mailFolders", api.MailInbox). - Reply(400). - JSON(parseableToMap(t, odErr(string(graph.ResourceNotFound)))) - }, - expectErr: func(t *testing.T, err error) { - assert.NoError(t, err, clues.ToCore(err)) - }, - }, - { - name: "default drive err - no sharepoint license", - setGocks: func(t *testing.T) { - interceptV1Path("users", "user", "drive"). - Reply(400). - JSON(parseableToMap(t, odErrMsg("anycode", string(graph.NoSPLicense)))) - interceptV1Path("users", "user", "mailFolders", api.MailInbox). - Reply(400). - JSON(parseableToMap(t, odErr(string(graph.ResourceNotFound)))) - }, - expectErr: func(t *testing.T, err error) { - assert.NoError(t, err, clues.ToCore(err)) - }, - }, - { - name: "default drive err - other error", - setGocks: func(t *testing.T) { - interceptV1Path("users", "user", "drive"). - Reply(400). - JSON(parseableToMap(t, odErrMsg("somecode", "somemessage"))) - }, - expectErr: func(t *testing.T, err error) { - assert.Error(t, err, clues.ToCore(err)) - }, - }, - { - name: "mail inbox err - user not found", - setGocks: func(t *testing.T) { - interceptV1Path("users", "user", "drive"). - Reply(200). - JSON(parseableToMap(t, models.NewDrive())) - interceptV1Path("users", "user", "mailFolders", api.MailInbox). - Reply(400). - JSON(parseableToMap(t, odErr(string(graph.RequestResourceNotFound)))) - }, - expectErr: func(t *testing.T, err error) { - assert.ErrorIs(t, err, graph.ErrResourceOwnerNotFound, clues.ToCore(err)) - }, - }, - { - name: "mail inbox err - user not found", - setGocks: func(t *testing.T) { - interceptV1Path("users", "user", "drive"). - Reply(200). - JSON(parseableToMap(t, models.NewDrive())) - interceptV1Path("users", "user", "mailFolders", api.MailInbox). - Reply(400). - JSON(parseableToMap(t, odErr(string(graph.MailboxNotEnabledForRESTAPI)))) - }, - expectErr: func(t *testing.T, err error) { - assert.NoError(t, err, clues.ToCore(err)) - }, - }, - { - name: "mail inbox err - authenticationError", - setGocks: func(t *testing.T) { - interceptV1Path("users", "user", "drive"). - Reply(200). - JSON(parseableToMap(t, models.NewDrive())) - interceptV1Path("users", "user", "mailFolders", api.MailInbox). - Reply(400). - JSON(parseableToMap(t, odErr(string(graph.AuthenticationError)))) - }, - expectErr: func(t *testing.T, err error) { - assert.NoError(t, err, clues.ToCore(err)) - }, - }, - { - name: "mail inbox err - other error", - setGocks: func(t *testing.T) { - interceptV1Path("users", "user", "drive"). - Reply(200). - JSON(parseableToMap(t, models.NewDrive())) - interceptV1Path("users", "user", "mailFolders", api.MailInbox). - Reply(400). - JSON(parseableToMap(t, odErrMsg("somecode", "somemessage"))) - }, - expectErr: func(t *testing.T, err error) { - assert.Error(t, err, clues.ToCore(err)) - }, - }, - } - for _, test := range table { - suite.Run(test.name, func() { - t := suite.T() - ctx, flush := tester.NewContext(t) - - defer flush() - defer gock.Off() - - test.setGocks(t) - - _, err := suite.its.gockAC.Users().GetInfo(ctx, "user") - test.expectErr(t, err) - }) - } -} - -func (suite *UsersIntgSuite) TestUsers_GetInfo_quotaExceeded() { - t := suite.T() - ctx, flush := tester.NewContext(t) - - defer flush() - defer gock.Off() - - gock.EnableNetworking() - gock.New(graphAPIHostURL). - // Wildcard match on the inbox folder ID. - Get(v1APIURLPath("users", suite.its.user.id, "mailFolders", "(.*)", "messages", "delta")). - Reply(403). - SetHeaders( - map[string]string{ - "Content-Type": "application/json; odata.metadata=minimal; " + - "odata.streaming=true; IEEE754Compatible=false; charset=utf-8", - }, - ). - BodyString(`{"error":{"code":"ErrorQuotaExceeded","message":"The process failed to get the correct properties."}}`) - - output, err := suite.its.gockAC.Users().GetInfo(ctx, suite.its.user.id) - require.NoError(t, err, clues.ToCore(err)) - - assert.True(t, output.Mailbox.QuotaExceeded) -} diff --git a/src/pkg/services/m365/users.go b/src/pkg/services/m365/users.go index 8a3935c25..19f16b67d 100644 --- a/src/pkg/services/m365/users.go +++ b/src/pkg/services/m365/users.go @@ -15,39 +15,17 @@ import ( "github.com/alcionai/corso/src/pkg/services/m365/api" ) -// User is the minimal information required to identify and display a user. -type User struct { - PrincipalName string - ID string - Name string - Info api.UserInfo -} - // UserNoInfo is the minimal information required to identify and display a user. -// TODO: Remove this once `UsersCompatNoInfo` is removed +// TODO(pandeyabs): Rename this to User now that `Info` support has been removed. type UserNoInfo struct { PrincipalName string ID string Name string } -// UsersCompat returns a list of users in the specified M365 tenant. -// TODO(ashmrtn): Remove when upstream consumers of the SDK support the fault -// package. -func UsersCompat(ctx context.Context, acct account.Account) ([]*User, error) { - errs := fault.New(true) - - us, err := Users(ctx, acct, errs) - if err != nil { - return nil, err - } - - return us, errs.Failure() -} - // UsersCompatNoInfo returns a list of users in the specified M365 tenant. -// TODO: Remove this once `Info` is removed from the `User` struct and callers -// have switched over +// TODO(pandeyabs): Rename this to Users now that `Info` support has been removed. Would +// need corresponding changes in SDK consumers. func UsersCompatNoInfo(ctx context.Context, acct account.Account) ([]*UserNoInfo, error) { errs := fault.New(true) @@ -95,8 +73,6 @@ func UserHasDrives(ctx context.Context, acct account.Account, userID string) (bo } // usersNoInfo returns a list of users in the specified M365 tenant - with no info -// TODO: Remove this once we remove `Info` from `Users` and instead rely on the `GetUserInfo` API -// to get user information func usersNoInfo(ctx context.Context, acct account.Account, errs *fault.Bus) ([]*UserNoInfo, error) { ac, err := makeAC(ctx, acct, path.UnknownService) if err != nil { @@ -128,47 +104,14 @@ func usersNoInfo(ctx context.Context, acct account.Account, errs *fault.Bus) ([] return ret, nil } -// Users returns a list of users in the specified M365 tenant -func Users(ctx context.Context, acct account.Account, errs *fault.Bus) ([]*User, error) { - ac, err := makeAC(ctx, acct, path.ExchangeService) - if err != nil { - return nil, clues.Stack(err).WithClues(ctx) - } - - us, err := ac.Users().GetAll(ctx, errs) - if err != nil { - return nil, err - } - - ret := make([]*User, 0, len(us)) - - for _, u := range us { - pu, err := parseUser(u) - if err != nil { - return nil, clues.Wrap(err, "formatting user data") - } - - userInfo, err := ac.Users().GetInfo(ctx, pu.ID) - if err != nil { - return nil, clues.Wrap(err, "getting user details") - } - - pu.Info = *userInfo - - ret = append(ret, pu) - } - - return ret, nil -} - // parseUser extracts information from `models.Userable` we care about -func parseUser(item models.Userable) (*User, error) { +func parseUser(item models.Userable) (*UserNoInfo, error) { if item.GetUserPrincipalName() == nil { return nil, clues.New("user missing principal name"). With("user_id", ptr.Val(item.GetId())) } - u := &User{ + u := &UserNoInfo{ PrincipalName: ptr.Val(item.GetUserPrincipalName()), ID: ptr.Val(item.GetId()), Name: ptr.Val(item.GetDisplayName()), @@ -176,24 +119,3 @@ func parseUser(item models.Userable) (*User, error) { return u, nil } - -// UserInfo returns the corso-specific set of user metadata. -// TODO(pandeyabs): Remove support for this API. SDK users would be using -// per service API calls - UserHasMailbox, UserGetMailboxInfo, UserHasDrive, etc. -func GetUserInfo( - ctx context.Context, - acct account.Account, - userID string, -) (*api.UserInfo, error) { - ac, err := makeAC(ctx, acct, path.ExchangeService) - if err != nil { - return nil, clues.Stack(err).WithClues(ctx) - } - - ui, err := ac.Users().GetInfo(ctx, userID) - if err != nil { - return nil, err - } - - return ui, nil -} diff --git a/src/pkg/services/m365/users_test.go b/src/pkg/services/m365/users_test.go index 344b9be7b..a78226d8f 100644 --- a/src/pkg/services/m365/users_test.go +++ b/src/pkg/services/m365/users_test.go @@ -14,8 +14,6 @@ import ( "github.com/alcionai/corso/src/internal/tester/tconfig" "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" "github.com/alcionai/corso/src/pkg/services/m365/api" ) @@ -41,7 +39,7 @@ func (suite *userIntegrationSuite) SetupSuite() { suite.acct = tconfig.NewM365Account(suite.T()) } -func (suite *userIntegrationSuite) TestUsers() { +func (suite *userIntegrationSuite) TestUsersCompat_HasNoInfo() { t := suite.T() ctx, flush := tester.NewContext(t) @@ -49,28 +47,6 @@ func (suite *userIntegrationSuite) TestUsers() { graph.InitializeConcurrencyLimiter(ctx, true, 4) - users, err := Users(ctx, suite.acct, fault.New(true)) - assert.NoError(t, err, clues.ToCore(err)) - assert.NotEmpty(t, users) - - for _, u := range users { - suite.Run("user_"+u.ID, func() { - t := suite.T() - - assert.NotEmpty(t, u.ID) - assert.NotEmpty(t, u.PrincipalName) - assert.NotEmpty(t, u.Name) - assert.NotEmpty(t, u.Info) - }) - } -} - -func (suite *userIntegrationSuite) TestUsersCompat_HasNoInfo() { - t := suite.T() - - ctx, flush := tester.NewContext(t) - defer flush() - acct := tconfig.NewM365Account(suite.T()) users, err := UsersCompatNoInfo(ctx, acct) @@ -129,19 +105,22 @@ func (suite *userIntegrationSuite) TestUserHasDrive() { userID := tconfig.M365UserID(t) table := []struct { - name string - user string - expect bool + name string + user string + expect bool + expectErr require.ErrorAssertionFunc }{ { - name: "user without drive", - user: "a53c26f7-5100-4acb-a910-4d20960b2c19", // User: testevents@10rqc2.onmicrosoft.com - expect: false, + name: "user without drive", + user: "a53c26f7-5100-4acb-a910-4d20960b2c19", // User: testevents@10rqc2.onmicrosoft.com + expect: false, + expectErr: require.NoError, }, { - name: "user with drive", - user: userID, - expect: true, + name: "user with drive", + user: userID, + expect: true, + expectErr: require.NoError, }, } for _, test := range table { @@ -152,7 +131,7 @@ func (suite *userIntegrationSuite) TestUserHasDrive() { defer flush() enabled, err := UserHasDrives(ctx, acct, test.user) - require.NoError(t, err, clues.ToCore(err)) + test.expectErr(t, err, clues.ToCore(err)) assert.Equal(t, test.expect, enabled) }) } @@ -161,6 +140,7 @@ func (suite *userIntegrationSuite) TestUserHasDrive() { func (suite *userIntegrationSuite) TestUserGetMailboxInfo() { t := suite.T() acct := tconfig.NewM365Account(t) + userID := tconfig.M365UserID(t) table := []struct { name string @@ -177,6 +157,15 @@ func (suite *userIntegrationSuite) TestUserGetMailboxInfo() { }, expectErr: require.NoError, }, + { + name: "user mailbox", + user: userID, + expect: func(t *testing.T, info api.MailboxInfo) { + require.NotNil(t, info) + assert.Equal(t, "user", info.Purpose) + }, + expectErr: require.NoError, + }, { name: "user with no mailbox", user: "a53c26f7-5100-4acb-a910-4d20960b2c19", // User: testevents@10rqc2.onmicrosoft.com @@ -186,6 +175,18 @@ func (suite *userIntegrationSuite) TestUserGetMailboxInfo() { }, expectErr: require.NoError, }, + { + name: "invalid user", + user: uuid.NewString(), + expect: func(t *testing.T, info api.MailboxInfo) { + mi := api.MailboxInfo{ + ErrGetMailBoxSetting: []error{}, + } + + require.Equal(t, mi, info) + }, + expectErr: require.Error, + }, } for _, test := range table { suite.Run(test.name, func() { @@ -233,109 +234,9 @@ func (suite *userIntegrationSuite) TestUsers_InvalidCredentials() { ctx, flush := tester.NewContext(t) defer flush() - users, err := Users(ctx, test.acct(t), fault.New(true)) + users, err := UsersCompatNoInfo(ctx, test.acct(t)) assert.Empty(t, users, "returned some users") assert.NotNil(t, err) }) } } - -func (suite *userIntegrationSuite) TestGetUserInfo() { - table := []struct { - name string - user string - expect *api.UserInfo - expectErr require.ErrorAssertionFunc - }{ - { - name: "standard test user", - user: tconfig.M365UserID(suite.T()), - expect: &api.UserInfo{ - ServicesEnabled: map[path.ServiceType]struct{}{ - path.ExchangeService: {}, - path.OneDriveService: {}, - }, - Mailbox: api.MailboxInfo{ - Purpose: "user", - ErrGetMailBoxSetting: nil, - }, - }, - expectErr: require.NoError, - }, - { - name: "user does not exist", - user: uuid.NewString(), - expect: &api.UserInfo{ - ServicesEnabled: map[path.ServiceType]struct{}{}, - Mailbox: api.MailboxInfo{}, - }, - expectErr: require.Error, - }, - } - for _, test := range table { - suite.Run(test.name, func() { - t := suite.T() - - ctx, flush := tester.NewContext(t) - defer flush() - - result, err := GetUserInfo(ctx, suite.acct, test.user) - test.expectErr(t, err, clues.ToCore(err)) - - if err != nil { - return - } - - assert.Equal(t, test.expect.ServicesEnabled, result.ServicesEnabled) - }) - } -} - -func (suite *userIntegrationSuite) TestGetUserInfo_userWithoutDrive() { - userID := tconfig.M365UserID(suite.T()) - - table := []struct { - name string - user string - expect *api.UserInfo - }{ - { - name: "user without drive and exchange", - user: "a53c26f7-5100-4acb-a910-4d20960b2c19", // User: testevents@10rqc2.onmicrosoft.com - expect: &api.UserInfo{ - ServicesEnabled: map[path.ServiceType]struct{}{}, - Mailbox: api.MailboxInfo{ - ErrGetMailBoxSetting: []error{api.ErrMailBoxNotFound}, - }, - }, - }, - { - name: "user with drive and exchange", - user: userID, - expect: &api.UserInfo{ - ServicesEnabled: map[path.ServiceType]struct{}{ - path.ExchangeService: {}, - path.OneDriveService: {}, - }, - Mailbox: api.MailboxInfo{ - Purpose: "user", - ErrGetMailBoxSetting: []error{}, - }, - }, - }, - } - for _, test := range table { - suite.Run(test.name, func() { - t := suite.T() - - ctx, flush := tester.NewContext(t) - defer flush() - - result, err := GetUserInfo(ctx, suite.acct, test.user) - require.NoError(t, err, clues.ToCore(err)) - 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) - }) - } -}