Skip to content

Commit

Permalink
Fix for Issue 11863 - Panic when creating/updating approle role with …
Browse files Browse the repository at this point in the history
…token_type (#11864)

* initializing resp variable with aa *logical.Response before using it to add warning for default-service or default-batch token type.  Also adding guard around code that sets resp to a new logical.Response further on in the function.

* adding changelog entry

* renaming changelog file to match PR number
  • Loading branch information
marcboudreau authored Jun 24, 2021
1 parent 25346e8 commit 2acf487
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 1 deletion.
6 changes: 5 additions & 1 deletion builtin/credential/approle/path_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -887,9 +887,11 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
switch tokenTypeRaw.(string) {
case "default-service":
data.Raw["token_type"] = "service"
resp = &logical.Response{}
resp.AddWarning("default-service has no useful meaning; adjusting to service")
case "default-batch":
data.Raw["token_type"] = "batch"
resp = &logical.Response{}
resp.AddWarning("default-batch has no useful meaning; adjusting to batch")
}
}
Expand Down Expand Up @@ -976,7 +978,9 @@ func (b *backend) pathRoleCreateUpdate(ctx context.Context, req *logical.Request
}

if role.TokenMaxTTL > b.System().MaxLeaseTTL() {
resp = &logical.Response{}
if resp == nil {
resp = &logical.Response{}
}
resp.AddWarning("token_max_ttl is greater than the backend mount's maximum TTL value; issued tokens' max TTL value will be truncated")
}

Expand Down
130 changes: 130 additions & 0 deletions builtin/credential/approle/path_role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1820,6 +1820,136 @@ func TestAppRole_RoleWithTokenBoundCIDRsCRUD(t *testing.T) {
}
}

func TestAppRole_RoleWithTokenTypeCRUD(t *testing.T) {
var resp *logical.Response
var err error
b, storage := createBackendWithStorage(t)

roleData := map[string]interface{}{
"policies": "p,q,r,s",
"secret_id_num_uses": 10,
"secret_id_ttl": 300,
"token_ttl": 400,
"token_max_ttl": 500,
"token_num_uses": 600,
"token_type": "default-service",
}
roleReq := &logical.Request{
Operation: logical.CreateOperation,
Path: "role/role1",
Storage: storage,
Data: roleData,
}

resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}
if 0 == len(resp.Warnings) {
t.Fatalf("bad:\nexpected warning in resp:%#v\n", resp.Warnings)
}

roleReq.Operation = logical.ReadOperation
resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}

expected := map[string]interface{}{
"bind_secret_id": true,
"policies": []string{"p", "q", "r", "s"},
"secret_id_num_uses": 10,
"secret_id_ttl": 300,
"token_ttl": 400,
"token_max_ttl": 500,
"token_num_uses": 600,
"token_type": "service",
}

var expectedStruct roleStorageEntry
err = mapstructure.Decode(expected, &expectedStruct)
if err != nil {
t.Fatal(err)
}

var actualStruct roleStorageEntry
err = mapstructure.Decode(resp.Data, &actualStruct)
if err != nil {
t.Fatal(err)
}

expectedStruct.RoleID = actualStruct.RoleID
if !reflect.DeepEqual(expectedStruct, actualStruct) {
t.Fatalf("bad:\nexpected:%#v\nactual:%#v\n", expectedStruct, actualStruct)
}

roleData = map[string]interface{}{
"role_id": "test_role_id",
"policies": "a,b,c,d",
"secret_id_num_uses": 100,
"secret_id_ttl": 3000,
"token_ttl": 4000,
"token_max_ttl": 5000,
"token_type": "default-service",
}
roleReq.Data = roleData
roleReq.Operation = logical.UpdateOperation

resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}
if 0 == len(resp.Warnings) {
t.Fatalf("bad:\nexpected a warning in resp:%#v\n", resp.Warnings)
}

roleReq.Operation = logical.ReadOperation
resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}

expected = map[string]interface{}{
"policies": []string{"a", "b", "c", "d"},
"secret_id_num_uses": 100,
"secret_id_ttl": 3000,
"token_ttl": 4000,
"token_max_ttl": 5000,
"token_type": "service",
}
err = mapstructure.Decode(expected, &expectedStruct)
if err != nil {
t.Fatal(err)
}

err = mapstructure.Decode(resp.Data, &actualStruct)
if err != nil {
t.Fatal(err)
}

if !reflect.DeepEqual(expectedStruct, actualStruct) {
t.Fatalf("bad:\nexpected:%#v\nactual:%#v\n", expectedStruct, actualStruct)
}

// Delete test for role
roleReq.Path = "role/role1"
roleReq.Operation = logical.DeleteOperation
resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}

roleReq.Operation = logical.ReadOperation
resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}

if resp != nil {
t.Fatalf("expected a nil response")
}
}

func createRole(t *testing.T, b *backend, s logical.Storage, roleName, policies string) {
roleData := map[string]interface{}{
"policies": policies,
Expand Down
3 changes: 3 additions & 0 deletions changelog/11864.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
auth/approle: fixing dereference of nil pointer
```

0 comments on commit 2acf487

Please sign in to comment.