pup: enable OAuth for DDSQL editor commands - #756
Conversation
Remove 3 dsql-editor tool endpoints from OAUTH_EXCLUDED_ENDPOINTS so raw_get/raw_post send the OAuth bearer token for ddsql::spec, ddsql::schema_tables, and ddsql::schema_columns commands. Depends on server-side PR: dd-source#66615
…th-ddsql-editor # Conflicts: # src/raw_client.rs
…nd CCM Remove 32 entries from OAUTH_EXCLUDED_ENDPOINTS for endpoint groups whose server-side routes already accept OAuth: - Fleet Automation (15): v2 routes already have ValidOAuthAccessToken - Cost/Billing (3): server already accepts OAuth (DAL-959) - Cloud Cost Management config (14): server already accepts OAuth (DAL-959) Also remove the brittle OAUTH_EXCLUDED_ENDPOINTS count assertion (test_oauth_excluded_count) which was prone to merge conflicts and tested no actual behavior. Update test_requires_api_key_fallback_fleet to test_no_fallback_for_fleet, and switch the test_other_oauth_excluded_endpoints example from Fleet to Profiling (which remains excluded). Remaining excluded entries (8): DDSQL editor (3, handled by #756), Profiling (4, no OAuth scope), Events intake (1, SDK codegen issue).
|
Hey @srosenthal-dd I need this done for other DDSQL pup stuff I'm doing. Planning to move out of draft soon? |
Yes was just waiting for the corresponding server side change to deploy. |
…th-ddsql-editor # Conflicts: # src/raw_client.rs
Without this, OAuth sessions get a 403 (permission denied) on the ddsql-editor tool routes even after the OAuth exclusion is removed -- confirmed against prod. Per DAL-960, both changes are needed together. Also drop the now-stale help text claiming these commands require DD_API_KEY/DD_APP_KEY.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa45ba8432
ℹ️ 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".
3 participants
Summary
Remove 3 ddsql-editor tool endpoints from
OAUTH_EXCLUDED_ENDPOINTSsoraw_get/raw_postsend the OAuth bearer token forddsql::spec,ddsql::schema_tables, andddsql::schema_columnscommands.Server-side dependency
Depended on a server-side change adding
ValidOAuthAccessTokento the 3 ddsql-editor routes (graphing-backend). Confirmed deployed to prod (verified via Lambo, all 3 routes now show{ValidFullAPIUser,ValidOAuthAccessToken}).Changes
src/raw_client.rs: removed 3 ddsql-editor entries fromOAUTH_EXCLUDED_ENDPOINTS, replaced the old positive fallback-required test with a negative one confirming OAuth is no longer excluded for these routessrc/auth/types.rs: addedddsql_editor_readtodefault_scopes()andread_only_scopes()-- without this, OAuth sessions get a 403 (not a 401) on these routes even with the exclusion removed, since the scope was never requested at login. Confirmed against prod: old session (missing the scope) -> 403; fresh session (with the scope) -> 200 on all 3 routes.src/main.rs: dropped stale help text claiming these commands require DD_API_KEY/DD_APP_KEYValidation
cargo test --bin pup-- 1912 passedcargo fmt --check-- passedpup ddsql spec,pup ddsql schema tables,pup ddsql schema columns, and a directpup apicall to/api/unstable/ddsql-editor/tools/table-names) -- all 200 with a fresh OAuth session includingddsql_editor_read