Add OAuth PKCE auth for Apps uploads#393
Conversation
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 490c3f3 | Docs | Datadog PR Page | Give us feedback! |
0aa7176 to
69d59f9
Compare
d92d780 to
53ad0e4
Compare
53ad0e4 to
490c3f3
Compare
yoannmoinet
left a comment
There was a problem hiding this comment.
Doing a quick review first, to tackle my biggest comments 😃
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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'>; |
There was a problem hiding this comment.
I believe this should be more of the form:
| 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'>>; |
There was a problem hiding this comment.
This won't be necessary with my previous comment.
Summary
auth.methodwith optionalauth.oauthOptionsAuthorization: Bearerwhile preserving API/app key uploadsVerification
yarn workspace @dd/apps-plugin typecheckyarn workspace @dd/core typecheckyarn workspace @dd/factory typecheckyarn workspace @datadog/vite-plugin typecheckyarn 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=falseyarn workspace @dd/tests test:unit packages/plugins/apps/src/vite/index.test.ts packages/plugins/apps/src/vite/dev-server.test.ts --watchman=falseyarn workspace @datadog/vite-plugin buildimport("oauth4webapi"), norequire("oauth4webapi"), nonew Function