Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OPS-9699] Remove mapbox token from mabox responses #662

Merged
merged 1 commit into from
Oct 10, 2023
Merged

Conversation

orakili
Copy link
Collaborator

@orakili orakili commented Oct 6, 2023

Refs: OPS-9699

This substitutes the mapbox token in the proxied mapbox responses.

Tests

  1. Checkout the branch
  2. Recreate the drupal container
  3. Visit /disasters
  4. Check the network tab (refresh if necessary), filter by mapbox
  5. Check the content of any json file, the mapbox token should not be present (instead there should be access_token=token)

@orakili orakili requested a review from cafuego October 6, 2023 03:47
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Coverage Report

Totals Coverage
Statements: 11.73% ( 1089 / 9286 )
Methods: 11.58% ( 82 / 708 )
Lines: 11.74% ( 1007 / 8578 )

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Build output

Composer Validate success

PHP Lint success

Docker Build success

Environment Setup success

Site Install success

PHP Code Sniffer success

Software Versions PHP 8.2.10 (cli) (built: Sep 2 2023 06:59:22) (NTS) Copyright (c) The PHP Group Zend Engine v4.2.10, Copyright (c) Zend Technologies with Zend OPcache v8.2.10, Copyright (c), by Zend Technologies with Xdebug v3.2.1, Copyright (c) 2002-2023, by Derick Rethans Composer version 2.6.4 2023-09-29 10:54:46
Drupal Logs

Pusher: @orakili, Action: pull_request, Workflow: Run tests

Copy link
Contributor

@cafuego cafuego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked it on unocha.org dev and the proxy_set_header Accept-Encoding ""; is obviously what I was missing. The replace rule then works, but the links to the tile servers do not get re-rewritten, as they're not proxied, so the library is unable to download tiles: {"message":"Not Authorized - Invalid Token"}

@orakili
Copy link
Collaborator Author

orakili commented Oct 6, 2023

@cafuego Where do you see this {"message":"Not Authorized - Invalid Token"} message?

From my tests, all the tiles load properly.

The token substitution is done in the response of /mapbox/v4/reliefweb.xxx.json?style=mapbox://styles/reliefweb/xxx&secure&access_token=token file for the tile URLs:

https://a.tiles.mapbox.com/v4/reliefweb.xxxx/{z}/{x}/{y}.vector.pbf?access_token=token&style=mapbox://styles/reliefweb/xxx
https://b.tiles.mapbox.com/v4/reliefweb.xxxx/{z}/{x}/{y}.vector.pbf?access_token=token&style=mapbox://styles/reliefweb/xxx

But those URLs are not called.

The tiles requests are like /mapbox/v4/reliefweb.xxx/1/1/0.vector.pbf?style=mapbox://styles/reliefweb/xxx&sku=yyy&access_token=token and are properly proxied.

Zooming or scrolling loads new tiles with the /mapbox/v4/reliefweb.xxx/x/y/z.vector.pbf?style=mapbox://styles/reliefweb/xxx&sku=yyy&access_token=token pattern.

@cafuego
Copy link
Contributor

cafuego commented Oct 6, 2023

Huh, not called?

Copy link
Contributor

@cafuego cafuego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging, since the tile server URLs that get rewritten appear to not actually get used.

@orakili orakili merged commit e425181 into develop Oct 10, 2023
@orakili orakili deleted the OPS-9699 branch December 8, 2023 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants