OSDLabs / Encore

Intranet Music Player
http://10.4.9.32:2000/
MIT License
18 stars 13 forks source link

Implement client playlists #17

Open coditva opened 7 years ago

coditva commented 7 years ago

Addresses #5 Do not merge. Not complete. Will keep adding commits as things are done. What works:

What I want to know is,

0xRampey commented 7 years ago

Just append /admin at the end of your URL to access the admin panel. A DB entry would be a good idea if Sharing playlists was a login-only feature. Then every registered user would have his own DB record of playlists. Is that what you had in mind?

coditva commented 7 years ago

Arey I know about the '/admin' thing. What I'm asking is, Is there a check in the code such that, say, a command is run only by an admin... Yes, it'll be basically one db for all 'user-playlists', and each user will have an array field for storing the playlist IDs...

SebastinSanty commented 7 years ago

Actually the socket is open for everyone (we need to close it to avoid breach). We have only created a check in the front-end template.

coditva commented 7 years ago

@SebastinSanty, Will do that too...

SebastinSanty commented 7 years ago

This is the check https://github.com/OSDLabs/Encore/blob/master/views/index.html#L23-L25

0xRampey commented 7 years ago

@SebastinSanty Hard scan is registered to one of the socket events in the backend right? @UtkarshMe Why don't you include your middleware functions inside the socket.on listener for all admin events?

SebastinSanty commented 7 years ago

@prampey Yes, it is

0xRampey commented 7 years ago

I think it pertains to socket.on('scan_update' in the socket.js file.

coditva commented 7 years ago

Isn't socket.js public? Also, I am unable to figure out a way to get user session data (ie username or id) in the socket events in the routes/index.js file... :/

SebastinSanty commented 7 years ago

@UtkarshMe Yeah, it is public and that's the downside. This can be exploited only if the enduser makes a socket-client and connects (rare case). After our initial release, we can create an authenticated socket connection, to avoid external interference

This? https://github.com/OSDLabs/Encore/blob/master/routes/index.js#L110-L132

0xRampey commented 7 years ago

Even though it's public, you can use the middleware functions to ensure access to only an admin or registered user right?

mukkachaitanya commented 7 years ago

I got two ways to solve this issue :

  1. Use neDBSessionStore in the sessionOpts and access user details in the socket calls. (Ref)
  2. Use passport.socketio to get the current user details in socket calls. (needs a little setup I think, but good option)

So based on above we can have two ways to store/get playlists:

  1. Store playlists for each user on ther server and access using either of the above methods.
  2. Store playlists on users system. ( Ignore this in fact)

Also we might be able to tackle the admin problems with this.

So yeah if you people agree with me on this then we can continue with either of the ideas or improve on them.

coditva commented 7 years ago

@SebastinSanty, that works for a function that takes req as an argument (ie a middleware), but it fails for socket... I think passport.socketio, like @mukkachaitanya suggested, should work...

SebastinSanty commented 7 years ago

Oh yes, I agree. I didn't notice the socket events you mentioned.

mukkachaitanya commented 7 years ago

I think neDBSessionStore would be better in long run as we will be able to recommend songs and even implement public/share playlists better this way (I think so!).
Though I am not against passport.socketio, as at first glance even I thought this would be best.