Repository navigation
Stop storing API keys as plaintext - #77
Closed
dyk1454683243-sudo wants to merge 1 commit into
Closed
dyk1454683243-sudo wants to merge 1 commit into
dyk1454683243-sudo wants to merge 1 commit into
Conversation
Hash server-side API keys with SHA-256 before they are written to SQLite, migrate existing plaintext potok_ values on startup, and move leftover config.json api_key fields into the OS keyring. Fixes mtiluk#67 Co-authored-by: David <dyk1454683243-sudo@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #67.
Issue #67 is labeled documentation / good-first-issue, but the live secret still sits in SQLite as plaintext. Hashing is the right server-side fix (the CLI must keep a retrievable key, so it stays in the OS keyring). This does not invent a cipher: it uses
crypto/sha256for high-entropy tokens and leaves password hashing on bcrypt.Why hashing, not keyring, on the server
zalando/go-keyring. That cannot be hashed:potok push/pull/doctorhave to send the original bearer token.crypto/randvalues, so a fast cryptographic hash is enough. This is the same approach GitHub uses for personal access tokens. User passwords remain on bcrypt.Changes
Server
CreateUserstoressha256:+ hex(SHA-256(key)) inusers.api_key_hashand returns the plaintext key once.UserByAPIKeyhashes the presented bearer token and looks up the digest. Lookups no longer return a key or hash.0004renamesapi_key→api_key_hash. On startup, any leftoverpotok_…(or other non-hash) values are hashed in place. The migrate step is idempotent.POST /registernow returns JSON{id,email,api_key,created_at}so the live key is shown once without dumpingpassword_hash.GET /meis tested to omitapi_key.Client
api_keyfields inconfig.jsonare moved into the OS keyring on the nextpotokcommand, then stripped from the file. An existing keyring entry wins.Docs
Compatibility
potokdstart.Test plan
go build ./...go test ./...sha256:digest and the returned key still authenticatespotok_keys and stays idempotentconfig.jsonapi_keymoves into the keyring and is stripped from the file