diff --git a/src/commands/api.rs b/src/commands/api.rs index 0b1603f5..569ebbbf 100644 --- a/src/commands/api.rs +++ b/src/commands/api.rs @@ -872,11 +872,11 @@ mod tests { cleanup_env(); } - /// OAuth-excluded endpoints (e.g. GET /api/unstable/fleet/some-id) must use API-key + /// OAuth-excluded endpoints (e.g. GET /api/v2/validate_keys) 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. /// - /// Uses the still-excluded unstable Fleet entry as the example. + /// Uses the indefinitely-exempt validate_keys entry as the example. #[tokio::test] async fn test_api_oauth_excluded_uses_api_keys() { let _lock = lock_env().await; @@ -886,7 +886,7 @@ mod tests { // must prefer the API keys. cfg.access_token = Some("bearer-token".into()); let _mock = server - .mock("GET", "/api/unstable/fleet/some-id") + .mock("GET", "/api/v2/validate_keys") .match_query(mockito::Matcher::Any) .match_header("DD-API-KEY", "test-api-key") .match_header("DD-APPLICATION-KEY", "test-app-key") @@ -899,7 +899,7 @@ mod tests { let result = super::run( &cfg, - "unstable/fleet/some-id", + "v2/validate_keys", "GET", &[], &[], @@ -927,7 +927,7 @@ mod tests { let mut cfg = test_config(&server.url()); cfg.access_token = Some("bearer-token".into()); let _mock = server - .mock("GET", "/api/unstable/fleet/some-id") + .mock("GET", "/api/v2/validate_keys") .match_query(mockito::Matcher::Any) .match_header("DD-API-KEY", "test-api-key") .match_header("authorization", mockito::Matcher::Missing) @@ -938,7 +938,7 @@ mod tests { .await; // Pass the fully-qualified URL, not a relative path. - let absolute = format!("{}/api/unstable/fleet/some-id", server.url()); + let absolute = format!("{}/api/v2/validate_keys", server.url()); let result = super::run( &cfg, &absolute, diff --git a/src/raw_client.rs b/src/raw_client.rs index 0fc8588e..c730ca56 100644 --- a/src/raw_client.rs +++ b/src/raw_client.rs @@ -107,6 +107,7 @@ pub fn get_auth_type(cfg: &Config) -> AuthType { // OAuth-excluded endpoint validation // --------------------------------------------------------------------------- +#[derive(Debug, PartialEq, Eq)] struct EndpointRequirement { path: &'static str, method: &'static str, @@ -124,17 +125,9 @@ pub(crate) fn requires_api_key_only(method: &str, path: &str) -> bool { } fn find_endpoint_requirement(method: &str, path: &str) -> Option<&'static EndpointRequirement> { - OAUTH_EXCLUDED_ENDPOINTS.iter().find(|req| { - if req.method != method { - return false; - } - // Trailing "/" means prefix match (for ID-parameterized paths) - if req.path.ends_with('/') { - path.starts_with(&req.path[..req.path.len() - 1]) - } else { - req.path == path - } - }) + OAUTH_EXCLUDED_ENDPOINTS + .iter() + .find(|req| req.method == method && req.path == path) } // --------------------------------------------------------------------------- @@ -142,31 +135,19 @@ 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. +/// +/// General policy: any endpoint that accepts API + App Key auth should also +/// accept OAuth bearer tokens; these are the current exceptions. Before +/// adding an entry, check whether the route is actually missing +/// `ValidOAuthAccessToken` server-side and prefer landing OAuth support +/// there over widening this table. static OAUTH_EXCLUDED_ENDPOINTS: &[EndpointRequirement] = &[ - // Fleet Automation unstable surface — doesn't support OAuth server-side - // yet. Current status, not a permanent contract; delete this entry (and - // the tests referencing it) once it does, rather than patching forward. - EndpointRequirement { - path: "/api/unstable/fleet/", - method: "GET", - }, - // Profiling (4) - // No OAuth scope is declared for Continuous Profiler endpoints; force API-key auth. - EndpointRequirement { - path: "/profiling/api/v1/", - method: "POST", - }, - EndpointRequirement { - path: "/profiling/api/v1/", - method: "GET", - }, - EndpointRequirement { - path: "/api/unstable/profiles/", - method: "POST", - }, + // Validate API/App key pair (1) + // Indefinitely exempt from OAuth server-side: it exists to validate the + // exact API + App key pair the caller already holds, so bearer auth would + // defeat its purpose. EndpointRequirement { - path: "/api/ui/profiling/", + path: "/api/v2/validate_keys", method: "GET", }, // Events intake (1) @@ -614,31 +595,37 @@ mod tests { } #[test] - fn test_no_fallback_for_logs() { - assert!(!requires_api_key_fallback("POST", "/api/v2/logs/events")); - assert!(!requires_api_key_fallback( - "POST", - "/api/v2/logs/events/search" - )); + fn test_excluded_table_is_exactly_the_expected_entries() { + // The general policy is that any endpoint accepting API + App Key + // auth also accepts OAuth. Any change to the exception list should + // be a deliberate, reviewed edit here — not an incidental + // re-addition of a route whose server-side OAuth has since landed. + assert_eq!( + OAUTH_EXCLUDED_ENDPOINTS, + &[ + EndpointRequirement { + path: "/api/v2/validate_keys", + method: "GET", + }, + EndpointRequirement { + path: "/api/v1/events", + method: "POST", + }, + ] + ); } #[test] - fn test_no_fallback_for_rum() { - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/rum/applications" - )); + fn test_excluded_endpoints_are_exact_matches() { + // Excluded endpoints are exact path matches; subpaths and + // same-prefix routes are not excluded. + assert!(requires_api_key_fallback("GET", "/api/v2/validate_keys")); assert!(!requires_api_key_fallback( "GET", - "/api/v2/rum/applications/abc-123" + "/api/v2/validate_keys/extra" )); } - #[test] - fn test_no_fallback_for_events_search() { - assert!(!requires_api_key_fallback("POST", "/api/v2/events/search")); - } - #[test] fn test_fallback_for_events_post() { // Posting an event (V1 intake) requires only the API key; reading events @@ -650,103 +637,6 @@ mod tests { assert!(!requires_api_key_only("POST", "/api/v1/events/12345")); } - #[test] - fn test_no_fallback_for_logs_saved_views() { - assert!(!requires_api_key_fallback("GET", "/api/v1/logs/views")); - assert!(!requires_api_key_fallback("GET", "/api/v1/logs/views/123")); - assert!(!requires_api_key_fallback("POST", "/api/v1/logs/views")); - assert!(!requires_api_key_fallback( - "DELETE", - "/api/v1/logs/views/123" - )); - } - - #[test] - fn test_no_fallback_for_standard_endpoints() { - assert!(!requires_api_key_fallback("GET", "/api/v1/monitor")); - assert!(!requires_api_key_fallback("GET", "/api/v1/dashboard")); - assert!(!requires_api_key_fallback("GET", "/api/v2/incidents")); - } - - #[test] - fn test_prefix_matching_with_id() { - // Trailing "/" in the pattern should match paths with IDs. - // Uses the still-excluded unstable Fleet entry as the example. - assert!(requires_api_key_fallback( - "GET", - "/api/unstable/fleet/some-id" - )); - } - - #[test] - fn test_method_must_match() { - // RUM events/search is POST-excluded, but GET should not match - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/rum/events/search" - )); - } - - #[test] - fn test_no_fallback_for_obs_pipelines() { - // Observability Pipelines routes already accept OAuth server-side; - // removing them from OAUTH_EXCLUDED_ENDPOINTS means raw_get/raw_post - // (used by `pup obs-pipelines diff` and the `pup api` passthrough) - // should send the OAuth bearer instead of forcing API-key fallback. - // Collection endpoint - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/obs-pipelines/pipelines" - )); - assert!(!requires_api_key_fallback( - "POST", - "/api/v2/obs-pipelines/pipelines" - )); - // ID-parameterized endpoints (prefix match via trailing "/") - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/obs-pipelines/pipelines/abc-123" - )); - assert!(!requires_api_key_fallback( - "PUT", - "/api/v2/obs-pipelines/pipelines/abc-123" - )); - assert!(!requires_api_key_fallback( - "DELETE", - "/api/v2/obs-pipelines/pipelines/abc-123" - )); - // Validation endpoint - assert!(!requires_api_key_fallback( - "POST", - "/api/v2/obs-pipelines/pipelines/validate" - )); - // Non-matching method on a formerly-excluded path - assert!(!requires_api_key_fallback( - "PATCH", - "/api/v2/obs-pipelines/pipelines" - )); - } - - #[test] - fn test_no_fallback_for_ddsql_editor_tools() { - // DDSQL editor tools now accept OAuth server-side (DAL-960); removing - // them from OAUTH_EXCLUDED_ENDPOINTS means `pup ddsql spec`/`schema - // tables`/`schema columns` should send the OAuth bearer instead of - // forcing API-key fallback. - assert!(!requires_api_key_fallback( - "GET", - "/api/unstable/ddsql-editor/tools/ddsql-docs" - )); - assert!(!requires_api_key_fallback( - "GET", - "/api/unstable/ddsql-editor/tools/table-names" - )); - assert!(!requires_api_key_fallback( - "POST", - "/api/unstable/ddsql-editor/tools/table-data" - )); - } - #[tokio::test] async fn test_raw_get_obs_pipelines_uses_oauth_bearer() { let _lock = lock_env().await; @@ -771,127 +661,6 @@ mod tests { cleanup_env(); } - #[test] - fn test_no_fallback_for_notebooks() { - assert!(!requires_api_key_fallback("GET", "/api/v1/notebooks")); - assert!(!requires_api_key_fallback("GET", "/api/v1/notebooks/12345")); - assert!(!requires_api_key_fallback("POST", "/api/v1/notebooks")); - } - - #[test] - fn test_no_fallback_for_fleet() { - // Fleet Automation v2 routes already accept OAuth server-side; - // the raw/generic `pup api` passthrough should use the OAuth bearer - // like the typed fleet commands do. - assert!(!requires_api_key_fallback("GET", "/api/v2/fleet/agents")); - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/fleet/agents/agent-123" - )); - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/fleet/deployments" - )); - assert!(!requires_api_key_fallback( - "POST", - "/api/v2/fleet/deployments/configure" - )); - assert!(!requires_api_key_fallback( - "POST", - "/api/v2/fleet/schedules/sched-123/trigger" - )); - } - - #[test] - fn test_no_fallback_for_cost_billing() { - // Cost/Billing routes already accept OAuth server-side (DAL-959); the - // raw/generic `pup api` passthrough should use the OAuth bearer - // instead of forcing API-key fallback. - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/usage/projected_cost" - )); - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/usage/cost_by_org" - )); - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/cost_by_tag/monthly_cost_attribution" - )); - } - - #[test] - fn test_no_fallback_for_ccm() { - // Cloud Cost Management config routes already accept OAuth - // server-side (DAL-959); the raw/generic `pup api` passthrough - // should use the OAuth bearer instead of forcing API-key fallback. - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/cost/aws_cur_config" - )); - assert!(!requires_api_key_fallback( - "POST", - "/api/v2/cost/aws_cur_config" - )); - assert!(!requires_api_key_fallback( - "DELETE", - "/api/v2/cost/aws_cur_config/config-123" - )); - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/cost/azure_uc_config" - )); - assert!(!requires_api_key_fallback( - "DELETE", - "/api/v2/cost/azure_uc_config/config-123" - )); - assert!(!requires_api_key_fallback( - "GET", - "/api/v2/cost/gcp_uc_config" - )); - assert!(!requires_api_key_fallback( - "DELETE", - "/api/v2/cost/gcp_uc_config/config-123" - )); - assert!(!requires_api_key_fallback("GET", "/api/v2/cost/oci_config")); - assert!(!requires_api_key_fallback("GET", "/api/v2/cost/anomalies")); - } - - #[test] - 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] - fn test_no_fallback_for_error_tracking() { - assert!(!requires_api_key_fallback( - "POST", - "/api/v2/error_tracking/issues/search" - )); - } - // Verify raw_request reaches the auth check (and fails there) for both the // empty-query and non-empty-query paths. This ensures the `if !query.is_empty()` // branch compiles and runs without panic. @@ -1003,74 +772,18 @@ mod tests { #[test] fn test_other_oauth_excluded_endpoints_still_require_both_keys() { - // Uses the still-excluded unstable Fleet entry as the example. + // Uses the indefinitely-exempt validate_keys entry as the example. let mut cfg = test_cfg(); cfg.app_key = None; - let req = - reqwest::Client::new().get("https://api.datadoghq.com/api/unstable/fleet/some-id"); + let req = reqwest::Client::new().get("https://api.datadoghq.com/api/v2/validate_keys"); - let err = match apply_auth(req, &cfg, "GET", "/api/unstable/fleet/some-id") { + let err = match apply_auth(req, &cfg, "GET", "/api/v2/validate_keys") { Ok(_) => panic!("excluded endpoint should require both keys"), Err(err) => err, }; assert!(err.to_string().contains("DD_API_KEY and DD_APP_KEY")); } - #[test] - fn test_requires_api_key_fallback_profiling() { - // /profiling/api/v1/* - assert!(requires_api_key_fallback( - "POST", - "/profiling/api/v1/aggregate" - )); - assert!(requires_api_key_fallback( - "GET", - "/profiling/api/v1/profiles/abc/info" - )); - assert!(requires_api_key_fallback( - "GET", - "/profiling/api/v1/profiles/abc/analysis" - )); - assert!(requires_api_key_fallback( - "POST", - "/profiling/api/v1/profiles/abc/breakdown" - )); - assert!(requires_api_key_fallback( - "POST", - "/profiling/api/v1/profiles/abc/timeline" - )); - // /api/unstable/profiles/* - assert!(requires_api_key_fallback( - "POST", - "/api/unstable/profiles/list" - )); - assert!(requires_api_key_fallback( - "POST", - "/api/unstable/profiles/analytics" - )); - assert!(requires_api_key_fallback( - "POST", - "/api/unstable/profiles/insights" - )); - assert!(requires_api_key_fallback( - "POST", - "/api/unstable/profiles/callgraph" - )); - assert!(requires_api_key_fallback( - "POST", - "/api/unstable/profiles/interactive-analytics/field" - )); - assert!(requires_api_key_fallback( - "POST", - "/api/unstable/profiles/save-favorite" - )); - // /api/ui/profiling/* - assert!(requires_api_key_fallback( - "GET", - "/api/ui/profiling/profiles/abc/download" - )); - } - /// Verifies that raw_request attaches query parameters and returns Ok when the /// server responds 200. Exercises the `!query.is_empty()` branch added to the function. #[tokio::test]