fix(carto): only attach Authorization header when accessToken is present#10373
fix(carto): only attach Authorization header when accessToken is present#10373jatorre wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23430ad9ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| credentials?: RequestCredentials; | ||
| } { | ||
| if (!accessToken) { | ||
| return {credentials: 'same-origin'}; |
There was a problem hiding this comment.
When accessToken is absent, every CARTO tile layer passes this object into mergeLoadOptions(super.getLoadOptions(), ...); because credentials is a primitive value, it overwrites any loadOptions.fetch.credentials the application supplied. This breaks configurations that intentionally use credentials: 'include' for cross-origin cookie auth or credentials: 'omit' for public requests, even though the intended fix only needs to omit the Authorization header. Returning an empty fetch-options object in the no-token case would avoid clobbering user fetch settings.
Useful? React with 👍 / 👎.
|
@felixpalmer hey mate. Could you take a look? |
|
coverage: 83.394% (+0.3%) from 83.112% — jatorre:fix/carto-conditional-auth-header into visgl:master |
3 participants
Background
TilejsonResult.accessTokenis becoming optional in@carto/api-client(CartoDB/carto-api-client#298 adds a session auth mode where requests are authenticated by a same-origin session cookie behind a proxy, and no token ever reaches JavaScript — tile URLs come back rewritten onto the proxy base).Today the CARTO tile layers and
fetchMap'screateLoadOptionsbuildAuthorization: Bearer ${tileJSON.accessToken}unconditionally, so a missing token produces the literal headerBearer undefined— which actively breaks cookie-session servers (they engage only when the Authorization header is absent) and sends a junk credential otherwise.Change List
getAuthFetchOptions(accessToken?)helper inmodules/carto/src/utils.ts: returns the Bearer header when a token is present, or{credentials: 'same-origin'}when not (so the cookie rides along on same-origin tile requests).getLoadOptions) andcreateLoadOptionsinparse-map.ts(param widened tostring | undefined).Verified: carto module suite green (28 files / 124 tests), monorepo lint 0 errors, prettier clean.
Note
Medium Risk
Touches authentication for all CARTO tile loads, but token-based flows are unchanged and the no-token path only avoids a broken header and enables cookie auth.
Overview
Centralizes tile and map fetch auth in
getAuthFetchOptions, so missingaccessTokenno longer sendsAuthorization: Bearer undefined.When a token exists, behavior is unchanged (
Bearerheader). When it does not—e.g. session/cookie auth via same-origin proxy URLs—the helper omitsAuthorizationand setscredentials: 'same-origin'so cookies can authenticate requests.parse-mapcreateLoadOptionsand all CARTO tile layers that setloadOptions.fetch(vector, H3, quadbin, raster, cluster) now use this helper instead of inlining the header.Reviewed by Cursor Bugbot for commit 96a4490. Bugbot is set up for automated code reviews on this repo. Configure here.