Fix parsing of remote policies

[tyrannosaurus-becks]

Fixes https://github.com/hashicorp/vault/issues/8256

Because an example remote policy is name:AliyunRDSReadOnlyAccess,type:System using a TypeCommaStringSlice was not appropriate because a comma is used as part of each remote policy. It caused incorrect behavior when only sending 1 remote policy.

[kalafut]

Change seems fine, though consider adding a test. Also, can you explain why it worked if more than one policy was sent in?

[tyrannosaurus-becks]

Hi! Thanks for the review!

When I add a log line here like this:

b.Logger().Info(fmt.Sprintf("raw: %s, len strPolicies: %d", raw, len(strPolicies)))

And then do a call like:

vault write alicloud/role/policy-based \
    remote_policies='name:AliyunOSSReadOnlyAccess,type:System'

I receive:

2020-01-28T16:15:32.984-0800 [INFO]  secrets.vault-plugin-secrets-alicloud.vault-plugin-secrets-alicloud_ab411914.vault-plugin-secrets-alicloud.vault-plugin-secrets-alicloud: raw: [name:AliyunOSSReadOnlyAccess type:System], len strPolicies: 2: timestamp=2020-01-28T16:15:32.984-0800

So basically, it splits it on the comma and then the code tries to turn it into a policy, which needs both fields, and it fails.

When I repeat the test with two policies like this:

vault write alicloud/role/policy-based \
    remote_policies='name:AliyunOSSReadOnlyAccess,type:System' \
    remote_policies='name:AliyunRDSReadOnlyAccess,type:System'

Bizarrely, it behaves as expected, which is just a matter of luck:

2020-01-28T16:19:01.796-0800 [INFO]  secrets.vault-plugin-secrets-alicloud.vault-plugin-secrets-alicloud_ab411914.vault-plugin-secrets-alicloud.vault-plugin-secrets-alicloud: raw: [name:AliyunOSSReadOnlyAccess,type:System name:AliyunRDSReadOnlyAccess,type:System], len strPolicies: 2: timestamp=2020-01-28T16:19:01.796-0800

I don't think having commas in the values themselves is an expected use case, so it makes sense its behavior would be strange.

Using the TypeStringSlice means it no longer pays any attention to commas, and so behavior comes through in an expected fashion.

I hope that makes sense but let me know if you have more questions.

[tyrannosaurus-becks]

Also FWIW, the backend test didn't catch it because it passed in the value how I thought it would be passed in, not how it was actually passed in. :-)

[kalafut]

OK, I understand what's happening now. It's two parts, really. First, if multiple instances of the same argument are listed, the vault CLI (not server) turns that into an array:

# single parameter
vault write -output-curl-string alicloud/role/policy-based \
    remote_policies='name:AliyunOSSReadOnlyAccess,type:System'
curl -X PUT -H "X-Vault-Request: true" -H "X-Vault-Token: $(vault print token)" -d '{"remote_policies":"name:AliyunOSSReadOnlyAccess,type:System"}' http://127.0.0.1:8200/v1/alicloud/role/policy-based

#multiple parameters
vault write -output-curl-string alicloud/role/policy-based \
    remote_policies='name:AliyunOSSReadOnlyAccess,type:System' \
    remote_policies='name:AliyunRDSReadOnlyAccess,type:System'

curl -X PUT -H "X-Vault-Request: true" -H "X-Vault-Token: $(vault print token)" -d '{"remote_policies":["name:AliyunOSSReadOnlyAccess,type:System","name:AliyunRDSReadOnlyAccess,type:System"]}' http://127.0.0.1:8200/v1/alicloud/role/policy-based

Next, the handling of CommaStringSlice is that the comma splitting is only done on a string input. So when an array is received--regardless of whether that array contains strings--it just takes the elements and puts them as-is into the return slice. Which is why using >1 policy (as distinct CLI parameters) worked.