Restrict chromium requests #425
Conversation
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Codecov Report
@@ Coverage Diff @@
## main #425 +/- ##
============================================
- Coverage 74.27% 74.12% -0.15%
Complexity 383 383
============================================
Files 94 94
Lines 3984 4008 +24
Branches 641 649 +8
============================================
+ Hits 2959 2971 +12
- Misses 891 903 +12
Partials 134 134
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Joshua Li <joshuali925@gmail.com>
Signed-off-by: Joshua Li <joshuali925@gmail.com>
| @@ -93,6 +93,8 @@ const ipv6Regex = /(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:) | |||
| const localhostRegex = /localhost:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5])/g; | |||
| const iframeRegex = /iframe/g; | |||
|
|
|||
| export const ALLOWED_HOSTS = /^(0|0.0.0.0|127.0.0.1|localhost|(.*\.)?(opensearch.org|aws.a2z.com))$/; | |||
There was a problem hiding this comment.
This should not have an Amazon URL. And how is this used? Why would opensearch.org be in this list?
There was a problem hiding this comment.
This is the allowed hosts for all requests and redirections in chromium. The two hosts listed are used by map tiles visualizations
There was a problem hiding this comment.
Those 2 are not hosts, rather domains. Perhaps narrow the scope to the specific ones we need like maps.search-services.aws.a2z.com?
There was a problem hiding this comment.
I also think this needs to be configurable because some users will run in intranets. Is there a GH issue/feature request for this ask? I can open one otherwise.
There was a problem hiding this comment.
Thansk @dblock for pointing this one out: https://github.com/opensearch-project/OpenSearch-Dashboards/pull/899/files
It looks like we don't need the a2z one. Could you please confirm, @joshuali925 ? If we can also make the opensearch.org more specific that'd be even better.
There was a problem hiding this comment.
I opened opensearch-project/documentation-website#983 to document what's going on with these URLs.
There was a problem hiding this comment.
Nit: is ALLOWED_HOSTS only used for maps? Maybe split it into ALLOWED_HOSTS and ALLOWED_MAPS_HOST to make it explicit?
Also this needs tests. I don't see anything that checks that ALLOWED_HOSTS is taken into consideration in tests.
There was a problem hiding this comment.
@davidlago yes, good idea to narrow down to maybe (.+\.)?maps.opensearch.org. Will remove the a2z one if someone is able to confirm in db.'s issue which dashboards versions use which endpoint.
@dblock ALLOWED_HOSTS is used for all requests and redirections happening inside chromium. Reporting plugin cannot tell if the request is coming from a map visualization, so I'm not sure if there are values to split them. Will add tests after the a2z issue is addressed
There was a problem hiding this comment.
Let's make sure all concerns are cleared from @davidlago before we merge this.
|
closing since we no longer use chromium |
Description
[Describe what this change achieves]
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.