match specific error message for resourcre locked (#4806)

Matches a specific error message returned by graph when an account does not have support for microsoft teams.

---

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

- [x]  No

#### Type of change

- [x] 🐛 Bugfix

#### Test Plan

- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2023-12-05 18:26:08 -07:00 committed by GitHub
parent 68a0bebd1a
commit 27ff840be8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 186 additions and 10 deletions

View File

@ -361,6 +361,22 @@ func newFilter(c comparator, targets, normTargets []string, negate bool) Filter
// Comparisons // Comparisons
// ---------------------------------------------------------------------------------------------------- // ----------------------------------------------------------------------------------------------------
// Must returns true if ALL provided filters match the input.
// Returns false if no filters are provided
func Must(input string, filters ...Filter) bool {
if len(filters) == 0 {
return false
}
for _, f := range filters {
if !f.Compare(input) {
return false
}
}
return true
}
// CompareAny checks whether any one of all the provided // CompareAny checks whether any one of all the provided
// inputs passes the filter. // inputs passes the filter.
// //

View File

@ -554,6 +554,64 @@ func (suite *FiltersSuite) TestPathEquals_NormalizedTargets() {
} }
} }
func (suite *FiltersSuite) TestMust() {
table := []struct {
name string
filters []filters.Filter
expect assert.BoolAssertionFunc
}{
{
name: "no filters",
filters: []filters.Filter{},
expect: assert.False,
},
{
name: "one matching filter",
filters: []filters.Filter{filters.Equal([]string{"fnords"})},
expect: assert.True,
},
{
name: "one non-matching filter",
filters: []filters.Filter{filters.Equal([]string{"smarf"})},
expect: assert.False,
},
{
name: "multiple matching filter",
filters: []filters.Filter{
filters.Equal([]string{"fnords"}),
filters.Equal([]string{"FNORDS"}),
filters.Equal([]string{"fNORDs"}),
},
expect: assert.True,
},
{
name: "one non-matching filter among many matching",
filters: []filters.Filter{
filters.Equal([]string{"fnords"}),
filters.Equal([]string{"FNORDS"}),
filters.Equal([]string{"smarf"}),
},
expect: assert.False,
},
{
name: "all non-matching filters",
filters: []filters.Filter{
filters.Equal([]string{"smarf"}),
filters.Equal([]string{"SMARF"}),
filters.Equal([]string{"sMARf"}),
},
expect: assert.False,
},
}
for _, test := range table {
suite.Run(test.name, func() {
test.expect(
suite.T(),
filters.Must("fnords", test.filters...))
})
}
}
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// pii handling // pii handling
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------

View File

