From 333b85b60d4e736917eafce707bcdf5b7d52d12f Mon Sep 17 00:00:00 2001 From: rohanpatel2002 Date: Mon, 20 Jul 2026 20:17:25 +0530 Subject: [PATCH] fix: prefer Site URL over origin-only Referer for email redirects --- internal/api/external.go | 2 +- internal/api/identity.go | 2 +- internal/api/mail.go | 8 ++++- internal/api/resend.go | 2 +- internal/api/signup.go | 13 ++++++- internal/utilities/request.go | 17 +++++++++- internal/utilities/request_test.go | 54 ++++++++++++++++++++++++++++++ 7 files changed, 92 insertions(+), 6 deletions(-) diff --git a/internal/api/external.go b/internal/api/external.go index 82cf724821..9b03e180f2 100644 --- a/internal/api/external.go +++ b/internal/api/external.go @@ -412,7 +412,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http. } else { emailConfirmationSent := false if decision.CandidateEmail.Email != "" { - if terr = a.sendConfirmation(r, tx, user, models.ImplicitFlow); terr != nil { + if terr = a.sendConfirmation(r, tx, user, models.ImplicitFlow, ""); terr != nil { return 0, nil, terr } emailConfirmationSent = true diff --git a/internal/api/identity.go b/internal/api/identity.go index 54cfe9accb..438106bec4 100644 --- a/internal/api/identity.go +++ b/internal/api/identity.go @@ -144,7 +144,7 @@ func (a *API) linkIdentityToUser(r *http.Request, ctx context.Context, tx *stora return nil, terr } if !userData.Metadata.EmailVerified { - if terr := a.sendConfirmation(r, tx, targetUser, models.ImplicitFlow); terr != nil { + if terr := a.sendConfirmation(r, tx, targetUser, models.ImplicitFlow, ""); terr != nil { return nil, terr } return nil, storage.NewCommitWithError(apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailNotConfirmed, "Unverified email with %v. A confirmation email has been sent to your %v email", providerType, providerType)) diff --git a/internal/api/mail.go b/internal/api/mail.go index 1c42ef787f..ce74e155b5 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -315,7 +315,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error { return sendJSON(w, http.StatusOK, resp) } -func (a *API) sendConfirmation(r *http.Request, tx *storage.Connection, u *models.User, flowType models.FlowType) error { +func (a *API) sendConfirmation(r *http.Request, tx *storage.Connection, u *models.User, flowType models.FlowType, redirectTo string) error { var err error config := a.config @@ -335,6 +335,7 @@ func (a *API) sendConfirmation(r *http.Request, tx *storage.Connection, u *model emailActionType: mail.SignupVerification, otp: otp, tokenHashWithPrefix: u.ConfirmationToken, + redirectTo: redirectTo, }); err != nil { u.ConfirmationToken = oldToken if errors.Is(err, EmailRateLimitExceeded) { @@ -754,12 +755,17 @@ type sendEmailParams struct { oldPhone string provider string factorType string + // redirectTo, when set and allow-listed, overrides GetReferrer (e.g. signup JSON body). + redirectTo string } func (a *API) sendEmail(r *http.Request, tx *storage.Connection, u *models.User, params sendEmailParams) error { ctx := r.Context() config := a.config referrerURL := utilities.GetReferrer(r, config) + if params.redirectTo != "" && utilities.IsRedirectURLValid(config, params.redirectTo) { + referrerURL = params.redirectTo + } externalURL := getExternalHost(ctx) otp := params.otp diff --git a/internal/api/resend.go b/internal/api/resend.go index 22bac98ece..d6cc2379b9 100644 --- a/internal/api/resend.go +++ b/internal/api/resend.go @@ -133,7 +133,7 @@ func (a *API) Resend(w http.ResponseWriter, r *http.Request) error { return terr } } - return a.sendConfirmation(r, tx, user, flowType) + return a.sendConfirmation(r, tx, user, flowType, "") case smsVerification: if terr := models.NewAuditLogEntry(config.AuditLog, r, tx, user, models.UserRecoveryRequestedAction, "", nil); terr != nil { return terr diff --git a/internal/api/signup.go b/internal/api/signup.go index 0af5a7c48e..49e16626fa 100644 --- a/internal/api/signup.go +++ b/internal/api/signup.go @@ -27,6 +27,17 @@ type SignupParams struct { Channel string `json:"channel"` CodeChallengeMethod string `json:"code_challenge_method"` CodeChallenge string `json:"code_challenge"` + // RedirectTo is the post-confirm redirect. Clients may also send email_redirect_to. + RedirectTo string `json:"redirect_to"` + EmailRedirectTo string `json:"email_redirect_to"` +} + +// GetRedirectTo returns the explicit redirect from the signup body, if any. +func (p *SignupParams) GetRedirectTo() string { + if p.RedirectTo != "" { + return p.RedirectTo + } + return p.EmailRedirectTo } func (a *API) validateSignupParams(ctx context.Context, p *SignupParams) error { @@ -247,7 +258,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error { return terr } } - if terr = a.sendConfirmation(r, tx, user, flowType); terr != nil { + if terr = a.sendConfirmation(r, tx, user, flowType, params.GetRedirectTo()); terr != nil { return terr } } diff --git a/internal/utilities/request.go b/internal/utilities/request.go index f9988745ea..eacbaee0e5 100644 --- a/internal/utilities/request.go +++ b/internal/utilities/request.go @@ -81,13 +81,28 @@ func GetReferrer(r *http.Request, config *conf.GlobalConfiguration) string { // instead try referrer header value reqref = r.Referer() - if IsRedirectURLValid(config, reqref) { + if IsRedirectURLValid(config, reqref) && !shouldPreferSiteURLOverReferer(config.SiteURL, reqref) { return reqref } return config.SiteURL } +// shouldPreferSiteURLOverReferer is true when Referer is origin-only (empty or "/") +// but SiteURL includes a non-root path. Cross-origin browser requests often strip the +// path from Referer (Referrer-Policy: strict-origin-when-cross-origin), which would +// otherwise drop project paths such as GitHub Pages /repo/. +func shouldPreferSiteURLOverReferer(siteURL, referer string) bool { + site, serr := url.Parse(siteURL) + ref, rerr := url.Parse(referer) + if serr != nil || rerr != nil { + return false + } + sitePath := strings.TrimSuffix(site.Path, "/") + refPath := strings.TrimSuffix(ref.Path, "/") + return sitePath != "" && refPath == "" +} + var decimalIPAddressPattern = regexp.MustCompile("^[0-9]+$") var regularHostname = regexp.MustCompile("^[a-zA-Z0-9]([a-zA-Z0-9.-]*[a-zA-Z0-9])?$") diff --git a/internal/utilities/request_test.go b/internal/utilities/request_test.go index b7b825fbbf..5542690ea5 100644 --- a/internal/utilities/request_test.go +++ b/internal/utilities/request_test.go @@ -336,3 +336,57 @@ func TestGetReferrer(t *tst.T) { }) } } + +func TestGetReferrerPrefersSiteURLOverOriginOnlyReferer(t *tst.T) { + cases := []struct { + desc string + siteURL string + referer string + query string + expected string + }{ + { + desc: "origin-only referer loses to site url with path", + siteURL: "https://user.github.io/repo/", + referer: "https://user.github.io/", + expected: "https://user.github.io/repo/", + }, + { + desc: "explicit redirect_to still wins", + siteURL: "https://user.github.io/repo/", + referer: "https://user.github.io/", + query: "https://user.github.io/repo/welcome", + expected: "https://user.github.io/repo/welcome", + }, + { + desc: "referer with path still used", + siteURL: "https://user.github.io/repo/", + referer: "https://user.github.io/repo/signup", + expected: "https://user.github.io/repo/signup", + }, + { + desc: "origin-only referer kept when site url has no path", + siteURL: "https://example.com", + referer: "https://example.com/", + expected: "https://example.com/", + }, + } + + for _, c := range cases { + t.Run(c.desc, func(t *tst.T) { + config := conf.GlobalConfiguration{ + SiteURL: c.siteURL, + JWT: conf.JWTConfiguration{Secret: "testsecret"}, + } + require.NoError(t, config.ApplyDefaults()) + + reqURL := "http://localhost/signup" + if c.query != "" { + reqURL += "?redirect_to=" + c.query + } + r := httptest.NewRequest("POST", reqURL, nil) + r.Header.Set("Referer", c.referer) + require.Equal(t, c.expected, GetReferrer(r, &config)) + }) + } +}