From f628d1f0b3461890b699781557ec4aa5d88caef8 Mon Sep 17 00:00:00 2001 From: Scott McKendry Date: Mon, 13 Oct 2025 21:55:43 +1300 Subject: [PATCH] fix(redirect): allow root cookie domain host redirects (#409) Previously IsRedirectSafe rejected redirects to the exact cookie domain when AppURL had multiple subdomain levels, because it stripped the first label twice. --- internal/utils/app_utils.go | 12 +++++------ internal/utils/app_utils_test.go | 37 +++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/internal/utils/app_utils.go b/internal/utils/app_utils.go index de0cff6..7d143ac 100644 --- a/internal/utils/app_utils.go +++ b/internal/utils/app_utils.go @@ -100,17 +100,17 @@ func IsRedirectSafe(redirectURL string, domain string) bool { return false } - cookieDomain, err := GetCookieDomain(redirectURL) + host := parsedURL.Hostname() + if host == domain { + return true + } + cookieDomain, err := GetCookieDomain(redirectURL) if err != nil { return false } - if cookieDomain != domain { - return false - } - - return true + return cookieDomain == domain } func GetLogLevel(level string) zerolog.Level { diff --git a/internal/utils/app_utils_test.go b/internal/utils/app_utils_test.go index d8abcab..e6cdaeb 100644 --- a/internal/utils/app_utils_test.go +++ b/internal/utils/app_utils_test.go @@ -164,7 +164,7 @@ func TestIsRedirectSafe(t *testing.T) { // Case with no subdomain redirectURL := "http://example.com/welcome" result := utils.IsRedirectSafe(redirectURL, domain) - assert.Equal(t, false, result) + assert.Equal(t, true, result) // Case with different domain redirectURL = "http://malicious.com/phishing" @@ -202,6 +202,41 @@ func TestIsRedirectSafe(t *testing.T) { assert.Equal(t, false, result) } +func TestIsRedirectSafeMultiLevel(t *testing.T) { + // Setup + cookieDomain := "tinyauth.example.com" + + // Case with 3rd level domain + redirectURL := "http://tinyauth.example.com/welcome" + result := utils.IsRedirectSafe(redirectURL, cookieDomain) + assert.Equal(t, true, result) + + // Case with root domain + redirectURL = "http://example.com/unsafe" + result = utils.IsRedirectSafe(redirectURL, cookieDomain) + assert.Equal(t, false, result) + + // Case with 4th level domain + redirectURL = "http://auth.tinyauth.example.com/post-login" + result = utils.IsRedirectSafe(redirectURL, cookieDomain) + assert.Equal(t, true, result) + + // Case with 5th level domain (should be unsafe) + redirectURL = "http://x.auth.tinyauth.example.com/deep" + result = utils.IsRedirectSafe(redirectURL, cookieDomain) + assert.Equal(t, false, result) + + // Case with different subdomain + redirectURL = "http://auth.tinyauth.example.net/attack" + result = utils.IsRedirectSafe(redirectURL, cookieDomain) + assert.Equal(t, false, result) + + // Case with malformed URL + redirectURL = "http://[::1]:namedport" + result = utils.IsRedirectSafe(redirectURL, cookieDomain) + assert.Equal(t, false, result) +} + func TestGetOAuthProvidersConfig(t *testing.T) { env := []string{"PROVIDERS_CLIENT1_CLIENT_ID=client1-id", "PROVIDERS_CLIENT1_CLIENT_SECRET=client1-secret"} args := []string{"/tinyauth/tinyauth", "--providers-client2-client-id=client2-id", "--providers-client2-client-secret=client2-secret"}