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

JBrowse: enable callbacks in style options #2442

Closed
wants to merge 5 commits into from
Closed

JBrowse: enable callbacks in style options #2442

wants to merge 5 commits into from

Conversation

loraine-gueguen
Copy link
Contributor

FOR CONTRIBUTOR:

  • - I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • - License permits unrestricted use (educational + commercial)
  • - This PR adds a new tool or tool collection
  • - This PR updates an existing tool or tool collection
  • - This PR does something else (explain below)

Fixes #2441

Copy link
Contributor

@abretaud abretaud left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!
Can you bump the tool +galaxy version number?

@loraine-gueguen
Copy link
Contributor Author

loraine-gueguen commented Jun 5, 2019

Can you bump the tool +galaxy version number?

ok, but I am not sure which number to bump. This one:
<token name="@WRAPPER_VERSION@">galaxy3</token>
replace galaxy3 by galaxy4 ?

@abretaud
Copy link
Contributor

abretaud commented Jun 5, 2019

yep!

Copy link
Member

@hexylena hexylena left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution @loraine-gueguen.

But I'm not sure we should merge this. We wrote those sanitisation routines to prevent exactly this, that user-input javascript code is executed.

Galaxy normally strips all such content from displayed HTML files to protect users. We wanted to provide more rich html content but still offer (our best attempt at) a similar protection.

Any code executing on such a page will have access to galaxy resources that they shouldn't, and can execute calls on behalf of the executing user. You could imagine a malicious jbrowse shared by a colleague which wiped out all of your datasets.

Edit: let me go verify these claims I'm making.

I can definitely see this being useful to some groups, but I'm not sure how to balance the security implications against the utility. I'm afraid I'm leaning towards the more secure option.

@abretaud
Copy link
Contributor

abretaud commented Jun 5, 2019

Ah yes, it could be dangerous indeed, hadn't thought about that...
Maybe some special syntax or yet another option to allow getting field XX from current/parent feature?

@hexylena
Copy link
Member

hexylena commented Jun 5, 2019

It's framed, so accessing current page content isn't as much of an issue. I'd worry about it making requests on behalf of the current user though.

With the JBrowse tool whitelisted, pages with this content will return the user's history.

<script type="text/javascript">
const request = new Request('/api/histories');

f = fetch(request)
  .then(response => {
    if (response.status === 200) {
       return response.json();
    }
  })
  .then(response => {
    alert(response[0].name +' ' + response[0].id);
  });
</script>

You can make GET, PUT, etc. Updating / deleting histories is pretty easy.

@loraine-gueguen
Copy link
Contributor Author

I agree this not very secure...

Note that Javascript is already enabled for custom config options (I tried with a fmtDetailValue option): https://github.com/galaxyproject/tools-iuc/blob/master/tools/jbrowse/jbrowse.py#L711 So would need to be secured as well...

So what would be the best solution?

Here are some use cases with callbacks, I can think of:

  • get parent's attribute:
         "style" : {
            "label": "function(featureObject,value) { return featureObject.parent().get('gene_id')}",
            "description" : "function(featureObject,value) { if (featureObject.get('note')!=null) {value=featureObject.get('note')} if (featureObject.get('description')!=null) {value=featureObject.get('description')} else { value=featureObject.parent().get('description')} return value} "
         },
  • choose the color based on feature type (in style.color):
         "style" : {
            "className" : "feature",
            "color" : "function( featureObject, variableName, glyphObject, trackObject ) { return featureObject.get(\"type\")==\"CDS\" ? \"red\" : \"blue\"; }",
            "description" : "product"
         },
  • Customizing the "view details" pop-up with fmtDetailValue* and fmtDetailField* (https://github.com/GMOD/jbrowse/wiki/JBrowse_Configuration_Guide#customizing-parts-of-the-view-details-pop-ups-with-callbacks) :

    • add hyperlink to external databases depending on an attribute value (with "fmtDetailValue" custom config):
      "fmtDetailValue_Protein_domains" : "function(name, feature) {if(name[0].length==1){ if(name.substring(0,2)==\"PF\"){name=\"<a href=http://pfam.xfam.org/family/\"+name+\" target=_blank>\"+name+\"</a>\";} if(name.substring(0,2)==\"PS\"){name=\"<a href=http://prosite.expasy.org/cgi-bin/prosite/nicedoc.pl?\"+name+\" target=_blank>\"+name+\"</a>\";} if(name.substring(0,3)==\"IPR\"){name=\"<a href=http://www.ebi.ac.uk/interpro/entry/\"+name+\" target=_blank>\"+name+\"</a>\";} if(name.substring(0,4)==\"TIGR\"){name=\"<a href=http://www.jcvi.org/cgi-bin/tigrfams/HmmReportPage.cgi?acc=\"+name+\" target=_blank>\"+name+\"</a>\";}} else {for (var i=0;i<name.length;i++) { name[i]=name[i];}}; return name; }",

    • change the field name :
      "fmtDetailField_Tigr_role" : "function(name) { return \"TIGR_role\"; }",

@hexylena
Copy link
Member

hexylena commented Jun 7, 2019

Those are all very valid use cases. If we (on the galaxy side) can ever get around to implementing galaxyproject/galaxy#1932 then we could safely add this feature without worrying.

As it is, this could be some time :( Could we maybe identify a subset of useful / interesting cases? Those are nice but are there maybe others?

Note that Javascript is already enabled for custom config options

Oh shoot, we missed that.

hyperlink to external databases

Your snipet looks so familiar :) I think I once wrote a snippet that looked very like that. I always wanted to include it more properly in JBrowse but never had the time. Definitely something jbrowse should be taking care of for users.

@loraine-gueguen loraine-gueguen closed this by deleting the head repository Nov 10, 2022
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.

JBrowse: cannot use callbacks in style options
3 participants