add authErr to users mailbox check (#3744)

license issues can produce an authenticationError when retrieving a user's inbox.  This catches that error and treats it as a "mailbox not availble" condition.

---

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

- [x]  No

#### Type of change

- [x] 🐛 Bugfix

#### Issue(s)

* #3743

#### Test Plan

- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2023-07-03 13:31:35 -06:00 committed by GitHub
parent 725de79f64
commit cf495a6c8f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 132 additions and 18 deletions

View File

@ -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

View File

@ -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

View File

@ -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,

View File

@ -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) {

View File

@ -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

View File

@ -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)