From bef9c2ee61581a86b6e78496d8e18ea94d3118e0 Mon Sep 17 00:00:00 2001
From: vishalnayak <vishalnayakv@gmail.com>
Date: Tue, 13 Sep 2016 16:03:15 -0400
Subject: [PATCH] Ensure at least one constraint on the role

---
 builtin/credential/approle/path_role.go      | 16 +++++++
 builtin/credential/approle/path_role_test.go | 45 ++++++++++++++++++++
 website/source/docs/auth/approle.html.md     |  6 ++-
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/builtin/credential/approle/path_role.go b/builtin/credential/approle/path_role.go
index d85815bb7..d2565f462 100644
--- a/builtin/credential/approle/path_role.go
+++ b/builtin/credential/approle/path_role.go
@@ -524,6 +524,22 @@ func (b *backend) pathRoleSecretIDList(req *logical.Request, data *framework.Fie
 // setRoleEntry grabs a write lock and stores the options on an role into the storage.
 // Also creates a reverse index from the role's RoleID to the role itself.
 func (b *backend) setRoleEntry(s logical.Storage, roleName string, role *roleStorageEntry, previousRoleID string) error {
+	if roleName == "" {
+		return fmt.Errorf("missing role name")
+	}
+
+	if role == nil {
+		return fmt.Errorf("nil role")
+	}
+
+	// At least one constraint should be enabled on the role
+	switch {
+	case role.BindSecretID:
+	case role.BoundCIDRList != "":
+	default:
+		return fmt.Errorf("at least one constraint should be enabled on the role")
+	}
+
 	// Create a storage entry for the role
 	entry, err := logical.StorageEntryJSON("role/"+strings.ToLower(roleName), role)
 	if err != nil {
diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go
index b728d182d..8e551c8b2 100644
--- a/builtin/credential/approle/path_role_test.go
+++ b/builtin/credential/approle/path_role_test.go
@@ -10,6 +10,51 @@ import (
 	"github.com/mitchellh/mapstructure"
 )
 
+func TestAppRole_RoleConstraints(t *testing.T) {
+	var resp *logical.Response
+	var err error
+	b, storage := createBackendWithStorage(t)
+
+	roleData := map[string]interface{}{
+		"role_id":  "role-id-123",
+		"policies": "a,b",
+	}
+
+	roleReq := &logical.Request{
+		Operation: logical.CreateOperation,
+		Path:      "role/testrole1",
+		Storage:   storage,
+		Data:      roleData,
+	}
+
+	// Set bind_secret_id, which is enabled by default
+	resp, err = b.HandleRequest(roleReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+
+	// Set bound_cidr_list alone by explicitly disabling bind_secret_id
+	roleReq.Operation = logical.UpdateOperation
+	roleData["bind_secret_id"] = false
+	roleData["bound_cidr_list"] = "0.0.0.0/0"
+	resp, err = b.HandleRequest(roleReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+
+	// Remove both constraints
+	roleReq.Operation = logical.UpdateOperation
+	roleData["bound_cidr_list"] = ""
+	roleData["bind_secret_id"] = false
+	resp, err = b.HandleRequest(roleReq)
+	if resp != nil && resp.IsError() {
+		t.Fatalf("resp:%#v", err, resp)
+	}
+	if err == nil {
+		t.Fatalf("expected an error")
+	}
+}
+
 func TestAppRole_RoleIDUniqueness(t *testing.T) {
 	var resp *logical.Response
 	var err error
diff --git a/website/source/docs/auth/approle.html.md b/website/source/docs/auth/approle.html.md
index 03a6a0b7e..f337c6cbb 100644
--- a/website/source/docs/auth/approle.html.md
+++ b/website/source/docs/auth/approle.html.md
@@ -211,8 +211,10 @@ $ curl -XPOST "http://127.0.0.1:8200/v1/auth/approle/login" -d '{"role_id":"50be
 <dl class="api">
   <dt>Description</dt>
   <dd>
-  Create a new AppRole or update an existing AppRole. This endpoint
-  supports both `create` and `update` capabilities.
+  Creates a new AppRole or updates an existing AppRole. This endpoint
+  supports both `create` and `update` capabilities. There can be one or more
+  constraints enabled on the role. It is required to have at least one of them
+  enabled while creating or updating a role.
   </dd>
 
   <dt>Method</dt>