NBISweden / encam

Encyclopedia of Cancer Microenvironment
http://encima.one
2 stars 2 forks source link

Feature/content management #100

Closed danr closed 3 years ago

danr commented 3 years ago

The idea is to put a link on the webpage to go to /api/login for the user to login via google. The user is then redirected to /. The endpoints that are protected to are POST to /api/contents.{,staging.}json and will only succeed if the user is logged in and has whitelisted email. The login status can be obtained from /api/login_status to be presented to the user. This is mostly for convenience to see if you will be able to save if you try.

@pontus does this look reasonable?

pontus commented 3 years ago

I'm not sure I understand the primary use case for this.

My primary impulse is that I'd prefer to rely on the flask session rather than doing the roundtrip to google for each call where identity needs to be verified. (Note though that the default, session on client gave me problems, causing me to switch Flask-Session for server based sessions for herdbook).

In principle this should work, though (but there are a few cases that are good to document the decisions for, e.g. expired token or issue connecting to google and so on).

danr commented 3 years ago

The use-case is for the project members to be able to update the contents on the webpage. A project member logs in on the webpage and can then edit text content on the page and make it live on their own. The edits are done in the frontend and POSTed to the protected endpoints.

Is there a roundtrip to google at each google.get? I thought that it should be able to verify using only known public keys. It doesn't matter much though since this will only be used sparingly by few users.

pontus commented 3 years ago

Looks reasonable in principle (see comments for some opinions). One thing to watch out for might be if other stuff is put in the session (not visible in this PR), it might grow beyond the cookie size limit and stop functioning.

pontus commented 3 years ago

The use-case is for the project members to be able to update the contents on the webpage. A project member logs in on the webpage and can then edit text content on the page and make it live on their own. The edits are done in the frontend and POSTed to the protected endpoints.

Is things saved automatically somehow or is the typical usage working for half an hour and then saving explicitly?

Is there a roundtrip to google at each google.get? I thought that it should be able to verify using only known public keys. It doesn't matter much though since this will only be used sparingly by few users.

I haven't verified the code but the way I read the documentation, yes. (google will be a LocalProxy for a requests object that's preloaded with details).

For extra clarity; I'm asking for two reasons; does the added latency matter (sounds like not) as well as if the behaviour on connection issues, token expiry and so on is what you want.

danr commented 3 years ago

Thanks! Many good points!

Is things saved automatically somehow or is the typical usage working for half an hour and then saving explicitly? I think let's have it auto-save periodically to contents.staged.json.

For extra clarity; I'm asking for two reasons; does the added latency matter (sounds like not)

No, I don't think so.

as well as if the behaviour on connection issues, token expiry and so on is what you want.

Connection issues: with this setup the user needs to reach the server and the server needs to reach google. I think it is ok to assume that both of these will work. (No working offline)

Token expiry: this I don't know much about. What is the default? I have just assumed that the token will be valid for a long time. I was under the working assumption that it would be essentially indefinite, but tokens that are valid thoughout one editing session is also OK (~ 1h). Does this work with this kind of auth?

danr commented 3 years ago

What about versioning config.ini.default instead of config.ini so that one does not commit secrets?

pontus commented 3 years ago

Token expiry: this I don't know much about. What is the default? I have just assumed that the token will be valid for a long time. I was under the working assumption that it would be essentially indefinite, but tokens that are valid thoughout one editing session is also OK (~ 1h). Does this work with this kind of auth?

I had to check. The token I get for herdbook login was valid one hour.

I believe it works, but would personally opt for a solution where the third party is only used for logging in and the session is kept on the server instead.

danr commented 3 years ago

I had to check. The token I get for herdbook login was valid one hour.

I believe it works, but would personally opt for a solution where the third party is only used for logging in and the session is kept on the server instead.

Thanks for checking!! I think only valid for one hour renders this solution unappealing. Do you have the resources to make it so that it is stored in the session instead? Or perhaps @dbampalikis ? /Dan

pontus commented 3 years ago

Thanks for checking!! I think only valid for one hour renders this solution unappealing. Do you have the resources to make it so that it is stored in the session instead? Or perhaps @dbampalikis ? /Dan

What I believe is simplest ("best") is using something to get server side sessions (e.g. flask-session, https://flask-session.readthedocs.io/en/latest/ and control lifetime for that. With that, one can register who has logged in in the session when done).

(Herdbook uses flask-login as well, but I'm not sure I see any benefits for your use case for that.)

By defaults flask-session uses uuid to generate session names, so they can be expected to be hard to guess (traffic needs to be encrypted to make sure cookies can't be stolen in transit).

danr commented 3 years ago

Added new commits from last week that updates the backend in particular putting the content in a bind mount. We're using a subdir /config/content in /config mount for simplicity. There are some permission issues with docker so we're trying to put liberal permissions on the content dir and its files.

danr commented 3 years ago

I removed the temporary files and moving them because it was to fiddly.