Address PR feedback about child namespaces

command/events.go

@@ -35,8 +35,9 @@ func (c *EventsSubscribeCommands) Help() string {
 	helpText := `
 Usage: vault events subscribe [-namespaces=ns1] [-timeout=XYZs] eventType
 
-  Subscribe to events of the given event type (topic). The events will be
-  output to standard out.
+  Subscribe to events of the given event type (topic), which may be a glob
+  pattern (with "*"" treated as a wildcard). The events will be sent to
+  standard out.
 
   The output will be a JSON object serialized using the default protobuf
   JSON serialization format, with one line per event received.
@@ -49,9 +50,10 @@ func (c *EventsSubscribeCommands) Flags() *FlagSets {
 	f := set.NewFlagSet("Subscribe Options")
 	f.StringSliceVar(&StringSliceVar{
 		Name: "namespaces",
-		Usage: `Specifies one or more patterns of child namespaces to subscribe
-                to. Patterns can include "*" characters to indicate wildcards.
-				The default is to subscribe only to the request's namespace.`,
+		Usage: `Specifies one or more patterns of additional child namespaces
+				to subscribe to. Patterns can include "*" characters to indicate
+                wildcards. The default is to subscribe only to the request's
+                namespace.`,
 		Default: []string{},
 		Target:  &c.namespaces,
 	})

http/events.go

@@ -18,6 +18,7 @@ import (
 	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/hashicorp/vault/vault"
 	"github.com/hashicorp/vault/vault/eventbus"
+	"github.com/ryanuber/go-glob"
 	"nhooyr.io/websocket"
 )
 
@@ -124,6 +125,12 @@ func handleEventsSubscribe(core *vault.Core, req *logical.Request) http.Handler
 		}
 
 		namespacePatterns := r.URL.Query()["namespaces"]
+		namespacePatterns, err = validateNamespacePatterns(namespacePatterns, ns)
+		if err != nil {
+			logger.Info("Error validating namespace subscription request", "error", err)
+			respondError(w, http.StatusBadRequest, err)
+			return
+		}
 		conn, err := websocket.Accept(w, r, nil)
 		if err != nil {
 			logger.Info("Could not accept as websocket", "error", err)
@@ -164,3 +171,45 @@ func handleEventsSubscribe(core *vault.Core, req *logical.Request) http.Handler
 		}
 	})
 }
+
+// validateNamespacePatterns validates that the namespace subscribe patterns only match
+// child namespaces of the request namespace. It also checks if the request namespace
+// is present, and if it is not, it adds it. It returns a (possibly modified) copy of
+// the patterns.
+func validateNamespacePatterns(patterns []string, requestNamespace *namespace.Namespace) ([]string, error) {
+	rns := strings.TrimSuffix(requestNamespace.Path, "/")
+	for _, pattern := range patterns {
+		if !isChildPattern(pattern, rns) {
+			return nil, fmt.Errorf("namespace pattern %v must match only child namespaces of %s", pattern, rns)
+		}
+	}
+
+	// check that the request namespace is included in at least one of the patterns
+	present := false
+	for _, pattern := range patterns {
+		if glob.Glob(pattern, rns) {
+			present = true
+			break
+		}
+	}
+	if present {
+		return patterns, nil
+	}
+	newPatterns := append(patterns, rns)
+	return newPatterns, nil
+}
+
+func isChildPattern(pattern string, ns string) bool {
+	// everything is a child of the root namespace
+	if ns == "" {
+		return true
+	}
+	// if they are exact matches and the pattern doesn't have a glob, then it only matches the namespace
+	if ns == pattern && !strings.Contains(pattern, "*") {
+		return true
+	}
+	if strings.HasPrefix(pattern, ns+"/") {
+		return true
+	}
+	return false
+}

http/events_test.go

@@ -161,7 +161,8 @@ func TestEventsSubscribeNamespaces(t *testing.T) {
 		"",
 		"ns1",
 		"ns2",
-		"other",
+		"ns1/ns13",
+		"ns1/ns13/ns134",
 	}
 
 	// send some events with the specified namespaces
@@ -208,13 +209,13 @@ func TestEventsSubscribeNamespaces(t *testing.T) {
 		namespaces     []string
 		expectedEvents int
 	}{
-		{"invalid", []string{"something"}, 0},
-		{"simple wildcard", []string{"ns*"}, 2},
-		{"two namespaces", []string{"ns1", "other"}, 2},
+		{"invalid", []string{"something"}, 1},
+		{"simple wildcard", []string{"ns*"}, 5},
+		{"two namespaces", []string{"ns1/ns13", "ns1/other"}, 2},
 		{"no namespace", []string{""}, 1},
-		{"all wildcard", []string{"*"}, 4},
-		{"mixed wildcard", []string{"ns*", "other"}, 3},
-		{"overlapping wildcard", []string{"ns*", "ns1"}, 2},
+		{"all wildcard", []string{"*"}, 5},
+		{"mixed wildcard", []string{"ns1/ns13*", "ns2"}, 4},
+		{"overlapping wildcard", []string{"ns*", "ns1"}, 5},
 	}
 
 	for _, testCase := range testCases {
@@ -255,6 +256,46 @@ func TestEventsSubscribeNamespaces(t *testing.T) {
 	}
 }
 
+func TestNamespaceValidation(t *testing.T) {
+	testCases := []struct {
+		requestNs string
+		patterns  []string
+		result    []string
+		err       bool
+	}{
+		{"", []string{"ns*"}, []string{"ns*", ""}, false},
+		{"ns1", []string{"ns*"}, nil, true},
+		{"ns1", []string{"ns1*"}, nil, true},
+		{"ns1", []string{"ns1/*"}, []string{"ns1/*", "ns1"}, false},
+		{"", []string{"ns1/ns13", "ns1/other"}, []string{"ns1/ns13", "ns1/other", ""}, false},
+		{"ns1", []string{"ns1/ns13", "ns1/other"}, []string{"ns1/ns13", "ns1/other", "ns1"}, false},
+		{"", []string{""}, []string{""}, false},
+		{"ns1", []string{""}, nil, true},
+		{"ns1", []string{"ns1"}, []string{"ns1"}, false},
+		{"", []string{"*"}, []string{"*"}, false},
+		{"ns1", []string{"*"}, nil, true},
+		{"", []string{"ns1/ns13*", "ns2"}, []string{"ns1/ns13*", "ns2", ""}, false},
+		{"ns1", []string{"ns1/ns13*", "ns2"}, nil, true},
+		{"", []string{"ns*", "ns1"}, []string{"ns*", "ns1", ""}, false},
+		{"ns1", []string{"ns*", "ns1"}, nil, true},
+		{"ns1", []string{"ns1*", "ns1"}, nil, true},
+		{"ns1", []string{"ns1/*", "ns1"}, []string{"ns1/*", "ns1"}, false},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.requestNs+" "+strings.Join(testCase.patterns, " "), func(t *testing.T) {
+			result, err := validateNamespacePatterns(testCase.patterns, &namespace.Namespace{ID: testCase.requestNs, Path: testCase.requestNs})
+			if err != nil {
+				if testCase.err {
+					return
+				} else {
+					t.Fatalf("Expected no error but got %v", err)
+				}
+			}
+			assert.Equal(t, testCase.result, result)
+		})
+	}
+}
+
 func checkRequiredCloudEventsFields(t *testing.T, event map[string]interface{}) {
 	t.Helper()
 	for _, attr := range []string{"id", "source", "specversion", "type"} {