tokens.api_key holds API keys in cleartext, and authentication compares them directly:
internal/migrations/migrations_next.go:194 declares api_key as a plain text column.internal/database/principal.go:56 authenticates with SELECT principals.id FROM principals JOIN tokens ON tokens.name = principals.id WHERE tokens.api_key = $1, reached from internal/server/server.go:787 on every API request.
There is no hashing anywhere on that path, and these keys map to roles up to administrator.
#144 already considered where tokens live and decided they are "put into the database, in expanded form", for two reasons: env vars would be surprising in a v1/tokens API (#15), and token definitions should be shared across OCP instances rather than depending on the environment each instance runs in. I am not arguing against either, and the cleartext column predates that PR in any case.
What I want to raise is orthogonal to that decision. Expanded-versus-env-var and cleartext-versus-hashed are separate axes, and #144 only settled the first. A digest is not an env var, so it does not reintroduce the surprise #144 was avoiding in #15, and a digest is identical across instances, so shared token definitions still work. Storing a hash rather than the key satisfies both goals in #144 and changes only the at-rest property.
The change looks small: GetPrincipalID does an exact-match lookup rather than a range query, so hashing the presented bearer token and comparing digests needs no change to the query shape.
The exposure is that read access to the database, a backup, or a read replica yields every key, and they are replayable as-is against the API.
Happy to send a PR if you would like one.
tokens.api_keyholds API keys in cleartext, and authentication compares them directly:internal/migrations/migrations_next.go:194declaresapi_keyas a plain text column.internal/database/principal.go:56authenticates withSELECT principals.id FROM principals JOIN tokens ON tokens.name = principals.id WHERE tokens.api_key = $1, reached frominternal/server/server.go:787on every API request.There is no hashing anywhere on that path, and these keys map to roles up to
administrator.#144 already considered where tokens live and decided they are "put into the database, in expanded form", for two reasons: env vars would be surprising in a
v1/tokensAPI (#15), and token definitions should be shared across OCP instances rather than depending on the environment each instance runs in. I am not arguing against either, and the cleartext column predates that PR in any case.What I want to raise is orthogonal to that decision. Expanded-versus-env-var and cleartext-versus-hashed are separate axes, and #144 only settled the first. A digest is not an env var, so it does not reintroduce the surprise #144 was avoiding in #15, and a digest is identical across instances, so shared token definitions still work. Storing a hash rather than the key satisfies both goals in #144 and changes only the at-rest property.
The change looks small:
GetPrincipalIDdoes an exact-match lookup rather than a range query, so hashing the presented bearer token and comparing digests needs no change to the query shape.The exposure is that read access to the database, a backup, or a read replica yields every key, and they are replayable as-is against the API.
Happy to send a PR if you would like one.