Merge pull request #139 from nats-io/validate-permissions

[fixed] missing validation for permissions

v2/types.go

@@ -325,13 +325,29 @@ func (p *Permission) Empty() bool {
 	return len(p.Allow) == 0 && len(p.Deny) == 0
 }
 
+func checkPermission(vr *ValidationResults, subj string, permitQueue bool) {
+	tk := strings.Split(subj, " ")
+	switch len(tk) {
+	case 1:
+		Subject(tk[0]).Validate(vr)
+	case 2:
+		Subject(tk[0]).Validate(vr)
+		Subject(tk[1]).Validate(vr)
+		if !permitQueue {
+			vr.AddError(`Permission Subject "%s" is not allowed to contain queue`, subj)
+		}
+	default:
+		vr.AddError(`Permission Subject "%s" contains too many spaces`, subj)
+	}
+}
+
 // Validate the allow, deny elements of a permission
-func (p *Permission) Validate(vr *ValidationResults) {
+func (p *Permission) Validate(vr *ValidationResults, permitQueue bool) {
 	for _, subj := range p.Allow {
-		Subject(subj).Validate(vr)
+		checkPermission(vr, subj, permitQueue)
 	}
 	for _, subj := range p.Deny {
-		Subject(subj).Validate(vr)
+		checkPermission(vr, subj, permitQueue)
 	}
 }
 
@@ -359,6 +375,8 @@ func (p *Permissions) Validate(vr *ValidationResults) {
 	if p.Resp != nil {
 		p.Resp.Validate(vr)
 	}
+	p.Sub.Validate(vr, true)
+	p.Pub.Validate(vr, false)
 }
 
 // StringList is a wrapper for an array of strings

v2/types_test.go

@@ -278,6 +278,69 @@ func TestSubjectContainment(t *testing.T) {
 	AssertEquals(false, s.IsContainedIn(o), t)
 }
 
+func TestPermissions_Validate(t *testing.T) {
+	p := Permissions{
+		Pub:  Permission{},
+		Sub:  Permission{},
+		Resp: nil,
+	}
+	vr := ValidationResults{}
+	resetAndValidate := func() {
+		vr = ValidationResults{}
+		p.Validate(&vr)
+	}
+	resetAndValidate()
+	AssertTrue(vr.IsEmpty(), t)
+
+	p.Resp = &ResponsePermission{
+		MaxMsgs: 0,
+		Expires: 0,
+	}
+	resetAndValidate()
+	AssertTrue(vr.IsEmpty(), t)
+
+	p.Pub.Allow.Add("foo")
+	p.Pub.Deny.Add("bar")
+	resetAndValidate()
+	AssertTrue(vr.IsEmpty(), t)
+
+	p.Pub.Allow.Add("foo queue")
+	p.Pub.Deny.Add("bar queue")
+	resetAndValidate()
+	AssertTrue(!vr.IsEmpty(), t)
+	AssertTrue(vr.IsBlocking(false), t)
+	AssertTrue(len(vr.Errors()) == 2, t)
+
+	p.Pub = Permission{}
+
+	p.Sub.Allow.Add("1")
+	p.Sub.Deny.Add("2")
+	resetAndValidate()
+	AssertTrue(vr.IsEmpty(), t)
+
+	p.Sub.Allow.Add("3 queue")
+	p.Sub.Deny.Add("4 queue")
+	resetAndValidate()
+	AssertTrue(vr.IsEmpty(), t)
+
+	p.Sub.Allow.Add("5.* queue.*.foo")
+	p.Sub.Deny.Add("6.* queue.*.bar")
+	resetAndValidate()
+	AssertTrue(vr.IsEmpty(), t)
+
+	p.Sub.Allow.Add("7.> queue.>")
+	p.Sub.Deny.Add("8.> queue.>")
+	resetAndValidate()
+	AssertTrue(vr.IsEmpty(), t)
+
+	p.Sub.Allow.Add("9 too many spaces")
+	p.Sub.Deny.Add("0 too many spaces")
+	resetAndValidate()
+	AssertTrue(!vr.IsEmpty(), t)
+	AssertTrue(vr.IsBlocking(false), t)
+	AssertTrue(len(vr.Errors()) == 2, t)
+}
+
 func TestRenamingSubject_ToSubject(t *testing.T) {
 	AssertEquals(RenamingSubject("foo.$2.$1.bar").ToSubject(), Subject("foo.*.*.bar"), t)
 	AssertEquals(RenamingSubject("foo.*.bar").ToSubject(), Subject("foo.*.bar"), t)
@@ -286,13 +349,13 @@ func TestRenamingSubject_ToSubject(t *testing.T) {
 
 func TestRenamigSubject_Validate(t *testing.T) {
 	for from, to := range map[string]string{
-		"foo":">",
-		"bar":"*",
-		"foo.*":"*.*",
-		"foo.>":"*.*",
-		"bar.>":"*.>",
-		"bar.*.*>":"*.>",
-		"*.bar":"$2",
+		"foo":      ">",
+		"bar":      "*",
+		"foo.*":    "*.*",
+		"foo.>":    "*.*",
+		"bar.>":    "*.>",
+		"bar.*.*>": "*.>",
+		"*.bar":    "$2",
 	} {
 		vr := ValidationResults{}
 		RenamingSubject(to).Validate(Subject(from), &vr)
@@ -301,13 +364,13 @@ func TestRenamigSubject_Validate(t *testing.T) {
 		}
 	}
 	for from, to := range map[string]string{
-		"foo":"bar",
-		"foo.bar":"baz",
-		"x":"x.y.z",
-		">":"foo.>",
-		"*":"$1.foo",
-		"*.*":"$1.foo.$2",
-		"*.bar":"$1",
+		"foo":     "bar",
+		"foo.bar": "baz",
+		"x":       "x.y.z",
+		">":       "foo.>",
+		"*":       "$1.foo",
+		"*.*":     "$1.foo.$2",
+		"*.bar":   "$1",
 	} {
 		vr := ValidationResults{}
 		RenamingSubject(to).Validate(Subject(from), &vr)