From 789d5803a57439f5925a6d23915fef2ed8ae2fdb Mon Sep 17 00:00:00 2001
From: Jason Ernst <ernstjason1@gmail.com>
Date: Fri, 25 Sep 2026 14:00:40 -0700
Subject: [PATCH] Build the GitHub button's URL in Go, not in four templates
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

#631 moved GitHub's authorize URL into Go so the escaping was decided
once. It left each theme assembling the URL that reaches it:

  href="/login/github{{ if .next }}{{ if ne .next "/" }}?next={{ .next | urlquery }}{{ end }}{{ end }}"

A nested conditional building a URL, copied into four repositories —
the same shape of problem, one step further out.

GithubLoginURL decides it instead, and the templates print the result:

  href="{{ .github_login_url }}"

Beyond being shorter, this makes the URL's shape the server's
business: adding to it later touches no theme.

next stays in the context because the email login still redirects to it
in the browser once its AJAX call succeeds; that is behaviour rather
than URL construction.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 blog/blog.go                        | 25 +++++++++++++++++++---
 blog/github_login_test.go           | 33 +++++++++++++++++++++++++++++
 themes/default/templates/login.html |  2 +-
 3 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/blog/blog.go b/blog/blog.go
index 75b8a81..09b11ac 100644
--- a/blog/blog.go
+++ b/blog/blog.go
@@ -1379,14 +1379,18 @@ func (b *Blog) Login(c *gin.Context) {
 	}
 
 	clientID := os.Getenv("client_id")
+	next := SafeNext(c.Query("next"))
 	b.Render(c, http.StatusOK, "login.html", gin.H{
 		"logged_in": b.auth.IsLoggedIn(c),
 		"is_admin":  b.auth.IsAdmin(c),
 		// The only page whose markup uses .btn-social, so the only one that
 		// loads bootstrap-social (#630).
-		"login_page":          true,
-		"client_id":           clientID,
-		"next":                SafeNext(c.Query("next")),
+		"login_page": true,
+		"client_id":  clientID,
+		// next is still handed over for the email login, which redirects to it
+		// in the browser once its AJAX call succeeds.
+		"next":                next,
+		"github_login_url":    GithubLoginURL(next),
 		"version":             b.Version,
 		"title":               "Login",
 		"email_login_enabled": b.auth.EmailLoginEnabled(),
@@ -1472,6 +1476,21 @@ func RequestOrigin(c *gin.Context) string {
 	return scheme + "://" + c.Request.Host
 }
 
+// GithubLoginURL is where the login page's GitHub button points: /login/github,
+// carrying where the visitor was heading so GithubLogin can put it in the
+// session.
+//
+// The themes each assembled this with a nested template conditional, which is
+// the sort of thing moving the authorize URL into Go was meant to stop. A
+// template prints this value instead, so the URL's shape stays the server's
+// business and adding to it later touches no theme.
+func GithubLoginURL(next string) string {
+	if next == "" || next == "/" {
+		return "/login/github"
+	}
+	return "/login/github?next=" + url.QueryEscape(next)
+}
+
 // GithubAuthorizeURL builds the URL that starts GitHub's OAuth flow.
 //
 // redirect_uri carries no query of its own. GitHub matches it against the
diff --git a/blog/github_login_test.go b/blog/github_login_test.go
index 1650ddb..76f4a06 100644
--- a/blog/github_login_test.go
+++ b/blog/github_login_test.go
@@ -166,3 +166,36 @@ func TestOAuthSessionKeys(t *testing.T) {
 		t.Errorf("session keys are not distinct and non-empty: %q %q", auth.OAuthStateKey, auth.OAuthNextKey)
 	}
 }
+
+// TestGithubLoginURL: the themes each assembled this with a nested template
+// conditional. It is one value now, so the escaping and the empty-next case
+// are decided here rather than four times over in markup.
+func TestGithubLoginURL(t *testing.T) {
+	for _, tc := range []struct{ next, want string }{
+		{"", "/login/github"},
+		{"/", "/login/github"},
+		{"/admin/settings", "/login/github?next=%2Fadmin%2Fsettings"},
+		// An unescaped & would start a second parameter rather than staying
+		// part of next.
+		{"/search?q=a&b=c", "/login/github?next=%2Fsearch%3Fq%3Da%26b%3Dc"},
+	} {
+		if got := blog.GithubLoginURL(tc.next); got != tc.want {
+			t.Errorf("GithubLoginURL(%q) = %q, want %q", tc.next, got, tc.want)
+		}
+	}
+}
+
+// TestGithubLoginURL_SurvivesARoundTrip: what the button points at must parse
+// back to the next it was built from, since GithubLogin reads it as a query
+// parameter.
+func TestGithubLoginURL_SurvivesARoundTrip(t *testing.T) {
+	for _, next := range []string{"/admin/settings", "/search?q=a&b=c", "/posts/2026/01/01/a b"} {
+		u, err := url.Parse(blog.GithubLoginURL(next))
+		if err != nil {
+			t.Fatalf("next %q: does not parse: %v", next, err)
+		}
+		if got := u.Query().Get("next"); got != next {
+			t.Errorf("next %q came back as %q", next, got)
+		}
+	}
+}
diff --git a/themes/default/templates/login.html b/themes/default/templates/login.html
index d7a24a0..c8224ce 100644
--- a/themes/default/templates/login.html
+++ b/themes/default/templates/login.html
@@ -52,7 +52,7 @@ <h4>Site Login</h4>
     </div>
 
     <div class="container">
-      <a id="github-button" class="btn btn-social btn-github" href="/login/github{{ if .next }}{{ if ne .next "/" }}?next={{ .next | urlquery }}{{ end }}{{ end }}">
+      <a id="github-button" class="btn btn-social btn-github" href="{{ .github_login_url }}">
         <i class="fab fa-github fa-1x"></i> Sign in with GitHub
       </a>