search

heroku / identity

[DEPRECATED] Login and OAuth management service for Heroku
https://id.heroku.com/
MIT License
246 stars 20 forks source link

Revert "Remove user id from cookie" #237

Closed hannesfostie closed 7 years ago

hannesfostie commented 7 years ago

This reverts commit 0317fa67d6c0c7a63d4ccd5240e14af4b089661f.

We're adding the user_id to the cookie used by Glostick (introduced here). This will give us greater confidence using the correct user records when tailoring content to specific users. An example that is a WIP is showing relevant Dev Center articles upon (logged in) users visiting the Dev Center homepage.

/cc @heroku/marketing-web-ops

raul commented 7 years ago

Trying to come up with worst-case scenarios: do we know how this change would affect the existing encrypted details @hannesfostie? Perhaps we should test if the user_id will be cleanly decrypted as nil and discard that this could mess up the existing encryption "corrupting" the other encrypted fields?

What can we do if that happens? Perhaps "revoke" the existing sessions changing the session's seed token forcing everyone to login to go through the new encryption process?

hannesfostie commented 7 years ago

@raul the fernet is a simple encrypted value, and decrypting it will either show the heroku user id if it's in there, or it won't. There's no schema or anything that would add the key but not the value like you fear (if I understand the first question)

From looking at the code I don't think anything would be overwritten. We could indeed change the session secret to trigger cookies to go invalid, but I'm not sure if it's worth it. I'll let others weigh in here.

I'm not opposed to just waiting for the 30d to pass (that's the default - I'm not sure what it's actually set as in the prod ENV vars) so all existing cookies expire, and only then switch over to user id instead of email in our apps.

elight commented 7 years ago

:+1: