Bug #17598
closed
[keep-web] should be tolerant of superfluous :443's in the arvados config
Added by Ward Vandewege almost 3 years ago.
Updated almost 3 years ago.
Release relationship:
Auto
Description
With a config like this:
WebDAVDownload:
ExternalURL: https://download.xxxx1.arvadosapi.com:443
keep-web does a literal comparison of ExternalURL.Host with a http.Request.Host in `services/keep-web/handler.go` line 251, and the superfluous :443 in the config trips that up, which causes it to go into "turtle mode" and fall back to the anonymous token. In the normal (i.e. no anonymous access) case, that breaks all downloads with a 401, for example it makes viewing collection contents in WB2 fail.
keep-web should handle the special cases of :443 and :80 correctly. (also :https and :http)
two other lessons learned that need to be fixed:
- if AnonymousUserToken is empty, don't add it to 'tokens' in handler.go:349 (this is #17597)
- if a token was provided but not used (credentialsOK is false on line 295), it should log a warning saying that it is refusing to accept credentials, give some indication why, and (if applicable) indicate it will fall back to trying AnonymousUserToken
- Status changed from New to In Progress
- Status changed from In Progress to New
- Subject changed from [keep-web to [keep-web] should be tolerant of superfluous :443's in the arvados config
- Description updated (diff)
- Description updated (diff)
- Description updated (diff)
- Description updated (diff)
- Assigned To set to Peter Amstutz
- Status changed from New to In Progress
17598-keep-web-url @ arvados|d48cff711af8255f3b2b69506b54a283c1aab776
- Remove :80 and :443 before doing comparison. This does mean it will treat :80 and :443 as the same vhost. This seems fine.
- Handle the case of "no anonymous user token" better
- Improve error response when an access token is being deliberately ignored by keep-web
- Update tests.
developer-run-tests: #2443 
StripDefaultPort()
The vhost explanation says "...did not match either Services.WebDAV or Services.WebDAVDownload" which (I think) will be confusing when it does in fact match Services.WebDAV url but doesn't contain a collection ID (cf parseCollectionIDFromDNSName).
Maybe this?
Authorization tokens are not accepted here: vhost %q does not specify a single collection ID or match Services.WebDAVDownload.ExternalURL %q, and Collections.TrustAllContent is false
Tom Clegg wrote:
StripDefaultPort()
- should be unexported stripDefaultPort (even though this is package main)
Fixed.
- will do weird things if host is like
[::1]:443 ... there is some code in net/url we could make use of here, like [...]
Oh, I completely forgot about IPv6 addresses. Fixed.
The vhost explanation says "...did not match either Services.WebDAV or Services.WebDAVDownload" which (I think) will be confusing when it does in fact match Services.WebDAV url but doesn't contain a collection ID (cf parseCollectionIDFromDNSName).
Maybe this?
Authorization tokens are not accepted here: vhost %q does not specify a single collection ID or match Services.WebDAVDownload.ExternalURL %q, and Collections.TrustAllContent is false
Updated message.
17598-keep-web-url @ arvados|c736087b86da2353965e5722a38cff5e7d891fd2
developer-run-tests: #2444 
- Status changed from In Progress to Resolved
- Status changed from Resolved to Feedback
Needs to also account for case because domain names are not supposed to be case sensitive
- Status changed from Feedback to Resolved
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by