From f4b1ba424f166f2124ee97296c1a125d495e5dba Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Thu, 22 Aug 2024 17:36:19 -0400 Subject: [PATCH 1/2] Add stop gap option to send document for channel removal - match the behavior of SG 2.8 & 3.1 /db/_changes with special option - user has explict * channel access - using channel filter='A' will now send a Rev if the document leaves channel 'A' - demonstrate behavior using /db/_changes feed --- db/blip_handler.go | 3 +- db/database.go | 29 ++++---- rest/blip_channel_filter_test.go | 112 +++++++++++++++++++++++++++++++ rest/blip_client_test.go | 32 ++++++++- 4 files changed, 159 insertions(+), 17 deletions(-) create mode 100644 rest/blip_channel_filter_test.go diff --git a/db/blip_handler.go b/db/blip_handler.go index 0265703ffb..4c4dec3459 100644 --- a/db/blip_handler.go +++ b/db/blip_handler.go @@ -461,7 +461,7 @@ func (bh *blipHandler) sendChanges(sender *blip.Sender, opts *sendChangesOptions // If change is a removal and we're running with protocol V3 and change change is not a tombstone // fall into 3.0 removal handling. // Changes with change.Revoked=true have already evaluated UserHasDocAccess in changes.go, don't check again. - if change.allRemoved && bh.blipContext.ActiveSubprotocol() == BlipCBMobileReplicationV3 && !change.Deleted && !change.Revoked { + if change.allRemoved && bh.blipContext.ActiveSubprotocol() == BlipCBMobileReplicationV3 && !change.Deleted && !change.Revoked && !bh.db.Options.UnsupportedOptions.BlipSendDocsWithChannelRemoval { // If client doesn't want removals / revocations, don't send change if !opts.revocations { continue @@ -472,7 +472,6 @@ func (bh *blipHandler) sendChanges(sender *blip.Sender, opts *sendChangesOptions if err == nil && userHasAccessToDoc { continue } - // If we can't determine user access due to an error, log error and fall through to send change anyway. // In the event of an error we should be cautious and send a revocation anyway, even if the user // may actually have an alternate access method. This is the safer approach security-wise and diff --git a/db/database.go b/db/database.go index 9c7af5d1f9..5cf25e3e40 100644 --- a/db/database.go +++ b/db/database.go @@ -248,20 +248,21 @@ type APIEndpoints struct { // UnsupportedOptions are not supported for external use type UnsupportedOptions struct { - UserViews *UserViewsOptions `json:"user_views,omitempty"` // Config settings for user views - OidcTestProvider *OidcTestProviderOptions `json:"oidc_test_provider,omitempty"` // Config settings for OIDC Provider - APIEndpoints *APIEndpoints `json:"api_endpoints,omitempty"` // Config settings for API endpoints - WarningThresholds *WarningThresholds `json:"warning_thresholds,omitempty"` // Warning thresholds related to _sync size - DisableCleanSkippedQuery bool `json:"disable_clean_skipped_query,omitempty"` // Clean skipped sequence processing bypasses final check (deprecated: CBG-2672) - OidcTlsSkipVerify bool `json:"oidc_tls_skip_verify,omitempty"` // Config option to enable self-signed certs for OIDC testing. - SgrTlsSkipVerify bool `json:"sgr_tls_skip_verify,omitempty"` // Config option to enable self-signed certs for SG-Replicate testing. - RemoteConfigTlsSkipVerify bool `json:"remote_config_tls_skip_verify,omitempty"` // Config option to enable self signed certificates for external JavaScript load. - GuestReadOnly bool `json:"guest_read_only,omitempty"` // Config option to restrict GUEST document access to read-only - ForceAPIForbiddenErrors bool `json:"force_api_forbidden_errors,omitempty"` // Config option to force the REST API to return forbidden errors - ConnectedClient bool `json:"connected_client,omitempty"` // Enables BLIP connected-client APIs - UseQueryBasedResyncManager bool `json:"use_query_resync_manager,omitempty"` // Config option to use Query based resync manager to perform Resync op - DCPReadBuffer int `json:"dcp_read_buffer,omitempty"` // Enables user to set their own DCP read buffer - KVBufferSize int `json:"kv_buffer,omitempty"` // Enables user to set their own KV pool buffer + UserViews *UserViewsOptions `json:"user_views,omitempty"` // Config settings for user views + OidcTestProvider *OidcTestProviderOptions `json:"oidc_test_provider,omitempty"` // Config settings for OIDC Provider + APIEndpoints *APIEndpoints `json:"api_endpoints,omitempty"` // Config settings for API endpoints + WarningThresholds *WarningThresholds `json:"warning_thresholds,omitempty"` // Warning thresholds related to _sync size + DisableCleanSkippedQuery bool `json:"disable_clean_skipped_query,omitempty"` // Clean skipped sequence processing bypasses final check (deprecated: CBG-2672) + OidcTlsSkipVerify bool `json:"oidc_tls_skip_verify,omitempty"` // Config option to enable self-signed certs for OIDC testing. + SgrTlsSkipVerify bool `json:"sgr_tls_skip_verify,omitempty"` // Config option to enable self-signed certs for SG-Replicate testing. + RemoteConfigTlsSkipVerify bool `json:"remote_config_tls_skip_verify,omitempty"` // Config option to enable self signed certificates for external JavaScript load. + GuestReadOnly bool `json:"guest_read_only,omitempty"` // Config option to restrict GUEST document access to read-only + ForceAPIForbiddenErrors bool `json:"force_api_forbidden_errors,omitempty"` // Config option to force the REST API to return forbidden errors + ConnectedClient bool `json:"connected_client,omitempty"` // Enables BLIP connected-client APIs + UseQueryBasedResyncManager bool `json:"use_query_resync_manager,omitempty"` // Config option to use Query based resync manager to perform Resync op + DCPReadBuffer int `json:"dcp_read_buffer,omitempty"` // Enables user to set their own DCP read buffer + KVBufferSize int `json:"kv_buffer,omitempty"` // Enables user to set their own KV pool buffer + BlipSendDocsWithChannelRemoval bool `json:"blip_send_docs_with_channel_removal,omitempty"` // Enables sending docs with channel removals using channel filters } type WarningThresholds struct { diff --git a/rest/blip_channel_filter_test.go b/rest/blip_channel_filter_test.go new file mode 100644 index 0000000000..5409f8e079 --- /dev/null +++ b/rest/blip_channel_filter_test.go @@ -0,0 +1,112 @@ +package rest + +import ( + "fmt" + "log" + "net/http" + "testing" + + "github.com/couchbase/sync_gateway/base" + "github.com/couchbase/sync_gateway/channels" + "github.com/couchbase/sync_gateway/db" + "github.com/stretchr/testify/require" +) + +func TestChannelFilterRemovalFromChannel(t *testing.T) { + for _, sendDocWithChannelRemoval := range []bool{true, false} { + t.Run(fmt.Sprintf("sendDocWithChannelRemoval=%v", sendDocWithChannelRemoval), func(t *testing.T) { + rt := NewRestTester(t, &RestTesterConfig{ + SyncFn: channels.DocChannelsSyncFunction, + PersistentConfig: true, + }) + defer rt.Close() + + dbConfig := rt.NewDbConfig() + dbConfig.Unsupported = &db.UnsupportedOptions{ + BlipSendDocsWithChannelRemoval: sendDocWithChannelRemoval, + } + rt.CreateDatabase("db", dbConfig) + rt.CreateUser("alice", []string{"*"}) + rt.CreateUser("bob", []string{"A"}) + + btc, err := NewBlipTesterClientOptsWithRT(t, rt, &BlipTesterClientOpts{ + Username: "alice", + Password: base.StringPtr(RestTesterDefaultUserPassword), + Channels: []string{"A"}, + SendRevocations: false, + }) + require.NoError(t, err) + defer btc.Close() + + const docID = "doc1" + revID1 := rt.PutDoc("doc1", `{"channels":["A"]}`).Rev + rt.WaitForPendingChanges() + + response := rt.SendUserRequest("GET", "/{{.keyspace}}/_changes?since=0&channels=A&include_docs=true", "", "alice") + RequireStatus(t, response, http.StatusOK) + + log.Printf("response: %s", response.BodyBytes()) + expectedChanges1 := fmt.Sprintf(` +{ + "results": [ + {"seq":1, "id": "_user/alice", "changes":[]}, + {"seq":3, "id": "doc1", "doc": {"_id": "doc1", "_rev":"%s", "channels": ["A"]}, "changes": [{"rev":"%s"}]} + ], + "last_seq": "3" +}`, revID1, revID1) + require.JSONEq(t, expectedChanges1, string(response.BodyBytes())) + + continuous := "false" + since := "0" + activeOnly := "false" + channels := "A" + err = btc.StartFilteredPullSince(continuous, since, activeOnly, channels) + require.NoError(t, err) + + _, ok := btc.WaitForRev(docID, revID1) + require.True(t, ok) + + // remove channel A from doc1 + revID2 := rt.UpdateDoc(docID, revID1, `{"channels":["B"]}`).Rev + rt.WaitForPendingChanges() + + // alice will see doc1 rev2 with body + response = rt.SendUserRequest("GET", "/{{.keyspace}}/_changes?since=2&channels=A&include_docs=true", "", "alice") + RequireStatus(t, response, http.StatusOK) + + aliceExpectedChanges2 := fmt.Sprintf(` +{ + "results": [ + {"seq":4, "id": "doc1", "doc": {"_id": "doc1", "_rev":"%s", "channels": ["B"]}, "changes": [{"rev":"%s"}]} + ], + "last_seq": "4" +}`, revID2, revID2) + require.JSONEq(t, aliceExpectedChanges2, string(response.BodyBytes())) + + err = btc.StartFilteredPullSince(continuous, since, activeOnly, channels) + require.NoError(t, err) + + if sendDocWithChannelRemoval { + data, ok := btc.WaitForRev(docID, revID2) + require.True(t, ok) + require.Equal(t, `{"channels":["B"]}`, string(data)) + } else { + btc.RequireRevNotExpected(docID, revID2) + } + + // bob will not see doc1 + response = rt.SendUserRequest("GET", "/{{.keyspace}}/_changes?since=2&channels=A&include_docs=true", "", "bob") + RequireStatus(t, response, http.StatusOK) + + log.Printf("response: %s", response.BodyBytes()) + bobExpectedChanges2 := fmt.Sprintf(` +{ + "results": [ + {"seq":4, "id": "doc1", "removed":["A"], "doc": {"_id": "doc1", "_rev":"%s", "_removed": true}, "changes": [{"rev":"%s"}]} + ], + "last_seq": "4" +}`, revID2, revID2) + require.JSONEq(t, bobExpectedChanges2, string(response.BodyBytes())) + }) + } +} diff --git a/rest/blip_client_test.go b/rest/blip_client_test.go index 922143dab9..786992a359 100644 --- a/rest/blip_client_test.go +++ b/rest/blip_client_test.go @@ -32,6 +32,7 @@ import ( type BlipTesterClientOpts struct { ClientDeltas bool // Support deltas on the client side Username string + Password *string Channels []string SendRevocations bool SupportedBLIPProtocols []string @@ -551,8 +552,13 @@ func (btc *BlipTesterCollectionClient) getLastReplicatedRev(docID string) (revID } func newBlipTesterReplication(tb testing.TB, id string, btc *BlipTesterClient, skipCollectionsInitialization bool) (*BlipTesterReplicator, error) { + password := "test" + if btc.Password != nil { + password = *btc.Password + } + bt, err := NewBlipTesterFromSpecWithRT(tb, &BlipTesterSpec{ - connectingPassword: "test", + connectingPassword: password, connectingUsername: btc.Username, connectingUserChannelGrants: btc.Channels, blipProtocols: btc.SupportedBLIPProtocols, @@ -1027,6 +1033,25 @@ func (btc *BlipTesterCollectionClient) WaitForRev(docID, revID string) (data []b } } +// RequireRevNotExpected waits for 10s and fails is the given revID does show up. +func (btc *BlipTesterCollectionClient) RequireRevNotExpected(docID, revID string) { + if _, found := btc.GetRev(docID, revID); found { + btc.parent.rt.TB.Fatalf("BlipTesterClient found unexpected doc ID: %v rev ID: %v", docID, revID) + } + ticker := time.NewTicker(50 * time.Millisecond) + timeout := time.After(10 * time.Second) + for { + select { + case <-timeout: + return + case <-ticker.C: + if _, found := btc.GetRev(docID, revID); found { + btc.parent.rt.TB.Fatalf("BlipTesterClient found unexpected doc ID: %v rev ID: %v", docID, revID) + } + } + } +} + // GetDoc returns a rev stored in the Client under the given docID. (if multiple revs are present, rev body returned is non-deterministic) func (btc *BlipTesterCollectionClient) GetDoc(docID string) (data []byte, found bool) { btc.docsLock.RLock() @@ -1150,6 +1175,11 @@ func (btc *BlipTesterClient) WaitForRev(docID string, revID string) ([]byte, boo return btc.SingleCollection().WaitForRev(docID, revID) } +// RequireRevNotExpected waits for 10s and fails is the given revID does show up. +func (btc *BlipTesterClient) RequireRevNotExpected(docID string, revID string) { + btc.SingleCollection().RequireRevNotExpected(docID, revID) +} + func (btc *BlipTesterClient) WaitForDoc(docID string) ([]byte, bool) { return btc.SingleCollection().WaitForDoc(docID) } From 0b364921cd78b1774bb4c2423026caec352fb831 Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Thu, 22 Aug 2024 19:54:30 -0400 Subject: [PATCH 2/2] add license and lint fix --- rest/blip_channel_filter_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/rest/blip_channel_filter_test.go b/rest/blip_channel_filter_test.go index 5409f8e079..e435d5b657 100644 --- a/rest/blip_channel_filter_test.go +++ b/rest/blip_channel_filter_test.go @@ -1,3 +1,11 @@ +// Copyright 2024-Present Couchbase, Inc. +// +// Use of this software is governed by the Business Source License included +// in the file licenses/BSL-Couchbase.txt. As of the Change Date specified +// in that file, in accordance with the Business Source License, use of this +// software will be governed by the Apache License, Version 2.0, included in +// the file licenses/APL2.txt. + package rest import ( @@ -40,7 +48,7 @@ func TestChannelFilterRemovalFromChannel(t *testing.T) { const docID = "doc1" revID1 := rt.PutDoc("doc1", `{"channels":["A"]}`).Rev - rt.WaitForPendingChanges() + require.NoError(t, rt.WaitForPendingChanges()) response := rt.SendUserRequest("GET", "/{{.keyspace}}/_changes?since=0&channels=A&include_docs=true", "", "alice") RequireStatus(t, response, http.StatusOK) @@ -68,7 +76,7 @@ func TestChannelFilterRemovalFromChannel(t *testing.T) { // remove channel A from doc1 revID2 := rt.UpdateDoc(docID, revID1, `{"channels":["B"]}`).Rev - rt.WaitForPendingChanges() + require.NoError(t, rt.WaitForPendingChanges()) // alice will see doc1 rev2 with body response = rt.SendUserRequest("GET", "/{{.keyspace}}/_changes?since=2&channels=A&include_docs=true", "", "alice")