1908 : HtsgetReader should not send HTSGet ticket server headers in data server requests

[andaca]

Fixes #1908 : HtsgetReader should not send HTSGet ticket server headers in data server requests

HtsgetReader should not include ticket server config / headers in requests to the data server. See issue #1908 for further details.

[jrobinso]

It does look like your change conforms to the spec, however I am not in a position to review this change or any htsget related issue.

@brainstorm I'm not sure if you use igv.js there, but do you have any comments on this? With this change the Authorization: bearer token will not be included in the data fetch requests unless it was returned in the headers property of the initial response. From reviewing the code, the IGV desktop implementation DOES NOT behave this way. Given how old this code is either (1) everyone is returning the authoriziation bearer headers in the response json, so the fact we are adding it is a no-op, or (2) servers that IGV has been accessing are non-conforming in this regard.

[lbergelson]

@andaca Is there an up to date Htsget server that can be used for reference / testing against? It's very hard to maintain Htsget code without a server implementation. I'm not clear on who is using Htsget in the wild. Are you hosting your own files or accessing them from some where else? It's been very hard to tell how active / in demand it is.

[jrobinso]

This should probably be just

const options = {headers: urlData.headers || {})

[brainstorm]

@jrobinso @andaca @lbergelson A public htsget-rs instance is being deployed very soon (including support for Crypt4GH) in a GA4GH official and public domain, administered by them.

Meanwhile, can I interest you in deploying our server locally if you need to test/implement anything htsget client related?:

https://github.com/umccr/htsget-rs

Happy to help with any troubleshooting, we hope that the docs are clear enough too? We'll fix them presto if not!

Public htsget instance announcement aside, I'll now look at this particular issue/question/code and answer in a separate message here, just wanted to get the server support question out of the way fast.

/cc @mmalenic

[jrobinso]

@brainstorm I think this PR is correct wrt the spec, but not all servers out there are spec-compliant. I'm just running it by you to see if you anticipate any issues at UMCCR. The main effect will be Authorization bearer tokens will not be passed unless the server response to the call to get the "tickets" return them.

I'm hesitant to change things that have been working for years untouched.

[brainstorm]

Revising the changes and discussing with my colleage @mmalenic, this change would not affect our implementation since:

htsget-rs has the ability to forward auth headers for the data server (and this can be turned off). For AWS/S3, it returns presigned urls anyway which don't need additional auth headers

So I'd say go ahead and merge @jrobinso, although before doing so, I'd like to know what @andaca is testing those changes against?

[mmalenic]

I think this PR is correct wrt the spec

Seconding this, yes, this behaviour should be correct spec-wise. This shouldn't be an issue for htsget-rs/UMCCR.

[andaca]

Thanks for the quick responses :)

I stumbled across the defect and tested against a private HTSGet implementation (I'm at Genomics England), but will gladly check out / verify with @brainstorm's implementation (nice one and congrats on the public go-live btw!), though I might not get around to it until next week.

Interestingly, seeing mention of pre-signed URLs - it was actually adding S3 presigned URLs to our internal implementation that highlighted this issue; when including an Authorization header to the request to the presigned URL you'd get an unhappy (400) response:

<?xml version="1.0" encoding="UTF-8"?>
<Error>
<Code>InvalidArgument</Code>
<Message>Only one auth mechanism allowed; only the X-Amz-Algorithm query parameter, Signature query string parameter or the Authorization header should be specified</Message><ArgumentName>Authorization</ArgumentName>
<ArgumentValue>Bearer **REDACTED**</ArgumentValue><RequestId>**FOO**</RequestId><HostId>**BAR**</HostId>
</Error>