search

junlarsen / league-connect

:electric_plug: Node.js HTTP/1.1, HTTP/2.0 and WebSocket interface to the League of Legends Client APIs
https://www.npmjs.com/package/league-connect
MIT License
156 stars 23 forks source link

feature: provide signed certificate for node #35

Closed junlarsen closed 3 years ago

junlarsen commented 3 years ago

Right now, the node version ignores any certification rejections for any requests.

The LCU docs provide a riotgames.pem certificate which could be passed along with the requests and in return remove the certification rejection flag for the http requests.

themaxdavitt commented 3 years ago

I know I'm bumping an old issue, and I only tested this for the official client API, but this should be pretty easy to implement. It looks like ws allows for using a custom https.Agent ("Any other option allowed in http.request() or https.request()") and so does node-fetch. I'll open up a PR if I end up using this package.

junlarsen commented 3 years ago

Sounds good, let me know if you want me to assign you this issue if you end up using the library.

themaxdavitt commented 3 years ago

I think I'll use it, feel free to assign me this issue. Do you want me to commit the certificate to the repo or download it in one of the npm scripts or..?

junlarsen commented 3 years ago

Great, glad to see that you're interested. As for the sourcing of the certificate I think it's best to download it through an npm script. We don't know if the URL or certificate may change and hard-coding that would become a maintainability burden and it means that programs could fail at any time.

To make the addition as backwards-compatible as possible I suggest we add a certificate field to the credentials object which is returned by authenticate and then each API will consume this field to access the certificate. We could then have authenticate() take an optional object accepting the path to the .pem file.

themaxdavitt commented 3 years ago

That sounds like a great way to handle it with respect to backwards-compatibility and maintainability. I'll have a PR ready sometime soon!

themaxdavitt commented 3 years ago

This issue can be closed now. :)

junlarsen commented 3 years ago

Resolved in #44 and released under 5.2.0