-
Notifications
You must be signed in to change notification settings - Fork 55
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
Refactor Authenticator to always get documentId, userId and token #8
Comments
@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 |
@Jeffail, I'm curious why does authentication happen at I see that |
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 (#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. |
Authentication has been refactored in #14 |
Many authentication strategies may wish to:
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:
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).
The text was updated successfully, but these errors were encountered: