tomrosenback / botvac

PHP library for Neato Botvac
GNU General Public License v2.0
38 stars 21 forks source link

Use oauth? #5

Open ostinelli opened 7 years ago

ostinelli commented 7 years ago

Hello @tomrosenback! Let me introduce myself, I am Roberto Ostinelli, Neato Robotics' Director of Cloud Services.

I'm impressed with the work that you've put into integrating with our robots! Great job!

Given the interest that we've seen from developers like yourself and around the forums we've listened and have just released the Neato Developer Network. In there, amongst other things you will find official documentation and SDKs for JavaScript, iOS and Android (for now, we plan on expanding to other languages too!).

This issue is for you to consider switching to proper OAuth instead of the internal logins mechanisms that you are using in this gem.

It would be the official way to proceed, and you would avoid some improper implementations. For example, you should not pass token and platform here: they are not needed, raise errors on our systems - since tokens are invalid - and this could eventually result in the blocking of user accounts by our automated systems if abused. We definitely wouldn't want that!

Please note that all of this is in Beta, we're a small team but are doing our best!

All the best, r.

PS: I've opened the same issue on the gem that, to my knowledge, started it all. :)

tomrosenback commented 7 years ago

This is great news! Will make the changes accordingly as soon as possible. Thank you for listening to the community!

ostinelli commented 7 years ago

Thank you @tomrosenback! Let me know if we can help!

tomrosenback commented 7 years ago

@ostinelli I looked through the docs, looks really good. Only thing which I see as an issue is the following.

  1. Creating an app require a redirect URI, as this is just a class library the URI varies between installations
  2. Requiring HTTPS as a redirect URI, I agree on this, but I think there will be many users which don´t have SSL certificates on the their home servers.

Other than that, very good!

@kangguru please have a look on this and give me your points.

ostinelli commented 7 years ago

That was quick, thank you @tomrosenback for the feedback :)

To address your points:

  1. Yes indeed, with the OAuth flow every implementation needs an OAuth application (so that every developer can manage their own).
  2. We are following the RFC 6749 recommendation; simply put, without a TLS layer the whole OAuth mechanism is compromised. I do understand the SSL certificate issue, that being said there are some hosts that do provide it for free on non-custom domain names (for instance, Heroku).

Hopefully this clears up :)

kangguru commented 7 years ago

i also guess that the certificate can even be self-signed or letsencrypt might help users out, as i think that most users are kind of tech-savvy this won't be a big obstacle :) at least i hope so

tomrosenback commented 7 years ago

Ok, agreed.

One more thought. Class libraries as this, can´t they have a shared OAuth App? Or does this compromise RFC 6749 too?

ostinelli commented 7 years ago

@tomrosenback you can't share an OAuth App anyways, since the redirect uri is different per app.

My recommendation would be that the library should take the OAuth application client_id and client_secret to be operable, then you can also host your own service with it somewhere and use your own app.

tomrosenback commented 7 years ago

That is what I thought since I saw the Redirect URI in the app settings. Some OAuth providers accept that you submit in a redirect URI and redirects to this when authentication is done.