Jeffail / leaps

A pair programming service using operational transforms
MIT License
751 stars 55 forks source link

Refactor Authenticator to always get documentId, userId and token #8

Closed jonasfj closed 8 years ago

jonasfj commented 8 years ago

Many authentication strategies may wish to: 1) Validate that token allows the client to assume a given username 2) Treat document ids as namespaces and allow access based on prefixes

In particular when considering JWT (which is seems like a very appropriate option), one could create a JWT token that is effectively an signed JSON object with properties such as:

{
  userId: "....", // So users can't assume a false userId
  readWrite: [
     // Documents that this token grants read-write access to
     "somefolder/mydocument", "somefolder/mydocument2", "somefolder2/mydocument", ...
  ],
  readWritePrefixes: [
     // List of document id prefixes token grants read-write access to
     "personal-folder/"
    // In this example token would grant read-write access to any document
    // where the id is on the form "personal-folder/....",
    // Example; "personal-folder/my-file", "personal-folder/f/file2", is granted
  ]
  readOnly: [
     // Documents that this token grants read-only access to
     "id1", "id2", "id3", ...
  ],
  // readOnlyPrefixes, create and createPrefixes similar to readWrite above
}

All of this logic can be implemented in authentication strategies... But the JWT option is clearly the best as it would allow us to keep all the authentication logic on a different server. Some people might want to use LDAP access, or github auth, or just public documents (or just a public namespace).

jonasfj commented 8 years ago

@Jeffail, if you agree I think we should refactor Authenticator to be on the form:

type AccessLevel int
const (
  CreateAccess AccessLevel = iota,
  EditAccess,
  ReadAccess,
  NoAccess
)
type Authenticator interface {
    Authorise(token, docId, userId string) AccessLevel
    RegisterHandlers(register register.PubPrivEndpointRegister) error
}

Such that CreateAccess implies both EditAccess and ReadAccess... and similarly EditAccess will imply ReadAccess.

jonasfj commented 8 years ago

@Jeffail, I'm curious why does authentication happen at binder level? Wouldn't it make a lot more sense to do this at curator level.

I see that store.Fetch/Store/Create has to be at binder level, but keeping auth there just seems to complicated...

Jeffail commented 8 years ago

This looks good. Adding read-only support was a very last minute consideration, but with it included it probably makes sense to refactor authentication, your suggestions seem great from first glance. I'd say this should be done before client API changes (https://github.com/Jeffail/leaps/issues/4) since they will be impacted by this.

The way that the binder uses authentication tokens is purely as a unique identifier for each websocket, you're right that this is an awkward coupling. I'll make a separate issue for this.

Jeffail commented 8 years ago

Authentication has been refactored in https://github.com/Jeffail/leaps/pull/14