Add fields 'ttl' and 'num_uses' to SecretID generation. (#14474)

* Add fields 'ttl' and 'num_uses' to SecretID generation.

Add fields 'ttl' and 'num_uses' when generating/obtaining a SecretID.
Rather than just being able to use the Role's SecretID ttl and num uses. #14390

* Add secret_id_num_uses response field to generating SecretID

Add the response field secret_id_num_uses to the endpoints for generating
SecretIDs. Used in testing but also to supply the vendor with this variable.

* Add tests for new ttl and num_uses SecretID generation fields

Add tests to assert the new TTL and NumUses option in the SecretID entry.
Separate test for testing with just parameters vs a -force example.

* Patch up test for ttl and num_uses fields

* Add changelog entry for auth/approle 'ttl' and 'num_uses' fields

* Add fields to API Docs and AppRole Auth Docs example

* Correct error message for failing test on missing field.
Change the error message produced when a test fails due to a missing field.
Previous values did not map to correct fields.

* Remove unnecessary int cast to int "secret_id_num_uses" field.
Unnecessary cast to int where type already is int.

* Move numUses field check to after assignment.

* Remove metadata entry in sample payload to limit change to changes made.
Remove metadata entry in sample payload for custom-secret-id. The metadata was not
changed in the features pull request.

* Bind fields 'ttl' and 'num_uses' to role's configuration.

Rather than implicitly overriding, error when the ttl is lower than and the num
uses higher than the role's configuration. #14390

* Update changelog 14474 with a more detailed description.

More elaborate description for the changelog. Specifying the per-request based fields.

* Elaborate more on the bounds of the 'ttl' and 'num_uses' field.

Specify in both the api-docs and the CLI the limits of the fields.
Specify that the role's configuration is still the leading factor.

* Upper bound ttl with role secret id ttl

Upper bound ttl with role secret id ttl when creating a secret id
Adding test cases for infinite ttl and num uses
Adding test cases for negative ttl and num uses
Validation on infinite ttl and num uses

* Formatting issues. Removed unnecessary newline

* Update documentation for AppRole Secret ID and Role

Changed that TTL is not allowed to be shorter to longer

* Cleanup approle secret ID test and impl

* Define ttl and num_uses in every test

Define ttl and num_uses in every test despite them not being tested.
This is to ensure that no unexpected behaviour comes to mind.

* Rename test RoleSecretID -> RoleSecretIDWithoutFields

* Test secret id generation defaults to Role's config

Test secret id generation defaults to Role's configuration entries.

* Change finit -> finite

Co-authored-by: Josh Black <raskchanky@users.noreply.github.com>

* Rephrase comments to the correct validation check

* Rephrase role-secret-id option description

* Remove "default" incorrect statement about ttl

* Remove "default" incorrect statement about ttl for custom secret id

* Touch up approle.mdx to align more with path_role documentation

Co-authored-by: Remco Buddelmeijer <r.buddelmeijer@fullstaq.com>
Co-authored-by: Josh Black <raskchanky@users.noreply.github.com>
This commit is contained in:
Remco Buddelmeijer 2022-09-02 18:29:59 +02:00 committed by GitHub
parent 1ea50db6c6
commit b93d6e44e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 349 additions and 28 deletions

View File

@ -481,6 +481,16 @@ the role.`,
Type: framework.TypeCommaStringSlice,
Description: defTokenFields["token_bound_cidrs"].Description,
},
"num_uses": {
Type: framework.TypeInt,
Description: `Number of times this SecretID can be used, after which the SecretID expires.
Overrides secret_id_num_uses role option when supplied. May not be higher than role's secret_id_num_uses.`,
},
"ttl": {
Type: framework.TypeDurationSecond,
Description: `Duration in seconds after which this SecretID expires.
Overrides secret_id_ttl role option when supplied. May not be longer than role's secret_id_ttl.`,
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: b.pathRoleSecretIDUpdate,
@ -591,6 +601,16 @@ the role.`,
Description: `Comma separated string or list of CIDR blocks. If set, specifies the blocks of
IP addresses which can use the returned token. Should be a subset of the token CIDR blocks listed on the role, if any.`,
},
"num_uses": {
Type: framework.TypeInt,
Description: `Number of times this SecretID can be used, after which the SecretID expires.
Overrides secret_id_num_uses role option when supplied. May not be higher than role's secret_id_num_uses.`,
},
"ttl": {
Type: framework.TypeDurationSecond,
Description: `Duration in seconds after which this SecretID expires.
Overrides secret_id_ttl role option when supplied. May not be longer than role's secret_id_ttl.`,
},
},
Callbacks: map[logical.Operation]framework.OperationFunc{
logical.UpdateOperation: b.pathRoleCustomSecretIDUpdate,
@ -1497,7 +1517,7 @@ func (b *backend) pathRoleFieldRead(ctx context.Context, req *logical.Request, d
"bound_cidr_list": role.BoundCIDRList,
},
}
resp.AddWarning(`The "bound_cidr_list" parameter is deprecated and will be removed. Please use "secret_id_bound_cidrs" instead.`)
resp.AddWarning(`The "bound_cidr_list" field is deprecated and will be removed. Please use "secret_id_bound_cidrs" instead.`)
return resp, nil
default:
// shouldn't occur IRL
@ -2355,9 +2375,38 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req
return nil, err
}
var numUses int
// Check whether or not specified num_uses is defined, otherwise fallback to role's secret_id_num_uses
if numUsesRaw, ok := data.GetOk("num_uses"); ok {
numUses = numUsesRaw.(int)
if numUses < 0 {
return logical.ErrorResponse("num_uses cannot be negative"), nil
}
// If the specified num_uses is higher than the role's secret_id_num_uses, throw an error rather than implicitly overriding
if (numUses == 0 && role.SecretIDNumUses > 0) || (role.SecretIDNumUses > 0 && numUses > role.SecretIDNumUses) {
return logical.ErrorResponse("num_uses cannot be higher than the role's secret_id_num_uses"), nil
}
} else {
numUses = role.SecretIDNumUses
}
var ttl time.Duration
// Check whether or not specified ttl is defined, otherwise fallback to role's secret_id_ttl
if ttlRaw, ok := data.GetOk("ttl"); ok {
ttl = time.Second * time.Duration(ttlRaw.(int))
// If the specified ttl is longer than the role's secret_id_ttl, throw an error rather than implicitly overriding
if (ttl == 0 && role.SecretIDTTL > 0) || (role.SecretIDTTL > 0 && ttl > role.SecretIDTTL) {
return logical.ErrorResponse("ttl cannot be longer than the role's secret_id_ttl"), nil
}
} else {
ttl = role.SecretIDTTL
}
secretIDStorage := &secretIDStorageEntry{
SecretIDNumUses: role.SecretIDNumUses,
SecretIDTTL: role.SecretIDTTL,
SecretIDNumUses: numUses,
SecretIDTTL: ttl,
Metadata: make(map[string]string),
CIDRList: secretIDCIDRs,
TokenBoundCIDRs: secretIDTokenCIDRs,
@ -2376,6 +2425,7 @@ func (b *backend) handleRoleSecretIDCommon(ctx context.Context, req *logical.Req
"secret_id": secretID,
"secret_id_accessor": secretIDStorage.SecretIDAccessor,
"secret_id_ttl": int64(b.deriveSecretIDTTL(secretIDStorage.SecretIDTTL).Seconds()),
"secret_id_num_uses": secretIDStorage.SecretIDNumUses,
},
}
@ -2476,11 +2526,11 @@ to be generated against only this specific role, it can be done via
'role/<role_name>/secret-id' and 'role/<role_name>/custom-secret-id' endpoints.
The properties of the SecretID created against the role and the properties
of the token issued with the SecretID generated against the role, can be
configured using the parameters of this endpoint.`,
configured using the fields of this endpoint.`,
},
"role-bind-secret-id": {
"Impose secret_id to be presented during login using this role.",
`By setting this to 'true', during login the parameter 'secret_id' becomes a mandatory argument.
`By setting this to 'true', during login the field 'secret_id' becomes a mandatory argument.
The value of 'secret_id' can be retrieved using 'role/<role_name>/secret-id' endpoint.`,
},
"role-bound-cidr-list": {
@ -2512,16 +2562,17 @@ defined on the role, can access the role.`,
},
"role-secret-id-num-uses": {
"Use limit of the SecretID generated against the role.",
`If the SecretIDs are generated/assigned against the role using the
'role/<role_name>/secret-id' or 'role/<role_name>/custom-secret-id' endpoints,
then the number of times that SecretID can access the role is defined by
this option.`,
`If a SecretID is generated/assigned against a role using the
'role/<role_name>/secret-id' or 'role/<role_name>/custom-secret-id' endpoint,
then the number of times this SecretID can be used is defined by this option.
However, this option may be overriden by the request's 'num_uses' field.`,
},
"role-secret-id-ttl": {
`Duration in seconds, representing the lifetime of the SecretIDs
that are generated against the role using 'role/<role_name>/secret-id' or
'role/<role_name>/custom-secret-id' endpoints.`,
``,
"Duration in seconds of the SecretID generated against the role.",
`If a SecretID is generated/assigned against a role using the
'role/<role_name>/secret-id' or 'role/<role_name>/custom-secret-id' endpoint,
then the lifetime of this SecretID is defined by this option.
However, this option may be overridden by the request's 'ttl' field.`,
},
"role-secret-id-lookup": {
"Read the properties of an issued secret_id",
@ -2584,8 +2635,8 @@ this endpoint.`,
`The SecretID generated using this endpoint will be scoped to access
just this role and none else. The properties of this SecretID will be
based on the options set on the role. It will expire after a period
defined by the 'secret_id_ttl' option on the role and/or the backend
mount's maximum TTL value.`,
defined by the 'ttl' field or 'secret_id_ttl' option on the role,
and/or the backend mount's maximum TTL value.`,
},
"role-custom-secret-id": {
"Assign a SecretID of choice against the role.",
@ -2593,8 +2644,8 @@ mount's maximum TTL value.`,
to do so. This will assign a client supplied SecretID to be used to access
the role. This SecretID will behave similarly to the SecretIDs generated by
the backend. The properties of this SecretID will be based on the options
set on the role. It will expire after a period defined by the 'secret_id_ttl'
option on the role and/or the backend mount's maximum TTL value.`,
set on the role. It will expire after a period defined by the 'ttl' field
or 'secret_id_ttl' option on the role, and/or the backend mount's maximum TTL value.`,
},
"role-period": {
"Updates the value of 'period' on the role",

View File

@ -1098,7 +1098,7 @@ func TestAppRole_RoleList(t *testing.T) {
}
}
func TestAppRole_RoleSecretID(t *testing.T) {
func TestAppRole_RoleSecretIDWithoutFields(t *testing.T) {
var resp *logical.Response
var err error
b, storage := createBackendWithStorage(t)
@ -1135,13 +1135,18 @@ func TestAppRole_RoleSecretID(t *testing.T) {
if resp.Data["secret_id"].(string) == "" {
t.Fatalf("failed to generate secret_id")
}
if resp.Data["secret_id_ttl"].(int64) != int64(roleData["secret_id_ttl"].(int)) {
t.Fatalf("secret_id_ttl has not defaulted to the role's secret id ttl")
}
if resp.Data["secret_id_num_uses"].(int) != roleData["secret_id_num_uses"].(int) {
t.Fatalf("secret_id_num_uses has not defaulted to the role's secret id num_uses")
}
roleSecretIDReq.Path = "role/role1/custom-secret-id"
roleCustomSecretIDData := map[string]interface{}{
"secret_id": "abcd123",
}
roleSecretIDReq.Data = roleCustomSecretIDData
roleSecretIDReq.Operation = logical.UpdateOperation
resp, err = b.HandleRequest(context.Background(), roleSecretIDReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
@ -1150,6 +1155,240 @@ func TestAppRole_RoleSecretID(t *testing.T) {
if resp.Data["secret_id"] != "abcd123" {
t.Fatalf("failed to set specific secret_id to role")
}
if resp.Data["secret_id_ttl"].(int64) != int64(roleData["secret_id_ttl"].(int)) {
t.Fatalf("secret_id_ttl has not defaulted to the role's secret id ttl")
}
if resp.Data["secret_id_num_uses"].(int) != roleData["secret_id_num_uses"].(int) {
t.Fatalf("secret_id_num_uses has not defaulted to the role's secret id num_uses")
}
}
func TestAppRole_RoleSecretIDWithValidFields(t *testing.T) {
type testCase struct {
name string
payload map[string]interface{}
}
var resp *logical.Response
var err error
b, storage := createBackendWithStorage(t)
roleData := map[string]interface{}{
"policies": "p,q,r,s",
"secret_id_num_uses": 0,
"secret_id_ttl": 0,
"token_ttl": 400,
"token_max_ttl": 500,
}
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)
}
testCases := []testCase{
{
name: "finite num_uses ttl",
payload: map[string]interface{}{"secret_id": "finite", "ttl": 5, "num_uses": 5},
},
{
name: "infinite num_uses and ttl",
payload: map[string]interface{}{"secret_id": "infinite", "ttl": 0, "num_uses": 0},
},
{
name: "finite num_uses and infinite ttl",
payload: map[string]interface{}{"secret_id": "mixed1", "ttl": 0, "num_uses": 5},
},
{
name: "infinite num_uses and finite ttl",
payload: map[string]interface{}{"secret_id": "mixed2", "ttl": 5, "num_uses": 0},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
roleSecretIDReq := &logical.Request{
Operation: logical.UpdateOperation,
Path: "role/role1/secret-id",
Storage: storage,
}
roleCustomSecretIDData := tc.payload
roleSecretIDReq.Data = roleCustomSecretIDData
resp, err = b.HandleRequest(context.Background(), roleSecretIDReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}
if resp.Data["secret_id"].(string) == "" {
t.Fatalf("failed to generate secret_id")
}
if resp.Data["secret_id_ttl"].(int64) != int64(tc.payload["ttl"].(int)) {
t.Fatalf("secret_id_ttl has not been set by the 'ttl' field")
}
if resp.Data["secret_id_num_uses"].(int) != tc.payload["num_uses"].(int) {
t.Fatalf("secret_id_num_uses has not been set by the 'num_uses' field")
}
roleSecretIDReq.Path = "role/role1/custom-secret-id"
roleSecretIDReq.Data = roleCustomSecretIDData
resp, err = b.HandleRequest(context.Background(), roleSecretIDReq)
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}
if resp.Data["secret_id"] != tc.payload["secret_id"] {
t.Fatalf("failed to set specific secret_id to role")
}
if resp.Data["secret_id_ttl"].(int64) != int64(tc.payload["ttl"].(int)) {
t.Fatalf("secret_id_ttl has not been set by the 'ttl' field")
}
if resp.Data["secret_id_num_uses"].(int) != tc.payload["num_uses"].(int) {
t.Fatalf("secret_id_num_uses has not been set by the 'num_uses' field")
}
})
}
}
func TestAppRole_ErrorsRoleSecretIDWithInvalidFields(t *testing.T) {
type testCase struct {
name string
payload map[string]interface{}
expected string
}
type roleTestCase struct {
name string
options map[string]interface{}
cases []testCase
}
infiniteTestCases := []testCase{
{
name: "infinite ttl",
payload: map[string]interface{}{"secret_id": "abcd123", "num_uses": 1, "ttl": 0},
expected: "ttl cannot be longer than the role's secret_id_ttl",
},
{
name: "infinite num_uses",
payload: map[string]interface{}{"secret_id": "abcd123", "num_uses": 0, "ttl": 1},
expected: "num_uses cannot be higher than the role's secret_id_num_uses",
},
}
negativeTestCases := []testCase{
{
name: "negative num_uses",
payload: map[string]interface{}{"secret_id": "abcd123", "num_uses": -1, "ttl": 0},
expected: "num_uses cannot be negative",
},
}
roleTestCases := []roleTestCase{
{
name: "infinite role secret id ttl",
options: map[string]interface{}{
"secret_id_num_uses": 1,
"secret_id_ttl": 0,
},
cases: []testCase{
{
name: "higher num_uses",
payload: map[string]interface{}{"secret_id": "abcd123", "ttl": 0, "num_uses": 2},
expected: "num_uses cannot be higher than the role's secret_id_num_uses",
},
},
},
{
name: "infinite role num_uses",
options: map[string]interface{}{
"secret_id_num_uses": 0,
"secret_id_ttl": 1,
},
cases: []testCase{
{
name: "longer ttl",
payload: map[string]interface{}{"secret_id": "abcd123", "ttl": 2, "num_uses": 0},
expected: "ttl cannot be longer than the role's secret_id_ttl",
},
},
},
{
name: "finite role ttl and num_uses",
options: map[string]interface{}{
"secret_id_num_uses": 2,
"secret_id_ttl": 2,
},
cases: infiniteTestCases,
},
{
name: "mixed role ttl and num_uses",
options: map[string]interface{}{
"secret_id_num_uses": 400,
"secret_id_ttl": 500,
},
cases: negativeTestCases,
},
}
var resp *logical.Response
var err error
b, storage := createBackendWithStorage(t)
for i, rc := range roleTestCases {
roleData := map[string]interface{}{
"policies": "p,q,r,s",
"token_ttl": 400,
"token_max_ttl": 500,
}
roleData["secret_id_num_uses"] = rc.options["secret_id_num_uses"]
roleData["secret_id_ttl"] = rc.options["secret_id_ttl"]
roleReq := &logical.Request{
Operation: logical.CreateOperation,
Path: fmt.Sprintf("role/role%d", i),
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)
}
for _, tc := range rc.cases {
t.Run(fmt.Sprintf("%s/%s", rc.name, tc.name), func(t *testing.T) {
roleSecretIDReq := &logical.Request{
Operation: logical.UpdateOperation,
Path: fmt.Sprintf("role/role%d/secret-id", i),
Storage: storage,
}
roleSecretIDReq.Data = tc.payload
resp, err = b.HandleRequest(context.Background(), roleSecretIDReq)
if err != nil || (resp != nil && !resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}
if resp.Data["error"].(string) != tc.expected {
t.Fatalf("expected: %q, got: %q", tc.expected, resp.Data["error"].(string))
}
roleSecretIDReq.Path = fmt.Sprintf("role/role%d/custom-secret-id", i)
resp, err = b.HandleRequest(context.Background(), roleSecretIDReq)
if err != nil || (resp != nil && !resp.IsError()) {
t.Fatalf("err:%v resp:%#v", err, resp)
}
if resp.Data["error"].(string) != tc.expected {
t.Fatalf("expected: %q, got: %q", tc.expected, resp.Data["error"].(string))
}
})
}
}
}
func TestAppRole_RoleCRUD(t *testing.T) {

4
changelog/14474.txt Normal file
View File

@ -0,0 +1,4 @@
```release-note:improvement
auth/approle: SecretIDs can now be generated with an per-request specified TTL and num_uses.
When either the ttl and num_uses fields are not specified, the role's configuration is used.
```

View File

@ -67,11 +67,13 @@ enabled while creating or updating a role.
blocks; if set, specifies blocks of IP addresses which can perform the login
operation.
- `secret_id_num_uses` `(integer: 0)` - Number of times any particular SecretID
can be used to fetch a token from this AppRole, after which the SecretID will
expire. A value of zero will allow unlimited uses.
can be used to fetch a token from this AppRole, after which the SecretID by default
will expire. A value of zero will allow unlimited uses.
However, this option may be overridden by the request's 'num_uses' field when generating a SecretID.
- `secret_id_ttl` `(string: "")` - Duration in either an integer number of
seconds (`3600`) or an integer time unit (`60m`) after which any SecretID
expires.
seconds (`3600`) or an integer time unit (`60m`) after which by default any SecretID
expires. A value of zero will allow the SecretID to not expire.
However, this option may be overridden by the request's 'ttl' field when generating a SecretID.
- `local_secret_ids` `(bool: false)` - If set, the secret IDs generated
using this role will be cluster local. This can only be set during role
creation and once set, it can't be reset later.
@ -272,12 +274,22 @@ itself, and also to delete the SecretID from the AppRole.
- `token_bound_cidrs` `(array: [])` - Comma-separated string or list of CIDR
blocks; if set, specifies blocks of IP addresses which can use the auth tokens
generated by this SecretID. Overrides any role-set value but must be a subset.
- `num_uses` `(integer: 0)` - Number of times this SecretID can be used, after which
the SecretID expires. A value of zero will allow unlimited uses.
Overrides secret_id_num_uses role option when supplied.
May not be higher than role's secret_id_num_uses.
- `ttl` `(string: "")` - Duration in seconds (`3600`) or an integer time unit (`60m`)
after which this SecretID expires. A value of zero will allow the SecretID to not expire.
Overrides secret_id_ttl role option when supplied.
May not be longer than role's secret_id_ttl.
### Sample Payload
```json
{
"metadata": "{ \"tag1\": \"production\" }"
"metadata": "{ \"tag1\": \"production\" }",
"ttl": 600,
"num_uses": 50
}
```
@ -301,7 +313,8 @@ $ curl \
"data": {
"secret_id_accessor": "84896a0c-1347-aa90-a4f6-aca8b7558780",
"secret_id": "841771dc-11c9-bbc7-bcac-6a3945a69cd9",
"secret_id_ttl": 600
"secret_id_ttl": 600,
"secret_id_num_uses": 50
},
"lease_duration": 0,
"renewable": false,
@ -501,12 +514,22 @@ Assigns a "custom" SecretID against an existing AppRole. This is used in the
- `token_bound_cidrs` `(array: [])` - Comma-separated string or list of CIDR
blocks; if set, specifies blocks of IP addresses which can use the auth tokens
generated by this SecretID. Overrides any role-set value but must be a subset.
- `num_uses` `(integer: 0)` - Number of times this SecretID can be used, after which
the SecretID expires. A value of zero will allow unlimited uses.
Overrides secret_id_num_uses role option when supplied.
May not be higher than role's secret_id_num_uses.
- `ttl` `(string: "")` - Duration in seconds (`3600`) or an integer time unit (`60m`)
after which this SecretID expires. A value of zero will allow the SecretID to not expire.
Overrides secret_id_ttl role option when supplied.
May not be longer than role's secret_id_ttl.
### Sample Payload
```json
{
"secret_id": "testsecretid"
"secret_id": "testsecretid",
"ttl": 600,
"num_uses": 50
}
```
@ -528,8 +551,10 @@ $ curl \
"warnings": null,
"wrap_info": null,
"data": {
"secret_id": "testsecretid",
"secret_id_accessor": "84896a0c-1347-aa90-a4f6-aca8b7558780",
"secret_id": "testsecretid"
"secret_id_ttl": 600,
"secret_id_num_uses": 50
},
"lease_duration": 0,
"renewable": false,

View File

@ -117,6 +117,7 @@ documentation.
secret_id 6a174c20-f6de-a53c-74d2-6018fcceff64
secret_id_accessor c454f7e5-996e-7230-6074-6ef26b7bcf86
secret_id_ttl 10m
secret_id_num_uses 40
```
### Via the API
@ -175,7 +176,8 @@ documentation.
"data": {
"secret_id_accessor": "45946873-1d96-a9d4-678c-9229f74386a5",
"secret_id": "37b74931-c4cd-d49a-9246-ccc62d682a25",
"secret_id_ttl": 600
"secret_id_ttl": 600,
"secret_id_num_uses": 40
}
}
```