From c65527e5c4d7066eb29eb7d604b25ed2e6c60013 Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Thu, 21 Dec 2023 11:28:55 -0800 Subject: [PATCH] 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] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [ ] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- CHANGELOG.md | 3 ++ .../m365/collection/drive/permission.go | 14 ++++-- src/pkg/services/m365/api/graph/errors.go | 7 +++ .../services/m365/api/graph/errors_test.go | 44 +++++++++++++++++++ 4 files changed, 64 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c1e5a9b9..409d988fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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`. - 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 ### Changed diff --git a/src/internal/m365/collection/drive/permission.go b/src/internal/m365/collection/drive/permission.go index 2bde73c96..8718b63bf 100644 --- a/src/internal/m365/collection/drive/permission.go +++ b/src/internal/m365/collection/drive/permission.go @@ -389,8 +389,13 @@ func UpdateLinkShares( continue } + if graph.IsErrSharingDisabled(err) { + logger.CtxErr(ictx, err).Info("unable to restore link share; sharing disabled") + continue + } + if err != nil { - el.AddRecoverable(ctx, clues.Stack(err)) + el.AddRecoverable(ictx, clues.Stack(err)) continue } @@ -499,9 +504,10 @@ func RestorePermissions( permRemoved, caches.OldPermIDToNewID, errs) - if err != nil { - return clues.Wrap(err, "updating permissions") + if graph.IsErrSharingDisabled(err) { + logger.CtxErr(ctx, err).Info("sharing disabled, not restoring permissions") + return nil } - return nil + return clues.Wrap(err, "updating permissions").OrNil() } diff --git a/src/pkg/services/m365/api/graph/errors.go b/src/pkg/services/m365/api/graph/errors.go index 6cc155841..f2c99774c 100644 --- a/src/pkg/services/m365/api/graph/errors.go +++ b/src/pkg/services/m365/api/graph/errors.go @@ -77,6 +77,9 @@ const ( // inner error codes const ( 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 @@ -293,6 +296,10 @@ func IsErrResourceLocked(err error) bool { filters.In([]string{"preventing tokens from being issued for it"})) } +func IsErrSharingDisabled(err error) bool { + return hasInnerErrorCode(err, sharingDisabled) +} + // --------------------------------------------------------------------------- // error parsers // --------------------------------------------------------------------------- diff --git a/src/pkg/services/m365/api/graph/errors_test.go b/src/pkg/services/m365/api/graph/errors_test.go index 6fc4ead43..1f838dd64 100644 --- a/src/pkg/services/m365/api/graph/errors_test.go +++ b/src/pkg/services/m365/api/graph/errors_test.go @@ -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)) + }) + } +}