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

fetchBinaryRange - fails on Firefox and Safari #1186

Closed
keiranmraine opened this Issue Aug 20, 2018 · 8 comments

Comments

Projects
None yet
3 participants
@keiranmraine
Copy link
Contributor

keiranmraine commented Aug 20, 2018

fetchBinaryRange doesn't work on Firefox or Safari giving the following errors:

fetchBinaryRange/<@http://cgp-jbrowse.internal.sanger.ac.uk/Homo_sapiens/GRCh37/JBrowse/dist/0.bundle.js:4166:65
notify/</run@http://cgp-jbrowse.internal.sanger.ac.uk/Homo_sapiens/GRCh37/JBrowse/dist/main.bundle.js:8842:22
notify/<@http://cgp-jbrowse.internal.sanger.ac.uk/Homo_sapiens/GRCh37/JBrowse/dist/main.bundle.js:8859:30
module.exports/flush@http://cgp-jbrowse.internal.sanger.ac.uk/Homo_sapiens/GRCh37/JBrowse/dist/main.bundle.js:2988:9
 Error: HTTP 401 when fetching http://cgp-jbrowse.internal.sanger.ac.uk/Homo_sapiens/GRCh37/JBrowse/auto/nfs_irods-cgp-sr02-fg/intproj/827/sample/PD4099a/PD4099a.v0.5.9-r16+rugo.bam.bai bytes 0-262143
Stack trace:
fetchBinaryRange/<@http://cgp-jbrowse.internal.sanger.ac.uk/Homo_sapiens/GRCh37/JBrowse/dist/0.bundle.js:4166:65
notify/</run@http://cgp-jbrowse.internal.sanger.ac.uk/Homo_sapiens/GRCh37/JBrowse/dist/main.bundle.js:8842:22
notify/<@http://cgp-jbrowse.internal.sanger.ac.uk/Homo_sapiens/GRCh37/JBrowse/dist/main.bundle.js:8859:30
module.exports/flush@http://cgp-jbrowse.internal.sanger.ac.uk/Homo_sapiens/GRCh37/JBrowse/dist/main.bundle.js:2988:9

JBrowse version: 1.15.2
Safari: 11.1.2 (13605.3.8)
Firefox: 52.2.1 (64bit) (current is 60/61)

@cmdcolin

This comment has been minimized.

Copy link
Contributor

cmdcolin commented Aug 20, 2018

Just relaying what was said in the gitter chat, but the fact that this is an HTTP 401 error suggests maybe a server configuration issue.

I don't think that the jbrowse.org sample browser exhibits this issue even with firefox or safari so it is not an overarching issue on all deployments

@rbuels rbuels added this to the 1.15.3 milestone Aug 20, 2018

@rbuels rbuels self-assigned this Aug 20, 2018

@rbuels rbuels added the bug label Aug 20, 2018

@rbuels

This comment has been minimized.

Copy link
Collaborator

rbuels commented Aug 20, 2018

@keiranmraine is this 401 caused by JBrowse not forwarding some auth credentials in a scenario in which it used to forward them?

@keiranmraine

This comment has been minimized.

Copy link
Contributor Author

keiranmraine commented Aug 21, 2018

@rbuels, so I've confirmed turning off the basic_auth ldap layer in nginx allows Firefox and Safari to work, however that doesn't explain why Chrome works fine with it on. I've just found chromium 66 also fails.

In Chrome I can see the header Authorization: Basic ....., this is absent from the request in Chromium and Safari.

I've used exactly the same URL in browsers, but for simplicity just shown the reference sequence track (fa + fa.fai). The flow fails when requesting the fai file using tenacious-fetch.

The nginx config is pretty simple for ldap basic auth:

user OUR_USER OUR_GROUP; # comment for testing
worker_processes  auto;
  
events {
    worker_connections  512;
}
  
