JohnCoker / thrustcurve3

ThrustCurve.org third revision
ISC License
12 stars 7 forks source link

Implement better security for JSON API #19

Open broofa opened 3 years ago

broofa commented 3 years ago

That the following endpoint actually works is concerning for a number of reasons:

http://www.thrustcurve.org/api/v1/getrockets.json?username=mapeki9543@dghetian.com&password=mypassword

(BTW, that's just a temporary account I made that I have no attachment to, so don't really care that the credentials are public here.)

Specifically ...

At the risk of being a bit dramatic, those credentials were almost certainly compromised even before I posted them here. 😦

There are several action items that flow from this:

JohnCoker commented 3 years ago

There are existing users of this API, most of which don't log in, so we should continue to allow HTTP access, at least for the XML endpoints other than getrockets and saverockets. For example, RockSim and OpenRocket both use it and I doubt they would handle a 30x return value properly.

The site already redirects all HTTP access other than the API to HTTPS, see app.js:69. Right now both modes are supported so it's up to the client. I agree any modern client will use HTTPS, but we have some legacy ones out there too.

broofa commented 3 years ago

My concern here is that the way the API is currently presented encourages people to develop new clients that rely on these bad-practices, which just exacerbates the problem. It'd be nice if there were a path forward that took this into account.

I agree any modern client will use HTTPS, but we have some legacy ones out there too.

I think you're on to the right approach in your comment in #18. Keep the current v1 API in place for legacy clients, and treat v2 as "the future". Once v2 is available, the v1 API can be deprecated (in documentation if nowhere else).

JohnCoker commented 3 years ago

I think you're on to the right approach in your comment in #18. Keep the current v1 API in place for legacy clients, and treat v2 as "the future". Once v2 is available, the v1 API can be deprecated (in documentation if nowhere else).

Feel free to take over the Swagger spec for the v2 API.