ipfs / pinning-services-api-spec

Standalone, vendor-agnostic Pinning Service API for IPFS ecosystem
https://ipfs.github.io/pinning-services-api-spec/
Creative Commons Zero v1.0 Universal
100 stars 27 forks source link

Authorization safety #37

Closed obo20 closed 3 years ago

obo20 commented 3 years ago

I noticed that we're allowing the user to pass their authentication tokens via query parameters to the API instead of using a bearer token. Is there a specific reason for this?

There's a few security issues with passing credentials via query parameters in certain environments. A few notable ones:

I'm not sure the full context in which this API is going to be utilized but this was something I at least wanted to point out.

lidel commented 3 years ago

IIRC it was suggested in https://github.com/ipfs/pinning-services-api-spec/issues/6#issuecomment-655056961:

Authorization: Bearer <key> has the benefit of limiting the problems you’re taking on. If you accept it via header and querystring you’d have a fast path to adoption in pretty much any client.

@mikeal how strongly do you feel about querystring? Removing qurystring and leaving http header only would simplify the spec and remove concerns mentioned above. I believe most of HTTP clients will be capable of setting Authorization: Bearer <key> header anyway, including fetch in browser contexts.

mikeal commented 3 years ago

There’s a tradeoff either way so I would just go with whatever is easiest.