search

rwieruch / favesound-redux

🎶 A SoundCloud Client in React + Redux running in production. Live Demo and Source Code to explore React + Redux as a beginner.
http://www.robinwieruch.de/the-soundcloud-client-in-react-redux/
1.59k stars 249 forks source link

Refresh signs me out #82

Open ohadschn opened 6 years ago

ohadschn commented 6 years ago

If I hit the refresh button, or close and re-open the page, I get signed out.

I saw issue https://github.com/rwieruch/favesound-redux/issues/44 which might be related, but if this was intentional I would humbly suggest you reconsider - practically all sites I can think of retain the OAuth cookie and keep you signed in as long as the token is valid...

chriszhangusc commented 6 years ago

Yeah, I agree. Currently, the cookie expires when the session is done, maybe we can set an expiration time for the cookie and automatically login when current oauth token in the cookie is valid?

ohadschn commented 6 years ago

I don't think you need to set the expiration for the cookie yourself, you can just use whatever expiration policy you get with the SoundCloud token. In other words just try to use the token they gave you until you get 401, at which point re-authenticate.

chriszhangusc commented 6 years ago

@ohadschn Yes, I think it should work to use the token to check for 401 by the /me endpoint and if we do get a 401, we can remove the oauth token from the cookie and logout the current user.

ohadschn commented 6 years ago

Logout and ideally open up the login window (to save the user from noticing he has been logged out and clicking login button again himself).

chriszhangusc commented 6 years ago

@rwieruch Any suggestions? I think this is a great improvement.

rwieruch commented 6 years ago

Sounds great. Would you like to do it?

chriszhangusc commented 6 years ago

Yeah, I'm on it.

rwieruch commented 6 years ago

@MiniPekka I added you as collaborator to the project. You introduced so many improvements by now, I think it would make sense to have someone else to help me out organizing the PRs :) Feel free to merge things on your own too!

chriszhangusc commented 6 years ago

I have a question about our session state. Currently, our session state holds an oauth_token and a dialog_session. I am not sure if it is necessary to store these two pieces of data because oauth_token is stored in Cookie and dialog_session is never used anywhere. The only place that used session object is checking if there is a current user logged in. I think if we remove oauth_token and dialog_session from the state, we can simply check if the user object is null or not to see if it is authed or not.

rwieruch commented 6 years ago

That's seems right without looking at the code. I don't remember how I implemented it back in the days. So if you have the feeling it could be simplified, please go for it! :)