CBG-4687 have sgcollect_info use a token (#7604)

Co-authored-by: Copilot <[email protected]>

Author: torcolvin

base/devmode.go

@@ -13,7 +13,7 @@ import (
 )
 
 const (
-	assertionFailedPrefix = "Assertion failed: "
+	AssertionFailedPrefix = "Assertion failed: "
 )
 
 // IsDevMode returns true when compiled with the `cb_sg_devmode` build tag
@@ -26,5 +26,5 @@ func IsDevMode() bool {
 // Note: Callers MUST ensure code is safe to continue executing after the Assert (e.g. by returning an error) and MUST NOT be used like a panic that will halt.
 func AssertfCtx(ctx context.Context, format string, args ...any) {
 	SyncGatewayStats.GlobalStats.ResourceUtilization.AssertionFailCount.Add(1)
-	assertLogFn(ctx, assertionFailedPrefix+format, args...)
+	assertLogFn(ctx, AssertionFailedPrefix+format, args...)
 }

base/main_test_bucket_pool.go

@@ -675,7 +675,7 @@ func TestBucketPoolMain(ctx context.Context, m *testing.M, bucketReadierFunc TBP
 
 	teardownFuncs = append(teardownFuncs, func() {
 		if numAssertionFails := SyncGatewayStats.GlobalStats.ResourceUtilizationStats().AssertionFailCount.Value(); numAssertionFails > 0 {
-			panic(fmt.Sprintf("Test harness failed due to %d assertion failures. Search logs for %q", numAssertionFails, assertionFailedPrefix))
+			panic(fmt.Sprintf("Test harness failed due to %d assertion failures. Search logs for %q", numAssertionFails, AssertionFailedPrefix))
 		}
 	})
 	// must be the last teardown function added to the list to correctly detect leaked goroutines

rest/admin_api.go

@@ -1673,9 +1673,6 @@ func (h *handler) handleSGCollect() error {
 		return base.HTTPErrorf(http.StatusBadRequest, "Invalid options used for sgcollect_info: %v", multiError)
 	}
 
-	// Populate username and password used by sgcollect_info script for talking to Sync Gateway.
-	params.syncGatewayUsername, params.syncGatewayPassword = h.getBasicAuth()
-
 	addr, err := h.server.getServerAddr(adminServer)
 	if err != nil {
 		return base.HTTPErrorf(http.StatusInternalServerError, "Error getting admin server address: %v", err)

rest/handler.go

@@ -36,7 +36,8 @@ import (
 )
 
 const (
-	minCompressibleJSONSize = 1000
+	minCompressibleJSONSize      = 1000
+	sgcollectTokenInvalidRequest = "sgcollect token auth present in non-sgcollect request"
 )
 
 var _ http.Flusher = &CountedResponseWriter{}
@@ -111,6 +112,7 @@ type handler struct {
 	httpLogLevel          *base.LogLevel // if set, always log HTTP information at this level, instead of the default
 	rqCtx                 context.Context
 	serverType            serverType
+	sgcollect             bool // is called by sgcollect
 }
 
 type authScopeFunc func(context.Context, *handler) (string, error)
@@ -183,6 +185,7 @@ type handlerOptions struct {
 	skipLogDuration   bool           // if true, will skip logging HTTP response status/duration
 	authScopeFunc     authScopeFunc  // if set, this callback function will be used to set the auth scope for a given request body
 	httpLogLevel      *base.LogLevel // if set, log HTTP requests to this handler at this level, instead of the usual info level
+	sgcollect         bool           // if true, this handler is being invoked as part of sgcollect
 }
 
 func newHandler(server *ServerContext, privs handlerPrivs, serverType serverType, r http.ResponseWriter, rq *http.Request, options handlerOptions) *handler {
@@ -199,6 +202,7 @@ func newHandler(server *ServerContext, privs handlerPrivs, serverType serverType
 		authScopeFunc:     options.authScopeFunc,
 		httpLogLevel:      options.httpLogLevel,
 		serverType:        serverType,
+		sgcollect:         options.sgcollect,
 	}
 
 	// initialize h.rqCtx
@@ -289,6 +293,23 @@ func (h *handler) invoke(method handlerMethod, accessPermissions []Permission, r
 	return method(h) // Call the actual handler code
 }
 
+// shouldCheckAdminRBAC returns true if the request needs to check the server for permissions to run
+func (h *handler) shouldCheckAdminRBAC() bool {
+	sgcollectToken := h.server.SGCollect.getToken(h.rq.Header)
+	if sgcollectToken != "" && h.sgcollect && h.server.SGCollect.hasValidToken(h.ctx(), sgcollectToken) {
+		return false
+	}
+	if h.privs == adminPrivs && *h.server.Config.API.AdminInterfaceAuthentication {
+		if sgcollectToken != "" && !h.sgcollect {
+			base.AssertfCtx(h.ctx(), sgcollectTokenInvalidRequest)
+		}
+		return true
+	} else if h.privs == metricsPrivs && *h.server.Config.API.MetricsInterfaceAuthentication {
+		return true
+	}
+	return false
+}
+
 // validateAndWriteHeaders sets up handler.db and validates the permission of the user and returns an error if there is not permission.
 func (h *handler) validateAndWriteHeaders(method handlerMethod, accessPermissions []Permission, responsePermissions []Permission) (err error) {
 	var isRequestLogged bool
@@ -380,7 +401,7 @@ func (h *handler) validateAndWriteHeaders(method handlerMethod, accessPermission
 
 	// If an Admin Request and admin auth enabled or a metrics request with metrics auth enabled we need to check the
 	// user credentials
-	shouldCheckAdminAuth := (h.privs == adminPrivs && *h.server.Config.API.AdminInterfaceAuthentication) || (h.privs == metricsPrivs && *h.server.Config.API.MetricsInterfaceAuthentication)
+	shouldCheckAdminAuth := h.shouldCheckAdminRBAC()
 
 	// If admin/metrics endpoint but auth not enabled, set admin_noauth log ctx
 	if !shouldCheckAdminAuth && (h.serverType == adminServer || h.serverType == metricsServer) {

rest/handler_test.go

@@ -13,10 +13,12 @@ package rest
 import (
 	"fmt"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/couchbase/sync_gateway/base"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestParseHTTPRangeHeader(t *testing.T) {
@@ -151,3 +153,72 @@ func Benchmark_parseKeyspace(b *testing.B) {
 		_, _, _, _ = ParseKeyspace("d.s.c")
 	}
 }
+
+func TestShouldCheckAdminRBAC(t *testing.T) {
+	for _, requireInterfaceAuth := range []bool{false, true} {
+		t.Run(fmt.Sprintf("requireInterfaceAuth=%t", requireInterfaceAuth), func(t *testing.T) {
+			config := BootstrapStartupConfigForTest(t)
+			config.API.AdminInterfaceAuthentication = base.Ptr(requireInterfaceAuth)
+			config.API.MetricsInterfaceAuthentication = base.Ptr(requireInterfaceAuth)
+			sc, closeFn := StartServerWithConfig(t, &config)
+			defer closeFn()
+
+			for _, sgcollectable := range []bool{true, false} {
+				t.Run(fmt.Sprintf("sgcollectable=%t", sgcollectable), func(t *testing.T) {
+					// make sure assertion counts are correct
+					require.Equal(t, int64(0), base.SyncGatewayStats.GlobalStats.ResourceUtilizationStats().AssertionFailCount.Value())
+					defer func() {
+						base.SyncGatewayStats.GlobalStats.ResourceUtilizationStats().AssertionFailCount.Set(0)
+					}()
+					adminHandler := newHandler(sc, adminPrivs, adminServer, httptest.NewRecorder(), &http.Request{}, handlerOptions{sgcollect: sgcollectable})
+					metricsHandler := newHandler(sc, metricsPrivs, metricsServer, httptest.NewRecorder(), &http.Request{}, handlerOptions{sgcollect: sgcollectable})
+					if requireInterfaceAuth {
+						require.True(t, adminHandler.shouldCheckAdminRBAC())
+						require.True(t, metricsHandler.shouldCheckAdminRBAC())
+					} else {
+						require.False(t, adminHandler.shouldCheckAdminRBAC())
+
+						require.False(t, metricsHandler.shouldCheckAdminRBAC())
+					}
+					invalidAuthHeader := http.Header{}
+					invalidAuthHeader.Add("Authorization", "SGCollect invalid")
+					// with invalid sgcollect token
+					adminHandler = newHandler(sc, adminPrivs, adminServer, httptest.NewRecorder(), &http.Request{Header: invalidAuthHeader}, handlerOptions{sgcollect: sgcollectable})
+					metricsHandler = newHandler(sc, metricsPrivs, metricsServer, httptest.NewRecorder(), &http.Request{Header: invalidAuthHeader}, handlerOptions{sgcollect: sgcollectable})
+					if requireInterfaceAuth {
+						if base.IsDevMode() && !sgcollectable {
+							require.PanicsWithValue(t, base.AssertionFailedPrefix+sgcollectTokenInvalidRequest, func() { adminHandler.shouldCheckAdminRBAC() })
+						} else {
+							require.True(t, adminHandler.shouldCheckAdminRBAC(), "expected invalid token to still require auth")
+						}
+						require.True(t, metricsHandler.shouldCheckAdminRBAC(), "expected invalid token to still require auth")
+					} else {
+						require.False(t, adminHandler.shouldCheckAdminRBAC())
+						require.False(t, metricsHandler.shouldCheckAdminRBAC())
+
+					}
+					// with valid sgcollect token, but sgcollect on the handler is disabled
+					require.NoError(t, sc.SGCollect.createNewToken())
+
+					validAuthHeader := http.Header{}
+					validAuthHeader.Add("Authorization", fmt.Sprintf("SGCollect %s", sc.SGCollect.Token))
+					adminHandler = newHandler(sc, adminPrivs, adminServer, httptest.NewRecorder(), &http.Request{Header: validAuthHeader}, handlerOptions{sgcollect: false})
+					metricsHandler = newHandler(sc, metricsPrivs, metricsServer, httptest.NewRecorder(), &http.Request{Header: validAuthHeader}, handlerOptions{sgcollect: false})
+					if requireInterfaceAuth {
+						if base.IsDevMode() {
+							require.PanicsWithValue(t, base.AssertionFailedPrefix+sgcollectTokenInvalidRequest, func() { adminHandler.shouldCheckAdminRBAC() })
+						} else {
+							require.True(t, adminHandler.shouldCheckAdminRBAC(), "expected invalid token to still require auth")
+						}
+						require.True(t, metricsHandler.shouldCheckAdminRBAC(), "expected invalid token to still require auth")
+					} else {
+						require.False(t, adminHandler.shouldCheckAdminRBAC())
+
+						require.False(t, metricsHandler.shouldCheckAdminRBAC())
+					}
+
+				})
+			}
+		})
+	}
+}

rest/routing.go

@@ -15,6 +15,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/couchbase/sync_gateway/base"
 	"github.com/gorilla/mux"
 )
 
@@ -40,7 +41,7 @@ func createCommonRouter(sc *ServerContext, privs handlerPrivs, serverType server
 	root = NewRouter(sc, serverType)
 
 	// Global operations:
-	root.Handle("/", makeHandler(sc, privs, nil, nil, (*handler).handleRoot)).Methods("GET", "HEAD")
+	root.Handle("/", makeHandlerWithOptions(sc, privs, nil, nil, (*handler).handleRoot, handlerOptions{sgcollect: true})).Methods("GET", "HEAD")
 
 	// Operations on databases:
 	root.Handle("/{db:"+dbRegex+"}/", makeOfflineHandler(sc, privs, []Permission{PermDevOps, PermGetDb}, nil, (*handler).handleGetDB)).Methods("GET", "HEAD")
@@ -216,7 +217,7 @@ func CreateAdminRouter(sc *ServerContext) *mux.Router {
 	dbr.Handle("/_replicationStatus/{replicationID}",
 		makeHandler(sc, adminPrivs, []Permission{PermWriteReplications}, nil, (*handler).putReplicationStatus)).Methods("PUT")
 	dbr.Handle("/_config",
-		makeOfflineHandler(sc, adminPrivs, []Permission{PermUpdateDb}, nil, (*handler).handleGetDbConfig)).Methods("GET")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermUpdateDb}, nil, (*handler).handleGetDbConfig, handlerOptions{runOffline: true, sgcollect: true})).Methods("GET")
 	dbr.Handle("/_config",
 		makeOfflineHandler(sc, adminPrivs, []Permission{PermUpdateDb, PermConfigureSyncFn, PermConfigureAuth}, []Permission{PermUpdateDb, PermConfigureSyncFn, PermConfigureAuth}, (*handler).handlePutDbConfig)).Methods("PUT", "POST")
 
@@ -260,15 +261,15 @@ func CreateAdminRouter(sc *ServerContext) *mux.Router {
 		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleSetLogging)).Methods("PUT", "POST")
 
 	r.Handle("/_config",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleGetConfig)).Methods("GET")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleGetConfig, handlerOptions{sgcollect: true})).Methods("GET")
 	r.Handle("/_config",
 		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePutConfig)).Methods("PUT")
 
 	r.Handle("/_cluster_info",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleGetClusterInfo)).Methods("GET")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleGetClusterInfo, handlerOptions{sgcollect: true})).Methods("GET")
 
 	r.Handle("/_status",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleGetStatus)).Methods("GET")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleGetStatus, handlerOptions{sgcollect: true})).Methods("GET")
 
 	r.Handle("/_sgcollect_info",
 		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleSGCollectStatus)).Methods("GET")
