From 7cd9780399d862bfaf4b2d21cf63cda132f75760 Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Mon, 25 Sep 2023 20:46:45 +0300 Subject: [PATCH] MM-54489 restrict mobile oauth/saml redirect to native app schemes (#24554) * MM-54489 restrict mobile oauth/saml redirect to native app schemes * replace slices package with contains function in utils * use the existing Contains utility function * Fix unit tests * more test cases * fix cfg.NativeAppSettings.AppCustomURLSchemes assignment * Append mmauth to cfg.NativeAppSettings.AppCustomURLSchemes in unit test --- server/channels/utils/api.go | 8 ++------ server/channels/web/oauth_test.go | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/server/channels/utils/api.go b/server/channels/utils/api.go index af46f36aaa8..ca865ab61c7 100644 --- a/server/channels/utils/api.go +++ b/server/channels/utils/api.go @@ -16,6 +16,7 @@ import ( "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/i18n" + "github.com/mattermost/mattermost/server/public/utils" ) func CheckOrigin(r *http.Request, allowedOrigins string) bool { @@ -112,13 +113,8 @@ func RenderMobileAuthComplete(w http.ResponseWriter, redirectURL string) { func RenderMobileError(config *model.Config, w http.ResponseWriter, err *model.AppError, redirectURL string) { var link = template.HTMLEscapeString(redirectURL) - var invalidSchemes = map[string]bool{ - "data": true, - "javascript": true, - "vbscript": true, - } u, redirectErr := url.Parse(redirectURL) - if redirectErr != nil || invalidSchemes[u.Scheme] { + if redirectErr != nil || !utils.Contains(config.NativeAppSettings.AppCustomURLSchemes, u.Scheme) { link = *config.ServiceSettings.SiteURL } RenderMobileMessage(w, ` diff --git a/server/channels/web/oauth_test.go b/server/channels/web/oauth_test.go index 2efd54370a7..838048fa9d8 100644 --- a/server/channels/web/oauth_test.go +++ b/server/channels/web/oauth_test.go @@ -402,7 +402,10 @@ func TestMobileLoginWithOAuth(t *testing.T) { } var siteURL = "http://localhost:8065" - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = siteURL }) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = siteURL + cfg.NativeAppSettings.AppCustomURLSchemes = append(cfg.NativeAppSettings.AppCustomURLSchemes, "mmauth://") + }) translationFunc := i18n.GetUserTranslations("en") c.AppContext.SetT(translationFunc) @@ -411,11 +414,19 @@ func TestMobileLoginWithOAuth(t *testing.T) { einterfaces.RegisterOAuthProvider(model.ServiceGitlab, provider) t.Run("Should include redirect URL in the output when valid URL Scheme is passed", func(t *testing.T) { + responseWriter := httptest.NewRecorder() + request, _ := http.NewRequest(http.MethodGet, th.App.GetSiteURL()+"/oauth/gitlab/mobile_login?redirect_to="+url.QueryEscape("mmauth://"), nil) + mobileLoginWithOAuth(c, responseWriter, request) + assert.Contains(t, responseWriter.Body.String(), "mmauth://") + assert.NotContains(t, responseWriter.Body.String(), siteURL) + }) + + t.Run("Should include SiteURL in the output when invalid URL Scheme is passed", func(t *testing.T) { responseWriter := httptest.NewRecorder() request, _ := http.NewRequest(http.MethodGet, th.App.GetSiteURL()+"/oauth/gitlab/mobile_login?redirect_to="+url.QueryEscape("randomScheme://"), nil) mobileLoginWithOAuth(c, responseWriter, request) - assert.Contains(t, responseWriter.Body.String(), "randomScheme://") - assert.NotContains(t, responseWriter.Body.String(), siteURL) + assert.NotContains(t, responseWriter.Body.String(), "randomScheme://") + assert.Contains(t, responseWriter.Body.String(), siteURL) }) t.Run("Should not include the redirect URL consisting of javascript protocol", func(t *testing.T) {