feat(auth): Add key previews to personal API keys #22362
Conversation
|
The mask was inspired by OpenAI API keys page, let me know if you want this changed |
|
Ah @benjackwhite This is not is clear in the code, but the |
There was a problem hiding this comment.
A few suggestions, but in principle sounds good to me!
It's effectively a sacrifice of ~24 bits of entropy, out the key's 256 bits total, so we could also increase the key's random part to 280 bits. Let's do that in the generate_random_token_personal() function, by changing generate_random_token() to generate_random_token(35)
Twixes
changed the title
There was a problem hiding this comment.
Made a few small tweaks – the PR looks good to me! Will wait for a second opinion from @benjackwhite though
There was a problem hiding this comment.
Ah yeah - shouldn't have been trying to review this on the go 😅 Yeah we need the extra field indeed and overall this looks good.
Only remaining issue will be the double migration. I think our CI requires a max of one migration per PR so needs to be combined into one
|
Hey @benjackwhite, I have updated the migration code 😄 |
|
I could imagine some of the tests failing as they check the returned payload so that might be the final thing to get this over the line https://github.com/PostHog/posthog/blob/master/posthog/api/test/test_personal_api_keys.py#L27 |
|
Have updated the tests as you pointed out, just in case 👍 |
* added a way to preview the key value without exposing the key * updated api keys preview * Make the UI a bit nicer * Write out the reasoning behind personal API key entropy * Include underscore in masked value * updated migrations for API key mask * updated personal api keys tests --------- Co-authored-by: Michael Matloka <michal@matloka.com>
Problem
Fixes issue #22360
Changes
Does this work well for both Cloud and self-hosted?
Works on self-hosted.
How did you test this code?
Ran in codespaces on localhost.