Token not included in Sprite request when `sprite` property in style is a full URL

gavinr-maps commented 1 year ago

Describe the bug

(original issue: #186)

This bug is regarding when using a protected vector tile service and thus passing a token property in the options object (second parameter) like this:

L.esri.Vector.vectorTileLayer("993b3e810d814760918716aa9633f713", {
  token: "YOUR-TOKEN-HERE",
}).addTo(map);

That token should be passed to the request for the sprites too, but in some cases it is not being included:

When the sprite property of the style JSON (root.json) is NOT a full URL starting with http....,
- Expected: the token gets added onto the sprite request
- Actual: the token gets added onto the sprite request ✅
When the sprite property of the style JSON (root.json) IS a full URL starting with http...,
- Expected: the token gets added onto the sprite request
- Actual: the token is not added onto the sprite request ❌

Reproduction

In ArcGIS Pro, add a vector Feature Class to the map
Right-click the layer > "Sharing" > "Share as web layer"
In the "Share as web layer" panel, select "Layer Type: Vector Tile"
Ensure that you are NOT sharing the layer with "Everyone"
Publish the layer, note the item ID
Create a new OAuth App https://developers.arcgis.com/applications/
Note the client ID and Client Secret
Create a new JS Bin, using the code in the "demo template" below, and replace the item ID, client ID, and client secret into the code.
Open the test application in the browser, and in the developer tools observe that the token is being included in the Sprite requests. (simialr to item 1 above)
Find the layer just created in https://developers.arcgis.com/layers/ and click "Open style editor" for that layer
Make a small change (any change), and save the layer as a new item ("Save As"). Note the Item ID
Create a new JS Bin, using the code in the "demo template" below, and replace the client ID, client secret, and this second item ID into the code.
Open this second test application in the browser, and in the developer tools observe that the token is NOT included in the Sprite requests. (similar to item 2 above)

Logs

No response

System Info

Leaflet version 1.9.4
Esri Leaflet version 3.0.10
Esri Leaflet Vector version 4.1.0

Additional Information

I think this is where the issue is: https://github.com/Esri/esri-leaflet-vector/blob/cad5b1967536486afb2f3f2507be1a6c33efd1d8/src/Util.js#L207-L216

The fix should be as simple as moving the line that adds the token (style.sprite += token ? '?token=' + token : '';) out of that if statement.

Template

Here is the demo template that is used in the replication steps above. See places where the string "XYZ" are for places where you need to replace values.

<!DOCTYPE html>
<html>

<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>Demo</title>
  <!-- Load Leaflet from CDN -->
  <link rel="stylesheet" href="https://unpkg.com/leaflet/dist/leaflet.css" />
  <script src="https://unpkg.com/leaflet/dist/leaflet.js"></script>

  <!-- Esri Leaflet and Esri Leaflet Vector -->
  <script src="https://unpkg.com/esri-leaflet/dist/esri-leaflet.js"></script>
  <script src="https://unpkg.com/esri-leaflet-vector@4/dist/esri-leaflet-vector.js"></script>

  <style>
    body {
      margin: 0;
      padding: 0;
    }

    #map {
      position: absolute;
      top: 0;
      bottom: 0;
      right: 0;
      left: 0;
    }
  </style>

</head>

<body>
    <div id="map"></div>
    <script>
      L.esri.post(
        "https://www.arcgis.com/sharing/rest/oauth2/token",
        {
          f: "json",
          client_id: "XYZ",
          // ***************************************
          // NEVER DO THIS (INCLUDE CLIENT SECRET IN JS CODE) - WE ARE ONLY DOING THIS FOR
          // EASIER DEBUGGING PURPOSES!!!!!!
          client_secret: "XYZ",
          grant_type: "client_credentials",
        },
        function (error, result) {
          if (error) {
            throw error;
          }
          const map = L.map("map", {
            minZoom: 2,
          });
          map.setView([-27, 27], 8);
          L.esri.Vector.vectorTileLayer("XYZ", {
            token: result.access_token,
          }).addTo(map);
        }
      );

    </script>
</body>

</html>

mstiglingh commented 1 year ago

Just checking in on the progress on this? We have a client with a pending production release that's dependent on this fix

patrickarlt commented 1 year ago

@gavinr-maps @mstiglingh there are security considerations here. We cannot blindly send a users token to an external URL over http:// since that isn't secure. We also should not blindly send a users token over https:// to a server or domain we don't trust. Right now the only way to verify that would be to confirm that the style and the sprites/glyph are on the same top level domain. If they are on different domains you will need to manually modify the style to incldue the token manually like so:

L.esri.Vector.vectorTileLayer("XYZ", {
  token: result.access_token,
  style: (style) => {
    // manually add the token here
    return style;
  }
}).addTo(map);

gavinr-maps commented 1 year ago

The fix for this was released in v4.2.0.

mstiglingh commented 5 months ago

@gavinr-maps the issue seems to have returned in the latest versions

gavinr-maps commented 5 months ago

@mstiglingh thanks for the note. The unit tests included in the fix for this (#192) seem to be passing, so I'm not sure what you're seeing. Could you please provide some more details including a replication case and what version the issue started happening? Thanks!

Esri / esri-leaflet-vector