add secret access key and secret key length check#30
Conversation
ddebko
left a comment
ddebko
left a comment
There was a problem hiding this comment.
Is there a strong need or use case for the validation checks on the access and secret key?
| if err != nil { | ||
| badFields[fmt.Sprintf("secrets.%s", ConstAccessKeyId)] = err.Error() | ||
| } else if len(accessKey) < 16 || len(accessKey) > 128 { | ||
| badFields[fmt.Sprintf("secrets.%s", ConstAccessKeyId)] = "value must be between 16 and 128 characters" |
There was a problem hiding this comment.
I'm not sure if we should enforce any strict checks on the given values of the access or secret key. I think it would be safer for AWS to return an error to the plugin and the plugin could bubble up the error to the user.
The reason why I think it would be safer to bubble up an error like this from AWS is because the user would be able to search AWS documentation by matching the error message. Otherwise the user could get stuck with our unique error message that may not be helpful with searching AWS documentation.
Also, AWS can change their behavior and this check becomes invalid, which would enable an edge case bug in the plugin that we would then need to patch.
2 participants
validates that
secrets.access_key_idis always between 16 and 128 characters long and matches the\w+pattern per https://docs.aws.amazon.com/IAM/latest/APIReference/API_AccessKey.htmlvalidates that
secrets.secret_access_keyis always 40 characters long