diff --git a/server/channels/utils/utils.go b/server/channels/utils/utils.go index d99b260eb5..b66b30a052 100644 --- a/server/channels/utils/utils.go +++ b/server/channels/utils/utils.go @@ -191,14 +191,17 @@ func AppendQueryParamsToURL(baseURL string, params map[string]string) string { // Validates RedirectURL passed during OAuth or SAML func IsValidWebAuthRedirectURL(config *model.Config, redirectURL string) bool { u, err := url.Parse(redirectURL) - if err == nil && (u.Scheme == "http" || u.Scheme == "https") { - if config.ServiceSettings.SiteURL != nil { - siteURL := *config.ServiceSettings.SiteURL - return strings.Index(strings.ToLower(redirectURL), strings.ToLower(siteURL)) == 0 - } + if err != nil || config.ServiceSettings.SiteURL == nil { return false } - return true + siteURL, err := url.Parse(*config.ServiceSettings.SiteURL) + if err != nil { + return false + } + if u.Scheme == siteURL.Scheme && u.Host == siteURL.Host { + return true + } + return false } // Validates Mobile Custom URL Scheme passed during OAuth or SAML diff --git a/server/channels/utils/utils_test.go b/server/channels/utils/utils_test.go index f43891766a..5314209e1e 100644 --- a/server/channels/utils/utils_test.go +++ b/server/channels/utils/utils_test.go @@ -9,6 +9,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + "github.com/mattermost/mattermost/server/public/model" ) func TestStringArrayIntersection(t *testing.T) { @@ -432,3 +434,173 @@ func TestRoundOffToZeroesResolution(t *testing.T) { }) } } + +func TestIsValidWebAuthRedirectURL(t *testing.T) { + t.Run("Valid redirect URL with matching scheme and host", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://example.com"), + }, + } + redirectURL := "https://example.com/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.True(t, result) + }) + + t.Run("Valid redirect URL with matching scheme and host with port", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://example.com:8080"), + }, + } + redirectURL := "https://example.com:8080/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.True(t, result) + }) + + t.Run("Invalid redirect URL with different scheme", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://example.com"), + }, + } + redirectURL := "http://example.com/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.False(t, result) + }) + + t.Run("Invalid redirect URL with different host", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://example.com"), + }, + } + redirectURL := "https://malicious.com/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.False(t, result) + }) + + t.Run("Invalid redirect URL with different port", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://example.com:8080"), + }, + } + redirectURL := "https://example.com:9090/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.False(t, result) + }) + + t.Run("Invalid redirect URL - malformed URL", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://example.com"), + }, + } + redirectURL := "not-a-valid-url" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.False(t, result) + }) + + t.Run("Invalid config - nil SiteURL", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: nil, + }, + } + redirectURL := "https://example.com/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.False(t, result) + }) + + t.Run("Invalid config - malformed SiteURL", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("not-a-valid-url"), + }, + } + redirectURL := "https://example.com/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.False(t, result) + }) + + t.Run("Valid redirect URL with subdomain", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://app.example.com"), + }, + } + redirectURL := "https://app.example.com/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.True(t, result) + }) + + t.Run("Invalid redirect URL with different subdomain", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://app.example.com"), + }, + } + redirectURL := "https://api.example.com/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.False(t, result) + }) + + t.Run("Valid redirect URL with path", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://example.com/mattermost"), + }, + } + redirectURL := "https://example.com/mattermost/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.True(t, result) + }) + + t.Run("Valid redirect URL with query parameters", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://example.com"), + }, + } + redirectURL := "https://example.com/oauth/callback?state=abc123&code=def456" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.True(t, result) + }) + + t.Run("Valid redirect URL with fragment", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://example.com"), + }, + } + redirectURL := "https://example.com/oauth/callback#token=abc123" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.True(t, result) + }) + + t.Run("Invalid redirect URL with @ symbol in host", func(t *testing.T) { + config := &model.Config{ + ServiceSettings: model.ServiceSettings{ + SiteURL: model.NewPointer("https://qa-release.test.mattermost.cloud"), + }, + } + redirectURL := "https://qa-release.test.mattermost.cloud@example.com/oauth/callback" + + result := IsValidWebAuthRedirectURL(config, redirectURL) + assert.False(t, result) + }) +}