CBG-3791: change getAuthScopeHandleCreateDB method to not expand envi…

…ronment variables (#6703)

* CBG-3791: change getAuthScopeHandleCreateDB method to not expand environment variables

* updates after review

* updates off review

* fix test for default collection

rest/admin_api.go

@@ -169,9 +169,9 @@ func (h *handler) handleCreateDB() error {
 func getAuthScopeHandleCreateDB(ctx context.Context, h *handler) (bucketName string, err error) {
 
 	// grab a copy of the request body and restore body buffer for later handlers
-	bodyJSON, err := h.readBody()
-	if err != nil {
-		return "", base.HTTPErrorf(http.StatusInternalServerError, "Unable to read body: %v", err)
+	bodyJSON, readErr := h.readBody()
+	if readErr != nil {
+		return "", base.HTTPErrorf(http.StatusInternalServerError, "Unable to read body: %v", readErr)
 	}
 	// mark the body as already read to avoid double-counting bytes for stats once it gets read again
 	h.requestBody.bodyRead = true
@@ -180,8 +180,14 @@ func getAuthScopeHandleCreateDB(ctx context.Context, h *handler) (bucketName str
 	var dbConfigBody struct {
 		Bucket string `json:"bucket"`
 	}
-	reader := bytes.NewReader(bodyJSON)
-	err = DecodeAndSanitiseConfig(ctx, reader, &dbConfigBody, false)
+
+	bodyJSON, err = sanitiseConfig(ctx, bodyJSON, h.server.Config.Unsupported.AllowDbConfigEnvVars)
+	if err != nil {
+		return "", err
+	}
+
+	d := base.JSONDecoder(bytes.NewBuffer(bodyJSON))
+	err = d.Decode(&dbConfigBody)
 	if err != nil {
 		return "", err
 	}

rest/adminapitest/admin_api_test.go

@@ -4069,3 +4069,83 @@ func TestDatabaseCreationErrorCode(t *testing.T) {
 		rest.RequireStatus(t, resp, http.StatusPreconditionFailed)
 	}
 }
+
+// TestDatabaseCreationWithEnvVariable:
+//   - Create rest tester that enables admin auth and disallows db config env vars
+//   - Create CBS user to authenticate with over admin port to force auth scope callback call
+//   - Create db with sync function that calls env variable
+//   - Assert that db is created
+func TestDatabaseCreationWithEnvVariable(t *testing.T) {
+	if base.UnitTestUrlIsWalrus() {
+		t.Skip("This test only works against Couchbase Server")
+	}
+
+	tb := base.GetTestBucket(t)
+	ctx := base.TestCtx(t)
+	defer tb.Close(ctx)
+
+	// disable AllowDbConfigEnvVars to avoid attempting to expand variables + enable admin auth
+	rt := rest.NewRestTester(t, &rest.RestTesterConfig{
+		PersistentConfig: true,
+		MutateStartupConfig: func(config *rest.StartupConfig) {
+			config.Unsupported.AllowDbConfigEnvVars = base.BoolPtr(false)
+		},
+		AdminInterfaceAuthentication: true,
+		SyncFn:                       `function (doc) { console.log("${environment}"); return true }`,
+		CustomTestBucket:             tb,
+	})
+	defer rt.Close()
+
+	// create a role to authenticate with in admin endpoint
+	eps, httpClient, err := rt.ServerContext().ObtainManagementEndpointsAndHTTPClient()
+	require.NoError(t, err)
+	rest.MakeUser(t, httpClient, eps[0], rest.MobileSyncGatewayRole.RoleName, "password", []string{fmt.Sprintf("%s[%s]", rest.MobileSyncGatewayRole.RoleName, tb.GetName())})
+	defer rest.DeleteUser(t, httpClient, eps[0], rest.MobileSyncGatewayRole.RoleName)
+
+	cfg := rt.NewDbConfig()
+	input, err := base.JSONMarshal(&cfg)
+	require.NoError(t, err)
+
+	// create db with config and assert it is successful
+	resp := rt.SendAdminRequestWithAuth(http.MethodPut, "/db/", string(input), rest.MobileSyncGatewayRole.RoleName, "password")
+	rest.RequireStatus(t, resp, http.StatusCreated)
+}
+
+func TestDatabaseCreationWithEnvVariableWithBackticks(t *testing.T) {
+	if base.UnitTestUrlIsWalrus() {
+		t.Skip("This test only works against Couchbase Server")
+	}
+
+	tb := base.GetTestBucket(t)
+	ctx := base.TestCtx(t)
+	defer tb.Close(ctx)
+
+	// disable AllowDbConfigEnvVars to avoid attempting to expand variables + enable admin auth
+	rt := rest.NewRestTester(t, &rest.RestTesterConfig{
+		PersistentConfig: true,
+		MutateStartupConfig: func(config *rest.StartupConfig) {
+			config.Unsupported.AllowDbConfigEnvVars = base.BoolPtr(false)
+		},
+		AdminInterfaceAuthentication: true,
+		SyncFn:                       `function (doc) { console.log("${environment}"); return true }`,
+		CustomTestBucket:             tb,
+	})
+	defer rt.Close()
+
+	// create a role to authenticate with in admin endpoint
+	eps, httpClient, err := rt.ServerContext().ObtainManagementEndpointsAndHTTPClient()
+	require.NoError(t, err)
+	rest.MakeUser(t, httpClient, eps[0], rest.MobileSyncGatewayRole.RoleName, "password", []string{fmt.Sprintf("%s[%s]", rest.MobileSyncGatewayRole.RoleName, tb.GetName())})
+	defer rest.DeleteUser(t, httpClient, eps[0], rest.MobileSyncGatewayRole.RoleName)
+
+	cfg := rt.NewDbConfig()
+	input, err := base.JSONMarshal(&cfg)
+	require.NoError(t, err)
+
+	// change config to include backticks
+	cfg.Bucket = base.StringPtr(fmt.Sprintf("`"+"%s"+"`", tb.GetName()))
+
+	// create db with config and assert it is successful
+	resp := rt.SendAdminRequestWithAuth(http.MethodPut, "/backticks/", string(input), rest.MobileSyncGatewayRole.RoleName, "password")
+	rest.RequireStatus(t, resp, http.StatusCreated)
+}

rest/api_test.go

@@ -1437,7 +1437,7 @@ func TestEventConfigValidationInvalid(t *testing.T) {
 
 	buf := bytes.NewBufferString(dbConfigJSON)
 	var dbConfig DbConfig
-	err := DecodeAndSanitiseConfig(base.TestCtx(t), buf, &dbConfig, true)
+	err := DecodeAndSanitiseStartupConfig(base.TestCtx(t), buf, &dbConfig, true)
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "document_scribbled_on")
 }

rest/config.go

@@ -1134,19 +1134,18 @@ func (config *DbConfig) redactInPlace(ctx context.Context) error {
 	return nil
 }
 
-// DecodeAndSanitiseConfig will sanitise a config from an io.Reader and unmarshal it into the given config parameter.
-func DecodeAndSanitiseConfig(ctx context.Context, r io.Reader, config interface{}, disallowUnknownFields bool) (err error) {
+// DecodeAndSanitiseStartupConfig will sanitise a config from an io.Reader and unmarshal it into the given config parameter.
+func DecodeAndSanitiseStartupConfig(ctx context.Context, r io.Reader, config interface{}, disallowUnknownFields bool) (err error) {
 	b, err := io.ReadAll(r)
 	if err != nil {
 		return err
 	}
 
 	// Expand environment variables.
-	b, err = expandEnv(ctx, b)
+	b, err = sanitiseConfig(ctx, b, base.BoolPtr(true))
 	if err != nil {
 		return err
 	}
-	b = base.ConvertBackQuotedStrings(b)
 
 	d := base.JSONDecoder(bytes.NewBuffer(b))
 	if disallowUnknownFields {
@@ -1156,6 +1155,23 @@ func DecodeAndSanitiseConfig(ctx context.Context, r io.Reader, config interface{
 	return base.WrapJSONUnknownFieldErr(err)
 }
 
+// sanitiseConfig will expand environment variables if needed and will convert any back quotes in the config
+func sanitiseConfig(ctx context.Context, configBytes []byte, allowEnvVars *bool) ([]byte, error) {
+	var err error
+	// Expand environment variables if needed
+	if base.BoolDefault(allowEnvVars, true) {
+		configBytes, err = expandEnv(ctx, configBytes)
+		if err != nil {
+			return nil, err
+		}
+	}
+	// Convert the back quotes into double-quotes, escapes literal
+	// backslashes, newlines or double-quotes with backslashes.
+	configBytes = base.ConvertBackQuotedStrings(configBytes)
+
+	return configBytes, nil
+}
+
 // expandEnv replaces $var or ${var} in config according to the values of the
 // current environment variables. The replacement is case-sensitive. References
 // to undefined variables will result in an error. A default value can

rest/config_legacy.go

@@ -370,7 +370,7 @@ func LoadLegacyServerConfig(ctx context.Context, path string) (config *LegacySer
 
 // readLegacyServerConfig returns a validated LegacyServerConfig from an io.Reader
 func readLegacyServerConfig(ctx context.Context, r io.Reader) (config *LegacyServerConfig, err error) {
-	err = DecodeAndSanitiseConfig(ctx, r, &config, true)
+	err = DecodeAndSanitiseStartupConfig(ctx, r, &config, true)
 	if err != nil {
 		return config, err
 	}

rest/config_startup.go

@@ -217,7 +217,7 @@ func LoadStartupConfigFromPath(ctx context.Context, path string) (*StartupConfig
 	defer func() { _ = rc.Close() }()
 
 	var sc StartupConfig
-	err = DecodeAndSanitiseConfig(ctx, rc, &sc, true)
+	err = DecodeAndSanitiseStartupConfig(ctx, rc, &sc, true)
 	return &sc, err
 }
 

rest/handler.go

@@ -1090,18 +1090,11 @@ func (h *handler) readSanitizeJSON(val interface{}) error {
 		return err
 	}
 
-	// Expand environment variables.
-	if base.BoolDefault(h.server.Config.Unsupported.AllowDbConfigEnvVars, true) {
-		content, err = expandEnv(h.ctx(), content)
-		if err != nil {
-			return err
-		}
+	content, err = sanitiseConfig(h.ctx(), content, h.server.Config.Unsupported.AllowDbConfigEnvVars)
+	if err != nil {
+		return err
 	}
 
-	// Convert the back quotes into double-quotes, escapes literal
-	// backslashes, newlines or double-quotes with backslashes.
-	content = base.ConvertBackQuotedStrings(content)
-
 	// Decode the body bytes into target structure.
 	decoder := base.JSONDecoder(bytes.NewReader(content))
 	decoder.DisallowUnknownFields()