Raise for a request that names nobody
A nil return is silent at a call site that forgets the check; the raise makes the miss loud.
Assisted-by: GLM-5.3 via pi
diff --git a/lib/pro_tacts/proxy_auth.rb b/lib/pro_tacts/proxy_auth.rb
index e62b38c..f54e168 100644
--- a/lib/pro_tacts/proxy_auth.rb
+++ b/lib/pro_tacts/proxy_auth.rb
@@ -8,11 +8,11 @@ module ProTacts
# (docs/plans/2026-09-15-identity-from-one-header.md, "Which
# header").
#
- # A request that names nobody is refused at the top of Web's route
- # (Web#unauthorized). That covers the two cases Tailscale documents as
- # having no identity: Funnel traffic, which is public, and traffic from
- # tagged devices. A family device that gets tagged will start seeing
- # 401s.
+ # A request that names nobody raises rather than passing a nil
+ # down (Web's error handler maps the raise to the 401). That
+ # covers the two cases Tailscale documents as having no identity:
+ # Funnel traffic, which is public, and traffic from tagged devices.
+ # A family device that gets tagged will start seeing 401s.
#
# Any login the proxy vouches for is accepted. Getting onto the tailnet
# is the access control; the login only picks which address book a
@@ -32,19 +32,28 @@ module ProTacts
HEADER = "Remote-User" #: String
ENV_KEY = "HTTP_#{HEADER.upcase.tr("-", "_")}" #: String
- # The login on a request, or nil for one that names nobody. Read as
- # UTF-8, which is the encoding a login reaches the store in
- # (AGENTS.md's note on STRICT columns); bytes that are not text at
- # all name nobody rather than raising at the bind, several layers
- # down, on a group named after them.
- #: (Rack::env env) -> String?
+ # What #login raises for a request that names nobody. Its own
+ # class, so Web's error handler can answer exactly it with the
+ # 401 and nothing wider is ever caught — an unexpected failure
+ # still propagates to Sentry (see web.rb).
+ class MissingLogin < StandardError
+ end
+
+ # The login on a request. Read as UTF-8, which is the encoding a
+ # login reaches the store in (AGENTS.md's note on STRICT
+ # columns); bytes that are not text at all name nobody rather
+ # than raising at the bind, several layers down, on a group
+ # named after them.
+ #: (Rack::env env) -> String
def self.login(env)
login = env[ENV_KEY].to_s.dup.force_encoding(Encoding::UTF_8)
# Checked before strip, which raises on invalid bytes.
- return unless login.valid_encoding?
+ raise MissingLogin, "the #{HEADER} header is not valid UTF-8" unless login.valid_encoding?
login = login.strip
- login unless login.empty?
+ raise MissingLogin, "no login on the #{HEADER} header" if login.empty?
+
+ login
end
end
end
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index e9ec6c7..26ab8f7 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -73,14 +73,40 @@ module ProTacts
"Not Found"
end
+ # The identity gate's refusal, as an exception mapped to a
+ # response: ProxyAuth.login raises MissingLogin for a request that
+ # names nobody, and this answers it with the 401. `classes:` keeps
+ # the plugin from touching any other raise — an unexpected failure
+ # still propagates to Sentry::Rack::CaptureExceptions outside the
+ # app, where it is reported, so this handler must not widen.
+ #
+ # A 401 must carry a challenge (RFC 9110 section 15.5.2), and
+ # `Proxy-Identity` is no registered scheme: it tells the client
+ # that the credential is the login its proxy vouches for, which
+ # nothing the client could send supplies.
+ #
+ # The body names the header that was read, because the failure it
+ # reports is almost always a proxy writing some other one. Naming
+ # it hands an attacker nothing: the proxy overwrites the header on
+ # every request, so knowing which one it is buys no way to forge
+ # it.
+ plugin :error_handler, classes: [ProxyAuth::MissingLogin] do |_e|
+ response.status = 401
+ response["WWW-Authenticate"] = "Proxy-Identity"
+ response["Content-Type"] = "text/plain"
+ "Unauthorized: no login on the #{ProxyAuth::HEADER} header.\n" \
+ "The proxy in front of this app writes that header.\n"
+ end
+
# The router's trunk: the identity gate, the static files, and the
# root the admin and DAV halves share. Every other first segment is
# a hash_branch in lib/pro_tacts/web/, run on this same instance, so
# what the trunk sets they see.
route do |r|
- # Every request names a tailnet user or is refused, the static
- # files included (ProTacts::ProxyAuth).
- @login = ProxyAuth.login(r.env) || unauthorized(r)
+ # Every request names a tailnet user or is refused — the static
+ # files included — because #login raises for one that names
+ # nobody and the error handler above answers it.
+ @login = ProxyAuth.login(r.env)
r.public
r.hash_branches
@@ -103,28 +129,6 @@ module ProTacts
private
- # The refusal of a request that names nobody. A 401 must carry a
- # challenge (RFC 9110 section 15.5.2), and `Proxy-Identity` is no
- # registered scheme: it tells the client that the credential is the
- # login its proxy vouches for, which nothing the client could send
- # supplies.
- #
- # The body names the header that was read, because the failure it
- # reports is almost always a proxy writing some other one. Naming it
- # hands an attacker nothing: the proxy overwrites the header on every
- # request, so knowing which one it is buys no way to forge it.
- #: (Roda::RodaRequest r) -> bot
- def unauthorized(r)
- response.status = 401
- response["WWW-Authenticate"] = "Proxy-Identity"
- response["Content-Type"] = "text/plain"
- response.write(
- "Unauthorized: no login on the #{ProxyAuth::HEADER} header.\n" \
- "The proxy in front of this app writes that header.\n"
- )
- r.halt
- end
-
#: () -> Store
def store
self.class.store
diff --git a/sig/gems/roda.rbs b/sig/gems/roda.rbs
index 6e77743..e235b49 100644
--- a/sig/gems/roda.rbs
+++ b/sig/gems/roda.rbs
@@ -6,7 +6,10 @@
# classes, regexps, symbols, hashes, and arrays of those — is open, and
# the block arguments a match yields depend on which matcher matched.
class Roda
- def self.plugin: (Symbol | Module plugin, *untyped args) ?{ () [self: instance] -> untyped } -> void
+ # The configure blocks vary by plugin — error_handler passes the
+ # exception, not_found and the routes pass nothing — so the block
+ # takes anything, like the matchers below.
+ def self.plugin: (Symbol | Module plugin, *untyped args, **untyped options) ?{ (*untyped) [self: instance] -> untyped } -> void
def self.use: (untyped middleware, **untyped options) -> void
diff --git a/test/pro_tacts/test_proxy_auth.rb b/test/pro_tacts/test_proxy_auth.rb
index 0d39e26..71d07c8 100644
--- a/test/pro_tacts/test_proxy_auth.rb
+++ b/test/pro_tacts/test_proxy_auth.rb
@@ -16,19 +16,19 @@ class ProxyAuthTest < Minitest::Test
end
def test_no_header_is_nobody
- assert_nil login(nil)
+ assert_raises(ProTacts::ProxyAuth::MissingLogin) { login(nil) }
end
def test_an_empty_login_is_nobody
- assert_nil login("")
+ assert_raises(ProTacts::ProxyAuth::MissingLogin) { login("") }
end
def test_a_blank_login_is_nobody
- assert_nil login(" ")
+ assert_raises(ProTacts::ProxyAuth::MissingLogin) { login(" ") }
end
def test_a_login_on_another_header_is_nobody
- assert_nil login(sent_as: "HTTP_TAILSCALE_USER_LOGIN")
+ assert_raises(ProTacts::ProxyAuth::MissingLogin) { login(sent_as: "HTTP_TAILSCALE_USER_LOGIN") }
end
# The proxy copies the value through, so a non-ASCII one arrives as the
@@ -38,6 +38,6 @@ class ProxyAuthTest < Minitest::Test
end
def test_a_login_that_is_not_utf8_is_nobody
- assert_nil login((+"zo\xEB").force_encoding(Encoding::BINARY))
+ assert_raises(ProTacts::ProxyAuth::MissingLogin) { login((+"zo\xEB").force_encoding(Encoding::BINARY)) }
end
end