From 2ce8233921d5cb38954501d327db3c1a6e35d9d7 Mon Sep 17 00:00:00 2001 From: ed Date: Wed, 27 Nov 2024 22:07:53 +0000 Subject: [PATCH] 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 --- copyparty/httpcli.py | 27 ++++++++++++++++++++------- tests/test_dots.py | 8 ++++---- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/copyparty/httpcli.py b/copyparty/httpcli.py index 76f8883a..cd5f5ada 100644 --- a/copyparty/httpcli.py +++ b/copyparty/httpcli.py @@ -1243,7 +1243,7 @@ class HttpCli(object): self.log("RSS %s @%s" % (self.req, self.uname)) if not self.can_read: - return self.tx_404() + return self.tx_404(True) vn = self.vn if not vn.flags.get("rss"): @@ -1484,7 +1484,7 @@ class HttpCli(object): t2 = " or 'infinity'" if self.args.dav_inf else "" 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,)) raise Pebkac(401, "authenticate") @@ -1766,7 +1766,7 @@ class HttpCli(object): if not self.can_write: 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(): return self.headers.get("content-length") == "0" @@ -4624,6 +4624,19 @@ class HttpCli(object): if "th" in self.ouparam: 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) qv = quotep(self.vpaths) + self.ourlq() html = self.j2s( @@ -5265,7 +5278,7 @@ class HttpCli(object): st = bos.stat(abspath) except: 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) if ret == "true": @@ -5276,9 +5289,9 @@ class HttpCli(object): try: st = bos.stat(abspath) except: - return self.tx_404() + return self.tx_404(not self.can_read) else: - return self.tx_404() + return self.tx_404(not self.can_read) if rem.startswith(".hist/up2k.") or ( 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 self.can_read and not fk_pass and "fk" in vn.flags: if not use_filekey: - return self.tx_404() + return self.tx_404(True) if add_og and not abspath.lower().endswith(".md"): if og_ua or self.host not in self.headers.get("referer", ""): diff --git a/tests/test_dots.py b/tests/test_dots.py index eeef0630..928a3d69 100644 --- a/tests/test_dots.py +++ b/tests/test_dots.py @@ -286,8 +286,8 @@ class TestDots(unittest.TestCase): self.assertIn('">folder', self.curl(zs, "u2")[1]) # fk enabled, so this should fail - self.assertIn('">e404', self.curl("dk,fk/f.t1?th=x", "u2")[1]) - self.assertIn('">e404', self.curl("dk,fk/s1/f.t2?th=x", "u2")[1]) + self.assertIn('">e403', self.curl("dk,fk/f.t1?th=x", "u2")[1]) + self.assertIn('">e403', self.curl("dk,fk/s1/f.t2?th=x", "u2")[1]) # but dk should return correct filekeys, so try that zs = "dk,fk/%s&th=x" % (zj["files"][0]["href"]) @@ -332,8 +332,8 @@ class TestDots(unittest.TestCase): self.assertIn('">folder', self.curl(zs, "u2")[1]) # fk enabled, so this should fail - self.assertIn('">e404', self.curl("dks,fk/f.t1?th=x", "u2")[1]) - self.assertIn('">e404', self.curl("dks,fk/s1/f.t2?th=x", "u2")[1]) + self.assertIn('">e403', self.curl("dks,fk/f.t1?th=x", "u2")[1]) + self.assertIn('">e403', self.curl("dks,fk/s1/f.t2?th=x", "u2")[1]) # but dk should return correct filekeys, so try that zs = "dks,fk/%s&th=x" % (zj["files"][0]["href"])