Repository navigation
feat(audit): accept a named role when creating or re-scoping an API key - #113
Merged
Merged
Conversation
The audit service has a named role catalogue for API keys (campus#918, campus PR #935): a role name is accepted wherever a scope list is accepted, and resolves to exactly one scope list. The SDK could not express that — APIKeys.new() required a scope list and there was nowhere to name a role — so every caller hand-assembled the scope list the catalogue exists to avoid. APIKeys.new() and APIKey.update() now take a role, and scopes becomes optional so a key can be created by role alone. The SDK does not validate the role, and does not duplicate the catalogue: the service owns it. An unknown role is refused by the server, which names the roles that exist. A role and a scope list together are refused server-side too (they leave it ambiguous which was meant), so this change sends both rather than silently preferring one — the server's answer is the right one.
The role parameter came with docstrings, but the README still described a package layout from before the audit client existed — the `audit/` package (apikeys, traces, the ingest circuit breaker) was missing from the tree, and the audit client was missing from the feature list. The apikeys module now also quotes the service's role catalogue, for orientation: it is explicitly not a contract, since the service is the authority and refuses an unknown role with the names of the roles it does have.
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.
Closes #112
Summary
The audit service has a named role catalogue for API keys (campus#918, campus PR #935): a role name is accepted wherever a scope list is accepted, and resolves to exactly one scope list.
traces-viewertraces:readtraces-searchertraces:read,traces:searchtraces-producertraces:read,traces:search,traces:writekey-operatorapikeys:read,apikeys:writeThe SDK could not express that:
APIKeys.new()required a scope list and there was nowhere to name a role, so every caller hand-assembled the scope list the catalogue exists to avoid.APIKeys.new()andAPIKey.update()now take arole, andscopesbecame optional so a key can be created by role alone.The SDK does not own the catalogue
That is deliberate: the catalogue is a service-side vocabulary, and the SDK is a client of it.
Testing
test_new_posts_a_role_instead_of_scopes— the payload carries the role and no scope list.test_new_sends_both_role_and_scopes_when_given— both reach the server, unmodified.test_update_patches_a_role— the re-scoping path.poetry run pytest: 295 passed. Ruff and pyright report the same counts asmain(43 and 37) — all pre-existing in this checkout, none from this change.Related
--rolesurface that will use this)