[APPS] Add core request auth plumbing#396
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
sdkennedy2
changed the title
![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: 05cdc83ce9
ℹ️ 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".
| if (req.url === '/__dd/debugBundle') { | ||
| await handleDebugBundle(req, res, functionsByName, bundle); | ||
| } else if (req.url === '/__dd/executeAction') { | ||
| await handleExecuteAction(req, res, functionsByName, bundle, { request }, log); |
There was a problem hiding this comment.
When DD_API_KEY/DD_APP_KEY are absent, this now enters handleExecuteAction unconditionally; the MissingRequestAuthError is only thrown when the wrapped request is invoked after validateAndBundle has already parsed and bundled the backend function. In that missing-auth scenario, a bundling error will surface as an unrelated 500 instead of the previous immediate 403, and even successful bundles do unnecessary work before rejecting. Please keep an auth-validity check before calling handleExecuteAction for /__dd/executeAction.
Useful? React with 👍 / 👎.
sdkennedy2
force-pushed
the
sdkennedy2/apps-request-auth-plumbing
branch
from
05cdc83 to
bf20dae
Compare
| const requestAuth: RequestAuthOptions | undefined = hasValidAppApiKey(auth) | ||
| ? { apiKey: auth.apiKey, appKey: auth.appKey } | ||
| : undefined; | ||
| let didWarn = false; |
There was a problem hiding this comment.
This seems odd. Should we just warn when we setup withApiAuth?
| if (!request.isAuthConfigured) { | ||
| sendError( | ||
| res, | ||
| 403, | ||
| 'Auth credentials not configured. Set DD_API_KEY and DD_APP_KEY to enable remote execution.', | ||
| ); | ||
| return; | ||
| } | ||
| await handleExecuteAction(req, res, functionsByName, bundle, { request }, log); |
There was a problem hiding this comment.
Could we handle this just by throwing instead of a property on request?
sdkennedy2
force-pushed
the
sdkennedy2/apps-request-auth-plumbing
branch
from
bf20dae to
371a68b
Compare
1 participant
Motivation
Apps upload and dev-server paths currently thread raw auth configuration through multiple modules. This makes the Apps auth surface harder to extend and mixes request construction with credential selection.
This is the downstack plumbing change for Apps request auth. OAuth support is intentionally left to the follow-up stacked PR.
Changes
Adds a core request-auth helper that composes request behavior around
doRequest:The helper validates API/app key credentials once when the wrapper is created, warns if they are missing, and throws a
MissingRequestAuthErroronly when the wrapped request path is invoked without credentials.Apps now constructs a single authenticated Datadog API request function in the Vite plugin entrypoint and passes that request function into upload and dev-server code. Upload and dev-server code use relative API paths and no longer need to receive raw auth config or derive API base URLs themselves. The App Builder UI link still uses
https://app.${site}separately.QA Instructions
No manual QA needed; this is an internal auth plumbing refactor with no intended behavior change for API/app key auth.
Blast Radius
This affects Apps plugin upload and local dev-server backend execution paths. The behavior should remain API/app-key only in this PR; OAuth is added in the stacked follow-up.
Documentation