search

influxdata / influxdb-relay

Service to replicate InfluxDB data for high availability
MIT License
855 stars 350 forks source link

Add authorization forwarding for HTTP #21

Closed goakley closed 8 years ago

goakley commented 8 years ago

I realize there is a discussion going on about auth for the relay ( #13 ) . This implementation proposes just blindly forwarding the Authorization header if it's present. It seems to encompass the spirit of the relay, which is just to forward the request to each server. Query-string auth currently works with the relay since the entire query-string is forwarded, but basic auth did not. Since Telegraf uses basic auth and not query-string auth, data from telegraf fails to pass through the relay without this fix, which is a major issue.

joelegasse commented 8 years ago

Thanks for submitting this. This was actually the solution I was thinking about suggesting for the auth discussion. This looks like a good start, but you're missing the logic in (*bufferList).add() to make sure the auth value matches the current node.

The line that currently reads: if (*cur).query != query || (*cur).full {

Should now read: if (*cur).query != query || (*cur).auth != auth || (*cur).full {

Without that change, only the first auth header for a given batch of a query string will be used, probably causing problems in a multi-user scenario.

Also, you'll need to sign the InfluxData CLA before I can merge this: https://influxdata.com/community/cla/

goakley commented 8 years ago

Understood and resolved. I didn't consider that different users could cause subtle differences in behaviour on the backend for the exact same query.

I'll sign the CLA as soon as I get an okay to do so on my end (I ran into this issue since we're prototyping InfluxDB at Thumbtack)

joelegasse commented 8 years ago

@goakley Any update on signing the CLA? If you're not able to, then I'll close this and re-implement the pass-through.

goakley commented 8 years ago

@joelegasse sorry about the delay, our legal process is new and thus ill-defined. If I can't have it by the beginning of next week then you can go ahead and re-implement it (I mostly care that it's there, not that I wrote it). I'll let you know.

jeffastorey commented 8 years ago

Any progress on getting this merged?

joelegasse commented 8 years ago

@goakley, can you either sign the CLA or close this PR with "the lawyers won't let me"? :-)

goakley commented 8 years ago

HTTP 408 Lawyers Timeout

If there's not a new one submitted by the time they get back to me I'll resubmit.

goakley commented 8 years ago

@joelegasse lo and behold, I signed the CLA.

joelegasse commented 8 years ago

Awesome! Also, for future reference, the proper HTTP status code is 451 :-)