From 6d377b5348429875ad61af70f6f4db051766b12d Mon Sep 17 00:00:00 2001 From: Sean Reifschneider Date: Tue, 1 Sep 2026 15:06:18 +0000 Subject: [PATCH] oidc: serve the registration confirmation page from a reloadable URL The interstitial was the body of /oidc/callback, the URL carrying the single-use code, so any reload re-entered the spent exchange. Redirect to GET /register/confirm/{auth_id}, also missing from the route table. --- hscontrol/app.go | 1 + hscontrol/handlers.go | 5 +- hscontrol/handlers_test.go | 2 +- hscontrol/oidc.go | 144 +++++++++-- hscontrol/oidc_callback_reload_test.go | 336 +++++++++++++++++++++++++ integration/scenario.go | 27 +- 6 files changed, 479 insertions(+), 36 deletions(-) create mode 100644 hscontrol/oidc_callback_reload_test.go diff --git a/hscontrol/app.go b/hscontrol/app.go index aad241c2..b763e3ac 100644 --- a/hscontrol/app.go +++ b/hscontrol/app.go @@ -478,6 +478,7 @@ func (h *Headscale) createRouter(apiV1Mux, apiV2Mux http.Handler) *chi.Mux { if provider, ok := h.authProvider.(*AuthProviderOIDC); ok { r.Get("/oidc/callback", provider.OIDCCallbackHandler) + r.Get("/register/confirm/{auth_id}", provider.RegisterConfirmGetHandler) r.Post("/register/confirm/{auth_id}", provider.RegisterConfirmHandler) } diff --git a/hscontrol/handlers.go b/hscontrol/handlers.go index 6ccfa611..7693ad94 100644 --- a/hscontrol/handlers.go +++ b/hscontrol/handlers.go @@ -73,7 +73,10 @@ func userMessageForStatusCode(code int) string { case code == http.StatusUnauthorized || code == http.StatusForbidden: return "You are not authorized. Please contact your administrator." case code == http.StatusGone: - return "Your session has expired. Please try again." + // Overwhelmingly a reload or a back button on a link the user + // already used, not a failure, so lead with that. + return "This link has already been used or has expired. " + + "If your device is connected you are done; otherwise start the login again." case code >= 400 && code < 500: return "The request could not be processed. Please try again." default: diff --git a/hscontrol/handlers_test.go b/hscontrol/handlers_test.go index 9eaae41e..668f545a 100644 --- a/hscontrol/handlers_test.go +++ b/hscontrol/handlers_test.go @@ -202,7 +202,7 @@ func TestHttpUserError(t *testing.T) { name: "gone_renders_session_expired", err: NewHTTPError(http.StatusGone, "login session expired, try again", nil), wantCode: http.StatusGone, - wantContains: "Your session has expired. Please try again.", + wantContains: "This link has already been used or has expired.", wantNotContain: "login session expired", }, { diff --git a/hscontrol/oidc.go b/hscontrol/oidc.go index fb2e6839..5737cda2 100644 --- a/hscontrol/oidc.go +++ b/hscontrol/oidc.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "slices" "strings" "time" @@ -349,7 +350,7 @@ func (a *AuthProviderOIDC) OIDCCallbackHandler( // /register/{auth_id} could silently complete a registration when // the IdP allows silent SSO. if authInfo.Registration { - a.renderRegistrationConfirmInterstitial(writer, req, authInfo.AuthID, user, nodeExpiry) + a.beginRegistrationConfirmation(writer, req, authInfo.AuthID, user, nodeExpiry) return } @@ -667,34 +668,70 @@ func (a *AuthProviderOIDC) createOrUpdateUserFromClaim( // browser do not collide. const registerConfirmCSRFCookie = "headscale_register_confirm" +// registrationLinkSpentMsg is logged when a user returns to a +// registration link whose session is gone, which is usually a reload or a +// back button after they already confirmed. The page the user sees comes +// from [userMessageForStatusCode]. +const registrationLinkSpentMsg = "registration link already used or expired" + +var errRegistrationLinkSpent = NewHTTPError(http.StatusGone, registrationLinkSpentMsg, nil) + +// registerConfirmURL is the browser-facing URL of the confirmation page. +// It is built from server_url, like [AuthProviderOIDC.RegisterURL] and the +// OIDC redirect URI, so a Headscale that a reverse proxy serves under a +// path prefix hands the browser a URL that resolves. +func (a *AuthProviderOIDC) registerConfirmURL(authID types.AuthID) string { + return authPathURL(a.serverURL, "register/confirm", authID) +} + // setRegisterConfirmCookie writes the per-session register-confirm CSRF // cookie. Pass the CSRF token and authCacheExpiration seconds to set it; // pass ("", -1) to clear it after the registration is finalised. -func setRegisterConfirmCookie( +func (a *AuthProviderOIDC) setRegisterConfirmCookie( writer http.ResponseWriter, req *http.Request, authID types.AuthID, value string, maxAge int, - secure bool, ) { + // Scope the cookie to the browser-facing path, which carries the + // reverse proxy's prefix; the routed path does not. + path := "/register/confirm/" + authID.String() + if u, err := url.Parse(a.registerConfirmURL(authID)); err == nil { //nolint:noinlineerr + path = u.Path + } + //nolint:gosec // G124: Secure from server_url scheme or req.TLS; HttpOnly + SameSite already set http.SetCookie(writer, &http.Cookie{ Name: registerConfirmCSRFCookie, Value: value, - Path: "/register/confirm/" + authID.String(), + Path: path, MaxAge: maxAge, - Secure: secure || req.TLS != nil, + Secure: a.cookiesSecure() || req.TLS != nil, HttpOnly: true, - SameSite: http.SameSiteStrictMode, + // Lax, not Strict: the callback sets this cookie and immediately + // redirects to the confirmation page. That hop ends a redirect + // chain which began cross-site at the IdP, and Firefox evaluates + // the whole chain, so a Strict cookie is withheld and the + // confirmation page 403s. Lax still never rides a cross-site + // POST, so the confirm submission stays protected. + SameSite: http.SameSiteLaxMode, }) } -// renderRegistrationConfirmInterstitial captures the resolved OIDC -// identity and node expiry into the cached [types.AuthRequest], sets the CSRF -// cookie, and renders the confirmation page that the user must -// explicitly submit before the registration is finalised. -func (a *AuthProviderOIDC) renderRegistrationConfirmInterstitial( +// beginRegistrationConfirmation captures the resolved OIDC identity and +// node expiry into the cached [types.AuthRequest], sets the CSRF cookie, and +// redirects the browser to the confirmation page. +// +// The interstitial is served from its own URL rather than written inline +// here, because this request carries the single-use OAuth authorization +// code. A page rendered on this response leaves the browser parked on the +// code-bearing URL, and anything that reloads it — an extension calling +// window.location.reload(), the back button, pull-to-refresh, a prerender +// — re-enters the callback with a spent code and paints an error over the +// interstitial. Redirecting keeps the code exchange one-shot and makes the +// page the user waits on safe to reload. +func (a *AuthProviderOIDC) beginRegistrationConfirmation( writer http.ResponseWriter, req *http.Request, authID types.AuthID, @@ -726,14 +763,78 @@ func (a *AuthProviderOIDC) renderRegistrationConfirmInterstitial( CSRF: csrf, }) - setRegisterConfirmCookie(writer, req, authID, csrf, int(authCacheExpiration.Seconds()), a.cookiesSecure()) + a.setRegisterConfirmCookie(writer, req, authID, csrf, int(authCacheExpiration.Seconds())) + + // 303 See Other so the browser issues a fresh GET for the + // confirmation page and leaves the code-bearing URL behind as a + // transient hop rather than a history entry it can return to. + http.Redirect(writer, req, a.registerConfirmURL(authID), http.StatusSeeOther) +} + +// RegisterConfirmGetHandler renders the OIDC registration confirmation +// interstitial. It is reached via the redirect that +// [AuthProviderOIDC.beginRegistrationConfirmation] issues from the OIDC +// callback, and it is safe to reload: it only reads the pending +// confirmation captured on the cached [types.AuthRequest] and never touches +// the one-time code exchange. +// +// Listens in GET /register/confirm/:auth_id. +func (a *AuthProviderOIDC) RegisterConfirmGetHandler( + writer http.ResponseWriter, + req *http.Request, +) { + authID, err := authIDFromRequest(req) + if err != nil { + httpUserError(writer, err) + + return + } + + authReq, ok := a.h.state.GetAuthCacheEntry(authID) + if !ok { + httpUserError(writer, errRegistrationLinkSpent) + + return + } + + pending := authReq.PendingConfirmation() + if pending == nil { + httpUserError(writer, NewHTTPError(http.StatusForbidden, "registration not OIDC-authorized", nil)) + + return + } + + // Only the browser that completed the OIDC flow holds this cookie, and + // holding it is what authorises the confirm POST. Requiring it here too + // keeps the device details, and the token that finalises the + // registration, away from anyone who merely knows the auth ID — which + // the node being registered does. + cookie, err := req.Cookie(registerConfirmCSRFCookie) + if err != nil { + httpUserError(writer, NewHTTPError(http.StatusForbidden, "missing csrf cookie", err)) + + return + } + + if cookie.Value != pending.CSRF { + httpUserError(writer, NewHTTPError(http.StatusForbidden, "csrf token mismatch", nil)) + + return + } + + user, err := a.h.state.GetUserByID(types.UserID(pending.UserID)) + if err != nil { + httpUserError(writer, fmt.Errorf("looking up user: %w", err)) + + return + } regData := authReq.RegistrationData() info := templates.RegisterConfirmInfo{ - FormAction: "/register/confirm/" + authID.String(), + FormAction: a.registerConfirmURL(authID), CSRFTokenName: registerConfirmCSRFCookie, - CSRFToken: csrf, + CSRFToken: pending.CSRF, User: user.Display(), Hostname: regData.Hostname, MachineKey: regData.MachineKey.ShortString(), @@ -742,6 +843,9 @@ func (a *AuthProviderOIDC) renderRegistrationConfirmInterstitial( info.OS = regData.Hostinfo.OS } + // The page carries the token that finalises the registration, so no + // shared cache or history restore may serve it back. + writer.Header().Set("Cache-Control", "no-store") writer.Header().Set("Content-Type", "text/html; charset=utf-8") writer.WriteHeader(http.StatusOK) @@ -758,12 +862,6 @@ func (a *AuthProviderOIDC) RegisterConfirmHandler( writer http.ResponseWriter, req *http.Request, ) { - if req.Method != http.MethodPost { - httpUserError(writer, errMethodNotAllowed) - - return - } - authID, err := authIDFromRequest(req) if err != nil { httpUserError(writer, err) @@ -804,7 +902,7 @@ func (a *AuthProviderOIDC) RegisterConfirmHandler( authReq, ok := a.h.state.GetAuthCacheEntry(authID) if !ok { - httpUserError(writer, NewHTTPError(http.StatusGone, "registration session expired", nil)) + httpUserError(writer, errRegistrationLinkSpent) return } @@ -832,7 +930,7 @@ func (a *AuthProviderOIDC) RegisterConfirmHandler( newNode, err := a.handleRegistration(user, authID, pending.NodeExpiry) if err != nil { if errors.Is(err, db.ErrNodeNotFoundRegistrationCache) { - httpUserError(writer, NewHTTPError(http.StatusGone, "registration session expired", err)) + httpUserError(writer, NewHTTPError(http.StatusGone, registrationLinkSpentMsg, err)) return } @@ -843,7 +941,7 @@ func (a *AuthProviderOIDC) RegisterConfirmHandler( } // Clear the CSRF cookie now that the registration is final. - setRegisterConfirmCookie(writer, req, authID, "", -1, a.cookiesSecure()) + a.setRegisterConfirmCookie(writer, req, authID, "", -1) content := renderRegistrationSuccessTemplate(user, newNode) diff --git a/hscontrol/oidc_callback_reload_test.go b/hscontrol/oidc_callback_reload_test.go new file mode 100644 index 00000000..335ef9f0 --- /dev/null +++ b/hscontrol/oidc_callback_reload_test.go @@ -0,0 +1,336 @@ +package hscontrol + +import ( + "context" + "io" + "net/http" + "net/http/cookiejar" + "net/http/httptest" + "net/url" + "regexp" + "strings" + "testing" + + "github.com/juanfont/headscale/hscontrol/types" + "github.com/oauth2-proxy/mockoidc" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "tailscale.com/types/key" +) + +// oidcBrowser drives the interactive OIDC registration flow the way a +// browser does: over real HTTP against the real route table, through a +// cookie jar that honours Path and expiry, following redirects. +// +// Both halves matter for this bug. The route table is where the +// confirmation page's missing GET route lives, and cookie Path decides +// whether the state cookie the callback deletes is actually gone on the +// next hit. +type oidcBrowser struct { + app *Headscale + idp *mockoidc.MockOIDC + srv *httptest.Server + client *http.Client +} + +func newOIDCBrowser(t *testing.T) *oidcBrowser { + t.Helper() + + idp, err := mockoidc.Run() + require.NoError(t, err) + + t.Cleanup(func() { + _ = idp.Shutdown() + }) + + app := createTestApp(t) + + // The provider derives its OIDC redirect_uri from the server URL, and + // the server serves the router the provider is registered on, so bind + // the router late to break the cycle. + var router http.Handler + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + router.ServeHTTP(w, r) + })) + t.Cleanup(srv.Close) + + provider, err := NewAuthProviderOIDC( + context.Background(), + app, + srv.URL, + &types.OIDCConfig{ + Issuer: idp.Issuer(), + ClientID: idp.ClientID, + ClientSecret: idp.ClientSecret, + Scope: []string{"openid", "profile", "email"}, + }, + ) + require.NoError(t, err) + + app.authProvider = provider + router = app.createRouter(nil, nil) + + jar, err := cookiejar.New(nil) + require.NoError(t, err) + + return &oidcBrowser{ + app: app, + idp: idp, + srv: srv, + client: &http.Client{Jar: jar}, + } +} + +// get fetches a URL, follows redirects, and returns the status, the URL +// the browser ended up on, and the page body. +func (b *oidcBrowser) get(t *testing.T, rawURL string) (int, *url.URL, string) { + t.Helper() + + resp, err := b.client.Get(rawURL) //nolint:noctx,bodyclose // test client; closed below + require.NoError(t, err) + + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + return resp.StatusCode, resp.Request.URL, string(body) +} + +// pendingNode mints a pending node registration and returns the URL the +// tailscale client would print for the user to open. +func (b *oidcBrowser) pendingNode(t *testing.T) (types.AuthID, string) { + t.Helper() + + authID := types.MustAuthID() + b.app.state.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{ + MachineKey: key.NewMachine().Public(), + NodeKey: key.NewNode().Public(), + Hostname: "reload-victim", + })) + + return authID, b.srv.URL + "/register/" + authID.String() +} + +var ( + csrfInputRe = regexp.MustCompile( + `name="` + registerConfirmCSRFCookie + `"[^>]*value="([^"]+)"`, + ) + formActionRe = regexp.MustCompile(`action="([^"]+)"`) +) + +// TestOIDCLoginDoesNotParkTheBrowserOnTheCodeURL reproduces +// https://github.com/juanfont/headscale/issues/3365. +// +// The interactive OIDC flow ends with the browser sitting on the +// confirmation interstitial, waiting for the user to click. Today that +// interstitial is written as the body of the /oidc/callback response, so +// the URL the browser is parked on is the one carrying the single-use +// OAuth authorization code. Reloading it re-enters the callback, which +// has already spent the code and deleted the state cookie, and the error +// page paints over the interstitial. The node is never registered. +// +// Adblock Plus is only the loudest trigger — it calls +// window.location.reload() to apply element-hiding filters. The back +// button, mobile pull-to-refresh and browser prerendering all aim at the +// same URL. +// +// The property asserted here is the one that fixes the whole class, +// stated without naming an implementation: wherever the flow leaves the +// browser, that URL must be free of the authorization code and safe to +// load again. +func TestOIDCLoginDoesNotParkTheBrowserOnTheCodeURL(t *testing.T) { + b := newOIDCBrowser(t) + + _, registerURL := b.pendingNode(t) + + status, landed, body := b.get(t, registerURL) + require.Equal(t, http.StatusOK, status, "the login flow must reach a page") + require.Contains(t, body, "Confirm node registration", + "the flow must end on the confirmation interstitial") + + assert.Empty(t, landed.Query().Get("code"), + "the browser must not be left parked on the URL carrying the one-time "+ + "OAuth code; any reload of it re-enters the spent callback") + + reloadedStatus, _, reloadedBody := b.get(t, landed.String()) + + require.Equal(t, http.StatusOK, reloadedStatus, + "reloading the page the flow left the browser on must re-render it") + assert.Contains(t, reloadedBody, "Confirm node registration", + "the reload must show the confirmation interstitial, not an error page") +} + +// TestOIDCLoginCompletesAfterReload is the reporters' scenario end to +// end: the confirmation page is reloaded before the user clicks, and the +// registration must still complete. One deployment measured login +// completion falling from 100% to 61-73% across this exact step. +func TestOIDCLoginCompletesAfterReload(t *testing.T) { + b := newOIDCBrowser(t) + + _, registerURL := b.pendingNode(t) + + status, landed, _ := b.get(t, registerURL) + require.Equal(t, http.StatusOK, status) + + // The spurious reload, on whatever URL the flow parked the browser on. + reloadedStatus, reloadedURL, body := b.get(t, landed.String()) + require.Equal(t, http.StatusOK, reloadedStatus, + "the page the user is sitting on must survive a reload") + + csrf := csrfInputRe.FindStringSubmatch(body) + require.Len(t, csrf, 2, "the reloaded page must still carry a usable confirm form") + + action := formActionRe.FindStringSubmatch(body) + require.Len(t, action, 2, "the reloaded page must still carry a form action") + + confirmURL, err := reloadedURL.Parse(action[1]) + require.NoError(t, err) + + //nolint:noctx,bodyclose // test client; closed below + confirmed, err := b.client.PostForm(confirmURL.String(), url.Values{ + registerConfirmCSRFCookie: {csrf[1]}, + }) + require.NoError(t, err) + + defer confirmed.Body.Close() + + confirmedBody, err := io.ReadAll(confirmed.Body) + require.NoError(t, err) + + require.Equal(t, http.StatusOK, confirmed.StatusCode, + "confirming after a reload must register the node") + assert.Contains(t, strings.ToLower(string(confirmedBody)), "registered", + "the user must get the registration success page") +} + +// TestRegisterConfirmGETIsNotADeadEnd covers the second, independent way +// a registration is lost, reported with no ad blocker involved: the +// confirmation endpoint is POST-only in the route table, so a user who +// refreshes or navigates back to it gets a bare 405 from the router with +// no way to recover, while the pending registration sits in the cache +// unreachable until it expires. +// +// This is a route-table gap, so it can only be observed through the real +// router — calling the handler directly cannot see it. +func TestRegisterConfirmGETIsNotADeadEnd(t *testing.T) { + b := newOIDCBrowser(t) + + authID, registerURL := b.pendingNode(t) + + // Complete the OIDC leg so there is a pending confirmation to render. + status, _, _ := b.get(t, registerURL) + require.Equal(t, http.StatusOK, status) + + confirmStatus, _, body := b.get(t, b.srv.URL+"/register/confirm/"+authID.String()) + + require.NotEqual(t, http.StatusMethodNotAllowed, confirmStatus, + "GET on the confirmation URL must not be a dead end for a user who "+ + "refreshes or goes back") + require.Equal(t, http.StatusOK, confirmStatus, + "the confirmation page must be reachable by GET") + assert.Contains(t, body, "Confirm node registration") +} + +// TestRegisterConfirmNeedsTheCallbackCookie locks the reason the +// confirmation step exists. The node being registered knows its own auth +// ID, so the auth ID alone must never be enough to view the device +// details or to finalise the registration — only the browser that +// completed the OIDC login holds the cookie the callback set, and holding +// it is what authorises the confirm. +// +// Without this, an attacker could hand a victim a /register/{auth_id} +// link for the attacker's own node, let the victim's IdP silently sign +// in, and then confirm the registration themselves under the victim's +// identity. +func TestRegisterConfirmNeedsTheCallbackCookie(t *testing.T) { + b := newOIDCBrowser(t) + + authID, registerURL := b.pendingNode(t) + + status, _, body := b.get(t, registerURL) + require.Equal(t, http.StatusOK, status) + + csrf := csrfInputRe.FindStringSubmatch(body) + require.Len(t, csrf, 2) + + // A second browser that knows the auth ID, and even the token from the + // rendered page, but never completed the OIDC login. + jar, err := cookiejar.New(nil) + require.NoError(t, err) + + attacker := &http.Client{Jar: jar} + confirmURL := b.srv.URL + "/register/confirm/" + authID.String() + + //nolint:noctx,bodyclose // test client; closed below + viewed, err := attacker.Get(confirmURL) + require.NoError(t, err) + + defer viewed.Body.Close() + + assert.Equal(t, http.StatusForbidden, viewed.StatusCode, + "the confirmation page must not render without the callback cookie") + + //nolint:noctx,bodyclose // test client; closed below + submitted, err := attacker.PostForm(confirmURL, url.Values{ + registerConfirmCSRFCookie: {csrf[1]}, + }) + require.NoError(t, err) + + defer submitted.Body.Close() + + assert.Equal(t, http.StatusForbidden, submitted.StatusCode, + "the registration must not finalise without the callback cookie") + + cached, ok := b.app.state.GetAuthCacheEntry(authID) + require.True(t, ok, "the pending registration must survive the attempt") + assert.NotNil(t, cached.PendingConfirmation(), + "the pending registration must still be waiting for the real user") +} + +// TestSetRegisterConfirmCookieSameSite pins SameSite=Lax. Strict is +// withheld by browsers that evaluate the whole redirect chain, and this +// cookie now has to survive the callback's redirect to the confirmation +// page — a chain that begins cross-site at the identity provider. Lax is +// still never attached to a cross-site POST, so the confirm submission +// keeps its protection. +func TestSetRegisterConfirmCookieSameSite(t *testing.T) { + a := &AuthProviderOIDC{serverURL: "https://hs.example.com"} + authID := types.MustAuthID() + + rec := httptest.NewRecorder() + a.setRegisterConfirmCookie(rec, + httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/oidc/callback", nil), + authID, "token", 900) + + cookies := rec.Result().Cookies() + require.Len(t, cookies, 1) + assert.Equal(t, http.SameSiteLaxMode, cookies[0].SameSite, + "the confirm cookie must survive the callback redirect") + assert.True(t, cookies[0].Secure, "https server_url must set Secure") + assert.Equal(t, "/register/confirm/"+authID.String(), cookies[0].Path) +} + +// TestRegisterConfirmURLFollowsServerURLPrefix covers the deployment +// where a reverse proxy serves Headscale under a path prefix. The +// redirect target, the form action and the cookie scope are all seen by +// the browser, so they carry the prefix even though the routed path does +// not. +func TestRegisterConfirmURLFollowsServerURLPrefix(t *testing.T) { + a := &AuthProviderOIDC{serverURL: "https://example.com/hs"} + authID := types.MustAuthID() + + assert.Equal(t, "https://example.com/hs/register/confirm/"+authID.String(), + a.registerConfirmURL(authID)) + + rec := httptest.NewRecorder() + a.setRegisterConfirmCookie(rec, + httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/oidc/callback", nil), + authID, "token", 900) + + cookies := rec.Result().Cookies() + require.Len(t, cookies, 1) + assert.Equal(t, "/hs/register/confirm/"+authID.String(), cookies[0].Path, + "the cookie must be scoped to the path the browser sees") +} diff --git a/integration/scenario.go b/integration/scenario.go index 7af28161..73eb73f2 100644 --- a/integration/scenario.go +++ b/integration/scenario.go @@ -1112,6 +1112,11 @@ func (j *debugJar) Dump(w io.Writer) { } } +// registerConfirmCSRFField is the name of both the hidden CSRF form field +// and the cookie on the OIDC registration confirmation interstitial. It +// mirrors registerConfirmCSRFCookie in hscontrol, which is unexported. +const registerConfirmCSRFField = "headscale_register_confirm" + func copyCookie(c *http.Cookie) *http.Cookie { cc := *c return &cc @@ -1225,10 +1230,11 @@ func doLoginURLWithClient(hostname string, loginURL *url.URL, hc *http.Client, f } } - // The OIDC registration flow now renders a confirmation interstitial - // (POST form) instead of completing immediately. Detect the form and + // The OIDC registration flow renders a confirmation interstitial + // (POST form) instead of completing immediately. Detect the form by its + // CSRF field, which does not move when the form action does, and // auto-submit it so integration tests behave like a real browser. - if followRedirects && strings.Contains(body, `action="/register/confirm/`) { + if followRedirects && strings.Contains(body, `name="`+registerConfirmCSRFField+`"`) { confirmBody, confirmURL, confirmErr := submitConfirmForm(hostname, body, resp, hc) if confirmErr != nil { return body, redirectURL, confirmErr @@ -1267,7 +1273,7 @@ func submitConfirmForm( // Extract hidden CSRF input value. The rendered has // attributes in name-type-value order so we grab the whole tag. - before, _, ok := strings.Cut(htmlBody, `name="headscale_register_confirm"`) + before, _, ok := strings.Cut(htmlBody, `name="`+registerConfirmCSRFField+`"`) if !ok { return "", nil, fmt.Errorf("%s confirm form: no CSRF input", hostname) //nolint:err113 } @@ -1293,18 +1299,17 @@ func submitConfirmForm( valEnd := strings.Index(inputTag[valStart:], `"`) csrfToken := inputTag[valStart : valStart+valEnd] - // Build the absolute POST URL from the response's request URL. - base := prevResp.Request.URL - confirmURL := &url.URL{ - Scheme: base.Scheme, - Host: base.Host, - Path: formAction, + // Resolve the form action against the page it was served from, so an + // absolute and a relative action both work. + confirmURL, err := prevResp.Request.URL.Parse(formAction) + if err != nil { + return "", nil, fmt.Errorf("%s confirm form: resolving action %q: %w", hostname, formAction, err) } log.Printf("%s auto-submitting confirm form: %s", hostname, confirmURL) formData := url.Values{ - "headscale_register_confirm": {csrfToken}, + registerConfirmCSRFField: {csrfToken}, } ctx := context.Background()