Skip to content

data-deletion: send OAuth bearer token, document scope requirement | DAL-547 - #693

Merged
platinummonkey merged 1 commit into
DataDog:mainfrom
srosenthal-dd:stephen.rosenthal/data-deletion-oauth
Jul 30, 2026
Merged

data-deletion: send OAuth bearer token, document scope requirement | DAL-547#693
platinummonkey merged 1 commit into
DataDog:mainfrom
srosenthal-dd:stephen.rosenthal/data-deletion-oauth

Conversation

@srosenthal-dd

@srosenthal-dd srosenthal-dd commented Jul 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Flip pup data-deletion requests {list,create,cancel} from make_api_no_auth! to make_api! so the OAuth bearer is actually sent.
  • Document in the command help text that all operations require logs_delete_data and/or rum_delete_data, which are not requested by default.

Scope check

Neither logs_delete_data nor rum_delete_data is in default_scopes() or read_only_scopes() today. All 3 routes require one of these permissions (OR), including the list/GET route, so there's no scope-free read path. Users must opt in with pup auth login --extra-scopes logs_delete_data,rum_delete_data.

Test plan

  • cargo test data_deletion passes
  • CI passes
  • Manually verify pup data-deletion requests list works after pup auth login --extra-scopes logs_delete_data,rum_delete_data

Jira: DAL-547

Flip from make_api_no_auth! to make_api! so the OAuth bearer is actually
sent, and document in the command help text that all operations require
logs_delete_data and/or rum_delete_data, neither of which is requested by
default.
@srosenthal-dd
srosenthal-dd marked this pull request as ready for review July 30, 2026 16:59
@srosenthal-dd
srosenthal-dd requested a review from a team as a code owner July 30, 2026 16:59
@platinummonkey
platinummonkey merged commit a2bea0f into DataDog:main Jul 30, 2026
6 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18e47f721b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


fn make_api(cfg: &Config) -> DataDeletionAPI {
crate::make_api_no_auth!(DataDeletionAPI, cfg)
crate::make_api!(DataDeletionAPI, cfg)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add coverage for the OAuth bearer path

The existing data-deletion tests build their configuration with access_token: None, so this macro change is exercised only with API keys and never verifies the behavior it is intended to fix. If bearer middleware stops being installed—or the token header is malformed—the original OAuth-only failure will return while these tests remain green; add a bearer-only test that matches Authorization: Bearer <token> and corresponding failure-path coverage.

AGENTS.md reference: AGENTS.md:L22-L22

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants