New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement authentication, access rights and user settings #55

Closed
elisee opened this Issue Jan 14, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@elisee
Member

elisee commented Jan 14, 2016

OK, I think we finally have a decent proposal for how to deal with user authentication and settings in Superpowers. Feedback welcome!

First of all, some goals:

  • Avoid single points of failure and costly (both money- and time-wise) centralized infrastructure
  • Maintain the ability to join a server temporarily without maintaining an account
  • Automate as much as possible so users don't have to do drudge work

And some non-goals:

  • We can't protect users from malicious server owners storing a user's password, because the client JS code is provided by the server. This is a core aspect of Superpowers's design and as such, users should only connect to servers they trust and for added security, use a different, unpredictable password on every server. See #53

Now, with that in mind, here's the proposed design:

Authentication

We'll use passport as the authentication framework, along with a text-based store (with in-memory cache) for user accounts and session objects. We could use sqlite but it seems overkill for the small amount of users and connections a Superpowers server will have. Plus being text-based means one could easily version or duplicate the server membership information, if desired.

The global server password will be re-purposed as an optional key for restricting access to the sign up form and stored as a bcrypt hash. A separate option will be added to open the server to outside connections or not and it will stay disabled by default, as it currently is, to avoid exposing users exposing their projects to the Internet accidentally.

Authentication options

A server owner will be able to enable various login options:

Temporary login: Clients can enter an available guest username, the account will be tagged visually as guest and will automatically expire after an hour. No one can log in to it without the session key.

Local password: Clients can choose an available username and a password. We should mention in the UI that a unique password should be used. The server will store a bcrypt hash of the user's password. They can also upload a profile picture.

Login with external service: Using OAuth, server owners can setup authentication with GitHub, Twitter or whatever other service they want. It is up to the server owner to setup an OAuth id/secret on the external service and fill them in the server config. After logging in with the service, the client will have still provide a local username (though the field can be pre-filled with the one from the external service used to log in). The OAuth-provided profile picture will automatically be used.

Each user account will be stored in a users folder, with the username as the folder name.
It will contain an auth.json that looks something like this:

{
  "auth": {
    "guest": { "expires": "some timestamp" },
    "local": { "passwordHash": "some bcrypt hash" },
    "github or twitter or whatever": {
      "token": "oauth token",
      "pictureURL": "url"
    },
    /* etc. */
  },
  "picture": "url of the active picture if any, otherwise it'll automatically use the local one"
}

We'll store a local picture.png file in the same folder if the user uploads one.

QUESTION: The primary key for the user account would then be its username. Is that a bad thing? Do we care about renaming accounts on servers? If so, where will we reference user accounts in Superpowers? do we need to have a never-changing user ID instead?

Access rights

The server can have one of several default settings:

  • No access to anything (not even the list of projects)
  • Read-only access to every project
  • Read/write access to every project for everyone (does not apply to temporary logins)

Each user can then have custom access rights for each project. That information should be stored in the user's folder, not in project's folder, to avoid versioning it accidentally.

User settings

User settings will be stored in localStorage. Each namespace (similar to resource IDs) will be stored as a separate key, something like superpowers.textEditor. It should have a format version for migration.

Editors interested should subscribe to localStorage events and react to changes as needed.

Each settings tool plugin will be able to add a user-specific section for user-specific settings.

In the browser, the settings tool will provide buttons to import and export user settings as a JSON file.

In the app, the launcher will provide those buttons in the settings instead. The settings tool of a project will notify Electron of changes to the settings so that it can keep the global copy updated and update the localStorage of other origins currently open. And it will update it when opening a server, too.

@rhin0cer0s

This comment has been minimized.

rhin0cer0s commented Jan 14, 2016

We can't protect users from malicious server owners storing a user's password

Like any other web service. The other problem would be "how to know that a server is really the one it annonces ?" and the best solution today remains PKI based even if it is far from enough. Anyway, this introduction is clearly acceptable.

We could use sqlite but it seems overkill

I agree. If you managed to avoid the use of a DBMS so far let's keep it that way.

The global server password will be re-purposed ...

Yep, a password just to access the server is definitely a great idea. I will be glad to offer a PR regarding the password storage and verification (as discuted in #52).

QUESTION: The primary key for the user account would then be its username. ...

I have absolutely no idea for this one. Fixing something is generaly not a good thing but here it doesn't seem really serious. If someone really wants to change its name why not just add a field "nickname" which can be modified freely ?

No access to anything (not even the list of projects)

? So no access to the server ?

This list makes me think : is there an activity log somewhere ?

@elisee

This comment has been minimized.

Member

elisee commented Jan 14, 2016

Thanks for the feedback!

? So no access to the server ?

Ah right, I had something in mind but didn't explain it. The idea would be that the person was invited to sign up, and once they've done it, the server owner defines which project they can access after the fact. It's useful if you want to let a user join a single project and nothing else.

(... We might want to have a more elaborate invite link system that comes with predefined access rights for a particular project at some point.)

This list makes me think : is there an activity log somewhere ?

There is not. We might want to build one though, it seems useful. At the very list it could log connections, maybe high-level changes like project creation or even more detailed like asset tree changes, etc.

@Xstoudi

This comment has been minimized.

Contributor

Xstoudi commented Jan 16, 2016

QUESTION: The primary key for the user account would then be its username. Is that a bad thing? Do we care about renaming accounts on servers? If so, where will we reference user accounts in Superpowers? do we need to have a never-changing user ID instead?

I think primary key should never be a concret information. Create an unique user ID can prevent many problems.

@elisee elisee referenced this issue Jan 17, 2016

Closed

Hash password #60

@elisee

This comment has been minimized.

Member

elisee commented Jan 17, 2016

@Xstoudi yeah alright, let's do that. It's probably for the best.

@elisee

This comment has been minimized.

Member

elisee commented Jul 29, 2016

A much simpler version of this spec has been implemented:

  • We now use passport-local for managing user sessions. The server password is no longer stored in a cookie for each user, which was a terrible temporary hack that stayed around for way too long.
  • We're sticking to just a global server password for now, since it has proven good enough. We might do access rights some day, but for now it's not a priority for anyone AFAICT.
  • Per-server user settings have been implemented using localStorage. It's good enough and much simpler than trying to synchronize them across all servers. We can improve on that later if needed.

So I think we can close this!

@elisee elisee closed this Jul 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment