search

ferristseng / rust-ipfs-api

IPFS HTTP client in Rust
Apache License 2.0
246 stars 68 forks source link

HTTP Basic authorization option #98

Closed blackghost1987 closed 1 year ago

blackghost1987 commented 2 years ago

This PR is a crude solution to add HTTP Basic auth headers to IPFS requests. This is not a full-fledged merge request, more like a quick hacky solution to show the intent, and to start a discussion on this topic. By the way it would be nice to enable the Discussions section in the repository, it would help with things like this in the future.

My use-case: The paid version of Infura IPFS uses Basic auth on top of the standard IPFS communication to identify clients

The solution is tested and works fine, which is enough for my use-case, but I wanted to see if this feature would be a welcome addition, and discuss what is needed to be done to flesh out this PR into a real solution. So what whould be the optimal way to support this? Should we add it to both Hyper and Actix backends? Where should the config data live? What would be the best way to configure the Backend? Should we add a Builder like solution or is a mutable function fine?

Also I had to add base64 as a new dependency, because Hyper doesn't have built in Basic auth support. Hyper is intentionally a low-level crate, but it's higher level counterpart Reqwest has support for it. I thought a single feature doesn't really worth the hassle of changing to a Reqwest backend (or adding it as a new backend), so instead I just added the base64 crate and implemented the Authorization header construction manually.

Let me know your thoughts!

ferristseng commented 2 years ago

Thank you! Will try to look at this soon. IIRC, there was an attempt to make global options configurable via. https://github.com/ferristseng/rust-ipfs-api/pull/87 that could be informative.

So what whould be the optimal way to support this? Should we add it to both Hyper and Actix backends?

I would like to support options for both Actix and Hyper

iamjpotts commented 1 year ago

@blackghost1987 I took a shot at implementing this in #116. Interested in your feedback on it.

blackghost1987 commented 1 year ago

@iamjpotts Thanks, I never got around to flesh this out into a proper MR. I'll check your solution and try it out in our project soon.

iamjpotts commented 1 year ago

@ferristseng thanks for merging #116; I think this one can be closed now.

@blackghost1987 I used the new basic authentication in my ipfs-cp utility for copying files from one IPFS node to another.

blackghost1987 commented 1 year ago

Thanks for your work @iamjpotts and sorry for never getting around to test your solution. The holiday season plus a long illness got in my way. I'm looking forward to migrate my solution to use this feature from the current master branch.