pebble-dev / rebblestore-api

Rebble Store api code.
MIT License
39 stars 8 forks source link

Add barebones account system + some fixes #28

Closed azertyfun closed 6 years ago

azertyfun commented 6 years ago

Okay boys, this is a big one.

Account system

The account system lays the foundations to addressing #21. Right now, it only supports creating, updating and logging in to accounts, but the foundations are there to add, quite easily, more features.

The implementation is described quite thoroughly in docs/account_system.md. Here are the relevant parts:

The account management is homebrew, except the password hashing which is handled by bcrypt (adaptative hashing algorithm).

Expected client behaviour

The user can choose to register an account or login to an existing one. Both return a session key (see API below), which must be stored locally (via cookies for instance).

Session keys expire after a certain time spent without being used, or if 5 other sessions have been opened since the last time the key has been used. If a key is expired, a re-login is needed.

The client should then, every time a page is loaded, check the current status of the session by hitting the /user/status endpoint (see API below). Actions which require an account to be completed will further require a session key.

Move to TLS only

For what I hope are obvious reasons, the backend now requires an https connection. Development is done using a self-signed certificate (the README has steps on how to create one).

Request for comments

I want to preface this by saying that I have no background on security. I did what I could to make the handling of user data as safe as possible (it currently doesn't store any email addresses, and passwords should be correctly hashed using bcrypt), but I can't guarantee I didn't fuck up somewhere. Therefore, I would like you to be extra careful and judgemental when reviewing this code.

Documentation

I have also done some documentation of the endpoints. This should allow the frontend guys to add features more easily now. It can be found in the docs/ folders, in markdown format.

Fixed database locking

All calls to db.Begin() (which creates a SQL transaction) are now followed by a defer tx.Rollback(). It will silently fail if the changes were already committed, and roll back any changes otherwise. The idea is to prevent database locking, as exiting a function before a rollback or commit would leave the transaction open, blocking any further transaction indefinitely.

ishotjr commented 6 years ago

@azertyfun this is fantastic!!!! :metal: :bowtie: :metal: :rocket:

azertyfun commented 6 years ago

I have now added OpenID support. It is trivial to add an OpenID-compliant provider, as all relevant endpoints are pulled from the discovery file (unless you are like yahoo and don't set CORS header, so right now their discovery file is hardcorded for the frontend). You just need to create an application and copy the keys in the relevant config files (the README has information on how to do this, as usual).

However, I am disappointed at the state of OpenID support; it seems that Fitbit doesn't support it (they recommend using plain OAuth2 for this purpose, which would make the code a lot less elegant), Microsoft requires a (very expensive) Azure subscription, and Yahoo don't allow http://localhost as a valid domain name for redirects (really?). At least this last one shouldn't be a problem once deployed.

We could also add Auth0 as an OpenID provider, as it should (as far as I understand it) act as a middle man between all varying OAuth2-based authentication methods, and our OpenID app.

If you have a free (as in freedom) OpenID provider to suggest, this could also be a good idea.

EDIT: The key verification step is incorrect for now, please don't merge until I fix it EDIT2: fixed!

azertyfun commented 6 years ago

And the authentication system is now its own service! The backend officially doesn't process any account information whatsoever anymore. The idea is that, now, the backend will call {auth}/user/info with the access token given to id by the frontend to authorize requests.

https://github.com/azertyfun/rebble-auth (this will need to be added to the rebble repo list before merging the two PRs)

azertyfun commented 6 years ago

We are going to use pebble-dev/rebble-auth-py instead. Closing this, as it is not relevant anymore.