mizzao / meteor-sharejs

Meteor smart package for transparently adding ShareJS editors to an app
MIT License
225 stars 53 forks source link

Adding authentication #4

Closed charandas closed 10 years ago

charandas commented 10 years ago

@mizzao Nice work on this package. Clean code!

Do you have plans to add authentication using something like this?

OR

Any caveats/recommendations if a brand new node programmer tries to put it in? I know javascript/coffee well enough and have been getting my hands dirty with JS frameworks recently. Meteor is my newest love and I am trying to build a pet project based on sharejs using your smart package.

mizzao commented 10 years ago

Hi @kbdaitch, thanks a lot for bringing this up. Sorry for the slow response, as I have been traveling.

I haven't looked carefully into how the ShareJS authentication system works, but I would love to integrate it with Meteor's accounts at some point in this smart package. Currently, the semi-secure way to deal with documents is to only publish the ids that each user should be able to access - it's very unlikely that they will be able to find an arbitrary document without knowing its id.

However, it would also be useful to integrate things like who has the document open, and who is currently looking at the document, and where their cursor position is - like in Google Docs. The first two things should be easy to do; the latter seems very difficult given my knowledge of ShareJS.

I'd be happy to discuss your thoughts for contributing something useful to improve this package and even help you out with it.

charandas commented 10 years ago

Hi again @mizzao, thanks for your elaborate response.

I took a look at user accounts and it was fairly easy to associate each document in the database with the additional userId key. The documents demo has given me a good start and I was able to remove insecure meteor package off and have this tested.

I feel this package's stripped down nature allows people to add custom features pretty easily. I am afraid, the other features could make it bloated. Your thoughts?

In any case, I would be happy to contribute if the opportunity presents itself.

mizzao commented 10 years ago

Hi @kbdaitch, what do you mean by associating each document with a userId? Would that be the 'owner' of the document? Are you talking about other stuff such as editing/viewing permissions?

How would you propose to use the authentication of ShareJS to restrict connections to arbitrary documents and stuff like that?

charandas commented 10 years ago

@mizzao Oops. I forgot about the ShareJS authentication while implementing some meteor abstractions last night, even though I posted about that method in the first place!

What I did last night was basically inserting a "userId" key into the mongo collection while creating a new document. I removed autopublish and published documents on an ownership basis.

Let me give ShareJS transactions a try soon and get back to you. It might offer me some advantages with auth tokens that might allow sharing docs between the users without me reinventing the wheel.

mizzao commented 10 years ago

@kbdaitch ok - I think we should keep this discussion going if you would like to merge these packages together. It's important to have some reasonable defaults for ShareJS documents (selecting between either globally visible or individually visible.)

charandas commented 10 years ago

@mizzao, I appreciate your openness to continue dialogue. I used sharejs auth mechanism successfully to authenticate via their token method: I just used the Meteor.userId() which I stored in the "documents" collection while storing the document in meteor-documents-demo.

This userId and invitedUsers attribute added to the document collection allows me to invite others to read and edit my docs. Relevant code: here. Please note that I removed the autopublish and insecure package while testing this. My UI for invitations is still pending, FYI.

Smart package changes here. While by no means the most modular or complete (it connects the demo code with the smart package, ugh!), its just one way to accomplish things. Let me know your thoughts.

charandas commented 10 years ago

Deployed to http://documents-users.meteor.com

charandas commented 10 years ago

Would be discussed further in the PR.