fix: simplify onboarding redirect link handling
- commit
- 6521b774e72bc8ea4fc79e8e90dba65afac1a300
- author
- brice · · 2026-04-01 12:27 UTC
- parent
- 39cdcd8
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +27 -15
M internal/adapters/in/http/middleware/cidr.go +2 -2
| @@ -79,8 +79,8 @@ func HTTPSAuthority(host string, httpPort, tlsPort int) string { | |||
| 79 | 79 | return host | |
| 80 | 80 | } | |
| 81 | 81 | ||
| 82 | - | port, _ := strconv.Atoi(portStr) // Ignore parse errors so invalid explicit ports fall through to the preserve-as-is branch. | |
| 83 | - | if port == httpPort { | |
| 82 | + | port, err := strconv.Atoi(portStr) | |
| 83 | + | if err == nil && port == httpPort { | |
| 84 | 84 | return net.JoinHostPort(hostname, strconv.Itoa(tlsPort)) | |
| 85 | 85 | } | |
| 86 | 86 | ||
| @@ -79,8 +79,8 @@ func HTTPSAuthority(host string, httpPort, tlsPort int) string { | |||||
| 79 | return host | 79 | return host | ||
| 80 | } | 80 | } | ||
| 81 | 81 | ||||
| 82 | - | port, _ := strconv.Atoi(portStr) // Ignore parse errors so invalid explicit ports fall through to the preserve-as-is branch. | 82 | + | port, err := strconv.Atoi(portStr) |
| 83 | - | if port == httpPort { | 83 | + | if err == nil && port == httpPort { |
| 84 | return net.JoinHostPort(hostname, strconv.Itoa(tlsPort)) | 84 | return net.JoinHostPort(hostname, strconv.Itoa(tlsPort)) | ||
| 85 | } | 85 | } | ||
| 86 | 86 | ||||
M internal/adapters/in/http/middleware/cidr_test.go +19 -0
| @@ -167,6 +167,25 @@ func TestHTTPSRedirect_UnknownExplicitPort_IsPreserved(t *testing.T) { | |||
| 167 | 167 | assert.Equal(t, "https://o2.bnema.dev:9999/", rr.Header().Get("Location")) | |
| 168 | 168 | } | |
| 169 | 169 | ||
| 170 | + | func TestHTTPSRedirect_InvalidExplicitPort_IsPreserved(t *testing.T) { | |
| 171 | + | log := testLogger() | |
| 172 | + | ok := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |
| 173 | + | w.WriteHeader(http.StatusOK) | |
| 174 | + | }) | |
| 175 | + | ||
| 176 | + | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | |
| 177 | + | ||
| 178 | + | req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) | |
| 179 | + | req.Host = "o2.bnema.dev:abcd" | |
| 180 | + | req.RemoteAddr = "203.0.113.10:12345" | |
| 181 | + | rr := httptest.NewRecorder() | |
| 182 | + | ||
| 183 | + | handler.ServeHTTP(rr, req) | |
| 184 | + | ||
| 185 | + | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | |
| 186 | + | assert.Equal(t, "https://o2.bnema.dev:abcd/", rr.Header().Get("Location")) | |
| 187 | + | } | |
| 188 | + | ||
| 170 | 189 | func TestProxyCIDRAllowlist(t *testing.T) { | |
| 171 | 190 | log := testLogger() | |
| 172 | 191 | redirect := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| @@ -167,6 +167,25 @@ func TestHTTPSRedirect_UnknownExplicitPort_IsPreserved(t *testing.T) { | |||||
| 167 | assert.Equal(t, "https://o2.bnema.dev:9999/", rr.Header().Get("Location")) | 167 | assert.Equal(t, "https://o2.bnema.dev:9999/", rr.Header().Get("Location")) | ||
| 168 | } | 168 | } | ||
| 169 | 169 | ||||
| 170 | + | func TestHTTPSRedirect_InvalidExplicitPort_IsPreserved(t *testing.T) { | |||
| 171 | + | log := testLogger() | |||
| 172 | + | ok := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | |||
| 173 | + | w.WriteHeader(http.StatusOK) | |||
| 174 | + | }) | |||
| 175 | + | ||||
| 176 | + | handler := HTTPSRedirect(nil, 8088, 8443, true, log)(ok) | |||
| 177 | + | ||||
| 178 | + | req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) | |||
| 179 | + | req.Host = "o2.bnema.dev:abcd" | |||
| 180 | + | req.RemoteAddr = "203.0.113.10:12345" | |||
| 181 | + | rr := httptest.NewRecorder() | |||
| 182 | + | ||||
| 183 | + | handler.ServeHTTP(rr, req) | |||
| 184 | + | ||||
| 185 | + | assert.Equal(t, http.StatusPermanentRedirect, rr.Code) | |||
| 186 | + | assert.Equal(t, "https://o2.bnema.dev:abcd/", rr.Header().Get("Location")) | |||
| 187 | + | } | |||
| 188 | + | ||||
| 170 | func TestProxyCIDRAllowlist(t *testing.T) { | 189 | func TestProxyCIDRAllowlist(t *testing.T) { | ||
| 171 | log := testLogger() | 190 | log := testLogger() | ||
| 172 | redirect := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 191 | redirect := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
M internal/adapters/in/http/onboarding/handler.go +2 -9
| @@ -69,18 +69,11 @@ func (h *Handler) ServeOnboardingPage(w http.ResponseWriter, r *http.Request) { | |||
| 69 | 69 | redirectURL := h.publicHTTPSURL(r) | |
| 70 | 70 | showSiteLink := !isRawIP(r.Host) | |
| 71 | 71 | ||
| 72 | - | // On HTTP (r.TLS == nil), omit Secure from the cookie so browsers persist it. | |
| 73 | - | // On HTTPS, do not set the cookie at all — it is an HTTP-bootstrap-only UX hint. | |
| 74 | - | cookieJS := "" | |
| 75 | - | if r.TLS == nil { | |
| 76 | - | cookieJS = `onclick="document.cookie='gordon-ca-installed=1;path=/;max-age=315360000;SameSite=Lax'"` | |
| 77 | - | } | |
| 78 | - | ||
| 79 | 72 | siteLink := "" | |
| 80 | 73 | if showSiteLink { | |
| 81 | 74 | siteLink = fmt.Sprintf( | |
| 82 | - | `<div class="skip"><p>Already installed? <a href="%s" %s>Go to site →</a></p></div>`, | |
| 83 | - | redirectURL, cookieJS, | |
| 75 | + | `<div class="skip"><p>Already installed? <a href="%s">Go to site →</a></p></div>`, | |
| 76 | + | redirectURL, | |
| 84 | 77 | ) | |
| 85 | 78 | } | |
| 86 | 79 | ||
| @@ -69,18 +69,11 @@ func (h *Handler) ServeOnboardingPage(w http.ResponseWriter, r *http.Request) { | |||||
| 69 | redirectURL := h.publicHTTPSURL(r) | 69 | redirectURL := h.publicHTTPSURL(r) | ||
| 70 | showSiteLink := !isRawIP(r.Host) | 70 | showSiteLink := !isRawIP(r.Host) | ||
| 71 | 71 | ||||
| 72 | - | // On HTTP (r.TLS == nil), omit Secure from the cookie so browsers persist it. | |||
| 73 | - | // On HTTPS, do not set the cookie at all — it is an HTTP-bootstrap-only UX hint. | |||
| 74 | - | cookieJS := "" | |||
| 75 | - | if r.TLS == nil { | |||
| 76 | - | cookieJS = `onclick="document.cookie='gordon-ca-installed=1;path=/;max-age=315360000;SameSite=Lax'"` | |||
| 77 | - | } | |||
| 78 | - | ||||
| 79 | siteLink := "" | 72 | siteLink := "" | ||
| 80 | if showSiteLink { | 73 | if showSiteLink { | ||
| 81 | siteLink = fmt.Sprintf( | 74 | siteLink = fmt.Sprintf( | ||
| 82 | - | `<div class="skip"><p>Already installed? <a href="%s" %s>Go to site →</a></p></div>`, | 75 | + | `<div class="skip"><p>Already installed? <a href="%s">Go to site →</a></p></div>`, |
| 83 | - | redirectURL, cookieJS, | 76 | + | redirectURL, |
| 84 | ) | 77 | ) | ||
| 85 | } | 78 | } | ||
| 86 | 79 | ||||
M internal/adapters/in/http/onboarding/handler_test.go +4 -4
| @@ -151,14 +151,14 @@ func TestHandler_OnboardingPage_IncludesFingerprintAndClientImportCopy(t *testin | |||
| 151 | 151 | assert.NotContains(t, body, "sudo gordon ca install") | |
| 152 | 152 | } | |
| 153 | 153 | ||
| 154 | - | func TestHandler_OnboardingPage_HTTPVariantDoesNotUseSecureCookieHint(t *testing.T) { | |
| 154 | + | func TestHandler_OnboardingPage_GoToSiteLinkIsPlainLink(t *testing.T) { | |
| 155 | 155 | srv, _, _ := newTestServer(t) | |
| 156 | 156 | ||
| 157 | - | // Default httptest server is HTTP, so request is non-TLS. | |
| 158 | 157 | body := getOnboardingBody(t, srv, "o2.bnema.dev") | |
| 159 | 158 | ||
| 160 | - | // The cookie JS must NOT include "Secure" on the HTTP variant. | |
| 161 | - | assert.NotContains(t, body, ";Secure") | |
| 159 | + | assert.Contains(t, body, `<a href="https://o2.bnema.dev/">Go to site →</a>`) | |
| 160 | + | assert.NotContains(t, body, "onclick=") | |
| 161 | + | assert.NotContains(t, body, "document.cookie") | |
| 162 | 162 | } | |
| 163 | 163 | ||
| 164 | 164 | func TestHandler_OnboardingPage_HEAD(t *testing.T) { | |
| @@ -151,14 +151,14 @@ func TestHandler_OnboardingPage_IncludesFingerprintAndClientImportCopy(t *testin | |||||
| 151 | assert.NotContains(t, body, "sudo gordon ca install") | 151 | assert.NotContains(t, body, "sudo gordon ca install") | ||
| 152 | } | 152 | } | ||
| 153 | 153 | ||||
| 154 | - | func TestHandler_OnboardingPage_HTTPVariantDoesNotUseSecureCookieHint(t *testing.T) { | 154 | + | func TestHandler_OnboardingPage_GoToSiteLinkIsPlainLink(t *testing.T) { |
| 155 | srv, _, _ := newTestServer(t) | 155 | srv, _, _ := newTestServer(t) | ||
| 156 | 156 | ||||
| 157 | - | // Default httptest server is HTTP, so request is non-TLS. | |||
| 158 | body := getOnboardingBody(t, srv, "o2.bnema.dev") | 157 | body := getOnboardingBody(t, srv, "o2.bnema.dev") | ||
| 159 | 158 | ||||
| 160 | - | // The cookie JS must NOT include "Secure" on the HTTP variant. | 159 | + | assert.Contains(t, body, `<a href="https://o2.bnema.dev/">Go to site →</a>`) |
| 161 | - | assert.NotContains(t, body, ";Secure") | 160 | + | assert.NotContains(t, body, "onclick=") |
| 161 | + | assert.NotContains(t, body, "document.cookie") | |||
| 162 | } | 162 | } | ||
| 163 | 163 | ||||
| 164 | func TestHandler_OnboardingPage_HEAD(t *testing.T) { | 164 | func TestHandler_OnboardingPage_HEAD(t *testing.T) { | ||