fix(website): use session cookie instead of ID token #188
Conversation
google-cla
bot
added
the
cla: yes
|
N.B: @engelke this does work with the API's |
| @@ -46,12 +46,16 @@ def login_post(): | |||
| # Only allow sign-ins with tokens generated in the past 5 minutes | |||
| return flask.abort(403, "Token deadline exceeded.") | |||
|
|
|||
| # Configure response to store ID token as a session cookie | |||
| # Create session cookie | |||
| # See https://firebase.google.com/docs/auth/admin/manage-cookies#create_session_cookie | |||
There was a problem hiding this comment.
follow-up: should we redirect to login if the session cookie is expired, per the documented sample?
There was a problem hiding this comment.
This route only executes upon POST /login.
(I don't think we currently require login for any views; we just hide links to them in the UI.)
There was a problem hiding this comment.
There are two layers of auth here: authenticating with the website and delegating your identity to the website to make API calls.
For the first one, I assume we are doing something like https://firebase.google.com/docs/auth/admin/manage-cookies#verify_session_cookie_and_check_permissions and if that fails, I'm suggesting like the sample we direct users to log in.
There was a problem hiding this comment.
That redirect makes sense on a non-/login page - but this code only executes on POST /login.
Basically, all that would happen is:
- User does
GET /login - Firebase JS does
POST /login - If auth fails, user is redirected to
GET /login
(Worse still, that may create an 🚨 infinite loop! 🚨 )
| response = make_response(redirect("/")) | ||
| response.set_cookie( | ||
| "session", | ||
| id_token, | ||
| session_cookie, |
There was a problem hiding this comment.
I'm glad we've found the right model to persist website authentication, but this doesn't seem like something the API should speak. Should the session cookies be used to create the expected Identity Token?
Every authentication mechanism the API supports represents testing, documentation, maintenance overhead.
There was a problem hiding this comment.
This page seems to say that session cookies are preferred for initial auth.
I'm not sure about minting fresh ID tokens, but a quick internet search didn't reveal anything along these lines - so I'd guess that this isn't supported.
(Perhaps we could confirm all this with a Firebase person?)
There was a problem hiding this comment.
Session cookies to me mean HTTP session cookies, which means the browser forgets them when it is closed. I think that Firebase here is using that term to mean something else: long-lived ID tokens. Which is fine. These cookies will still eventually expire, so we might want to eventually have the website tell the user clearly that they have been logged out (after 5 days or whatever) and to try again.
This would explain why the API has to try to verify them as OAuth2 tokens or Firebase tokens. They have similar structures but are signed with different keys.
These tokens won't be accepted by Google APIs, which is perfect. That's what we want, a token that only provides identity which our own API will then use to decide authorization.
We need to test these over hours or longer, but I think you've found the solution.
There was a problem hiding this comment.
@engelke agreed. (I didn't think clear-on-close was the default, but I stand corrected.)
RE testing: we can always mint session cookies with a short expiry period (e.g. 5 minutes) and test those.
There was a problem hiding this comment.
These cookies have a 2 week expiration, which is a pretty standard "login period" for a website. That seems good to me.
I'm less sure we should directly use that cookie value to make API requests. It seems to me that a token that lasts for 2 weeks should be moved over the wire as little as possible. This might make a good, specific question to ask an expert.
|
I'm not sure what this is doing. Is the Firebase session cookie value being updated automatically and transparently by Firebase? Because it looks like the value in the request received by the website is the ID_TOKEN, just as before. Have we checked that this is updated when the token expires? |
|
If this header is refreshed upon login, that's better than before, where the cookie value persisted. But it's still going to expired. Will Firebase just refresh it transparently, or will this force the user to log in again when it expires? |
The intention is the cookie is valid for 2 weeks after login, then on two weeks when you go to the page, you need to re-login. A two week timer is pretty typical, this is what happens when google periodically asks you to log in. (Hence my point about login redirect as a next step, the browser sends the cookie, when the website realizes it's invalid we should send the user to a login page instead of throwing an error) |
|
Revisiting this - after chatting with the Firebase folks, I think the right move here is actually to use ID tokens, and to auto-renew those periodically (e.g. call @engelke I'm 👎 on merging this, unless it unblocks your progress towards Internal Preview. (If it does, we can merge + do a follow-up PR later.) |
Session cookies are longer-lived, and are what we should've been using anyway.