fix(search): fill a repeated root placeholder everywhere, reject a repeated or grouped scope

services/search/pkg/search/scope.go

@@ -8,18 +8,32 @@ import (
 	"github.com/opencloud-eu/opencloud/pkg/kql"
 )
 
-// ErrNestedScope is returned when the scope sits inside a group: it narrows
-// the whole search to a folder and cannot take part in a boolean expression.
-var ErrNestedScope = errors.New("scope: must be a top-level restriction")
+// A scope narrows the whole search to one folder, so it cannot take part in a
+// boolean expression and cannot be given twice.
+var (
+	ErrNestedScope   = errors.New("scope: must be a top-level restriction")
+	ErrRepeatedScope = errors.New("scope: must be given once")
+)
 
 // ExtractScope takes the scope restriction out of the query, together with
 // the operator that joined it, and returns the remaining nodes and the scope.
 func ExtractScope(nodes []ast.Node) ([]ast.Node, string, error) {
+	scopes := 0
 	for _, n := range nodes {
-		if group, ok := n.(*ast.GroupNode); ok && hasKey(group.Nodes, "scope") {
-			return nil, "", ErrNestedScope
+		switch node := n.(type) {
+		case *ast.GroupNode:
+			if strings.EqualFold(node.Key, "scope") || hasKey(node.Nodes, "scope") {
+				return nil, "", ErrNestedScope
+			}
+		case *ast.StringNode:
+			if strings.EqualFold(node.Key, "scope") {
+				scopes++
+			}
 		}
 	}
+	if scopes > 1 {
+		return nil, "", ErrRepeatedScope
+	}
 
 	for i, n := range nodes {
 		node, ok := n.(*ast.StringNode)
@@ -67,7 +81,7 @@ func hasKey(nodes []ast.Node, key string) bool {
 				return true
 			}
 		case *ast.GroupNode:
-			if hasKey(node.Nodes, key) {
+			if strings.EqualFold(node.Key, key) || hasKey(node.Nodes, key) {
 				return true
 			}
 		}

services/search/pkg/search/scope_test.go

@@ -38,8 +38,17 @@ var _ = DescribeTable("ExtractScope rejects a nested scope",
 	Entry("inside a group", `name:x AND (scope:1$2!3 OR tag:y)`),
 	Entry("inside a nested group", `name:x AND (tag:y OR (scope:1$2!3))`),
 	Entry("after a top-level scope", `scope:1$2!3 AND (scope:1$2!4)`),
+	Entry("as a group value", `name:x AND scope:(1$2!3)`),
+	Entry("as a group value inside a group", `name:x AND (tag:y OR scope:(1$2!3))`),
 )
 
+var _ = Describe("ExtractScope", func() {
+	It("rejects a second scope", func() {
+		_, _, err := search.ExtractScope(parse(`scope:1$2!3 AND name:x AND scope:1$2!4`).Nodes)
+		Expect(err).To(MatchError(search.ErrRepeatedScope))
+	})
+})
+
 var _ = DescribeTable("ExpandFavorites",
 	func(query, want string) {
 		a := parse(query)

services/search/pkg/search/template.go

@@ -17,12 +17,10 @@ import (
 // nodes where the user's query and the space roots go. An empty template
 // expands to the bare query.
 type QueryTemplate struct {
-	nodes    []ast.Node
-	query    *ast.GroupNode
-	queries  int
-	stray    bool
-	personal *ast.StringNode
-	project  *ast.StringNode
+	nodes   []ast.Node
+	query   *ast.GroupNode
+	queries int
+	stray   bool
 }
 
 const (
@@ -75,13 +73,8 @@ func (t *QueryTemplate) findMarkers(nodes []ast.Node) {
 			}
 			t.findMarkers(node.Nodes)
 		case *ast.StringNode:
-			switch node.Value {
-			case queryMarker:
+			if node.Value == queryMarker {
 				t.stray = true
-			case personalRootMarker:
-				t.personal = node
-			case projectRootMarker:
-				t.project = node
 			}
 		}
 	}
@@ -102,20 +95,25 @@ func (t *QueryTemplate) Expand(query *ast.Ast, space *provider.StorageSpace) *as
 		return config.NoResourceID
 	}
 
+	// the user's nodes are copied without replace, so a marker in the user's
+	// query stays what it is
 	return &ast.Ast{Nodes: cloneNodes(t.nodes, func(n ast.Node) ast.Node {
-		switch n {
-		case t.query:
+		if n == ast.Node(t.query) {
 			c := *t.query
 			c.Nodes = cloneNodes(query.Nodes, nil)
 			return &c
-		case t.personal:
-			c := *t.personal
-			c.Value = rootID(_spaceTypePersonal)
-			return &c
-		case t.project:
-			c := *t.project
-			c.Value = rootID(_spaceTypeProject)
-			return &c
+		}
+		if s, ok := n.(*ast.StringNode); ok {
+			switch s.Value {
+			case personalRootMarker:
+				c := *s
+				c.Value = rootID(_spaceTypePersonal)
+				return &c
+			case projectRootMarker:
+				c := *s
+				c.Value = rootID(_spaceTypeProject)
+				return &c
+			}
 		}
 		return nil
 	})}

services/search/pkg/search/template_test.go

@@ -95,6 +95,23 @@ var _ = Describe("QueryTemplate", func() {
 		Expect(render(expanded.Nodes)).To(Equal(render(parse(`(NOT mediatype:folder AND (name:x OR name:y)) OR tag:keep`).Nodes)))
 	})
 
+	It("fills a root placeholder that is used twice", func() {
+		t, err := search.ParseQueryTemplate(`({{.QUERY}}) AND NOT id:{{.SPACE_ROOT_PERSONAL}} AND NOT parentid:{{.SPACE_ROOT_PERSONAL}}`)
+		Expect(err).ToNot(HaveOccurred())
+		space := &sprovider.StorageSpace{SpaceType: "personal", Root: &sprovider.ResourceId{StorageId: "1", SpaceId: "2", OpaqueId: "2"}}
+
+		expanded := t.Expand(parse(`name:x`), space)
+		Expect(render(expanded.Nodes)).To(Equal(render(parse(`(name:x) AND NOT id:"1$2!2" AND NOT parentid:"1$2!2"`).Nodes)))
+	})
+
+	It("leaves a marker in the user's query alone", func() {
+		t, err := search.ParseQueryTemplate(`({{.QUERY}}) AND NOT id:{{.SPACE_ROOT_PERSONAL}}`)
+		Expect(err).ToNot(HaveOccurred())
+
+		expanded := t.Expand(parse(`name:opencloudpersonalrootmarker`), &sprovider.StorageSpace{})
+		Expect(render(expanded.Nodes)).To(Equal(render(parse(`(name:opencloudpersonalrootmarker) AND NOT id:"none$none!none"`).Nodes)))
+	})
+
 	It("keeps the key of the query group", func() {
 		t, err := search.ParseQueryTemplate(`name:({{.QUERY}})`)
 		Expect(err).ToNot(HaveOccurred())