fix(maps): one basemap layer, with a configurable key - #843
Merged
Conversation
Every map rendered with CARTO's "API KEY REQUIRED" watermark. CARTO requires a
key on basemap requests now and watermarks unkeyed ones, but still answers 200
with usable tiles -- the watermark is drawn into the image, so nothing logs and
no health check notices. It shows only to whoever is looking at the map.
The URL was hardcoded at five call sites, not the four the deployed bundle
suggested: two of them share a chunk. Fixing four and stopping would have left
the admin geofence submissions map watermarked.
BasemapService owns it now, reading basemap_url, basemap_key and
basemap_attribution from site settings. {key} is substituted here rather than
handed to Leaflet, whose template helper throws on a placeholder it has no value
for, and because the encoding is ours to get right. The CARTO parameter is `key`
and not `api_key`; the wrong name also returns 200 and also watermarks, so it is
easy to "fix" this and change nothing.
The five sites had drifted. Three carried no attribution at all, which
centralising fixes on its own, and one caps at maxZoom 18 -- createLayer takes an
override so that one keeps its cap rather than being quietly changed.
missingKey is deliberately narrower than "no key is set": it asks whether the
configured template wants one. An operator who has pointed basemap_url at a
keyless provider is not warned about a key their URL never asked for.
Refs #842.
A 200 response means nothing here -- CARTO returns working tiles without a key and draws the watermark into the image -- so the only way anyone learns about this is by looking at a map and recognising what they are seeing. The warning goes on the admin settings page rather than beside each map, because that is where the person who can fix it already is. It reads off BasemapService.missingKey, so it stays silent for an operator whose configured tile URL wants no key. Also adds the test that would have caught a typo in this change. A group or setting whose labelKey is absent from en.json renders the raw key as a heading, and nothing catches that today: locale-parity compares the eleven locales against each other, so a key missing from all of them is consistent and passes. Refs #842.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #842.
Every map rendered with CARTO's "API KEY REQUIRED" watermark. CARTO requires a key on basemap requests now and watermarks unkeyed ones, but still answers 200 with usable tiles, so nothing logged and no health check noticed. It showed only to whoever was looking at a map.
BasemapServicenow owns the tile layer, readingbasemap_url,basemap_keyandbasemap_attributionfrom site settings. Admin settings grows a Maps group for the three, in all eleven locales. Set the key and the watermark goes; leave it blank and you get exactly the maps you have today.In the built output the CARTO URL now appears once, in one chunk, as
?key={key}.Three things the issue didn't know
It is five call sites, not four. The issue counted chunks in the deployed image; two of the five share a chunk. Fixing four and stopping would have left the admin geofence submissions map watermarked.
The five had drifted. Three carried no
attributionat all, which is a licensing problem independent of the watermark and which centralising fixes on its own. One caps atmaxZoom: 18, socreateLayertakes an override rather than quietly moving it to 19.missingKeyasks whether the template wants a key, not merely whether one is set. An operator who has pointedbasemap_urlat a keyless provider is not warned about a key their URL never asked for.Where the key lives
Admin settings rather than
.env, which is a deliberate departure from the issue's suggestion. A CARTO basemap key travels in every tile URL the browser requests, so unlikePORACLE_API_SECRETorDISCORD_BOT_TOKENit is not a secret and storing it beside them would imply a protection it cannot have. More practically, URL, key and attribution are one unit: an operator able to switch providers in the UI but not supply the matching key has half a control, and the attribution has to change with the provider or it is simply wrong.Tests
13 for the service, 2 new ones asserting every
SETTING_GROUPSlabel and description resolves inen.json. 1438 passing, clean lint, prettier and production build.Both new sets were checked by breaking the code first. Dropping
encodeURIComponentand looseningmissingKeyfails two; typoingGROUP_MAPSfails one. That last one closes a real gap —locale-paritycompares the eleven locales against each other, so a key missing from all of them is consistent and passes, and the raw key would render as a heading.Not in scope
All five sites hardcoded
light_all, so maps stay light in dark mode. The service is now the right place to fix that, but it needs layer re-creation on theme change across five components and it is not the watermark.