Skip to content

Commit

Permalink
Add fields 'ttl' and 'num_uses' to SecretID generation. (#14474)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>

* 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 <[email protected]>
Co-authored-by: Josh Black <[email protected]>
  • Loading branch information
@RemcoBuddelmeijer @raskchanky
3 people authored Sep 2, 2022
1 parent f7a50f3 commit 3e6f7a3
Show file tree
Hide file tree
Showing 5 changed files with 349 additions and 28 deletions.
85 changes: 68 additions & 17 deletions builtin/credential/approle/path_role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
},
}

Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -2584,17 +2635,17 @@ 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.",
`This option is not recommended unless there is a specific need
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",
Expand Down
Loading

0 comments on commit 3e6f7a3

Please sign in to comment.