diff --git a/docs/EXTENSIONS.md b/docs/EXTENSIONS.md index 1cb84859..9a3e64a4 100644 --- a/docs/EXTENSIONS.md +++ b/docs/EXTENSIONS.md @@ -151,7 +151,7 @@ The examples above hand-roll HTTP and auth. You usually don't need to. Because p ### Make authenticated API calls with `pup api` -`pup api ` reuses pup's full auth handler: it chooses OAuth bearer vs. API-key auth, applies the per-endpoint fallback for endpoints that don't accept OAuth (e.g. `/api/v2/api_keys`, fleet, cost), sets the branded User-Agent, and resolves the site. The extension already has a fresh `DD_ACCESS_TOKEN` (or the `DD_API_KEY`/`DD_APP_KEY` pair) in its environment, so these calls are authenticated automatically. +`pup api ` reuses pup's full auth handler: it chooses OAuth bearer vs. API-key auth, applies the per-endpoint fallback for endpoints that don't accept OAuth, sets the branded User-Agent, and resolves the site. The extension already has a fresh `DD_ACCESS_TOKEN` (or the `DD_API_KEY`/`DD_APP_KEY` pair) in its environment, so these calls are authenticated automatically. ```bash #!/bin/bash diff --git a/src/commands/api.rs b/src/commands/api.rs index 8a09dc85..2cd24960 100644 --- a/src/commands/api.rs +++ b/src/commands/api.rs @@ -714,9 +714,13 @@ 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. + /// + /// 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 +730,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 +743,7 @@ mod tests { let result = super::run( &cfg, - "v2/api_keys", + "v2/fleet/agents", "GET", &[], &[], @@ -767,7 +771,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 +782,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/config.rs b/src/config.rs index 0c9e766f..f0b55653 100644 --- a/src/config.rs +++ b/src/config.rs @@ -359,18 +359,6 @@ impl Config { Ok(()) } - /// Validate that both DD_API_KEY and DD_APP_KEY are configured. - /// Used for endpoints that require API key auth and do not accept OAuth2 tokens. - pub fn validate_api_and_app_keys(&self) -> Result<()> { - if self.api_key.is_none() || self.app_key.is_none() { - bail!( - "this command requires both DD_API_KEY and DD_APP_KEY — \ - OAuth2 bearer tokens are not supported here" - ); - } - Ok(()) - } - /// Validate that DD_API_KEY is configured for API-key-only endpoints. pub fn validate_api_key_only(&self) -> Result<()> { if self.api_key.is_none() { @@ -972,24 +960,6 @@ mod tests { assert_eq!(OutputFormat::Tsv.to_string(), "tsv"); } - #[test] - fn test_validate_api_and_app_keys_ok() { - let cfg = make_cfg(Some("key"), Some("app"), None); - assert!(cfg.validate_api_and_app_keys().is_ok()); - } - - #[test] - fn test_validate_api_and_app_keys_bearer_only_fails() { - let cfg = make_cfg(None, None, Some("token")); - assert!(cfg.validate_api_and_app_keys().is_err()); - } - - #[test] - fn test_validate_api_and_app_keys_missing_app_key_fails() { - let cfg = make_cfg(Some("key"), None, None); - assert!(cfg.validate_api_and_app_keys().is_err()); - } - #[test] fn test_validate_api_key_only_accepts_api_key_without_app_key() { let cfg = make_cfg(Some("key"), None, None); diff --git a/src/main.rs b/src/main.rs index e84a7716..5b84c721 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)] @@ -405,7 +408,7 @@ enum Commands { /// # List your application keys /// pup app-keys list /// - /// # List all application keys in the org (requires API keys) + /// # List all application keys in the org /// pup app-keys list --all /// /// # Get application key details @@ -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 -- this command only lists, so org_app_keys_write + /// is not needed). Neither scope is requested by default -- opt in with: + /// pup auth login --extra-scopes user_app_keys,org_app_keys_read #[command(name = "app-keys", verbatim_doc_comment)] AppKeys { #[command(subcommand)] @@ -2182,6 +2188,10 @@ enum Commands { /// /// AUTHENTICATION: /// Requires either OAuth2 authentication or API keys with org management permissions. + /// The policies/policy-overrides/policy-configs subcommands require the + /// org_group_read (read) and org_group_write (write) scopes, which are + /// not requested by default -- opt in with: + /// pup auth login --extra-scopes org_group_read,org_group_write #[command(verbatim_doc_comment)] Organizations { #[command(subcommand)] @@ -2899,6 +2909,10 @@ 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 #[command(verbatim_doc_comment)] Users { #[command(subcommand)] @@ -6325,11 +6339,11 @@ enum AppKeyActions { help = "Sort field (name, -name, created_at, -created_at)" )] sort: String, - /// List all org keys (requires API keys, not OAuth) + /// List all org keys (OAuth requires the org_app_keys_read scope; see --extra-scopes) #[arg( long, default_value_t = false, - help = "List all org keys (requires API keys, not OAuth)" + help = "List all org keys (OAuth requires the org_app_keys_read scope; see --extra-scopes)" )] all: bool, }, @@ -14424,7 +14438,6 @@ async fn main_inner() -> anyhow::Result<()> { all, } => { if all { - cfg.validate_api_and_app_keys()?; commands::app_keys::list_all(&cfg, page_size, page_number, &filter, &sort) .await? } else { diff --git a/src/raw_client.rs b/src/raw_client.rs index b70e4b73..ed14c965 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", @@ -856,10 +819,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" @@ -877,7 +836,7 @@ mod tests { #[test] fn test_oauth_excluded_count() { - assert_eq!(OAUTH_EXCLUDED_ENDPOINTS.len(), 55); + assert_eq!(OAUTH_EXCLUDED_ENDPOINTS.len(), 46); } #[test] @@ -897,13 +856,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] @@ -1041,12 +1016,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"));