Skip to content

Commit

Permalink
Add role patching test case (#15545)
Browse files Browse the repository at this point in the history 
* Add tests for role patching

Signed-off-by: Alexander Scheel <[email protected]>

* Prevent bad issuer names on update

Signed-off-by: Alexander Scheel <[email protected]>

* Add documentation on PATCH operations

Signed-off-by: Alexander Scheel <[email protected]>
  • Loading branch information
@cipherboy
cipherboy authored May 20, 2022
1 parent 74ac757 commit a38678d
Show file tree
Hide file tree
Showing 3 changed files with 305 additions and 8 deletions.
6 changes: 6 additions & 0 deletions builtin/logical/pki/path_fetch_issuers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da
// When the new name is in use but isn't this name, throw an error.
return logical.ErrorResponse(err.Error()), nil
}
if len(newName) > 0 && !nameMatcher.MatchString(newName) {
return logical.ErrorResponse("new key name outside of valid character limits"), nil
}

newPath := data.Get("manual_chain").([]string)
rawLeafBehavior := data.Get("leaf_not_after_behavior").(string)
Expand Down Expand Up @@ -374,6 +377,9 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat
// When the new name is in use but isn't this name, throw an error.
return logical.ErrorResponse(err.Error()), nil
}
if len(newName) > 0 && !nameMatcher.MatchString(newName) {
return logical.ErrorResponse("new key name outside of valid character limits"), nil
}
if newName != issuer.Name {
oldName = issuer.Name
issuer.Name = newName
Expand Down
281 changes: 281 additions & 0 deletions builtin/logical/pki/path_roles_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package pki

import (
"context"
"fmt"
"testing"
"time"

"github.com/hashicorp/go-secure-stdlib/strutil"
"github.com/hashicorp/vault/sdk/logical"
"github.com/mitchellh/mapstructure"
"github.com/stretchr/testify/require"
)

func TestPki_RoleGenerateLease(t *testing.T) {
Expand Down Expand Up @@ -738,3 +740,282 @@ func TestPki_CertsLease(t *testing.T) {
t.Fatalf("expected a response that contains a secret")
}
}

func TestPki_RolePatch(t *testing.T) {
type TestCase struct {
Field string
Before interface{}
Patched interface{}
}

testCases := []TestCase{
{
Field: "ttl",
Before: int64(5),
Patched: int64(10),
},
{
Field: "max_ttl",
Before: int64(5),
Patched: int64(10),
},
{
Field: "allow_localhost",
Before: true,
Patched: false,
},
{
Field: "allowed_domains",
Before: []string{"alex", "bob"},
Patched: []string{"sam", "alex", "frank"},
},
{
Field: "allowed_domains_template",
Before: false,
Patched: true,
},
{
Field: "allow_bare_domains",
Before: true,
Patched: false,
},
{
Field: "allow_subdomains",
Before: false,
Patched: true,
},
{
Field: "allow_glob_domains",
Before: true,
Patched: false,
},
{
Field: "allow_wildcard_certificates",
Before: false,
Patched: true,
},
{
Field: "allow_any_name",
Before: true,
Patched: false,
},
{
Field: "enforce_hostnames",
Before: false,
Patched: true,
},
{
Field: "allow_ip_sans",
Before: true,
Patched: false,
},
{
Field: "allowed_uri_sans",
Before: []string{"gopher://*"},
Patched: []string{"https://*"},
},
{
Field: "allowed_uri_sans_template",
Before: false,
Patched: true,
},
{
Field: "allowed_other_sans",
Before: []string{"1.2.3.4;UTF8:magic"},
Patched: []string{"4.3.2.1;UTF8:cigam"},
},
{
Field: "allowed_serial_numbers",
Before: []string{"*"},
Patched: []string{""},
},
{
Field: "server_flag",
Before: true,
Patched: false,
},
{
Field: "client_flag",
Before: false,
Patched: true,
},
{
Field: "code_signing_flag",
Before: true,
Patched: false,
},
{
Field: "email_protection_flag",
Before: false,
Patched: true,
},
// key_type, key_bits, and signature_bits can't be tested in this setup
// due to their non-default stored nature.
{
Field: "key_usage",
Before: []string{"DigitialSignature"},
Patched: []string{"DigitalSignature", "KeyAgreement"},
},
{
Field: "ext_key_usage",
Before: []string{"ServerAuth"},
Patched: []string{"ClientAuth"},
},
{
Field: "ext_key_usage_oids",
Before: []string{"1.2.3.4"},
Patched: []string{"4.3.2.1"},
},
{
Field: "use_csr_common_name",
Before: true,
Patched: false,
},
{
Field: "use_csr_sans",
Before: false,
Patched: true,
},
{
Field: "ou",
Before: []string{"crypto"},
Patched: []string{"cryptosec"},
},
{
Field: "organization",
Before: []string{"hashicorp"},
Patched: []string{"dadgarcorp"},
},
{
Field: "country",
Before: []string{"US"},
Patched: []string{"USA"},
},
{
Field: "locality",
Before: []string{"Orange"},
Patched: []string{"Blue"},
},
{
Field: "province",
Before: []string{"CA"},
Patched: []string{"AC"},
},
{
Field: "street_address",
Before: []string{"101 First"},
Patched: []string{"202 Second", "Unit 020"},
},
{
Field: "postal_code",
Before: []string{"12345"},
Patched: []string{"54321-1234"},
},
{
Field: "generate_lease",
Before: false,
Patched: true,
},
{
Field: "no_store",
Before: true,
Patched: false,
},
{
Field: "require_cn",
Before: false,
Patched: true,
},
{
Field: "policy_identifiers",
Before: []string{"1.2.3.4.5"},
Patched: []string{"5.4.3.2.1"},
},
{
Field: "basic_constraints_valid_for_non_ca",
Before: true,
Patched: false,
},
{
Field: "not_before_duration",
Before: int64(30),
Patched: int64(300),
},
{
Field: "not_after",
Before: "9999-12-31T23:59:59Z",
Patched: "1230-12-31T23:59:59Z",
},
{
Field: "issuer_ref",
Before: "default",
Patched: "missing",
},
}

b, storage := createBackendWithStorage(t)

for index, testCase := range testCases {
var resp *logical.Response
var roleDataResp *logical.Response
var afterRoleDataResp *logical.Response
var err error

// Create the role
roleData := map[string]interface{}{}
roleData[testCase.Field] = testCase.Before

roleReq := &logical.Request{
Operation: logical.UpdateOperation,
Path: "roles/testrole",
Storage: storage,
Data: roleData,
}

resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad [%d/%v] create: err: %v resp: %#v", index, testCase.Field, err, resp)
}

// Read the role after creation
roleReq.Operation = logical.ReadOperation
roleDataResp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (roleDataResp != nil && roleDataResp.IsError()) {
t.Fatalf("bad [%d/%v] read: err: %v resp: %#v", index, testCase.Field, err, resp)
}

beforeRoleData := roleDataResp.Data

// Patch the role
roleReq.Operation = logical.PatchOperation
roleReq.Data[testCase.Field] = testCase.Patched
resp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad [%d/%v] patch: err: %v resp: %#v", index, testCase.Field, err, resp)
}

// Re-read and verify the role
roleReq.Operation = logical.ReadOperation
afterRoleDataResp, err = b.HandleRequest(context.Background(), roleReq)
if err != nil || (afterRoleDataResp != nil && afterRoleDataResp.IsError()) {
t.Fatalf("bad [%d/%v] read: err: %v resp: %#v", index, testCase.Field, err, resp)
}

afterRoleData := afterRoleDataResp.Data

for field, before := range beforeRoleData {
switch typed := before.(type) {
case *bool:
before = *typed
afterRoleData[field] = *(afterRoleData[field].(*bool))
}

if field != testCase.Field {
require.Equal(t, before, afterRoleData[field], fmt.Sprintf("bad [%d/%v] compare: non-modified field %v should not be changed", index, testCase.Field, field))
} else {
require.Equal(t, before, testCase.Before, fmt.Sprintf("bad [%d] compare: modified field %v before should be correct", index, field))
require.Equal(t, afterRoleData[field], testCase.Patched, fmt.Sprintf("bad [%d] compare: modified field %v after should be correct", index, field))
}
}
}
}
26 changes: 18 additions & 8 deletions website/content/api-docs/secret/pki.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1676,13 +1676,18 @@ modes are set on this issuer.
Note that it is not possible to change the certificate of this issuer; to
do so, import a new issuer and a new `issuer_id` will be assigned.

