CBG-4025 Invalidate user roles after resync #6942
Conversation
As part of this change, modified access invalidation functions to perform invalidation in a single update to the principal document, instead of once per collection (and one additional time for roles). To support this: - Switched existing invalidation functions (invalRoleChannels, invalUserChannels, etc) to have *DatabaseContext receiver instead of *DatabaseCollection, and added a ScopeAndCollectionNames parameter to specify the set of collections that should have access invalidated. - For ease of use, maintained the existing invalidation functions on *DatabaseCollection - they now just call in to the *DatabaseContext functions with their single collection - Added a new invalUserRolesAndChannels to invalidate a user’s roles and channels in a single user doc update. Only currently used by resync Query based resync still processes a single collection’s updates at a time - it’s structured a bit differently and didn’t seem to be worth refactoring at this point. It has been updated to properly invalidate user roles. The new test TestResyncInvalidatePrincipals covers the fix - have verified it with SG_TEST_USE_DEFAULT_COLLECTION=true/false. Also made a test utility change to remove the password parameter from GetRolePayload since roles don’t have passwords.
| } | ||
|
|
||
| if subdocStore, ok := base.AsSubdocStore(auth.datastore); ok { | ||
| err := subdocStore.SubdocInsert(auth.LogCtx, docID, subdocPath, 0, invalSeq) |
There was a problem hiding this comment.
sgbucket.SubDocStore is part of sgbucket.DataStore so we don't need to do this cast.
There was a problem hiding this comment.
AsSubdocStore is also doing a check on ds.IsSupported(sgbucket.BucketStoreFeatureSubdocOperations) (which I believe is still relevant for walrus support for backports to 3.1 that support walrus). I think I'd prefer to leave this code as it was in this PR since we expect a backport.
| // Attempt to use subdoc if we're only modifying a single collection | ||
| if len(collections) == 1 { |
There was a problem hiding this comment.
This is more of an optimization than might be necessary, but can we change the API of SubdocInsert to be map[string]any so we can do multiple subdoc operations at once?
Later we are going to run through the logic of updating the document, though not the write operation.
There was a problem hiding this comment.
I considered adding that functionality in this PR, but decided against it due to the increase in scope/risk. Since the more common usage of InvalidateChannels (at write time) is always single collection, I thought it was reasonable to omit.
CBG-4025
As part of this change, modified access invalidation functions to perform invalidation in a single update to the principal document, instead of once per collection (and one additional time for roles). To support this:
*DatabaseCollection, and added aScopeAndCollectionNamesparameter to specify the set of collections that should have access invalidated.*DatabaseCollection- they now just call in to the*DatabaseContextfunctions with their single collectionQuery based resync still processes a single collection’s updates at a time - it’s structured a bit differently and didn’t seem to be worth refactoring at this point. It has been updated to properly invalidate user roles.
The new test TestResyncInvalidatePrincipals covers the fix - have verified it with
SG_TEST_USE_DEFAULT_COLLECTION=true/false. Also made a test utility change to remove the password parameter from GetRolePayload since roles don’t have passwords.Integration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGateway-Integration/2551/