catch and sentinel resource locked error case (#4465)
handle cases where a resource is found, but is not accessible due to being locked out by an administrator or msoft process. --- #### Does this PR need a docs update or release note? - [ ] ✅ Yes, it's included #### Type of change - [x] 🐛 Bugfix #### Issue(s) * #4464 #### Test Plan - [x] ⚡ Unit test - [x] 💚 E2E
This commit is contained in:
parent
eb0299d316
commit
7196b5d278
@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
### Added
|
### Added
|
||||||
- Skips graph calls for expired item download URLs.
|
- Skips graph calls for expired item download URLs.
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
- Catch and report cases where a protected resource is locked out of access. SDK consumers have a new errs sentinel that allows them to check for this case.
|
||||||
|
|
||||||
## [v0.14.0] (beta) - 2023-10-09
|
## [v0.14.0] (beta) - 2023-10-09
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|||||||
@ -263,6 +263,10 @@ func (r resourceClient) GetResourceIDAndNameFrom(
|
|||||||
return nil, clues.Stack(graph.ErrResourceOwnerNotFound, err)
|
return nil, clues.Stack(graph.ErrResourceOwnerNotFound, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if graph.IsErrResourceLocked(err) {
|
||||||
|
return nil, clues.Stack(graph.ErrResourceLocked, err)
|
||||||
|
}
|
||||||
|
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -15,6 +15,7 @@ import (
|
|||||||
"github.com/pkg/errors"
|
"github.com/pkg/errors"
|
||||||
|
|
||||||
"github.com/alcionai/corso/src/internal/common/ptr"
|
"github.com/alcionai/corso/src/internal/common/ptr"
|
||||||
|
"github.com/alcionai/corso/src/internal/common/str"
|
||||||
"github.com/alcionai/corso/src/pkg/fault"
|
"github.com/alcionai/corso/src/pkg/fault"
|
||||||
"github.com/alcionai/corso/src/pkg/filters"
|
"github.com/alcionai/corso/src/pkg/filters"
|
||||||
)
|
)
|
||||||
@ -50,6 +51,7 @@ const (
|
|||||||
// nameAlreadyExists occurs when a request with
|
// nameAlreadyExists occurs when a request with
|
||||||
// @microsoft.graph.conflictBehavior=fail finds a conflicting file.
|
// @microsoft.graph.conflictBehavior=fail finds a conflicting file.
|
||||||
nameAlreadyExists errorCode = "nameAlreadyExists"
|
nameAlreadyExists errorCode = "nameAlreadyExists"
|
||||||
|
NotAllowed errorCode = "notAllowed"
|
||||||
noResolvedUsers errorCode = "noResolvedUsers"
|
noResolvedUsers errorCode = "noResolvedUsers"
|
||||||
QuotaExceeded errorCode = "ErrorQuotaExceeded"
|
QuotaExceeded errorCode = "ErrorQuotaExceeded"
|
||||||
RequestResourceNotFound errorCode = "Request_ResourceNotFound"
|
RequestResourceNotFound errorCode = "Request_ResourceNotFound"
|
||||||
@ -61,6 +63,11 @@ const (
|
|||||||
syncStateNotFound errorCode = "SyncStateNotFound"
|
syncStateNotFound errorCode = "SyncStateNotFound"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// inner error codes
|
||||||
|
const (
|
||||||
|
ResourceLocked errorCode = "resourceLocked"
|
||||||
|
)
|
||||||
|
|
||||||
type errorMessage string
|
type errorMessage string
|
||||||
|
|
||||||
const (
|
const (
|
||||||
@ -113,6 +120,11 @@ var (
|
|||||||
// replies, no error should get returned.
|
// replies, no error should get returned.
|
||||||
ErrMultipleResultsMatchIdentifier = clues.New("multiple results match the identifier")
|
ErrMultipleResultsMatchIdentifier = clues.New("multiple results match the identifier")
|
||||||
|
|
||||||
|
// ErrResourceLocked occurs when a resource has had its access locked.
|
||||||
|
// Example case: https://learn.microsoft.com/en-us/sharepoint/manage-lock-status
|
||||||
|
// This makes the resource inaccessible for any Corso operations.
|
||||||
|
ErrResourceLocked = clues.New("resource has been locked and must be unlocked by an administrator")
|
||||||
|
|
||||||
// ErrServiceNotEnabled identifies that a resource owner does not have
|
// ErrServiceNotEnabled identifies that a resource owner does not have
|
||||||
// access to a given service.
|
// access to a given service.
|
||||||
ErrServiceNotEnabled = clues.New("service is not enabled for that resource owner")
|
ErrServiceNotEnabled = clues.New("service is not enabled for that resource owner")
|
||||||
@ -267,6 +279,12 @@ func IsErrSiteNotFound(err error) bool {
|
|||||||
return hasErrorMessage(err, requestedSiteCouldNotBeFound)
|
return hasErrorMessage(err, requestedSiteCouldNotBeFound)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func IsErrResourceLocked(err error) bool {
|
||||||
|
return errors.Is(err, ErrResourceLocked) ||
|
||||||
|
hasInnerErrorCode(err, ResourceLocked) ||
|
||||||
|
hasErrorCode(err, NotAllowed)
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// error parsers
|
// error parsers
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@ -294,6 +312,34 @@ func hasErrorCode(err error, codes ...errorCode) bool {
|
|||||||
return filters.Equal(cs).Compare(code)
|
return filters.Equal(cs).Compare(code)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func hasInnerErrorCode(err error, codes ...errorCode) bool {
|
||||||
|
if err == nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
var oDataError odataerrors.ODataErrorable
|
||||||
|
if !errors.As(err, &oDataError) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
inner := oDataError.GetErrorEscaped().GetInnerError()
|
||||||
|
if inner == nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
code, err := str.AnyValueToString("code", inner.GetAdditionalData())
|
||||||
|
if err != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
cs := make([]string, len(codes))
|
||||||
|
for i, c := range codes {
|
||||||
|
cs[i] = string(c)
|
||||||
|
}
|
||||||
|
|
||||||
|
return filters.Equal(cs).Compare(code)
|
||||||
|
}
|
||||||
|
|
||||||
// only use this as a last resort. Prefer the code or statuscode if possible.
|
// only use this as a last resort. Prefer the code or statuscode if possible.
|
||||||
func hasErrorMessage(err error, msgs ...errorMessage) bool {
|
func hasErrorMessage(err error, msgs ...errorMessage) bool {
|
||||||
if err == nil {
|
if err == nil {
|
||||||
|
|||||||
@ -813,3 +813,57 @@ func (suite *GraphErrorsUnitSuite) TestIsErrItemNotFound() {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (suite *GraphErrorsUnitSuite) TestIsErrResourceLocked() {
|
||||||
|
innerMatch := odErr("not-match")
|
||||||
|
merr := odataerrors.NewMainError()
|
||||||
|
inerr := odataerrors.NewInnerError()
|
||||||
|
inerr.SetAdditionalData(map[string]any{
|
||||||
|
"code": string(ResourceLocked),
|
||||||
|
})
|
||||||
|
merr.SetInnerError(inerr)
|
||||||
|
merr.SetCode(ptr.To("not-match"))
|
||||||
|
innerMatch.SetErrorEscaped(merr)
|
||||||
|
|
||||||
|
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: odErrMsg("InvalidRequest", "resource is locked"),
|
||||||
|
expect: assert.False,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "matching oDataErr code",
|
||||||
|
err: odErr(string(NotAllowed)),
|
||||||
|
expect: assert.True,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "matching oDataErr inner code",
|
||||||
|
err: innerMatch,
|
||||||
|
expect: assert.True,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "matching err sentinel",
|
||||||
|
err: ErrResourceLocked,
|
||||||
|
expect: assert.True,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
for _, test := range table {
|
||||||
|
suite.Run(test.name, func() {
|
||||||
|
test.expect(suite.T(), IsErrResourceLocked(test.err))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@ -30,6 +30,10 @@ func IsServiceEnabled(
|
|||||||
return false, clues.Stack(graph.ErrResourceOwnerNotFound, err)
|
return false, clues.Stack(graph.ErrResourceOwnerNotFound, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if graph.IsErrResourceLocked(err) {
|
||||||
|
return false, clues.Stack(graph.ErrResourceLocked, err)
|
||||||
|
}
|
||||||
|
|
||||||
return false, clues.Stack(err)
|
return false, clues.Stack(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -105,6 +105,17 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
|
|||||||
assert.Error(t, err, clues.ToCore(err))
|
assert.Error(t, err, clues.ToCore(err))
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "resource locked",
|
||||||
|
mock: func(ctx context.Context) getDefaultDriver {
|
||||||
|
odErr := odErrMsg(string(graph.NotAllowed), "resource")
|
||||||
|
return mockDGDD{nil, graph.Stack(ctx, odErr)}
|
||||||
|
},
|
||||||
|
expect: assert.False,
|
||||||
|
expectErr: func(t *testing.T, err error) {
|
||||||
|
assert.Error(t, err, clues.ToCore(err))
|
||||||
|
},
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "arbitrary error",
|
name: "arbitrary error",
|
||||||
mock: func(ctx context.Context) getDefaultDriver {
|
mock: func(ctx context.Context) getDefaultDriver {
|
||||||
|
|||||||
@ -16,6 +16,7 @@ const (
|
|||||||
ApplicationThrottled errEnum = "application-throttled"
|
ApplicationThrottled errEnum = "application-throttled"
|
||||||
BackupNotFound errEnum = "backup-not-found"
|
BackupNotFound errEnum = "backup-not-found"
|
||||||
RepoAlreadyExists errEnum = "repository-already-exists"
|
RepoAlreadyExists errEnum = "repository-already-exists"
|
||||||
|
ResourceNotAccessible errEnum = "resource-not-accesible"
|
||||||
ResourceOwnerNotFound errEnum = "resource-owner-not-found"
|
ResourceOwnerNotFound errEnum = "resource-owner-not-found"
|
||||||
ServiceNotEnabled errEnum = "service-not-enabled"
|
ServiceNotEnabled errEnum = "service-not-enabled"
|
||||||
)
|
)
|
||||||
@ -27,6 +28,7 @@ var internalToExternal = map[errEnum][]error{
|
|||||||
ApplicationThrottled: {graph.ErrApplicationThrottled},
|
ApplicationThrottled: {graph.ErrApplicationThrottled},
|
||||||
BackupNotFound: {repository.ErrorBackupNotFound},
|
BackupNotFound: {repository.ErrorBackupNotFound},
|
||||||
RepoAlreadyExists: {repository.ErrorRepoAlreadyExists},
|
RepoAlreadyExists: {repository.ErrorRepoAlreadyExists},
|
||||||
|
ResourceNotAccessible: {graph.ErrResourceLocked},
|
||||||
ResourceOwnerNotFound: {graph.ErrResourceOwnerNotFound},
|
ResourceOwnerNotFound: {graph.ErrResourceOwnerNotFound},
|
||||||
ServiceNotEnabled: {graph.ErrServiceNotEnabled},
|
ServiceNotEnabled: {graph.ErrServiceNotEnabled},
|
||||||
}
|
}
|
||||||
|
|||||||
@ -29,6 +29,7 @@ func (suite *ErrUnitSuite) TestInternal() {
|
|||||||
{BackupNotFound, []error{repository.ErrorBackupNotFound}},
|
{BackupNotFound, []error{repository.ErrorBackupNotFound}},
|
||||||
{ServiceNotEnabled, []error{graph.ErrServiceNotEnabled}},
|
{ServiceNotEnabled, []error{graph.ErrServiceNotEnabled}},
|
||||||
{ResourceOwnerNotFound, []error{graph.ErrResourceOwnerNotFound}},
|
{ResourceOwnerNotFound, []error{graph.ErrResourceOwnerNotFound}},
|
||||||
|
{ResourceNotAccessible, []error{graph.ErrResourceLocked}},
|
||||||
}
|
}
|
||||||
for _, test := range table {
|
for _, test := range table {
|
||||||
suite.Run(string(test.get), func() {
|
suite.Run(string(test.get), func() {
|
||||||
@ -46,6 +47,7 @@ func (suite *ErrUnitSuite) TestIs() {
|
|||||||
{BackupNotFound, repository.ErrorBackupNotFound},
|
{BackupNotFound, repository.ErrorBackupNotFound},
|
||||||
{ServiceNotEnabled, graph.ErrServiceNotEnabled},
|
{ServiceNotEnabled, graph.ErrServiceNotEnabled},
|
||||||
{ResourceOwnerNotFound, graph.ErrResourceOwnerNotFound},
|
{ResourceOwnerNotFound, graph.ErrResourceOwnerNotFound},
|
||||||
|
{ResourceNotAccessible, graph.ErrResourceLocked},
|
||||||
}
|
}
|
||||||
for _, test := range table {
|
for _, test := range table {
|
||||||
suite.Run(string(test.target), func() {
|
suite.Run(string(test.target), func() {
|
||||||
|
|||||||
@ -184,6 +184,10 @@ func EvaluateMailboxError(err error) error {
|
|||||||
return clues.Stack(graph.ErrResourceOwnerNotFound, err)
|
return clues.Stack(graph.ErrResourceOwnerNotFound, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if graph.IsErrResourceLocked(err) {
|
||||||
|
return clues.Stack(graph.ErrResourceLocked, err)
|
||||||
|
}
|
||||||
|
|
||||||
if graph.IsErrExchangeMailFolderNotFound(err) || graph.IsErrAuthenticationError(err) {
|
if graph.IsErrExchangeMailFolderNotFound(err) || graph.IsErrAuthenticationError(err) {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|||||||
@ -85,6 +85,13 @@ func (suite *UsersUnitSuite) TestEvaluateMailboxError() {
|
|||||||
assert.ErrorIs(t, err, graph.ErrResourceOwnerNotFound, clues.ToCore(err))
|
assert.ErrorIs(t, err, graph.ErrResourceOwnerNotFound, clues.ToCore(err))
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "mail inbox err - resoruceLocked",
|
||||||
|
err: odErr(string(graph.NotAllowed)),
|
||||||
|
expect: func(t *testing.T, err error) {
|
||||||
|
assert.ErrorIs(t, err, graph.ErrResourceLocked, clues.ToCore(err))
|
||||||
|
},
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "mail inbox err - user not found",
|
name: "mail inbox err - user not found",
|
||||||
err: odErr(string(graph.MailboxNotEnabledForRESTAPI)),
|
err: odErr(string(graph.MailboxNotEnabledForRESTAPI)),
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user