@ -276,7 +276,12 @@ func IsErrSiteNotFound(err error) bool {
func IsErrResourceLocked(err error) bool { func IsErrResourceLocked(err error) bool {
return errors.Is(err, ErrResourceLocked) || return errors.Is(err, ErrResourceLocked) ||
hasInnerErrorCode(err, ResourceLocked) || hasInnerErrorCode(err, ResourceLocked) ||
hasErrorCode(err, NotAllowed) hasErrorCode(err, NotAllowed) ||
errMessageMatchesAllFilters(
err,
filters.In([]string{"the service principal for resource"}),
filters.In([]string{"this indicate that a subscription within the tenant has lapsed"}),
filters.In([]string{"preventing tokens from being issued for it"}))
} }
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@ -358,6 +363,25 @@ func hasErrorMessage(err error, msgs ...errorMessage) bool {
return filters.In(cs).Compare(msg) return filters.In(cs).Compare(msg)
} }
// only use this as a last resort. Prefer the code or statuscode if possible.
func errMessageMatchesAllFilters(err error, fs ...filters.Filter) bool {
if err == nil {
return false
}
var oDataError odataerrors.ODataErrorable
if !errors.As(err, &oDataError) {
return false
}
msg, ok := ptr.ValOK(oDataError.GetErrorEscaped().GetMessage())
if !ok {
return false
}
return filters.Must(msg, fs...)
}
// Wrap is a helper function that extracts ODataError metadata from // Wrap is a helper function that extracts ODataError metadata from
// the error. If the error is not an ODataError type, returns the error. // the error. If the error is not an ODataError type, returns the error.
func Wrap(ctx context.Context, e error, msg string) *clues.Err { func Wrap(ctx context.Context, e error, msg string) *clues.Err {

View File

@ -877,6 +877,17 @@ func (suite *GraphErrorsUnitSuite) TestIsErrResourceLocked() {
err: innerMatch, err: innerMatch,
expect: assert.True, expect: assert.True,
}, },
{
name: "matching oDataErr message",
err: graphTD.ODataErrWithMsg(
string(AuthenticationError),
"AADSTS500014: The service principal for resource 'beefe6b7-f5df-413d-ac2d-abf1e3fd9c0b' "+
"is disabled. This indicate that a subscription within the tenant has lapsed, or that the "+
"administrator for this tenant has disabled the application, preventing tokens from being "+
"issued for it. Trace ID: dead78e1-0830-4edf-bea7-f0a445620100 Correlation ID: "+
"deadbeef-7f1e-4578-8215-36004a2c935c Timestamp: 2023-12-05 19:31:01Z"),
expect: assert.True,
},
{ {
name: "matching err sentinel", name: "matching err sentinel",
err: ErrResourceLocked, err: ErrResourceLocked,

View File

@ -110,20 +110,18 @@ func (c Groups) GetByID(
identifier string, identifier string,
_ CallConfig, // matching standards _ CallConfig, // matching standards
) (models.Groupable, error) { ) (models.Groupable, error) {
service, err := c.Service(c.counter)
if err != nil {
return nil, err
}
ctx = clues.Add(ctx, "resource_identifier", identifier) ctx = clues.Add(ctx, "resource_identifier", identifier)
var group models.Groupable var (
group models.Groupable
err error
)
// prefer lookup by id, but fallback to lookup by display name, // prefer lookup by id, but fallback to lookup by display name,
// even in the case of a uuid, just in case the display name itself // even in the case of a uuid, just in case the display name itself
// is a uuid. // is a uuid.
if uuidRE.MatchString(identifier) { if uuidRE.MatchString(identifier) {
group, err = service. group, err = c.Stable.
Client(). Client().
Groups(). Groups().
ByGroupId(identifier). ByGroupId(identifier).
@ -149,7 +147,7 @@ func (c Groups) GetByID(
}, },
} }
resp, err := service.Client().Groups().Get(ctx, opts) resp, err := c.Stable.Client().Groups().Get(ctx, opts)
if err == nil { if err == nil {
return getGroupFromResponse(ctx, resp) return getGroupFromResponse(ctx, resp)
} }
@ -169,7 +167,7 @@ func (c Groups) GetByID(
}, },
} }
resp, err := service.Client().Groups().Get(ctx, opts) resp, err := c.Stable.Client().Groups().Get(ctx, opts)
if err != nil { if err != nil {
if graph.IsErrResourceLocked(err) { if graph.IsErrResourceLocked(err) {
err = clues.Stack(graph.ErrResourceLocked, err) err = clues.Stack(graph.ErrResourceLocked, err)

View File

@ -5,6 +5,7 @@ import (
"github.com/alcionai/clues" "github.com/alcionai/clues"
"github.com/google/uuid" "github.com/google/uuid"
"github.com/h2non/gock"
"github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -15,6 +16,7 @@ import (
"github.com/alcionai/corso/src/internal/tester/tconfig" "github.com/alcionai/corso/src/internal/tester/tconfig"
"github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph" "github.com/alcionai/corso/src/pkg/services/m365/api/graph"
graphTD "github.com/alcionai/corso/src/pkg/services/m365/api/graph/testdata"
) )
type GroupUnitSuite struct { type GroupUnitSuite struct {
@ -218,3 +220,70 @@ func (suite *GroupsIntgSuite) TestGroups_GetByID() {
}) })
} }
} }
func (suite *GroupsIntgSuite) TestGroups_GetByID_mockResourceLockedErrs() {
gID := uuid.NewString()
table := []struct {
name string
id string
setup func(t *testing.T)
}{
{
name: "by name",
id: "g",
setup: func(t *testing.T) {
err := graphTD.ODataErr(string(graph.NotAllowed))
interceptV1Path("groups").
Reply(403).
JSON(graphTD.ParseableToMap(t, err))
},
},
{
name: "by id",
id: gID,
setup: func(t *testing.T) {
err := graphTD.ODataErr(string(graph.NotAllowed))
interceptV1Path("groups", gID).
Reply(403).
JSON(graphTD.ParseableToMap(t, err))
},
},
{
name: "by id - matches error message",
id: gID,
setup: func(t *testing.T) {
err := graphTD.ODataErrWithMsg(
string(graph.AuthenticationError),
"AADSTS500014: The service principal for resource 'beefe6b7-f5df-413d-ac2d-abf1e3fd9c0b' "+
"is disabled. This indicate that a subscription within the tenant has lapsed, or that the "+
"administrator for this tenant has disabled the application, preventing tokens from being "+
"issued for it. Trace ID: dead78e1-0830-4edf-bea7-f0a445620100 Correlation ID: "+
"deadbeef-7f1e-4578-8215-36004a2c935c Timestamp: 2023-12-05 19:31:01Z")
interceptV1Path("groups", gID).
Reply(403).
JSON(graphTD.ParseableToMap(t, err))
},
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
defer gock.Off()
test.setup(t)
_, err := suite.its.gockAC.
Groups().
GetByID(ctx, test.id, CallConfig{})
require.ErrorIs(t, err, graph.ErrResourceLocked, clues.ToCore(err))
})
}
}