Generate key material when minting an API key for a user - #2903
Merged
Conversation
The mint endpoint called the store directly, which requires Key to be set already and rejects an empty one with "key not specified" — so every call 500'd. Caught probing the live endpoint. Use the controller, which generates the key and stamps the owner from the user passed in, exactly as the self-serve create path does. Spec-Ref: helix-specs@6789a48:002391_fix-these-critical-bugs
DefaultWrapper flattens every error to 500, so the endpoint's 404 for "this person has no Helix user" was indistinguishable from a genuine failure. An orchestrator has to tell those apart: no account is the expected case for someone who signed up there but never used Helix, and it should fall back quietly, not log an outage. Spec-Ref: helix-specs@6789a48:002391_fix-these-critical-bugs
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
2 participants
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.
Follow-up to #2902 — that endpoint 500s on every call.
It went straight to
Store.CreateAPIKey, which requiresKeyto be populatedalready and rejects an empty one:
Caught by probing the deployed endpoint on meta.helix.ml rather than by the unit
tests, which mocked the store and so happily accepted a key with no key in it.
That's the tell: the mock was asserting my assumption back at me.
Controller.CreateAPIKeyis the correct path and what the self-serve createhandler already uses — it calls
system.GenerateAPIKey()and stampsOwner/OwnerTypefrom the user passed in, so handing ittargetUserkeeps the"owned by them, not by the admin who asked" property that #2902 was built around.
Testing
go test -run TestAdmin ./pkg/server/passes. The mint test now builds a realControlleraround the mock store instead of calling the store directly, andasserts the created key actually has key material — the thing whose absence the
previous version couldn't see.
Verified against the live endpoint that the route is deployed and reached my
handler; will re-probe once this ships to confirm a key actually comes back.