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

Add an option to hardcode TileJSON tiles URL by providing a header in requests #6021

Closed

Conversation

optionsome
Copy link
Member

Summary

The request URL that OTP knows differs in many ways from what the users use through the Digitransit APIs. Therefore, we need to control the tiles URL in the TileJSON file on a request basis through a header. This PR introduces X-OTP-Tilejson-Url to achieve that.

Issue

Minor sandbox addition, no issue.

Unit tests

Added tests.

Documentation

Updated documentation.

Changelog

Sandbox changelog will be updated.

Copy link

codecov bot commented Aug 26, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.73%. Comparing base (8164d41) to head (ec39af0).
Report is 200 commits behind head on dev-2.x.

Files with missing lines Patch % Lines
...ipplanner/ext/vectortiles/VectorTilesResource.java 88.88% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #6021      +/-   ##
=============================================
- Coverage      69.73%   69.73%   -0.01%     
  Complexity     17314    17314              
=============================================
  Files           1960     1960              
  Lines          74267    74269       +2     
  Branches        7603     7604       +1     
=============================================
- Hits           51793    51791       -2     
- Misses         19831    19834       +3     
- Partials        2643     2644       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@t2gran
Copy link
Member

t2gran commented Aug 28, 2024

Not sure I understand this PR. Why is it needed, could this be done by a Gateway and is this vulnerable to X-Site attacks?

@optionsome
Copy link
Member Author

Not sure I understand this PR. Why is it needed, could this be done by a Gateway and is this vulnerable to X-Site attacks?

The problem I'm trying to solve is that the users send their requests to cdn.digitransit.fi domain but the cdn sends the requests to api.digitransit.fi domain, which is also what shows up as the host when the request arrives to OTP. Additionally, since our cdn doesn't support caching based on headers, we have different paths for different languages but our proxy translates those paths into accept-language headers. Also, our API requires API keys in requests.

OTP on the other hand, by default, creates the tiles url in the TileJSON file from the request host and path. So for instance, https://cdn.digitransit.fi/map/v3/hsl/fi/stops,stations/tilejson.json?digitransit-subscription-key=some-key returns https://api.digitransit.fi/otp/routers/hsl/vectorTiles/stops,stations/{z}/{x}/{y}.pbf as the tiles url, but it should return https://cdn.digitransit.fi/map/v3/hsl/fi/stops,stations/{z}/{x}/{y}.pbf?digitransit-subscription-key=some-key instead (I have not yet validated how well the url parameter works with client libraries).

I thought about different ways to exploit this and I think your concern is valid. If the tilejson files are cached somewhere, it's possible that someone can "corrupt" the cache with tiles url that points to some malicious endpoint. However, this problem wouldn't affect my project since we would always rewrite the header in our proxy before it enters OTP and we wouldn't cache this file.

I think this leaves three options:

  1. Some feature flag to turn this feature on so it's not in use in deployments where it's unnecessary and/or can cause a vulnerability.
  2. Somewhat complex "digitransit mapper" which constructs the tiles url with a combination of configuration of the host in OTP side and reading request path/headers/query parameters and constructing the tiles URL from those.
  3. We just don't support this feature. For most clients, the lack of a working tilejson file is not really a problem.

@optionsome optionsome marked this pull request as draft August 29, 2024 13:50
@optionsome
Copy link
Member Author

I will discover if it's possible for us to patch the tiles url in our proxy instead of having OTP do it.

@t2gran t2gran added this to the 2.7 (next release) milestone Sep 18, 2024
@optionsome
Copy link
Member Author

I found a way to solve this issue by patching the response in our proxy, so I will close this pr.

@optionsome optionsome closed this Sep 23, 2024
@t2gran t2gran modified the milestones: 2.7 (next release), Rejected Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants