webdav: auth-challenge clients correctly:

* return 403 instead of 404 in the following sitations:
  * viewing an RSS feed without necessary auth
  * accessing a file with the wrong filekey
  * accessing a file/folder without necessary auth
     (would previously 404 for intentional ambiguity)

* only allow PROPFIND if user has either read or write;
   previously a blank response was returned if user has
   get-access, but this could confuse webdav clients into
   skipping authentication (for example AuthPass)

* return 401 basic-challenge instead of 403 if the client
   appears to be non-graphical, because many webdav clients
   do not provide the credentials until they're challenged.
   There is a heavy bias towards assuming the client is a
   browser, because browsers must NEVER EVER get a 401
   (tricky state that is near-impossible to deal with)

* return 401 basic-challenge instead of 403 if a PUT
   is attempted without any credentials included; this
   should be safe, as graphical browsers never do that

this fixes the interoperability issues mentioned in
https://github.com/authpass/authpass/issues/379
where AuthPass would GET files without providing the
password because it expected a 401 instead of a 403;
AuthPass is behaving correctly, this is not a bug
This commit is contained in:
ed 2024-11-27 22:07:53 +00:00
parent 697a4fa8a4
commit 2ce8233921
2 changed files with 24 additions and 11 deletions

View file

@ -1243,7 +1243,7 @@ class HttpCli(object):
self.log("RSS %s @%s" % (self.req, self.uname)) self.log("RSS %s @%s" % (self.req, self.uname))
if not self.can_read: if not self.can_read:
return self.tx_404() return self.tx_404(True)
vn = self.vn vn = self.vn
if not vn.flags.get("rss"): if not vn.flags.get("rss"):
@ -1484,7 +1484,7 @@ class HttpCli(object):
t2 = " or 'infinity'" if self.args.dav_inf else "" t2 = " or 'infinity'" if self.args.dav_inf else ""
raise Pebkac(412, t.format(depth, t2)) raise Pebkac(412, t.format(depth, t2))
if not self.can_read and not self.can_write and not self.can_get and not fgen: if not self.can_read and not self.can_write and not fgen:
self.log("inaccessible: [%s]" % (self.vpath,)) self.log("inaccessible: [%s]" % (self.vpath,))
raise Pebkac(401, "authenticate") raise Pebkac(401, "authenticate")
@ -1766,7 +1766,7 @@ class HttpCli(object):
if not self.can_write: if not self.can_write:
t = "user %s does not have write-access under /%s" t = "user %s does not have write-access under /%s"
raise Pebkac(403, t % (self.uname, self.vn.vpath)) raise Pebkac(403 if self.pw else 401, t % (self.uname, self.vn.vpath))
if not self.args.no_dav and self._applesan(): if not self.args.no_dav and self._applesan():
return self.headers.get("content-length") == "0" return self.headers.get("content-length") == "0"
@ -4624,6 +4624,19 @@ class HttpCli(object):
if "th" in self.ouparam: if "th" in self.ouparam:
return self.tx_svg("e" + pt[:3]) return self.tx_svg("e" + pt[:3])
# most webdav clients will not send credentials until they
# get 401'd, so send a challenge if we're Absolutely Sure
# that the client is not a graphical browser
if (
rc == 403
and not self.pw
and not self.ua.startswith("Mozilla/")
and "sec-fetch-site" not in self.headers
and "referer" not in self.headers
):
rc = 401
self.out_headers["WWW-Authenticate"] = 'Basic realm="a"'
t = t.format(self.args.SR) t = t.format(self.args.SR)
qv = quotep(self.vpaths) + self.ourlq() qv = quotep(self.vpaths) + self.ourlq()
html = self.j2s( html = self.j2s(
@ -5265,7 +5278,7 @@ class HttpCli(object):
st = bos.stat(abspath) st = bos.stat(abspath)
except: except:
if "on404" not in vn.flags: if "on404" not in vn.flags:
return self.tx_404() return self.tx_404(not self.can_read)
ret = self.on40x(vn.flags["on404"], vn, rem) ret = self.on40x(vn.flags["on404"], vn, rem)
if ret == "true": if ret == "true":
@ -5276,9 +5289,9 @@ class HttpCli(object):
try: try:
st = bos.stat(abspath) st = bos.stat(abspath)
except: except:
return self.tx_404() return self.tx_404(not self.can_read)
else: else:
return self.tx_404() return self.tx_404(not self.can_read)
if rem.startswith(".hist/up2k.") or ( if rem.startswith(".hist/up2k.") or (
rem.endswith("/dir.txt") and rem.startswith(".hist/th/") rem.endswith("/dir.txt") and rem.startswith(".hist/th/")
@ -5411,7 +5424,7 @@ class HttpCli(object):
if not is_dir and (self.can_read or self.can_get): if not is_dir and (self.can_read or self.can_get):
if not self.can_read and not fk_pass and "fk" in vn.flags: if not self.can_read and not fk_pass and "fk" in vn.flags:
if not use_filekey: if not use_filekey:
return self.tx_404() return self.tx_404(True)
if add_og and not abspath.lower().endswith(".md"): if add_og and not abspath.lower().endswith(".md"):
if og_ua or self.host not in self.headers.get("referer", ""): if og_ua or self.host not in self.headers.get("referer", ""):

View file

@ -286,8 +286,8 @@ class TestDots(unittest.TestCase):
self.assertIn('">folder</text>', self.curl(zs, "u2")[1]) self.assertIn('">folder</text>', self.curl(zs, "u2")[1])
# fk enabled, so this should fail # fk enabled, so this should fail
self.assertIn('">e404</text>', self.curl("dk,fk/f.t1?th=x", "u2")[1]) self.assertIn('">e403</text>', self.curl("dk,fk/f.t1?th=x", "u2")[1])
self.assertIn('">e404</text>', self.curl("dk,fk/s1/f.t2?th=x", "u2")[1]) self.assertIn('">e403</text>', self.curl("dk,fk/s1/f.t2?th=x", "u2")[1])
# but dk should return correct filekeys, so try that # but dk should return correct filekeys, so try that
zs = "dk,fk/%s&th=x" % (zj["files"][0]["href"]) zs = "dk,fk/%s&th=x" % (zj["files"][0]["href"])
@ -332,8 +332,8 @@ class TestDots(unittest.TestCase):
self.assertIn('">folder</text>', self.curl(zs, "u2")[1]) self.assertIn('">folder</text>', self.curl(zs, "u2")[1])
# fk enabled, so this should fail # fk enabled, so this should fail
self.assertIn('">e404</text>', self.curl("dks,fk/f.t1?th=x", "u2")[1]) self.assertIn('">e403</text>', self.curl("dks,fk/f.t1?th=x", "u2")[1])
self.assertIn('">e404</text>', self.curl("dks,fk/s1/f.t2?th=x", "u2")[1]) self.assertIn('">e403</text>', self.curl("dks,fk/s1/f.t2?th=x", "u2")[1])
# but dk should return correct filekeys, so try that # but dk should return correct filekeys, so try that
zs = "dks,fk/%s&th=x" % (zj["files"][0]["href"]) zs = "dks,fk/%s&th=x" % (zj["files"][0]["href"])