feat(auth): support Personal and Service Access Tokens (DD_PAT / DD_SAT)#497
Draft
srosenthal-dd wants to merge 3 commits into
Draft
feat(auth): support Personal and Service Access Tokens (DD_PAT / DD_SAT)#497srosenthal-dd wants to merge 3 commits into
srosenthal-dd wants to merge 3 commits into
Conversation
Add Personal Access Token and Service Access Token as a third authentication tier, sitting between OAuth2 (preferred) and the classic DD_API_KEY + DD_APP_KEY (mildly discouraged). PATs and SATs share the same wire protocol -- sent as Authorization: Bearer on OAuth-compatible endpoints, and as DD-APPLICATION-KEY on the OAuth-excluded endpoints (api_keys, application_keys, ddsql-editor) using Datadog's migration form. This means a PAT or SAT covers every endpoint that API+App Key covers. Runtime auth precedence: OAuth access_token > PAT/SAT > API+App Key. If both DD_PAT and DD_SAT are set, DD_PAT wins. The PatKind enum records which env var supplied the token so pup auth status can display "Personal Access Token" vs "Service Access Token". Docs updated: - README.md restructured Authentication into three tiers, with PAT and SAT both shown as preferred over API+App Key. - docs/OAUTH2.md comparison table is now a three-way table with PAT/SAT pairs treated as one column. - Environment variables and WASM sections list DD_PAT and DD_SAT.
Audited PAT/SAT support across all auth code paths and fixed the following issues raised by Claude, Codex, and Gemini reviewers: - apply_org_override no longer clobbers an explicit env-supplied PAT/SAT (or DD_ACCESS_TOKEN) with a stored OAuth session. Otherwise --org could silently switch the request principal. - raw_post_lenient and raw_delete now route through the shared apply_auth so PAT/SAT callers reach debugger and cost-CCM paths. - client.rs and make_dd_config now check PAT before API+App Key on OAuth-excluded paths, matching documented precedence (OAuth > PAT/SAT > API+App Key). - Config::from_env now checks all env vars before any file values, so DD_SAT in env beats pat: in the config file (and vice versa). - make_dd_config populates apiKeyAuth=PAT alongside appKeyAuth=PAT for PAT-only callers, since some SDK ops require the slot. - commands/api.rs, commands/incidents.rs, commands/bits.rs, and commands/acp.rs no longer have PAT-blind auth blocks. - commands/api.rs and src/api.rs route PAT through DD-APPLICATION-KEY for OAuth-excluded paths instead of Authorization: Bearer. - extensions/exec.rs forwards DD_PAT or DD_SAT to child processes based on cfg.pat_kind and clears stale values otherwise. - Extracted OAUTH_EXCLUDED_ENDPOINTS and requires_api_key_fallback to a new dependency-free src/oauth_excluded.rs so both client.rs and the browser/WASM api.rs share one canonical list (previously drifted to 11 vs 52 entries). Tests added for each new branch (PAT preserved across --org, env override of file PAT/SAT, PAT routed to DD-APPLICATION-KEY on excluded endpoints, raw_post_lenient + raw_delete with PAT).
Eliminated four near-duplicate auth-routing ladders by making client::apply_auth pub and delegating from the command-level helpers. Net diff: -80 LoC. - commands/api.rs::run -- replaced 45-line inline auth ladder with a single apply_auth call. - commands/incidents.rs::attachments_delete -- ditto. - commands/bits.rs::resolve_agent_id -- changed signature from three Option<&str> params to &Config, eliminating the call-site PAT-into- access_token coalesce. - AuthType::AccessToken now carries PatKind directly instead of Option<PatKind>. The None arm was unreachable since pat and pat_kind are always set together. - commands/auth.rs::build_non_oauth_status uses kind.display_name() instead of re-implementing the Personal/Service -> label mapping. - config.rs::apply_org_override uses PatKind::env_var() instead of hardcoded "DD_PAT"/"DD_SAT" strings. - extensions/exec.rs::inject_auth_env collapses the two PatKind match arms into a single env-set + unconditional unset pair. - Removed unused Config::has_pat helper.
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
1 participant
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 Personal and Service Access Tokens as a third auth tier between OAuth2 (preferred) and
DD_API_KEY+DD_APP_KEY(mildly discouraged). PATs and SATs are wire-identical; both go inAuthorization: Beareron OAuth-compatible endpoints andDD-APPLICATION-KEYon OAuth-excluded ones.Recommended order:
DD_PAT/DD_SAT) -- supported anywhereDD_API_KEY+DD_APP_KEYisDD_API_KEY+DD_APP_KEY-- works everywhere; mildly discouragedNote: env var names (
DD_PAT/DD_SAT) are not standardized across Datadog tooling. Worth confirming the naming before merge.Notable behavior
DD_ACCESS_TOKEN> stored OAuth >DD_PAT>DD_SAT>DD_API_KEY+DD_APP_KEY.--orgno longer silently swaps an explicit env-supplied PAT/SAT/access-token for a stored OAuth session.src/oauth_excluded.rsso the WASM build shares one source of truth.Test plan
cargo fmt,cargo clippy --all-targets -- -D warnings,cargo test -- --test-threads=1(1092 pass)