From 259cb09ba613f45293909ba1d746d1b73c72658a Mon Sep 17 00:00:00 2001 From: Stephen Rosenthal Date: Mon, 3 Aug 2026 11:04:48 -0700 Subject: [PATCH 1/2] Document opt-in OAuth scopes for api-keys, app-keys, service-accounts These commands already send the OAuth bearer via make_api!, but the required scopes (api_keys_*, user_app_keys, org_app_keys_*, service_account_write) aren't in default_scopes(). Document them using the same --extra-scopes opt-in pattern used for logs-restriction, so users know how to unlock OAuth for these commands instead of hitting an unexplained permission failure. service-accounts (create/app-keys) also needs server-side OAuth support, tracked separately in DAL-958 -- noted in the help text so --extra-scopes alone isn't mistaken for a full fix. --- src/main.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index b429f8c..df04274 100644 --- a/src/main.rs +++ b/src/main.rs @@ -290,6 +290,9 @@ enum Commands { /// AUTHENTICATION: /// Requires OAuth2 (via 'pup auth login') or a valid API key + Application key /// combination. Note: You cannot use an API key to delete itself. + /// OAuth2 requires the api_keys_read/api_keys_write/api_keys_delete scopes, + /// which are not requested by default -- opt in with: + /// pup auth login --extra-scopes api_keys_read,api_keys_write,api_keys_delete #[command(name = "api-keys", verbatim_doc_comment)] ApiKeys { #[command(subcommand)] @@ -425,8 +428,11 @@ enum Commands { /// /// AUTHENTICATION: /// Most commands use the current_user endpoints and support OAuth2 (via - /// 'pup auth login'). The 'list --all' command uses the org-wide endpoint - /// and requires API + Application keys (DD_API_KEY + DD_APP_KEY). + /// 'pup auth login'), gated by the user_app_keys scope. The 'list --all' + /// command uses the org-wide endpoint (also OAuth2-capable, gated by + /// org_app_keys_read/org_app_keys_write). None of these scopes are + /// requested by default -- opt in with: + /// pup auth login --extra-scopes user_app_keys,org_app_keys_read,org_app_keys_write #[command(name = "app-keys", verbatim_doc_comment)] AppKeys { #[command(subcommand)] @@ -2888,6 +2894,12 @@ enum Commands { /// /// AUTHENTICATION: /// Requires either OAuth2 authentication or API keys. + /// list/get/roles-list work with default OAuth scopes. service-accounts + /// (create and app-keys) require the service_account_write scope, which + /// is not requested by default -- opt in with: + /// pup auth login --extra-scopes service_account_write + /// Server-side OAuth support for service-accounts routes is being added + /// under DAL-958; --extra-scopes alone won't help until that ships. #[command(verbatim_doc_comment)] Users { #[command(subcommand)] From e3f9d8cfdcd827926fa4302599616b0de3db774d Mon Sep 17 00:00:00 2001 From: Stephen Rosenthal Date: Mon, 3 Aug 2026 11:36:27 -0700 Subject: [PATCH 2/2] Remove stale API/App Keys entries from OAUTH_EXCLUDED_ENDPOINTS /api/v2/api_keys and /api/v2/application_keys (org-wide) already accept OAuth server-side (DAL-514), but 9 entries for these paths were still sitting in the raw-client OAuth-exclusion table, forcing the generic `pup api` passthrough to use API+App key auth on them even when a bearer token is present. The typed api-keys/app-keys commands (make_api!-based) were never affected, but this table is the only auth routing the passthrough command has. Updated the tests that used these paths as examples of "still excluded" behavior to use Fleet Automation instead, since it's still genuinely excluded today. --- src/commands/api.rs | 19 +++++++---- src/raw_client.rs | 77 +++++++++++++++++---------------------------- 2 files changed, 41 insertions(+), 55 deletions(-) diff --git a/src/commands/api.rs b/src/commands/api.rs index 8a09dc8..bbac4ac 100644 --- a/src/commands/api.rs +++ b/src/commands/api.rs @@ -714,9 +714,16 @@ mod tests { cleanup_env(); } - /// OAuth-excluded endpoints (e.g. GET /api/v2/api_keys) must use API-key auth - /// even when a bearer token is present. This exercises the reuse of + /// OAuth-excluded endpoints (e.g. GET /api/v2/fleet/agents) must use API-key + /// auth even when a bearer token is present. This exercises the reuse of /// raw_client::apply_auth's per-endpoint fallback table. + /// + /// Note: GET /api/v2/api_keys used to be the example endpoint here, but it + /// (and /api/v2/application_keys) now accept OAuth server-side (DAL-514) and + /// were removed from the fallback table -- see raw_client's + /// test_no_fallback_for_api_keys. Fleet Automation is just today's example + /// of a still-excluded endpoint, not a claim it's meant to stay that way -- + /// update this test if/when Fleet gets OAuth support too. #[tokio::test] async fn test_api_oauth_excluded_uses_api_keys() { let _lock = lock_env().await; @@ -726,7 +733,7 @@ mod tests { // must prefer the API keys. cfg.access_token = Some("bearer-token".into()); let _mock = server - .mock("GET", "/api/v2/api_keys") + .mock("GET", "/api/v2/fleet/agents") .match_query(mockito::Matcher::Any) .match_header("DD-API-KEY", "test-api-key") .match_header("DD-APPLICATION-KEY", "test-app-key") @@ -739,7 +746,7 @@ mod tests { let result = super::run( &cfg, - "v2/api_keys", + "v2/fleet/agents", "GET", &[], &[], @@ -767,7 +774,7 @@ mod tests { let mut cfg = test_config(&server.url()); cfg.access_token = Some("bearer-token".into()); let _mock = server - .mock("GET", "/api/v2/api_keys") + .mock("GET", "/api/v2/fleet/agents") .match_query(mockito::Matcher::Any) .match_header("DD-API-KEY", "test-api-key") .match_header("authorization", mockito::Matcher::Missing) @@ -778,7 +785,7 @@ mod tests { .await; // Pass the fully-qualified URL, not a relative path. - let absolute = format!("{}/api/v2/api_keys", server.url()); + let absolute = format!("{}/api/v2/fleet/agents", server.url()); let result = super::run( &cfg, &absolute, diff --git a/src/raw_client.rs b/src/raw_client.rs index 81ad0f9..a8f07e1 100644 --- a/src/raw_client.rs +++ b/src/raw_client.rs @@ -120,39 +120,6 @@ fn find_endpoint_requirement(method: &str, path: &str) -> Option<&'static Endpoi /// Endpoints that don't support OAuth. /// Trailing "/" means prefix match for ID-parameterized paths. static OAUTH_EXCLUDED_ENDPOINTS: &[EndpointRequirement] = &[ - // API/App Keys (8) - EndpointRequirement { - path: "/api/v2/api_keys", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/api_keys/", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/api_keys", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/api_keys/", - method: "DELETE", - }, - EndpointRequirement { - path: "/api/v2/application_keys", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/application_keys/", - method: "GET", - }, - EndpointRequirement { - path: "/api/v2/application_keys/", - method: "POST", - }, - EndpointRequirement { - path: "/api/v2/application_keys/", - method: "PATCH", - }, // DDSQL editor tools (3) EndpointRequirement { path: "/api/unstable/ddsql-editor/tools/ddsql-docs", @@ -166,10 +133,6 @@ static OAUTH_EXCLUDED_ENDPOINTS: &[EndpointRequirement] = &[ path: "/api/unstable/ddsql-editor/tools/table-data", method: "POST", }, - EndpointRequirement { - path: "/api/v2/application_keys/", - method: "DELETE", - }, // Fleet Automation (15) EndpointRequirement { path: "/api/v2/fleet/agents", @@ -866,10 +829,6 @@ mod tests { #[test] fn test_prefix_matching_with_id() { // Trailing "/" in the pattern should match paths with IDs - assert!(requires_api_key_fallback( - "DELETE", - "/api/v2/api_keys/key-123" - )); assert!(requires_api_key_fallback( "GET", "/api/v2/fleet/agents/agent-123" @@ -887,7 +846,7 @@ mod tests { #[test] fn test_oauth_excluded_count() { - assert_eq!(OAUTH_EXCLUDED_ENDPOINTS.len(), 57); + assert_eq!(OAUTH_EXCLUDED_ENDPOINTS.len(), 48); } #[test] @@ -907,13 +866,29 @@ mod tests { } #[test] - fn test_requires_api_key_fallback_api_keys() { - assert!(requires_api_key_fallback("GET", "/api/v2/api_keys")); - assert!(requires_api_key_fallback("POST", "/api/v2/api_keys")); - assert!(requires_api_key_fallback( + fn test_no_fallback_for_api_keys() { + // /api/v2/api_keys and /api/v2/application_keys already accept OAuth + // server-side (DAL-514); the raw/generic `pup api` passthrough should + // use the OAuth bearer like the typed api-keys/app-keys commands do, + // not force an API+Application key fallback. + assert!(!requires_api_key_fallback("GET", "/api/v2/api_keys")); + assert!(!requires_api_key_fallback("POST", "/api/v2/api_keys")); + assert!(!requires_api_key_fallback( "DELETE", "/api/v2/api_keys/key-123" )); + assert!(!requires_api_key_fallback( + "GET", + "/api/v2/application_keys" + )); + assert!(!requires_api_key_fallback( + "DELETE", + "/api/v2/application_keys/key-123" + )); + assert!(!requires_api_key_fallback( + "PATCH", + "/api/v2/application_keys/key-123" + )); } #[test] @@ -1052,12 +1027,16 @@ mod tests { #[test] fn test_other_oauth_excluded_endpoints_still_require_both_keys() { + // Uses Fleet Automation as a currently-still-excluded example. This is + // just today's state of OAUTH_EXCLUDED_ENDPOINTS, not a claim that Fleet + // (or anything else in the table) is meant to stay that way -- update + // this example if/when its entries get OAuth support and are removed. let mut cfg = test_cfg(); cfg.app_key = None; - let req = reqwest::Client::new().post("https://api.datadoghq.com/api/v2/api_keys"); + let req = reqwest::Client::new().get("https://api.datadoghq.com/api/v2/fleet/agents"); - let err = match apply_auth(req, &cfg, "POST", "/api/v2/api_keys") { - Ok(_) => panic!("API key management should require both keys"), + let err = match apply_auth(req, &cfg, "GET", "/api/v2/fleet/agents") { + Ok(_) => panic!("Fleet Automation should require both keys"), Err(err) => err, }; assert!(err.to_string().contains("DD_API_KEY and DD_APP_KEY"));