From 7e50a457d9fdda119f62e698d1771ae0757a96fb Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Tue, 7 Jul 2015 11:07:37 -0600
Subject: [PATCH] acl: allow omitting keyring policy, add tests

---
 acl/acl.go         |  9 +++++++--
 acl/acl_test.go    | 30 ++++++++++++++++++++++++++++++
 acl/policy.go      |  1 +
 acl/policy_test.go |  6 +++++-
 4 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/acl/acl.go b/acl/acl.go
index 29a7569d81..f18be42b55 100644
--- a/acl/acl.go
+++ b/acl/acl.go
@@ -350,14 +350,19 @@ func (p *PolicyACL) KeyringRead() bool {
 	switch p.keyringRule {
 	case KeyringPolicyRead, KeyringPolicyWrite:
 		return true
-	default:
+	case KeyringPolicyDeny:
 		return false
+	default:
+		return p.parent.KeyringRead()
 	}
 }
 
 // KeyringWrite determines if the keyring can be manipulated.
 func (p *PolicyACL) KeyringWrite() bool {
-	return p.keyringRule == KeyringPolicyWrite
+	if p.keyringRule == KeyringPolicyWrite {
+		return true
+	}
+	return p.parent.KeyringWrite()
 }
 
 // ACLList checks if listing of ACLs is allowed
diff --git a/acl/acl_test.go b/acl/acl_test.go
index 5bd77dc8b2..6872b04f13 100644
--- a/acl/acl_test.go
+++ b/acl/acl_test.go
@@ -47,6 +47,18 @@ func TestStaticACL(t *testing.T) {
 	if !all.ServiceWrite("foobar") {
 		t.Fatalf("should allow")
 	}
+	if !all.EventRead("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !all.EventWrite("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !all.KeyringRead() {
+		t.Fatalf("should allow")
+	}
+	if !all.KeyringWrite() {
+		t.Fatalf("should allow")
+	}
 	if all.ACLList() {
 		t.Fatalf("should not allow")
 	}
@@ -78,6 +90,12 @@ func TestStaticACL(t *testing.T) {
 	if none.EventWrite("") {
 		t.Fatalf("should not allow")
 	}
+	if none.KeyringRead() {
+		t.Fatalf("should now allow")
+	}
+	if none.KeyringWrite() {
+		t.Fatalf("should not allow")
+	}
 	if none.ACLList() {
 		t.Fatalf("should not allow")
 	}
@@ -97,6 +115,18 @@ func TestStaticACL(t *testing.T) {
 	if !manage.ServiceWrite("foobar") {
 		t.Fatalf("should allow")
 	}
+	if !manage.EventRead("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !manage.EventWrite("foobar") {
+		t.Fatalf("should allow")
+	}
+	if !manage.KeyringRead() {
+		t.Fatalf("should allow")
+	}
+	if !manage.KeyringWrite() {
+		t.Fatalf("should allow")
+	}
 	if !manage.ACLList() {
 		t.Fatalf("should allow")
 	}
diff --git a/acl/policy.go b/acl/policy.go
index d9e62792e8..9009ee76b5 100644
--- a/acl/policy.go
+++ b/acl/policy.go
@@ -114,6 +114,7 @@ func Parse(rules string) (*Policy, error) {
 	case KeyringPolicyRead:
 	case KeyringPolicyWrite:
 	case KeyringPolicyDeny:
+	case "": // Special case to allow omitting the keyring policy
 	default:
 		return nil, fmt.Errorf("Invalid keyring policy: %#v", p.Keyring)
 	}
diff --git a/acl/policy_test.go b/acl/policy_test.go
index 11f815da2b..043dc9c946 100644
--- a/acl/policy_test.go
+++ b/acl/policy_test.go
@@ -34,6 +34,7 @@ event "foo" {
 event "bar" {
 	policy = "deny"
 }
+keyring = "deny"
 	`
 	exp := &Policy{
 		Keys: []*KeyPolicy{
@@ -78,6 +79,7 @@ event "bar" {
 				Policy: EventPolicyDeny,
 			},
 		},
+		Keyring: KeyringPolicyDeny,
 	}
 
 	out, err := Parse(inp)
@@ -124,7 +126,8 @@ func TestParse_JSON(t *testing.T) {
 		"bar": {
 			"policy": "deny"
 		}
-	}
+	},
+	"keyring": "deny"
 }`
 	exp := &Policy{
 		Keys: []*KeyPolicy{
@@ -169,6 +172,7 @@ func TestParse_JSON(t *testing.T) {
 				Policy: EventPolicyDeny,
 			},
 		},
+		Keyring: KeyringPolicyDeny,
 	}
 
 	out, err := Parse(inp)