diff --git a/src/internal/m365/graph/errors.go b/src/internal/m365/graph/errors.go index a408c61a2..f0df6b4ec 100644 --- a/src/internal/m365/graph/errors.go +++ b/src/internal/m365/graph/errors.go @@ -26,6 +26,9 @@ import ( type errorCode string const ( + // this auth error is a catch-all used by graph in a variety of cases: + // users without licenses, bad jwts, missing account permissions, etc. + AuthenticationError errorCode = "AuthenticationError" // cannotOpenFileAttachment happen when an attachment is // inaccessible. The error message is usually "OLE conversion // failed for an attachment." @@ -103,6 +106,10 @@ var ( ErrResourceOwnerNotFound = clues.New("resource owner not found in tenant") ) +func IsErrAuthenticationError(err error) bool { + return hasErrorCode(err, AuthenticationError) +} + func IsErrDeletedInFlight(err error) bool { if errors.Is(err, ErrDeletedInFlight) { return true diff --git a/src/internal/m365/graph/errors_test.go b/src/internal/m365/graph/errors_test.go index eea3b7ddd..86415076d 100644 --- a/src/internal/m365/graph/errors_test.go +++ b/src/internal/m365/graph/errors_test.go @@ -73,6 +73,40 @@ func (suite *GraphErrorsUnitSuite) TestIsErrConnectionReset() { } } +func (suite *GraphErrorsUnitSuite) TestIsErrAuthenticationError() { + table := []struct { + name string + err error + expect assert.BoolAssertionFunc + }{ + { + name: "nil", + err: nil, + expect: assert.False, + }, + { + name: "non-matching", + err: assert.AnError, + expect: assert.False, + }, + { + name: "non-matching oDataErr", + err: odErr("fnords"), + expect: assert.False, + }, + { + name: "authenticationError oDataErr", + err: odErr(string(AuthenticationError)), + expect: assert.True, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + test.expect(suite.T(), IsErrAuthenticationError(test.err)) + }) + } +} + func (suite *GraphErrorsUnitSuite) TestIsErrDeletedInFlight() { table := []struct { name string diff --git a/src/pkg/services/m365/api/users.go b/src/pkg/services/m365/api/users.go index 8b50a86bb..39013dfca 100644 --- a/src/pkg/services/m365/api/users.go +++ b/src/pkg/services/m365/api/users.go @@ -195,16 +195,9 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) { // if they cannot, we can assume they are ineligible for exchange backups. inbx, err := c.GetMailInbox(ctx, userID) if err != nil { - err = graph.Stack(ctx, err) - - if graph.IsErrUserNotFound(err) { - logger.CtxErr(ctx, err).Error("user not found") - return nil, clues.Stack(graph.ErrResourceOwnerNotFound, err) - } - - if !graph.IsErrExchangeMailFolderNotFound(err) { + if err := EvaluateMailboxError(graph.Stack(ctx, err)); err != nil { logger.CtxErr(ctx, err).Error("getting user's mail folder") - return nil, clues.Stack(err) + return nil, err } logger.Ctx(ctx).Info("resource owner does not have a mailbox enabled") @@ -253,6 +246,26 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) { 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. +func EvaluateMailboxError(err error) error { + if err == nil { + return nil + } + + // must occur before MailFolderNotFound, due to overlapping cases. + if graph.IsErrUserNotFound(err) { + return clues.Stack(graph.ErrResourceOwnerNotFound, err) + } + + if graph.IsErrExchangeMailFolderNotFound(err) || graph.IsErrAuthenticationError(err) { + return nil + } + + return err +} + func (c Users) getMailboxSettings( ctx context.Context, userID string, diff --git a/src/pkg/services/m365/api/users_test.go b/src/pkg/services/m365/api/users_test.go index fc786991e..a6a8aa253 100644 --- a/src/pkg/services/m365/api/users_test.go +++ b/src/pkg/services/m365/api/users_test.go @@ -66,6 +66,55 @@ func (suite *UsersUnitSuite) TestValidateUser() { } } +func (suite *UsersUnitSuite) TestEvaluateMailboxError() { + table := []struct { + name string + err error + expect func(t *testing.T, err error) + }{ + { + name: "nil", + err: nil, + expect: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "mail inbox err - user not found", + err: odErr(string(graph.RequestResourceNotFound)), + expect: func(t *testing.T, err error) { + assert.ErrorIs(t, err, graph.ErrResourceOwnerNotFound, clues.ToCore(err)) + }, + }, + { + name: "mail inbox err - user not found", + err: odErr(string(graph.MailboxNotEnabledForRESTAPI)), + expect: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "mail inbox err - authenticationError", + err: odErr(string(graph.AuthenticationError)), + expect: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "mail inbox err - other error", + err: odErrMsg("somecode", "somemessage"), + expect: func(t *testing.T, err error) { + assert.Error(t, err, clues.ToCore(err)) + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + test.expect(suite.T(), api.EvaluateMailboxError(test.err)) + }) + } +} + type UsersIntgSuite struct { tester.Suite its intgTesterSetup @@ -156,6 +205,20 @@ func (suite *UsersIntgSuite) TestUsers_GetInfo_errors() { 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", "inbox"). + 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) { diff --git a/src/pkg/services/m365/m365.go b/src/pkg/services/m365/m365.go index 1a7f8f5da..f4c568e3a 100644 --- a/src/pkg/services/m365/m365.go +++ b/src/pkg/services/m365/m365.go @@ -80,17 +80,11 @@ func UserHasMailbox(ctx context.Context, acct account.Account, userID string) (b _, err = ac.Users().GetMailInbox(ctx, userID) if err != nil { - // we consider this a non-error case, since it - // answers the question the caller is asking. - if graph.IsErrExchangeMailFolderNotFound(err) { - return false, nil + if err := api.EvaluateMailboxError(err); err != nil { + return false, clues.Stack(err) } - if graph.IsErrUserNotFound(err) { - return false, clues.Stack(graph.ErrResourceOwnerNotFound, err) - } - - return false, clues.Stack(err) + return false, nil } return true, nil diff --git a/src/pkg/services/m365/m365_test.go b/src/pkg/services/m365/m365_test.go index 1ccf2be3a..d3de1399e 100644 --- a/src/pkg/services/m365/m365_test.go +++ b/src/pkg/services/m365/m365_test.go @@ -2,6 +2,7 @@ package m365 import ( "context" + "fmt" "testing" "github.com/alcionai/clues" @@ -506,6 +507,8 @@ func (suite *DiscoveryIntgSuite) TestGetUserInfo() { } for _, test := range table { suite.Run(test.name, func() { + fmt.Printf("\n-----\n%+v\n-----\n", test.name) + t := suite.T() ctx, flush := tester.NewContext(t)