Skip to content

Add OAuth PKCE auth for Apps uploads#393

Open
otorrillas wants to merge 4 commits into
masterfrom
oriol.torrillas/apps-oauth-pkce-upload
Open

Add OAuth PKCE auth for Apps uploads#393
otorrillas wants to merge 4 commits into
masterfrom
oriol.torrillas/apps-oauth-pkce-upload

Conversation

@otorrillas
Copy link
Copy Markdown

Summary

  • add OAuth Authorization Code + PKCE support for Apps bundle uploads
  • store and refresh OAuth tokens through OS credential storage
  • move upload auth selection to auth.method with optional auth.oauthOptions
  • send OAuth uploads with Authorization: Bearer while preserving API/app key uploads

Verification

  • yarn workspace @dd/apps-plugin typecheck
  • yarn workspace @dd/core typecheck
  • yarn workspace @dd/factory typecheck
  • yarn workspace @datadog/vite-plugin typecheck
  • yarn workspace @dd/tests test:unit packages/plugins/apps/src/validate.test.ts packages/plugins/apps/src/index.test.ts packages/plugins/apps/src/oauth.test.ts packages/plugins/apps/src/upload.test.ts packages/core/src/helpers/request.test.ts --watchman=false
  • yarn workspace @dd/tests test:unit packages/plugins/apps/src/vite/index.test.ts packages/plugins/apps/src/vite/dev-server.test.ts --watchman=false
  • yarn workspace @datadog/vite-plugin build
  • ESLint / Prettier checks on touched files
  • CJS bundle sanity check: native import("oauth4webapi"), no require("oauth4webapi"), no new Function

@otorrillas otorrillas requested review from a team and yoannmoinet as code owners June 2, 2026 06:52
@datadog-prod-us1-4
Copy link
Copy Markdown

datadog-prod-us1-4 Bot commented Jun 2, 2026

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 490c3f3 | Docs | Datadog PR Page | Give us feedback!

@otorrillas otorrillas force-pushed the oriol.torrillas/apps-oauth-pkce-upload branch from 0aa7176 to 69d59f9 Compare June 2, 2026 07:41
@otorrillas otorrillas force-pushed the oriol.torrillas/apps-oauth-pkce-upload branch 2 times, most recently from d92d780 to 53ad0e4 Compare June 2, 2026 08:15
@otorrillas otorrillas force-pushed the oriol.torrillas/apps-oauth-pkce-upload branch from 53ad0e4 to 490c3f3 Compare June 2, 2026 08:22
Copy link
Copy Markdown
Member

@yoannmoinet yoannmoinet left a comment

Choose a reason for hiding this comment

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

Doing a quick review first, to tackle my biggest comments 😃

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be factorised around the doRequest helper as much as possible.

What if another plugin needs to do oauth, will they have to re-implement most of this? (honest question, not rhetorical 😄)

export type AuthOptions = {
apiKey?: string;
appKey?: string;
accessToken?: string;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why's this user facing? Not sure I understand the need for it, from the user's perspective.

export type RequestOpts = {
url: string;
auth?: Pick<AuthOptions, 'apiKey' | 'appKey'>;
auth?: Pick<AuthOptions, 'apiKey' | 'appKey' | 'accessToken'>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe this should be more of the form:

Suggested change
auth?: Pick<AuthOptions, 'apiKey' | 'appKey' | 'accessToken'>;
auth?: Assign<Pick<AuthOptions, 'apiKey' | 'appKey'>, { accessToken: string }>;

So accessToken isn't leaked into user facing API (unless we actually need it there).

const DEV_VIRTUAL_PREFIX = 'virtual:dd-backend-dev:';

type AuthConfig = Required<AuthOptionsWithDefaults>;
type AuthConfig = Required<Pick<AuthOptionsWithDefaults, 'apiKey' | 'appKey' | 'site'>>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This won't be necessary with my previous comment.

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.

2 participants