http {
    include       mime.types;
    disable_symlinks off;
    default_type  application/octet-stream;
    sendfile        on;
    keepalive_timeout  65;

    auth_ldap_cache_enabled on;
    auth_ldap_cache_expiration_time 3600000;
    auth_ldap_cache_size 1000;
  
    ldap_server cancer {
          # user search base.
        url "ldap://ldap-ro.internal.sanger.ac.uk/dc=sanger,dc=ac,dc=uk?uid?sub?(objectClass=person)";
          # bind as
        binddn "uid=cgppipe,ou=people,dc=sanger,dc=ac,dc=uk";
          # bind pw
        binddn_passwd "SOME_PASSWORD";
          # group attribute name which contains member object
        group_attribute member;
          # search for full DN in member object
        group_attribute_is_dn on;
          # matching algorithm (any / all)
        satisfy any;
          # list of allowed groups
        require group "CN=cancer,OU=group,DC=sanger,DC=ac,DC=uk";
        require valid_user;
    }
  
    server {
        listen              80; # change to 8000+ for non-root testing
        listen              443 ssl; # comment for non-root testing
        server_name         cgp-jbrowse; # set appropriately
        ssl_certificate     /jbrowse/nginx/server.crt; # set this to match $NGINX_DIR/server.crt
        ssl_certificate_key /jbrowse/nginx/server.key; # set this to match $NGINX_DIR/server.crt
  
        location / {
            root   html;
            index  index.html index.htm;
            auth_ldap "Restricted access cancer members only";
            auth_ldap_servers cancer;
        }
  
        location ~* "\.(json|txt)z$" {
           add_header Content-Encoding  gzip;
           gzip off;
           types { application/json jsonz; }
        }
  
        #error_page  404              /404.html;
  
        # redirect server error pages to the static page /50x.html
        #
        error_page   500 502 503 504  /50x.html;
        location = /50x.html {
            root   html;
        }
    }
}
@cmdcolin

This comment has been minimized.

Copy link
Contributor

cmdcolin commented Aug 21, 2018

If I add some AuthType Basic with an htpasswd and try opening with Safari I did see it fail with a 401

I dug a little bit into this and I saw a possible clue related to the fetch API, but I haven't validated it yet

https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch

"By default, fetch won't send or receive any cookies from the server, resulting in unauthenticated requests if the site relies on maintaining a user session (to send cookies, the credentials init option must be set).
Since Aug 25, 2017. The spec changed the default credentials policy to same-origin. Firefox changed since 61.0b13."


diff --git a/src/crossFetchBinaryRange.js b/src/crossFetchBinaryRange.js
index e41fcb5..15fc788 100644
--- a/src/crossFetchBinaryRange.js
+++ b/src/crossFetchBinaryRange.js
@@ -4,7 +4,7 @@ function crossFetchBinaryRange(url, start, end) {
   const requestDate = new Date()
   return crossFetch(url, {
     method: 'GET',
-    headers: { range: `bytes=${start}-${end}` },
+    headers: { range: `bytes=${start}-${end}`, credentials: 'same-origin' },
   }).then(res => {
     const responseDate = new Date()
     if (res.status !== 206 && res.status !== 200)

I think that this patch should fix it but I can't manually confirm it haven't gotten yarn link to work yet

rbuels added a commit that referenced this issue Aug 21, 2018

@rbuels

This comment has been minimized.

Copy link
Collaborator

rbuels commented Aug 21, 2018

@keiranmraine if you apply the change in 87cb632 to your local copy, does that fix the problem?

@keiranmraine

This comment has been minimized.

Copy link
Contributor Author

keiranmraine commented Aug 22, 2018

@rbuels, I can confirm the patch fixes Safari, Firefox and Chromium. 🥇

@cmdcolin

This comment has been minimized.

Copy link
Contributor

cmdcolin commented Aug 22, 2018

Nice!

@cmdcolin

This comment has been minimized.

Copy link
Contributor

cmdcolin commented Aug 22, 2018

I guess close for now :)

@cmdcolin cmdcolin closed this Aug 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.