opensim-org / opensim-viewer

Front end of web based viewer of OpenSim models, data
Apache License 2.0
13 stars 3 forks source link

Implemented login, logout and register. #76

Closed AlbertoCasasOrtiz closed 1 year ago

AlbertoCasasOrtiz commented 1 year ago

I have implemented frontend pages for login, logout and register, and views in the backend to do this.

I have a question. I have implemented login functionality based in tokens. When a user logs in, the backend creates a token (and stores it in the database), and sends it to the frontend. This way, a session is linked to a specific token per user and the frontend knows if an user is authenticated or not based on the existence of the token. I initially created a state called isLoggedin, that is set to true when you log in, and to false when you log out, but I feel it is useless now. Should we just create a function called isLoggedin, that checks if the token exists?

Note: It is necessary to do makemigrations/migrate, since I used django tokens for the login functionality. Note 2: This is a simple implementation that works. We need to implement timeout dates for tokens as a security measure (so users are not logged in forever), a refresh token mechanism, and ways of checking if the token exists or not in the backend. We could create a function that takes as input the target URL, the parameters, and takes care of everything. Note 3: This function from Note 2 would also take care of detecting if the token is consistent or not with what we have in the backend, since any axios call should return 401 or 403 if the token does not exist, or the access to a resource is forbidden.

netlify[bot] commented 1 year ago

Deploy Preview for comforting-speculoos-d9e100 ready!

Name Link
Latest commit 98d8090ea3ac11baea63201fd2f6fbcddbe96e2e
Latest deploy log https://app.netlify.com/sites/comforting-speculoos-d9e100/deploys/64e3bc2ccf8ee2000874cd19
Deploy Preview https://deploy-preview-76--comforting-speculoos-d9e100.netlify.app/
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

aymanhab commented 1 year ago

Thanks @AlbertoCasasOrtiz I agree the state is unnecessary if we can do without would be better. Let me know if you remove it, and fix lint errors and then I'll merge and build on it for my other downstream PRs. Thank you

AlbertoCasasOrtiz commented 1 year ago

Hi Ayman, I think I have fixed everything. Only two things to mention:

  1. As expected, the backend does not work in the deploy preview, but you can see the frontend views in the following URLS: https://deploy-preview-76--comforting-speculoos-d9e100.netlify.app/log_in https://deploy-preview-76--comforting-speculoos-d9e100.netlify.app/log_out https://deploy-preview-76--comforting-speculoos-d9e100.netlify.app/register (If not logged in, logout redirects to login, so it will show login).

  2. I tried removing the isLoggedIn state, but the rendering of the AppBar depends of the state. I think by now it is good to keep it until I figure out how to render conditionally depending of the value in localStorage. There was also a warning that I supressed, the reason is explained here: https://github.com/opensim-org/opensim-viewer/blob/98d8090ea3ac11baea63201fd2f6fbcddbe96e2e/src/components/pages/LogoutPage.tsx#L53-L57