hmis-tools / hmis-api-server

Version 2 of OpenHMIS
Mozilla Public License 2.0
15 stars 11 forks source link

Cache the Google identity associated with an id token, for a set period of time. #68

Open kfogel opened 7 years ago

kfogel commented 7 years ago

In src/main/java/org/openhmis/util/Authentication.java, in the method Authentication:googleAuthenticate(), we call token.getPayload().getEmail() to get the human-readable Google identity (i.e., email address) associated with a particular id token (i.e., value received via an HTTP "Authorization" header).

This means we do a round-trip with Google every time we need to authorize. That is, since our authorization code is based on the email address (in the TMP_USER table), but what we receive in the Authorization header of an incoming request is just an id token, we must expensively re-resolve the id token into an email address on every request.

Instead, we should cache the id_token->real_identity mapping and just look it up locally.

Implementation:

In Authentication.java, make a new method, resolveIdentity(idToken), that returns an externalID as a string. That method looks up the idToken in the database. If the entry is present and still valid -- has not timed out -- then just return the corresponding externalID. If the entry is absent or has timed out, then do the lookup with Google and insert or update the database appropriately.

Finally, expose this method via an endpoint so that issue #67 can be fixed in the obvious way :-).

cecilia-donnelly commented 7 years ago

This proposed update could also handle the fact that we need to choose a timeout length for sessions. Google id tokens do expire (see an explanation here), but Authentication.java simply checks whether the token is related to a certain user. @kfogel and I talked about this a bit this morning and @slifty and I talked about it back in March:

<slifty>    we may want to create some kind of extra thing though that does require regular refresh of that [id] token
<cdonnelly> Oh you think so?
<slifty> i.e. storing a local "when is the last time they verified with google who they were"
<cdonnelly> mmhmm.  gotcha.
<slifty> but... idk... I do think having to do the oauth cycle every single query isn't the intent of oauth
<cdonnelly> not every single query, no
<cdonnelly> or I wouldn't think so
<cdonnelly> but I could see like "your session expired" once a week or something
<slifty>     yeah -- I think that would all be on our end though
<slifty>     if I understand correctly
<cdonnelly> I've been using the same id token for a month without a problem, so yes I think so
<slifty>    so we would require reauthentication explicitly on our end
[...]
<slifty> I'm saying, some day when we do real authentication I can just build in a local "last authenticated" field in our user table
<slifty> and if that value is older than a week, we reject their query
cecilia-donnelly commented 7 years ago

Later, we'll add a timeout for items in the cache so that users will need to re-log in at some predetermined interval. When we do so, we'll change the workflow to the following: