Add account-based zone authorization via PowerDNS#3
Open
mbchristoff wants to merge 4 commits into
Open
Conversation
Adds an `accounts` list to ProxyConfigEnvironment. When set, the proxy resolves zone access by reading the zone's `account` field from PowerDNS on each request, granting read/write within matched zones (no admin, no cryptokeys). Additive with the existing static `zones` list; either or both may be configured. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`/info/allowed` now lists statically-configured zones plus any zones in PowerDNS whose `account` matches the environment's configured accounts. `/info/zone-allowed` now uses the async resolver so account-based grants are reflected in the result, including a synthetic ProxyConfigZone with RW/no-admin permissions. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Promotes account-lookup flow messages to INFO so users can diagnose denied account-based access without enabling DEBUG globally. Now logs: - when the static path misses and account fallback begins - the account value PowerDNS returned for the zone - whether the account matched the environment's allowed accounts Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PowerDNS stores zone names with a trailing dot. Requests for a non-canonical name (e.g. 'example.com' instead of 'example.com.') get back an empty stub rather than the real zone, so the account lookup returned '' and account-based access was denied for any client that omitted the trailing dot. Normalize the zone id to canonical form before the upstream call so account-based authorization works regardless of trailing-dot input. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds dynamic, account-based zone authorization on top of the existing static zones: allowlist. When an environment configures accounts: [...], the proxy can grant access to zones whose PowerDNS account field matches, avoiding the need to enumerate zones in proxy config.
Changes:
- Adds
accounts: list[str]toProxyConfigEnvironmentand updates hashing behavior accordingly. - Introduces an async zone resolver that can authorize a zone either via static config or by looking up the zone’s PowerDNS
account. - Updates proxy endpoints and docs/tests so
/info/*and selected zone endpoints can use account-based access.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
powerdns_api_proxy/models.py |
Adds accounts to environment model and includes it in __hash__. |
powerdns_api_proxy/config.py |
Adds PowerDNS account lookup + zone resolution logic; extends zone listing filter to include account matches. |
powerdns_api_proxy/proxy.py |
Switches several endpoints to use the new async resolver; /info/allowed now includes account-matched zones. |
tests/unit/config_test.py |
Adds unit tests for account parsing, filtering, resolver behavior, and account lookup. |
tests/unit/proxy_test.py |
Adds tests ensuring /info/allowed and /info/zone-allowed reflect account-based authorization. |
README.md |
Documents the new accounts configuration and behavior/limitations. |
config-example.yml |
Adds accounts example configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+179
to
+217
| Resolve a zone for an environment, allowing two access paths: | ||
|
|
||
| 1. Static config: the zone is matched by the environment's `zones` list. | ||
| 2. Account-based: the environment declares one or more `accounts`, | ||
| and the zone's `account` field in PowerDNS matches one of them. | ||
|
|
||
| The static path is consulted first. The account path issues an extra | ||
| upstream call to read the zone's metadata; it is intentionally | ||
| uncached so access reflects PowerDNS state at request time. | ||
|
|
||
| Account-matched zones get RW permissions within the zone (no admin, | ||
| no cryptokeys) via a synthetic ProxyConfigZone. | ||
|
|
||
| Raises ZoneNotAllowedException if neither path grants access. | ||
| """ | ||
| try: | ||
| return environment.get_zone_if_allowed(zone) | ||
| except ZoneNotAllowedException: | ||
| pass | ||
|
|
||
| if not environment.accounts: | ||
| raise ZoneNotAllowedException() | ||
|
|
||
| logger.info( | ||
| f"Static zones do not allow '{zone}' for environment " | ||
| f"'{environment.name}'; checking accounts {environment.accounts}" | ||
| ) | ||
| account = await get_zone_account_from_pdns(pdns, server_id, zone) | ||
| if account and account in environment.accounts: | ||
| logger.info( | ||
| f"Zone '{zone}' granted to environment '{environment.name}' " | ||
| f"via account '{account}'" | ||
| ) | ||
| return ProxyConfigZone(name=zone) | ||
|
|
||
| logger.info( | ||
| f"Zone '{zone}' not granted via accounts: PowerDNS account " | ||
| f"'{account}' not in environment.accounts={environment.accounts}" | ||
| ) |
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.
Summary
Adds a new
accounts: [str]field onProxyConfigEnvironment. When set, the proxy grants read/write access to any zone in PowerDNS whoseaccountfield matches one of the environment's configured accounts — additive with the existing staticzones:list.Why
Lets a single token's access track PowerDNS's own zone tenancy without having to enumerate every zone in proxy config. Re-tagging a zone in PowerDNS is reflected immediately on the next request.
What changed
Model (
powerdns_api_proxy/models.py): newaccounts: list[str] = []onProxyConfigEnvironment(also included in__hash__).Resolver (
powerdns_api_proxy/config.py):get_zone_account_from_pdns(...)— fetches a zone'saccountfrom PowerDNS. Normalizes to canonical form (trailing dot) since PowerDNS returns an empty stub for non-canonical names.resolve_zone_for_environment(...)— tries static config first, then on miss queries PowerDNS for the zone's account. On match, returns a syntheticProxyConfigZone(name=zone)withall_records=True, admin=False, cryptokeys=False(RW within the zone, nothing else).get_only_pdns_zones_allowedextended to include account-matched zones in the/zoneslisting using theaccountfield PowerDNS already returns inline (no extra upstream call for listing).Endpoints (
powerdns_api_proxy/proxy.py): the following now consult the async resolver, granting account-based RW:GET /api/v1/servers/{id}/zones/{zone}PATCH /api/v1/servers/{id}/zones/{zone}(RRset update)PUT /api/v1/servers/{id}/zones/{zone}/notifyPUT /api/v1/servers/{id}/zones/{zone}/rectifyGET /info/allowedGET /info/zone-allowedAdmin-only endpoints (
POST/PUT/DELETEzone metadata, cryptokeys, tsigkeys) intentionally stay on the static path — account-based access does not grant admin or cryptokey rights.Logging: INFO-level messages emitted on the account-resolution path (static miss → account check → upstream call → grant/deny) so operators can debug without enabling DEBUG globally.
Docs:
README.mdandconfig-example.ymlupdated with the new field.Trade-offs
GET /zones/{id}per request, so authorization always reflects current PowerDNS state. Listings stay free (the inlineaccountfield is already in the response).accountfield directly. If you manage tenancy in PowerDNS-Admin (or similar GUI) that stores its mapping out-of-band, you must ensure the underlying PowerDNS zone has itsaccountpopulated (e.g.PUT /zones/{id}with{"account":"..."}).Test plan
accountsaccounts: [""]does not match unaccounted zonesget_zone_account_from_pdns: success, empty/None, upstream error, trailing-dot normalization/info/allowedand/info/zone-allowedintegration paths for account-matched and account-missruff checkandruff formatclean🤖 Generated with Claude Code