Mark a page behind an auth filter private

Only the login page carried a Cache-Control, so a page an auth filter
had let a visitor see could be kept by a cache shared with the next
visitor. The page also varies on the cookie that got them in.

5 files changed, 42 insertions, 14 deletions

diff --git a/custom/servers/apache.conf b/custom/servers/apache.conf
index f25c444..d95d76f 100644
--- a/custom/servers/apache.conf
+++ b/custom/servers/apache.conf
@@ -204,11 +204,11 @@ AddType text/plain               .txt
     # Site-wide security headers are set here so they also cover the static
     # assets Apache serves. cgit itself sends only the headers the proxy
     # cannot supply. Those are Status, Content-Type, Content-Length and
-    # Content-Disposition on downloads, Location on redirects, a no-store
-    # Cache-Control on unauthenticated responses, the auth filter's Set-Cookie,
-    # and on raw repository bytes a nosniff of its own next to the stricter
-    # policy "default-src 'none'". Everything else, this policy included, is
-    # the proxy's job.
+    # Content-Disposition on downloads, Location on redirects, a Cache-Control
+    # marking the login page no-store and a page behind an auth filter private,
+    # the auth filter's Set-Cookie, and on raw repository bytes a nosniff of
+    # its own next to the stricter policy "default-src 'none'". Everything
+    # else, this policy included, is the proxy's job.
     #
     # The word setifempty is load bearing on the two headers cgit can also
     # emit. "Header always set" replaces a same-named header even when the
diff --git a/custom/servers/lighttpd.conf b/custom/servers/lighttpd.conf
index c2a478f..c81306f 100644
--- a/custom/servers/lighttpd.conf
+++ b/custom/servers/lighttpd.conf
@@ -75,11 +75,11 @@ $HTTP["host"] == "git.example.org" {
     # Site-wide security headers are set here so they also cover the static
     # assets lighttpd serves. cgit itself sends only the headers the server
     # cannot supply. Those are Status, Content-Type, Content-Length and
-    # Content-Disposition on downloads, Location on redirects, a no-store
-    # Cache-Control on unauthenticated responses, the auth filter's Set-Cookie,
-    # and on raw repository bytes a nosniff of its own next to the stricter
-    # policy "default-src 'none'". Everything else, this policy included, is
-    # the server's job.
+    # Content-Disposition on downloads, Location on redirects, a Cache-Control
+    # marking the login page no-store and a page behind an auth filter private,
+    # the auth filter's Set-Cookie, and on raw repository bytes a nosniff of
+    # its own next to the stricter policy "default-src 'none'". Everything
+    # else, this policy included, is the server's job.
     #
     # The add in add-response-header is load bearing. It appends a second
     # copy next to what cgit emitted, so a raw page carries both policies and
diff --git a/custom/servers/nginx.conf b/custom/servers/nginx.conf
index 7049368..8d4b86f 100644
--- a/custom/servers/nginx.conf
+++ b/custom/servers/nginx.conf
@@ -154,10 +154,11 @@ http {
         # static assets nginx serves directly. cgit itself sends only the
         # headers the proxy cannot supply. Those are Status, Content-Type,
         # Content-Length and Content-Disposition on downloads, Location on
-        # redirects, a no-store Cache-Control on unauthenticated responses, the
-        # auth filter's Set-Cookie, and on raw repository bytes a nosniff of its
-        # own next to the stricter policy "default-src 'none'". Everything else,
-        # this policy included, is the proxy's job.
+        # redirects, a Cache-Control marking the login page no-store and a page
+        # behind an auth filter private, the auth filter's Set-Cookie, and on
+        # raw repository bytes a nosniff of its own next to the stricter policy
+        # "default-src 'none'". Everything else, this policy included, is the
+        # proxy's job.
         #
         # add_header appends and never replaces what cgit sent, so a raw page
         # carries both policies and the browser enforces the stricter one,
diff --git a/source/ui-shared.c b/source/ui-shared.c
index a112fb2..2c4db2c 100644
--- a/source/ui-shared.c
+++ b/source/ui-shared.c
@@ -1465,8 +1465,13 @@ void cgit_print_http_headers(void)
 		html("X-Content-Type-Options: nosniff\n");
 		html("Content-Security-Policy: default-src 'none'\n");
 	}
+	// A page behind an auth filter is for the visitor who was let in, so a
+	// cache shared with other visitors must not keep it, and it varies on
+	// the cookie that got them in.
 	if (!ctx.env.authenticated)
 		html("Cache-Control: no-cache, no-store\n");
+	else if (ctx.cfg.auth_filter)
+		html("Cache-Control: private\nVary: Cookie\n");
 	html("\n");
 	// Some pages follow the headers with output written by git itself, not
 	// through html_raw, so the buffer is emptied to keep the headers first.
diff --git a/tests/t0303-robustness.sh b/tests/t0303-robustness.sh
index fd1c02e..8dff969 100755
--- a/tests/t0303-robustness.sh
+++ b/tests/t0303-robustness.sh
@@ -545,6 +545,28 @@ test_expect_success 'a client that disconnects ends the request quietly' '
 	! grep "die()" err
 '
 
+# A page an auth filter let a visitor see is theirs alone, so a cache
+# shared with other visitors has to be told, while a site without a filter
+# keeps its pages free of any such header.
+test_expect_success 'pages behind an auth filter are marked private' '
+	cat >letin.sh <<-\EOF &&
+	#!/bin/sh
+	exit 1
+	EOF
+	chmod +x letin.sh &&
+	{
+		echo "auth-filter=exec:$PWD/letin.sh" &&
+		cat robrc
+	} >letinrc &&
+	CGIT_CONFIG="$PWD/letinrc" QUERY_STRING="url=rob/log/" cgit >tmp &&
+	grep "^Status: 200" tmp &&
+	grep "^Cache-Control: private$" tmp &&
+	grep "^Vary: Cookie$" tmp &&
+	robq "url=rob/log/" >tmp &&
+	! grep "^Cache-Control" tmp &&
+	! grep "^Vary" tmp
+'
+
 # The about page redirects to its trailing-slash form so relative links
 # resolve, and the branch asked for has to survive that hop, as does the
 # hop back to the summary of a repository without a readme.