Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

golang v0.7.0 api not passing acl token #2403

Closed
aaronhurt opened this issue Oct 7, 2016 · 11 comments
Closed

golang v0.7.0 api not passing acl token #2403

aaronhurt opened this issue Oct 7, 2016 · 11 comments
Labels
type/enhancement Proposed improvement or new feature
Milestone

Comments

@aaronhurt
Copy link
Contributor

aaronhurt commented Oct 7, 2016

Running Consul v0.7.0 with a simple single instance dev cluster it appears that the current consul/api package is not passing ACL tokens as expected. The below example code tries to set the token in both the client config and as a query option. Both methods fail to pass the ?token parameter to the call made by the api package.

Example Code

package main

import (
    "fmt"
    "github.com/hashicorp/consul/api"
)

func cTest() api.KVPairs {
    config := api.DefaultConfig()
    config.Token = "testToken"
    client, _ := api.NewClient(config)
    kvps, _, _ := client.KV().List("/", nil)
    return kvps
}

func optTest() api.KVPairs {
    client, _ := api.NewClient(api.DefaultConfig())
    opts := &api.QueryOptions{
        Token: "testToken",
    }
    kvps, _, _ := client.KV().List("/", opts)
    return kvps
}

func main() {
    fmt.Printf("Client Config Test: %+v\n", cTest())
    fmt.Printf("Client QueryOptions Test: %+v\n", optTest())
}

Shell Output

charlie:test ahurt$ go run test.go
Client Config Test: []
Client QueryOptions Test: []

Consul Log Output

    2016/10/07 15:58:34 [DEBUG] http: Request GET /v1/kv/?recurse= (68.192µs) from=127.0.0.1:59158
    2016/10/07 15:58:34 [DEBUG] http: Request GET /v1/kv/?recurse= (53.313µs) from=127.0.0.1:59159

CURL With Query Parameter

