Add support for token passed Authorization Bearer header #5397
Conversation
uepoch
force-pushed
the
rfc-6750
branch
2 times, most recently
from
045a6db
to
0dac0b0
Compare
There was a problem hiding this comment.
@uepoch this is great, thank you for doing it! Just a couple of little things.
http/handler.go
Outdated
| if v := r.Header.Get("Authorization"); v != "" { | ||
| // Reference for Authorization header format: https://tools.ietf.org/html/rfc7236#section-3 | ||
|
|
||
| if !strings.HasPrefix(v, "Bearer") { |
There was a problem hiding this comment.
RFC 6750 gives only one example of value v: Bearer mF_9.B5f-4.1JqM. I wonder if it would be safe to check for "Bearer " here.
http/handler.go
Outdated
| if !strings.HasPrefix(v, "Bearer") { | ||
| return "", fmt.Errorf("the Authorization header scheme should be Bearer") | ||
| } | ||
| vals := strings.Split(strings.TrimSpace(strings.TrimPrefix(v, "Bearer")), " ") |
There was a problem hiding this comment.
If we checked for the full "Bearer " prefix above with a space, could this just become return v[7:], nil? https://play.golang.org/p/Nttq9bx15lu
There was a problem hiding this comment.
One might send space separated value (not that it would make sense tho)
Still have to split(s, ' ')[0]
but if the aim is to optimize, we can probably check for the presence of a space in the string without allocation, return an error if present, return value if not
https://play.golang.org/p/NgRoBv-8Y1N
This address the comment of Calvin too I believe
There was a problem hiding this comment.
I updated the PR with a new version, with less allocations
http/handler_test.go
Outdated
| } | ||
|
|
||
| rWithVault, err := http.NewRequest("GET", "v1/test/path", nil) | ||
| rWithVault.Header.Set(consts.AuthHeaderName, token) |
There was a problem hiding this comment.
Could this be moved down to after the above error is checked? In case rWithVault is nil due to an error?
http/handler.go
Outdated
| return "", fmt.Errorf("the Authorization header scheme should be Bearer") | ||
| } | ||
| vals := strings.Split(strings.TrimSpace(strings.TrimPrefix(v, "Bearer")), " ") | ||
| if len(vals) < 1 { |
There was a problem hiding this comment.
Checking with len(vals) != 1 would also guard against (invalid formatted) tokens that might contain spaces.
|
Ok, I'll make the documentation PR this week, I've been busy on another project lately; sorry for the delay! |
Fixes: #5396
Similar to what has been done in hashicorp/consul#4502
Most credits go to @mbag for the consul PR
This let users pass their token as an Authorization: Bearer XXX header RFC-6750, which is an Oauth standard.
Some software only can be configured to use Authorization headers, and don't let users specify arbitrary (i.e X-Vault-Token) header names.
Doc will be further updated by this week in the PR