Update token gen

[yakkomajuri]

Changes

On a call right now so might still need to look into tradeoffs. A hex token should not contain invalid chars for cookies though, at the expense of being longer.

Checklist

• All querysets/queries filter by Organization, by Team, and by User
• Django backend tests
• Jest frontend tests
• Cypress end-to-end tests
• Migrations are safe to run at scale (e.g. PostHog Cloud) – present proof if not obvious
• Frontend/CSS is usable at 320px (iPhone SE) and decent at 360px (most phones)
• Breaking changes are backwards-compatible. Ensure old/new frontend requests work with new/old backends, and vice versa.

[yakkomajuri]

hmm seems the hex token is a bit too long for the column as is

Also, here's the relevant line: https://github.com/PostHog/posthog-js/blob/cbb31c2e64d7f8f47921b801ec1a5b62b1052ff4/src/posthog-persistence.js#L51

And issue: PostHog/posthog-js#249

[Twixes]

It could be a bit shorter if instead of secrests.token_foo we used secrets.randbits(32*8) to get an integer, and then saved that integer in base 36, maybe?

[yakkomajuri]

A few approaches here:

Could diverge Personal API Key (needs to be more secure) and Project API key so that personal keys can be base 64. I'm against this though because we might want to put personal api keys on a cookie some day (if even test keys for Docs autofill, like Stripe).

Another approach is to keep as is and update personal api keys to 64 char column.

And yet another (just saw @Twixes' comment as I started to write) is indeed base36. That's probably a good idea.

[yakkomajuri]

Base 36 indeed would allow us to keep the 50 char limit as well

[yakkomajuri]

I guess we could have posthog-js convert the b64 token to b36 to address this issue for projects create before this change

[Twixes]

V nice. I modified the number_to_b36 function a bit to actually convert to base62 – even shorter keys. And added token prefixes: phc_ for project tokens used by client libraries, 'phx_` for personal API keys offering full account access – inspired by GitHub's latest changes (though definitely not as rigorous, checksum and all, as they did). All completely under the 50 characters limit still.

Project

Personal

[yakkomajuri]

Nice! Base62 works and is shorter. Neat solution.

[yakkomajuri]

Also, thanks for adding the prefix. I spent a bit of time last night wondering about some changes to be made to API authentication, and having this prefix helps as we introduce new keys with different prefixes (I'm about to spec this out in an issue).

[yakkomajuri]

unrelated cypress component test - merging