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
This commit is contained in:
Elias Nahum 2023-09-25 20:46:45 +03:00 committed by GitHub
parent 1e121eb7f3
commit 7cd9780399
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 9 deletions

View file

@ -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, `

View file

@ -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) {