Add the option to specify a specific key id format that is generated … #2888
Conversation
|
I think if we're going to add this we need to not assume that everyone will always want the display name (since in most cases it's relatively useless). A better approach would be to use something a bit more templated, as in the SQL databases where you can provide a string with Potential values could be |
|
I agree with your recommendation, so I implemented it! |
|
Looks good. I realize the SSH CA tests aren't the easiest to deal with but could you try adding a test for this? |
2.) Add test for enforcement of allow_user_key_ids when signing user keys
|
Hi @jefferai - I added the requested test and an additional test that checks the enforcement of |
builtin/logical/ssh/path_sign.go
Outdated
| } | ||
|
|
||
| keyId = fmt.Sprintf("vault-%s", keyId) | ||
| keyId := substQuery(keyIdFormat, map[string]string{ | ||
| "token_display_name": req.DisplayName, |
There was a problem hiding this comment.
What happens when role's KeyIDFormat mentions {{token_display_name}} and req.DisplayName is empty? I think we'll end up having a {{}} substring in keyId. Given that the role specified it, it seems reasonable to have an {{}} block. Regardless, can we have a comment here explaining that?
There was a problem hiding this comment.
If req.DisplayName is an empty string, the return value should be the same. Here's an example: https://play.golang.org/p/um-WzsD8RE
builtin/logical/ssh/path_sign.go
Outdated
| } | ||
|
|
||
| keyId = fmt.Sprintf("vault-%s", keyId) | ||
| keyId := substQuery(keyIdFormat, map[string]string{ |
There was a problem hiding this comment.
Nitpick: Can we replace Id by ID?
…on the backend