charlie:test ahurt$ curl -v "http://localhost:8500/v1/kv/?recurse&token=testToken"
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8500 (#0)
> GET /v1/kv/?recurse&token=testToken HTTP/1.1
> Host: localhost:8500
> User-Agent: curl/7.49.1
> Accept: */*
>
< HTTP/1.1 200 OK
< Content-Type: application/json
< X-Consul-Index: 18
< X-Consul-Knownleader: true
< X-Consul-Lastcontact: 0
< Date: Fri, 07 Oct 2016 21:19:35 GMT
< Content-Length: 216
<
* Connection #0 to host localhost left intact
[{"LockIndex":0,"Key":"testKey","Flags":0,"Value":"dGVzdEtleSB2YWx1ZQ==","CreateIndex":15,"ModifyIndex":15},{"LockIndex":0,"Key":"testKey2","Flags":0,"Value":"dGVzdEtleSB2YWx1ZTI=","CreateIndex":18,"ModifyIndex":18}]

CURL With Header

charlie:test ahurt$ curl -v -H "X-Consul-Token: testToken" "http://localhost:8500/v1/kv/?recurse"
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8500 (#0)
> GET /v1/kv/?recurse HTTP/1.1
> Host: localhost:8500
> User-Agent: curl/7.49.1
> Accept: */*
> X-Consul-Token: testToken
>
< HTTP/1.1 200 OK
< Content-Type: application/json
< X-Consul-Index: 18
< X-Consul-Knownleader: true
< X-Consul-Lastcontact: 0
< Date: Fri, 07 Oct 2016 21:27:45 GMT
< Content-Length: 216
<
* Connection #0 to host localhost left intact
[{"LockIndex":0,"Key":"testKey","Flags":0,"Value":"dGVzdEtleSB2YWx1ZQ==","CreateIndex":15,"ModifyIndex":15},{"LockIndex":0,"Key":"testKey2","Flags":0,"Value":"dGVzdEtleSB2YWx1ZTI=","CreateIndex":18,"ModifyIndex":18}]
@aaronhurt
Copy link
Contributor Author

aaronhurt commented Oct 7, 2016

Using the v0.6.4 api the token is passed as a query parameter and the call returns the expected result. This breakage in the api must have happened somewhere between the v0.6.4 and v0.7.0 release.

@aaronhurt aaronhurt changed the title golang api not passing acl token golang v0.7.0 api not passing acl token Oct 7, 2016
@slackpad
Copy link
Contributor

slackpad commented Oct 7, 2016

There was a change to use an http header instead of the query parameter. Do you see the token coming in as a header?

On Oct 7, 2016, at 2:45 PM, Aaron Hurt notifications@github.com wrote:

One last update ... using the v0.6.4 api the token is passed as a query parameter and the call returns the expected result. This breakage in the api must have happened somewhere between the v0.6.4 and v0.7.0 release.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@aaronhurt
Copy link
Contributor Author

@slackpad I could sniff the transaction and/or point the test code above to a dummy webserver. What I have tested:

  • Curl with a header or a query parameter works against a v0.7.0 server
  • The v0.7.0 API does not work with client config set to use a token against a v0.7.0 server
  • The v0.7.0 API does not work with QueryOptions passed with a token to a v0.7.0 server
  • The v0.6.4 API using the exact same code above works against a 0.7.0 server

I suspect the header is not being passed by the v0.7.0 API with either the client config or the QueryOptions set. I will try and get confirmation though by using a dummy http server.

@slackpad
Copy link
Contributor

slackpad commented Oct 7, 2016

Linking to #2233 which was the header change.

@aaronhurt
Copy link
Contributor Author

I've worked up a really simple test that doesn't even use a consul server. It really looks like the 0.7.0 api client isn't sending the header.

https://github.com/leprechau/api-test

-- New Request --
Request URL: /v1/kv/?recurse=
Request Header: User-Agent => [Go-http-client/1.1]
Request Header: Referer => [http://localhost:9900/v1/kv//?recurse=]
Request Header: Accept-Encoding => [gzip]
-- End-Of-Request --

-- New Request --
Request URL: /v1/kv/?recurse=
Request Header: User-Agent => [Go-http-client/1.1]
Request Header: Referer => [http://localhost:9900/v1/kv//?recurse=]
Request Header: Accept-Encoding => [gzip]
-- End-Of-Request --

@aaronhurt
Copy link
Contributor Author

Here is a curl against the same test listener ...

The Request

charlie:api-test ahurt$ curl -v -H "X-Consul-Token: testToken" "http://localhost:9900/v1/kv/?recurse"
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 9900 (#0)
> GET /v1/kv/?recurse HTTP/1.1
> Host: localhost:9900
> User-Agent: curl/7.49.1
> Accept: */*
> X-Consul-Token: testToken
>
< HTTP/1.1 200 OK
< Date: Fri, 07 Oct 2016 23:26:14 GMT
< Content-Length: 0
< Content-Type: text/plain; charset=utf-8
<
* Connection #0 to host localhost left intact

The Listener

-- New Request --
Request URL: /v1/kv/?recurse
Request Header: X-Consul-Token => [testToken]
Request Header: User-Agent => [curl/7.49.1]
Request Header: Accept => [*/*]
-- End-Of-Request --

@slackpad
Copy link
Contributor

slackpad commented Oct 7, 2016

@leprechau thanks for the repro case. This is super weird as I see the token header going into the Do() call for the request in both examples. Will keep tracing this.

@slackpad
Copy link
Contributor

slackpad commented Oct 11, 2016

Hi @leprechau I think the root cause of this one is golang/go#4800. With older versions of Go, the HTTP client will drop the headers when it is following redirects. It looks like a fix for this is teed up in master and will go out with the next release of Go, but in the meantime, if you get rid of the slash you won't get the redirect and the headers will display:

diff --git a/test.go b/test.go
index afd24fd..d4c3685 100644
--- a/test.go
+++ b/test.go
@@ -9,7 +9,7 @@ func cTest() api.KVPairs {
        config := api.DefaultConfig()
        config.Token = "testToken"
        client, _ := api.NewClient(config)
-       kvps, _, _ := client.KV().List("/", nil)
+       kvps, _, _ := client.KV().List("", nil)
        return kvps
 }

@@ -18,7 +18,7 @@ func optTest() api.KVPairs {
        opts := &api.QueryOptions{
                Token: "testToken",
        }
-       kvps, _, _ := client.KV().List("/", opts)
+       kvps, _, _ := client.KV().List("", opts)
        return kvps
 }

Your sample app happened to have the same behavior as Consul because it was querying with a double slash. I think we should make the API client smarter and drop any leading slash in the KV path, since it's not necessary.

@slackpad slackpad added the type/enhancement Proposed improvement or new feature label Oct 11, 2016
@aaronhurt
Copy link
Contributor Author

@slackpad Interesting ... that makes sense. I also agree about enhancing the API client to not hard-code the prefixing slash when making requests. That was a rabbit hole.

@slackpad
Copy link
Contributor

Yeah I don't think there are other redirect spots in the API other than if you are getting the UI, so this shouldn't affect folks except for this KV case. Appreciate the repro case - that helped me narrow this down after a lot of head scratching - finally ended up doing a request against nc -l 9900 to see the header was there, then running wireshark and seeing the 301 against your test server, then running it against Consul and seeing the redirect there as well.

@aaronhurt
Copy link
Contributor Author

Nice work and glad I could help. Looking through that golang issue this has bitten quite a few people. It's always nice to have company in these types of issues.

aaronhurt added a commit to myENA/consul-backinator that referenced this issue Oct 12, 2016
Updating the TLS options to use the API features provided with the v0.7.0 API.
We previouslly updated to 0.7.0 release but then rolled back due to an issue
with the KV store and token passing.  However we found a workaround in
hashicorp/consul#2403 that should correct the issue.
@slackpad slackpad added this to the 0.7.1 milestone Oct 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Proposed improvement or new feature
Projects
None yet
Development

No branches or pull requests

2 participants