Added middleware header - #1087
Conversation
- Changed API base URL from 'https://api.descope.com' to 'https://api.descope.org' for preview environments. - Updated fetch cache option from 'force-cache' to 'no-store' to ensure fresh data retrieval. feat(vercel): add cache control headers - Introduced Cache-Control headers in vercel.json to prevent caching at various levels.
- Added regex for static file extensions to bypass middleware for asset requests. - Implemented no-cache headers for static assets and project configuration responses to improve security and data freshness. - Updated the default response headers to include no-cache settings when iframe embedding is allowed.
…ndling - Updated tests to ensure no-cache headers are applied correctly for static assets. - Corrected API base URL in tests to reflect the change from 'api.descope.com' to 'api.descope.org'. - Improved test coverage for middleware behavior regarding iframe embedding and cache settings.
… logic - Removed static asset cache control headers and implemented a matcher to exclude static file requests from middleware processing. - Updated API base URL for project configuration from 'https://api.descope.org' to 'https://api.descope.com'. - Changed fetch cache option to 'force-cache' for project configuration requests. - Enhanced middleware to conditionally omit X-Frame-Options header based on project configuration settings.
- Updated API base URL for preview environments from 'https://api.descope.com' to 'https://api.descope.org'. - Modified middleware to include a custom header 'moshe' based on project configuration for iframe embedding.
- Updated middleware to include project configuration in the response headers when iframe embedding is allowed, enhancing the context provided to the next middleware function.
|
✅ Code review completed successfully |
|
Your Vercel team Allen Zhou is not permitted to deploy from this git repository. Contact an administrator to add github organization descope as a Protected Git Scope in Allen Zhou on Vercel. Once added, commit again to see your changes. Learn more: https://vercel.com/docs/security/protected-git-scopes |
![shuni-bot-dev[bot]](https://avatars.githubusercontent.com/u/97479186?s=60&v=4){kind=small}
There was a problem hiding this comment.
🐕 Shuni's Review
Adds x-descope-middleware response header and fixes preview API base URL (.com → .org). But the tests weren't updated — they'll break on every changed behavior.
Sniffed out 2 issues:
- 1 🟠 HIGH: All existing tests will fail — assertions don't account for the new header or the URL change
- 1 🟡 MEDIUM: Test description text references stale domain
These bones are buried in the wrong yard! Woof!
| import { projectRegex } from './src/shared/projectRegex'; | ||
|
|
||
| const FETCH_TIMEOUT_MS = 2000; | ||
| const DESCOPE_MIDDLEWARE_HEADER = 'x-descope-middleware'; |
There was a problem hiding this comment.
🟠 HIGH: Tests not updated — all assertions will fail.
The test file src/middleware.test.ts was not modified in this PR, but every assertion is now stale:
-
"Embedding allowed" path (lines 85, 97): Tests assert
expect(mockedNext).toHaveBeenCalledWith()(zero args), but the code now passesnext({ headers: { 'x-descope-middleware': 'true' } }). These two tests will fail. -
expectXFrameOptionshelper (lines 22-26): Assertsnext()is called with{ headers: { 'X-Frame-Options': 'SAMEORIGIN' } }, but the code now also includes'x-descope-middleware': 'false'in the headers object.toHaveBeenCalledWithdoes an exact match on the argument shape, so every test using this helper will fail (6+ tests). -
Preview URL tests (lines 160-162, 175-177): Assert
api.descope.comin the fetch URL, but the code now usesapi.descope.org. Both config-base-URL tests will fail.
Please update src/middleware.test.ts to match the new behavior.
There was a problem hiding this comment.
Pull request overview
This PR updates the Vercel middleware to explicitly signal iframe-embedding decisions via a custom response header, and corrects the fallback configuration API base URL used for .preview.descope.org deployments.
Changes:
- Add
x-descope-middleware: true|falseheader to indicate whether iframe embedding is allowed. - Update
.preview.descope.orgconfig lookup fallback fromhttps://api.descope.comtohttps://api.descope.org.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return next({ | ||
| headers: { | ||
| [DESCOPE_MIDDLEWARE_HEADER]: 'false', | ||
| 'X-Frame-Options': 'SAMEORIGIN' | ||
| } |
There was a problem hiding this comment.
Default response headers now include x-descope-middleware: false in addition to X-Frame-Options. Update existing assertions that expect only X-Frame-Options (e.g., the expectXFrameOptions() helper in src/middleware.test.ts) so they account for the additional header.
itaihanski
left a comment
itaihanski
left a comment
There was a problem hiding this comment.
Looks good! Some tests got broken
| import { projectRegex } from './src/shared/projectRegex'; | ||
|
|
||
| const FETCH_TIMEOUT_MS = 2000; | ||
| const DESCOPE_MIDDLEWARE_HEADER = 'x-descope-middleware'; |
There was a problem hiding this comment.
I don't think we really need this - it's always false with the x-frame-header and always true otherwise. WDYT?
This pull request makes targeted improvements to the
middleware.tsfile, primarily to enhance how the middleware communicates its decision about iframe embedding to downstream handlers. The main changes introduce a new custom header and correct a configuration URL.Middleware response improvements:
x-descope-middleware, to responses to explicitly indicate whether iframe embedding is allowed ('true'or'false'). This provides clearer signaling to downstream consumers or middleware. [1] [2] [3]Configuration correction:
https://api.descope.comtohttps://api.descope.orgwhen handling.preview.descope.orghostnames.