-
Notifications
You must be signed in to change notification settings - Fork 435
feat: Proxy handler support enabling My Account and My Org #2400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2400 +/- ##
==========================================
+ Coverage 87.90% 88.71% +0.80%
==========================================
Files 37 39 +2
Lines 4127 4466 +339
Branches 818 883 +65
==========================================
+ Hits 3628 3962 +334
- Misses 496 501 +5
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ope instead of auth0-scope
…t variable correctly
ade1a6f to
25b1669
Compare
f7487b9 to
d3ca61a
Compare
| } | ||
|
|
||
| async handleMyAccount(req: NextRequest): Promise<NextResponse> { | ||
| return this.#handleProxy(req, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/me and /my-org use DPoP. When DPoP is disabled, #handleProxy falls back to Bearer. Does it make sense to return an error immediately when /me or /my-org are accessed when DPoP is disabled ?
| } | ||
| }; | ||
|
|
||
| const updateProfile = async (updates) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the page makes a My Account request (updateProfile in this case) during page render, and that page is linked using the Next.js <Link> component, it could trigger an actual API request during prefetch in production.
I think we should either fix this or document this.
| !HOP_BY_HOP_HEADERS.has(lowerKey); | ||
|
|
||
| if (shouldForward) { | ||
| forwardedHeaders.set(key, value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a minor one.
- If headers are case in-sensitive, we can set
forwardedHeaders.set(lowerKey, value);anyway. - If headers are case-sensitive, we should not do
key.toLowerCase();
But, headers are case in-sensitive, so this should be fine.
This PR enables support for secure browser-initiated My-Account and My-Org API calls through a
handleProxyhandler, a transparent and secure authenticating proxy to handle secure API calls originating from browser.Changes
auth-client.ts: added methodshandleMyAccountandhandleMyOrg, added matchers for these inhandler.utils/proxy.ts: header filters, url transform, proxy matcherTests
proxy-handler.test.ts: Added tests for proxy handler covering:proxy.test.ts: Tests for header filters, url transform, proxy matcherauth-client.proxy.test.ts: proxy tests specific to my-account and my-orgauth-client.test.ts: updated test casesReferences
Implementation details
The proxy matcher uses the following rules:
scopeheader is used to supply scope (notauth0-scope)