smallstep / cli

🧰 A zero trust swiss army knife for working with X509, OAuth, JWT, OATH OTP, etc.
https://smallstep.com/cli
Apache License 2.0
3.65k stars 249 forks source link

[Bug]: Cannot remove provisioners with name containing "://" #743

Open spacedub opened 2 years ago

spacedub commented 2 years ago

Steps to Reproduce

step ca provisioner add "testthis://foo" --type JWK --create

step ca provisioner remove "testthis://foo"

Your Environment

Expected Behavior

Remove the newly created provisioner.

Actual Behavior

json: cannot unmarshal number into Go value of type ca.AdminClientError

(and the provisioner cannot be removed)

Additional Context

No response

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

maraino commented 1 year ago

Moving to v0.24.0, the changes are more than expected:

So it seems that we need to set the path to the escaped version like:

path.Join("/foo/bar", url.PathEscape("testthis://foo"))
// => /foo/bar/testthis:%2F%2Ffoo

The final path after u.String() will be escaped again and will become /foo/bar/testthis:%252F%252Ffoo. That will be ok, but the name in the API would be testthis:%2F%2Ffoo so we need to do url.PathUnescape(name) to get the proper name.

Another solution would be to use u.JoinPath()

u := baseURL.JoinPath("foo", "bar", url.PathEscape("testthis://foo"))
// Path => /foo/bar/testthis://foo
// RawPath => /foo/bar/testthis:%2F%2Ffoo

The final string will have probably the cleanest path /foo/bar/testthis:%2F%2Ffoo but as happens with the previous version we will need to use url.PathUnescape(name).

So if there's no other solution, we will need to change most of the endpoints in the client and all the admin methods that can have a name/reference like this.

And obviously, we can always restrict which characters can use in all those names and references.