fix(dns): authenticate Umbrella with OAuth2 client credentials, not Basic (#3271) - #3275
Open
bdunncompany wants to merge 1 commit into
Open
fix(dns): authenticate Umbrella with OAuth2 client credentials, not Basic (#3271)#3275bdunncompany wants to merge 1 commit into
bdunncompany wants to merge 1 commit into
Conversation
…asic (LanternOps#3271) Cisco retired direct Basic Auth on the Umbrella APIs. The provider was sending `Authorization: Basic <key:secret>` straight at the endpoints, which now returns 401 for every key type — which is why the reporter's four key categories all failed identically, and why a direct curl outside Breeze reproduced it. The credentials were never the problem; the auth scheme was. The key/secret now buy a short-lived bearer token from Cisco's OAuth2 client-credentials endpoint, and every call carries that token. Verified against Cisco's published API docs rather than inferred: - token URL `https://api.umbrella.com/auth/v2/token`, POST, key/secret as HTTP Basic on the token request only, `application/x-www-form-urlencoded`, body `grant_type=client_credentials` - response `{ token_type, access_token, expires_in }`, lifetime 3600s - ONE token covers every Umbrella surface, so this fixes the four `policies/v2/destinationlists` call sites as well as the reporting one the issue names. A reports-only fix would have left block/allow-list sync broken in exactly the same way. Token is cached on the provider instance and refreshed 60s before expiry, with concurrent callers sharing one in-flight exchange rather than stampeding the token endpoint. The retry status is the trap here: an EXPIRED Umbrella token comes back as **400 `invalid_request`**, not 401. A conventional retry-on-401 would never fire for the one case a cache makes possible. `withAuth` refreshes once on 401 OR on a 400 whose body marks it as an auth failure, and deliberately leaves genuine validation 400s alone so a bad destination isn't retried into a duplicate write. Tests: 9 covering the exchange shape, bearer on both API surfaces, caching, refresh-and-retry on 400-invalid_request and on 401, the no-retry control for a validation 400, the preserved apiSecret requirement, and a malformed token response. All were run against the un-fixed code by reverting `withAuth` to the old Basic header — 8 of the 9 fail, the ninth being the apiSecret check both versions share. NOT verified end to end: I have no Umbrella tenant, so this is correct by construction and unit tests, not by a live sync. Asked the reporter on the issue to retest against a build. Gate: tsc exit 0; vitest 1286 files / 20538 tests / 0 failures; eslint clean.
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 #3271.
Cisco retired direct Basic Auth on the Umbrella APIs. The provider sent
Authorization: Basic <key:secret>straight at the endpoints, which now 401s for every key type — which is exactly why @cisspUser01's four key categories all failed identically and why a direct curl outside Breeze reproduced it. The credentials were never the problem; the auth scheme was.Verified against Cisco's docs, not inferred
https://api.umbrella.com/auth/v2/token,POST, key/secret as HTTP Basic on the token request only,Content-Type: application/x-www-form-urlencoded, bodygrant_type=client_credentials{ token_type, access_token, expires_in }, lifetime 3600sScope: five call sites, not one
The issue names the reporting endpoint. Cisco's docs state one token covers every Umbrella surface, so I checked the rest of the file —
basicAuthHeader()was also feeding fourpolicies/v2/destinationlistscalls. A reports-only fix would have left block/allow-list sync broken in precisely the same way, with no second bug report to explain why. All five now go throughwithAuth.The retry status is the trap
An expired Umbrella token comes back as
400 invalid_request, not 401. A conventional retry-on-401 would never fire for the one failure mode that caching makes possible — the token lapsing mid-run.withAuthrefreshes once on a 401, or on a 400 whose body marks it as an auth failure, and deliberately leaves genuine validation 400s alone. That last part matters for the destination-list writes: blanket-retrying a 400 risks turning a rejected write into a duplicate one.Token is cached on the provider instance, refreshed 60s before expiry, and concurrent callers share one in-flight exchange rather than stampeding the token endpoint.
Tests
Nine, covering the exchange shape, bearer on both API surfaces, caching, refresh-and-retry on
400 invalid_requestand on 401, a no-retry control for a validation 400, the preservedapiSecretrequirement, and a malformed token response.All were run against the un-fixed code — by reverting
withAuthto the old Basic header rather than by pattern-matching call sites — and 8 of the 9 fail, the ninth being theapiSecretcheck both versions share.What I could not verify
I have no Umbrella tenant, so this is correct by construction and by unit tests, not by a live sync. The token flow matches Cisco's documented contract, but only a real tenant proves Cisco accepts it end to end.
@cisspUser01 — if you can retest against a build from this branch, that would close the loop properly. Particularly useful: whether the destination-list (block/allow) sync now works too, since that half was broken silently and isn't something the original 401 report would have surfaced.
Gate
tsc --noEmitexit 0 ·vitest run1286 files / 20538 tests / 0 failures ·eslintclean on both changed files.