fix token interceptor for https #1524
Conversation
There was a problem hiding this comment.
Pull request overview
Updates frontend URL handling so the token interceptor doesn’t incorrectly rewrite absolute HTTPS URLs, and adjusts development environment endpoints.
Changes:
- Extend
TokenInterceptor.normalizeURL()to treathttps://URLs as already-normalized (same ashttp://). - Update
environment.dev.tsto pointapiRoot/saasURLtohttps://app.rocketadmin.com.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/environments/environment.dev.ts | Switches “development” build endpoints to app.rocketadmin.com. |
| frontend/src/app/services/token.interceptor.ts | Prevents base URL prefixing when request URLs are absolute HTTPS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (url.startsWith('http://')) { | ||
| return url; | ||
| } | ||
| if (url.startsWith('https://')) { | ||
| return url; | ||
| } |
There was a problem hiding this comment.
The new https:// branch changes URL normalization behavior but the existing spec only checks interceptor creation. Please add unit tests that exercise normalizeURL() for http:// and https:// absolute URLs to ensure they are not prefixed with baseURL (and to guard against regressions).
| if (url.startsWith('http://')) { | ||
| return url; | ||
| } | ||
| if (url.startsWith('https://')) { | ||
| return url; | ||
| } |
There was a problem hiding this comment.
The http:// and https:// checks are duplicated branches. Consider collapsing them into a single protocol check (e.g., https?://) to reduce the chance of future divergence and keep the normalization logic easier to maintain.
| if (url.startsWith('http://')) { | |
| return url; | |
| } | |
| if (url.startsWith('https://')) { | |
| return url; | |
| } | |
| if (url.startsWith('http://') || url.startsWith('https://')) { | |
| return url; | |
| } |
| apiRoot: 'https://app.rocketadmin.com/api', | ||
| saasURL: 'https://app.rocketadmin.com', |
There was a problem hiding this comment.
environment.dev.ts now points apiRoot/saasURL at the production host (app.rocketadmin.com). Since this file is used by the Angular build:development configuration (see frontend/angular.json), this will make “development” builds talk to production, which is risky and can also bypass the local /api proxy setup. If the goal is only to fix https URL handling, consider reverting these URLs back to the dev/staging host (or using relative /api + proxy) and introducing a separate environment file/config for production endpoints if needed.
No description provided.