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

[Feature]: Additional configuration to prevent the creation of a new tab and use of file upload controls #1211

Open
Binrix opened this issue Feb 23, 2023 · 7 comments

Comments

@Binrix
Copy link
Contributor

Binrix commented Feb 23, 2023

Requirement

As a developer I would like to have the possibility to set an option for the creation of a new page and another one to disable the use of the file upload controls in the configuration so that an user will not be able to access the file system via the development tools or the file upload controls.

Problem

The issue is that in a few cases a link will lead to a new tab. We use Jaeger-UI with electron on our instruments and a new tab (e.g. target of link is _blank) leads to a popup of a new window. Now the user is able to access the file system with the development tools. This problem leads to a security issue because the user can access data that shouldn't be accessible for everyone. There are also file upload controls in the Jaeger UI, which can be used to access the file system without opening new windows or developer tools and hence these controls would need to be removed.

Proposal

The proposal is to add two new boolean properties to the configuration file. These properties will later be used to evaluate if a link will lead to a new tab (for us to a new popup window) or not. The same for the file upload controls. To be more precise: When a case occurs that a target is _blank or a standalone link will be displayed, the new option in the configuration will prevent those two cases.

This solution was already discussed in the slack channel with Javier Jiménez Roda.

Open questions

No response

@yurishkuro
Copy link
Member

  1. what link / new page are you referring to?
  2. I don't follow the issue with "local file system access" - how is it different with any other web page?

@Binrix
Copy link
Contributor Author

Binrix commented Feb 24, 2023

  1. When the target of a link is _blank, normally a new tab is being created. We use Jaeger-UI on our instruments with electron and electron creates a new window. When a new window is created, the upper option menu is accessible which allows the user to open the developer tools and then the file system. This is why we don't want the application to open new tabs (new window for electron).
  2. Normally on our instruments users don't have access to the file system (software is already written this way), but on the search page you can choose between the parameter based search or searching with a json file. When I select the option with the json file I can upload a file giving me access to the file system. I know, for normal web pages this is not an issue at all but on our instruments are sensitiv data and we don't want the user to have access to it.

Both issues are a security issue for us and we would like to prevent this by adding two boolean properties to the configuration. Sorry for any uncertainties due to my description. Hopefully everything is clear now, if not, please reach out.

@yurishkuro
Copy link
Member

Thanks. I would be very curious to see a photo of your devices with Jaeger running on them :-) I take it these are some portable devices?

Regarding the feature - sounds reasonable. I'll transfer the issue to UI repo, as this will need to be implemented as a couple of new UI config options.

@yurishkuro yurishkuro transferred this issue from jaegertracing/jaeger Feb 24, 2023
@Binrix
Copy link
Contributor Author

Binrix commented Feb 24, 2023

Thanks a lot! I'm currently working on the implementation of the feature.

@Binrix
Copy link
Contributor Author

Binrix commented Mar 1, 2023

@yurishkuro To push my changes do I need special rights from you to do so? When I try to push my changes git says that permission have been denied.

@yurishkuro
Copy link
Member

You need to fork the repo, push changes to a branch in the fork, and create a pull request. When you push to a fork, git will print a URL to create a PR.

@yurishkuro
Copy link
Member

@Binrix see https://github.com/jaegertracing/jaeger/blob/main/CONTRIBUTING_GUIDELINES.md#making-a-change

yurishkuro added a commit that referenced this issue Jun 4, 2023
* This PR splits one single feature from #1229 by @Binrix, which is too
large
* Adds a new configuration option `disableFileUploadControl`
* Addresses part of #1211

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
ramumanam pushed a commit to ramumanam/jaeger-ui that referenced this issue Jun 6, 2023
* This PR splits one single feature from jaegertracing#1229 by @Binrix, which is too
large
* Adds a new configuration option `disableFileUploadControl`
* Addresses part of jaegertracing#1211

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: RAMU MANAM <manam.ramu@uber.com>
yurishkuro added a commit that referenced this issue Jun 6, 2023
- This PR splits one single feature from #1229 
- Adds a new configuration option `disableJsonView`
- Addresses part of #1211

---------

Signed-off-by: Benjamin Klein <benjamin.klein.bk1@roche.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Benjamin Klein <benjamin.klein.bk1@roche.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <github@ysh.us>
yurishkuro added a commit that referenced this issue Jun 27, 2023
This pull request resolves the issue #1211 

This property is later used to prevent the popup of a new page. When set
to true, it will alter all _blank targets for links to empty. A utils
function is provided.

---------

Signed-off-by: Benjamin Klein <benjamin.klein.bk1@roche.com>
Signed-off-by: Benjamin Klein <45211351+Binrix@users.noreply.github.com>
Co-authored-by: Benjamin Klein <benjamin.klein.bk1@roche.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants