Log and ignore sharing disabled errors when doing permissions restores (#4901)

We've encountered an error where the user previously had sharing for
one or more external users on an item and then did a backup. They
subsequently disabled sharing for the SharePoint site in either the
tenant-wide config or for the site in particular. Once sharing was
disabled, they attempted a restore which subsequently failed while
trying to restore sharing for the external user

This PR logs and ignores errors about sharing being disabled when
doing a restore

This has been tested manually

---

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

- [x]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ]  No

#### Type of change

- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Test Plan

- [x] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-12-21 11:28:55 -08:00 committed by GitHub
parent 753ed1a075
commit c65527e5c4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 64 additions and 4 deletions

View File

@ -13,6 +13,9 @@ this case, Corso will skip over the item but report this in the backup summary.
- Guarantee Exchange email restoration when restoring multiple attachments. Some previous restores were failing with `ErrorItemNotFound`. - Guarantee Exchange email restoration when restoring multiple attachments. Some previous restores were failing with `ErrorItemNotFound`.
- Avoid Graph SDK `Requests must contain extension changes exclusively.` errors by removing server-populated field from restored event items. - Avoid Graph SDK `Requests must contain extension changes exclusively.` errors by removing server-populated field from restored event items.
### Known issues
- Restoring OneDrive, SharePoint, or Teams & Groups items shared with external users while the tenant or site is configured to not allow sharing with external users will not restore permissions.
## [v0.17.0] (beta) - 2023-12-11 ## [v0.17.0] (beta) - 2023-12-11
### Changed ### Changed

View File

@ -389,8 +389,13 @@ func UpdateLinkShares(
continue continue
} }
if graph.IsErrSharingDisabled(err) {
logger.CtxErr(ictx, err).Info("unable to restore link share; sharing disabled")
continue
}
if err != nil { if err != nil {
el.AddRecoverable(ctx, clues.Stack(err)) el.AddRecoverable(ictx, clues.Stack(err))
continue continue
} }
@ -499,9 +504,10 @@ func RestorePermissions(
permRemoved, permRemoved,
caches.OldPermIDToNewID, caches.OldPermIDToNewID,
errs) errs)
if err != nil { if graph.IsErrSharingDisabled(err) {
return clues.Wrap(err, "updating permissions") logger.CtxErr(ctx, err).Info("sharing disabled, not restoring permissions")
return nil
} }
return nil return clues.Wrap(err, "updating permissions").OrNil()
} }

View File

@ -77,6 +77,9 @@ const (
// inner error codes // inner error codes
const ( const (
ResourceLocked errorCode = "resourceLocked" ResourceLocked errorCode = "resourceLocked"
// Returned when either the tenant-wide or site settings don't permit sharing
// for external users to some extent.
sharingDisabled errorCode = "sharingDisabled"
) )
type errorMessage string type errorMessage string
@ -293,6 +296,10 @@ func IsErrResourceLocked(err error) bool {
filters.In([]string{"preventing tokens from being issued for it"})) filters.In([]string{"preventing tokens from being issued for it"}))
} }
func IsErrSharingDisabled(err error) bool {
return hasInnerErrorCode(err, sharingDisabled)
}
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// error parsers // error parsers
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------

View File

@ -939,3 +939,47 @@ func (suite *GraphErrorsUnitSuite) TestIsErrResourceLocked() {
}) })
} }
} }
func (suite *GraphErrorsUnitSuite) TestIsErrSharingDisabled() {
innerMatch := graphTD.ODataErr("not-match")
merr := odataerrors.NewMainError()
inerr := odataerrors.NewInnerError()
inerr.SetAdditionalData(map[string]any{
"code": string(sharingDisabled),
})
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: graphTD.ODataErrWithMsg("InvalidRequest", "resource is locked"),
expect: assert.False,
},
{
name: "matching oDataErr inner code",
err: innerMatch,
expect: assert.True,
},
}
for _, test := range table {
suite.Run(test.name, func() {
test.expect(suite.T(), IsErrSharingDisabled(test.err))
})
}
}