| Method | Path |
| :----- | :------------------------ |
| `POST` | `/pki/issuer/:issuer_ref` |
| Method | Path |
| :------ | :------------------------ |
| `POST` | `/pki/issuer/:issuer_ref` |
| `PATCH` | `/pki/issuer/:issuer_ref` |

~> **Note** `POST`ing to this endpoint causes Vault to overwrite the previous
contents of the issuer, using the provided request data (and any defaults
for elided parameters). It does not update only the provided fields.
for elided parameters). It does not update only the provided fields.<br /><br />
Since Vault 1.11.0, Vault supports the PATCH operation to this endpoint,
using the [JSON patch format](https://datatracker.ietf.org/doc/html/rfc6902)
supported by KVv2, allowing update of specific fields. Note that
`vault write` uses `POST`.

#### Parameters

Expand Down Expand Up @@ -2018,14 +2023,19 @@ multiple roles nearly any issuing policy can be accommodated. `server_flag`,
requests a certificate that is not allowed by the CN policy in the role, the
request is denied.

| Method | Path |
| :----- | :----------------- |
| `POST` | `/pki/roles/:name` |
| Method | Path |
| :------ | :----------------- |
| `POST` | `/pki/roles/:name` |
| `PATCH` | `/pki/roles/:name` |

~> **Note** `POST`ing to this endpoint when the role already exists causes
Vault to overwrite the contents of the role, using the provided request
data (and any defaults for elided parameters). It does not update only
the provided fields.
the provided fields.<br /><br />
Since Vault 1.11.0, Vault supports the PATCH operation to this endpoint,
using the [JSON patch format](https://datatracker.ietf.org/doc/html/rfc6902)
supported by KVv2, allowing update of specific fields. Note that
`vault write` uses `POST`.

#### Parameters

Expand Down

0 comments on commit a38678d

Please sign in to comment.