@@ -281,33 +282,36 @@ func CreateAdminRouter(sc *ServerContext) *mux.Router {
 	r.Handle("/_stats",
 		makeHandler(sc, adminPrivs, []Permission{PermStatsExport}, nil, (*handler).handleStats)).Methods("GET")
 	r.Handle(kDebugURLPathPrefix,
-		makeSilentHandler(sc, adminPrivs, []Permission{PermStatsExport}, nil, (*handler).handleExpvar)).Methods("GET")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermStatsExport}, nil, (*handler).handleExpvar, handlerOptions{
+			httpLogLevel: base.Ptr(base.LevelDebug), // silent handler
+			sgcollect:    true,
+		})).Methods("GET")
 	r.Handle("/_profile/{profilename}",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleProfiling)).Methods("POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleProfiling, handlerOptions{sgcollect: true})).Methods("POST")
 	r.Handle("/_profile",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleProfiling)).Methods("POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleProfiling, handlerOptions{sgcollect: true})).Methods("POST")
 	r.Handle("/_heap",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleHeapProfiling)).Methods("POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleHeapProfiling, handlerOptions{sgcollect: true})).Methods("POST")
 	r.Handle("/_debug/pprof/goroutine",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofGoroutine)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofGoroutine, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 	r.Handle("/_debug/pprof/cmdline",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofCmdline)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofCmdline, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 	r.Handle("/_debug/pprof/symbol",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofSymbol)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofSymbol, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 	r.Handle("/_debug/pprof/heap",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofHeap)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofHeap, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 	r.Handle("/_debug/pprof/profile",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofProfile)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofProfile, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 	r.Handle("/_debug/pprof/block",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofBlock)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofBlock, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 	r.Handle("/_debug/pprof/threadcreate",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofThreadcreate)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofThreadcreate, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 	r.Handle("/_debug/pprof/mutex",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofMutex)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofMutex, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 	r.Handle("/_debug/pprof/trace",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofTrace)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePprofTrace, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 	r.Handle("/_debug/fgprof",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleFgprof)).Methods("GET", "POST")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleFgprof, handlerOptions{sgcollect: true})).Methods("GET", "POST")
 
 	r.Handle("/_post_upgrade",
 		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handlePostUpgrade)).Methods("POST")
@@ -332,7 +336,7 @@ func CreateAdminRouter(sc *ServerContext) *mux.Router {
 		makeMetadataDBOfflineHandler(sc, adminPrivs, []Permission{PermDeleteDb}, nil, (*handler).handleDeleteDB)).Methods("DELETE")
 
 	r.Handle("/_all_dbs",
-		makeHandler(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleAllDbs)).Methods("GET", "HEAD")
+		makeHandlerWithOptions(sc, adminPrivs, []Permission{PermDevOps}, nil, (*handler).handleAllDbs, handlerOptions{sgcollect: true})).Methods("GET", "HEAD")
 
 	return r
 }