auth: document opt-in scopes for api-keys, app-keys, service-accounts, org-group | DAL-958 DAL-973

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 <ENDPOINT>` 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 <ENDPOINT>` 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

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,

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);

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 {

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"));