diff --git a/i18n/en.json b/i18n/en.json index de84aa2405..39adc8cec8 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -171,6 +171,10 @@ "id": "api.admin.upload_brand_image.too_large.app_error", "translation": "Unable to upload file. File is too large." }, + { + "id": "api.back_to_app", + "translation": "Back to {{.SiteName}}" + }, { "id": "api.bot.create_disabled", "translation": "Bot creation has been disabled." @@ -1448,6 +1452,14 @@ "id": "api.invalid_channel", "translation": "Channel listed in the request doesn't belong to the user" }, + { + "id": "api.invalid_custom_url_scheme", + "translation": "Invalid custom url scheme has been provided" + }, + { + "id": "api.invalid_redirect_url", + "translation": "Invalid redirect url has been provided" + }, { "id": "api.io_error", "translation": "input/output error" @@ -1564,10 +1576,18 @@ "id": "api.oauth.allow_oauth.turn_off.app_error", "translation": "The system admin has turned off OAuth2 Service Provider." }, + { + "id": "api.oauth.auth_complete", + "translation": "Authentication complete" + }, { "id": "api.oauth.authorize_oauth.disabled.app_error", "translation": "The system admin has turned off OAuth2 Service Provider." }, + { + "id": "api.oauth.close_browser", + "translation": "You can close this browser tab now." + }, { "id": "api.oauth.get_access_token.bad_client_id.app_error", "translation": "invalid_request: Bad client_id." @@ -1628,6 +1648,10 @@ "id": "api.oauth.invalid_state_token.app_error", "translation": "Invalid state token." }, + { + "id": "api.oauth.redirecting_back", + "translation": "Redirecting you back to the app." + }, { "id": "api.oauth.register_oauth_app.turn_off.app_error", "translation": "The system admin has turned off OAuth2 Service Provider." @@ -6698,6 +6722,10 @@ "id": "ent.user.complete_switch_with_oauth.blank_email.app_error", "translation": "Unable to complete SAML login with an empty email address." }, + { + "id": "error", + "translation": "Error" + }, { "id": "group_not_associated_to_synced_team", "translation": "Group cannot be associated to the channel until it is first associated to the parent group-synced team." diff --git a/model/config.go b/model/config.go index 5a0b3189cc..fd9b2f193a 100644 --- a/model/config.go +++ b/model/config.go @@ -237,6 +237,10 @@ const ( LOCAL_MODE_SOCKET_PATH = "/var/tmp/mattermost_local.socket" ) +func GetDefaultAppCustomURLSchemes() []string { + return []string{"mmauth://", "mmauthbeta://"} +} + var ServerTLSSupportedCiphers = map[string]uint16{ "TLS_RSA_WITH_RC4_128_SHA": tls.TLS_RSA_WITH_RC4_128_SHA, "TLS_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, @@ -2427,9 +2431,10 @@ func (s *SamlSettings) SetDefaults() { } type NativeAppSettings struct { - AppDownloadLink *string `access:"site,write_restrictable,cloud_restrictable"` - AndroidAppDownloadLink *string `access:"site,write_restrictable,cloud_restrictable"` - IosAppDownloadLink *string `access:"site,write_restrictable,cloud_restrictable"` + AppCustomURLSchemes []string `access:"site,write_restrictable,cloud_restrictable"` + AppDownloadLink *string `access:"site,write_restrictable,cloud_restrictable"` + AndroidAppDownloadLink *string `access:"site,write_restrictable,cloud_restrictable"` + IosAppDownloadLink *string `access:"site,write_restrictable,cloud_restrictable"` } func (s *NativeAppSettings) SetDefaults() { @@ -2444,6 +2449,10 @@ func (s *NativeAppSettings) SetDefaults() { if s.IosAppDownloadLink == nil { s.IosAppDownloadLink = NewString(NATIVEAPP_SETTINGS_DEFAULT_IOS_APP_DOWNLOAD_LINK) } + + if s.AppCustomURLSchemes == nil { + s.AppCustomURLSchemes = GetDefaultAppCustomURLSchemes() + } } type ElasticsearchSettings struct { diff --git a/utils/api.go b/utils/api.go index f85b34b004..ce045376f2 100644 --- a/utils/api.go +++ b/utils/api.go @@ -74,3 +74,68 @@ func RenderWebError(config *model.Config, w http.ResponseWriter, r *http.Request fmt.Fprintln(w, `...`) fmt.Fprintln(w, ``) } + +func RenderMobileAuthComplete(w http.ResponseWriter, redirectUrl string) { + RenderMobileMessage(w, ` +
+ +
+

`+T("api.oauth.auth_complete")+`

+

`+T("api.oauth.redirecting_back")+`

+ + + + `) +} + +func RenderMobileError(config *model.Config, w http.ResponseWriter, err *model.AppError, redirectURL string) { + RenderMobileMessage(w, ` +
+ +
+

`+T("error")+`

+

`+err.Message+`

+ + `+T("api.back_to_app", map[string]interface{}{"SiteName": config.TeamSettings.SiteName})+` + + `) +} + +func RenderMobileMessage(w http.ResponseWriter, message string) { + w.Header().Set("Content-Type", "text/html") + fmt.Fprintln(w, ` + + + + + + + + + + +
+
+ `+message+` +
+
+ + + `) +} diff --git a/utils/utils.go b/utils/utils.go index ef181d8b19..5135dbfc07 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -11,6 +11,8 @@ import ( "strings" "github.com/pkg/errors" + + "github.com/mattermost/mattermost-server/v5/model" ) func StringInSlice(a string, slice []string) bool { @@ -173,3 +175,43 @@ func GetUrlWithCache(url string, cache *RequestCache, skip bool) ([]byte, error) cache.Date = resp.Header.Get("Date") return cache.Data, err } + +// Append tokens to passed baseUrl as query params +func AppendQueryParamsToURL(baseUrl string, params map[string]string) string { + u, err := url.Parse(baseUrl) + if err != nil { + return "" + } + q, err := url.ParseQuery(u.RawQuery) + if err != nil { + return "" + } + for key, value := range params { + q.Add(key, value) + } + u.RawQuery = q.Encode() + return u.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 + } + return false + } + return true +} + +// Validates Mobile Custom URL Scheme passed during OAuth or SAML +func IsValidMobileAuthRedirectURL(config *model.Config, redirectURL string) bool { + for _, URLScheme := range config.NativeAppSettings.AppCustomURLSchemes { + if strings.Index(strings.ToLower(redirectURL), strings.ToLower(URLScheme)) == 0 { + return true + } + } + return false +} diff --git a/utils/utils_test.go b/utils/utils_test.go index 3956ffef79..6bd10106ea 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -160,3 +160,13 @@ func TestRemoveStringFromSlice(t *testing.T) { assert.Equal(t, RemoveStringFromSlice("four", a), expected) } + +func TestAppendQueryParamsToURL(t *testing.T) { + url := "mattermost://callback" + redirectUrl := AppendQueryParamsToURL(url, map[string]string{ + "key1": "value1", + "key2": "value2", + }) + expected := url + "?key1=value1&key2=value2" + assert.Equal(t, redirectUrl, expected) +} diff --git a/web/oauth.go b/web/oauth.go index cb9ae848df..29b9354af6 100644 --- a/web/oauth.go +++ b/web/oauth.go @@ -7,7 +7,6 @@ import ( "net/http" "net/url" "path/filepath" - "strconv" "strings" "github.com/mattermost/mattermost-server/v5/app" @@ -274,18 +273,34 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) { body, teamId, props, tokenUser, err := c.App.AuthorizeOAuthUser(w, r, service, code, state, uri) action := "" + hasRedirectURL := false + isMobile := false + redirectURL := "" if props != nil { action = props["action"] + isMobile = action == model.OAUTH_ACTION_MOBILE + if val, ok := props["redirect_to"]; ok { + redirectURL = val + hasRedirectURL = redirectURL != "" + } + } + + renderError := func(err *model.AppError) { + if isMobile { + if hasRedirectURL { + utils.RenderMobileError(c.App.Config(), w, err, redirectURL) + } else { + w.Write([]byte(err.ToJson())) + } + } else { + utils.RenderWebAppError(c.App.Config(), w, r, err, c.App.AsymmetricSigningKey()) + } } if err != nil { err.Translate(c.App.T) c.LogErrorByCode(err) - if action == model.OAUTH_ACTION_MOBILE { - w.Write([]byte(err.ToJson())) - } else { - utils.RenderWebAppError(c.App.Config(), w, r, err, c.App.AsymmetricSigningKey()) - } + renderError(err) return } @@ -293,49 +308,48 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) { if err != nil { err.Translate(c.App.T) c.LogErrorByCode(err) - if action == model.OAUTH_ACTION_MOBILE { - w.Write([]byte(err.ToJson())) - } else { - utils.RenderWebAppError(c.App.Config(), w, r, err, c.App.AsymmetricSigningKey()) - } + renderError(err) return } - var redirectUrl string if action == model.OAUTH_ACTION_EMAIL_TO_SSO { - redirectUrl = c.GetSiteURLHeader() + "/login?extra=signin_change" + redirectURL = c.GetSiteURLHeader() + "/login?extra=signin_change" } else if action == model.OAUTH_ACTION_SSO_TO_EMAIL { - redirectUrl = app.GetProtocol(r) + "://" + r.Host + "/claim?email=" + url.QueryEscape(props["email"]) + redirectURL = app.GetProtocol(r) + "://" + r.Host + "/claim?email=" + url.QueryEscape(props["email"]) } else { - isMobile, parseErr := strconv.ParseBool(props[model.USER_AUTH_SERVICE_IS_MOBILE]) - if parseErr != nil { - mlog.Debug("Error parsing boolean property from props", mlog.Err(parseErr)) - } err = c.App.DoLogin(w, r, user, "", isMobile, false, false) if err != nil { err.Translate(c.App.T) - c.Err = err - if action == model.OAUTH_ACTION_MOBILE { - w.Write([]byte(err.ToJson())) - } + mlog.Error(err.Error()) + renderError(err) return } - c.App.AttachSessionCookies(w, r) + // Old mobile version + if isMobile && !hasRedirectURL { + c.App.AttachSessionCookies(w, r) + return + } else + // New mobile version + if isMobile && hasRedirectURL { + redirectURL = utils.AppendQueryParamsToURL(redirectURL, map[string]string{ + model.SESSION_COOKIE_TOKEN: c.App.Session().Token, + model.SESSION_COOKIE_CSRF: c.App.Session().GetCSRF(), + }) + utils.RenderMobileAuthComplete(w, redirectURL) + return + } else { // For web + c.App.AttachSessionCookies(w, r) - if _, ok := props["redirect_to"]; ok { - redirectUrl = props["redirect_to"] - } else { - redirectUrl = c.GetSiteURLHeader() + // If no redirect url is passed, get the default one + if !hasRedirectURL { + redirectURL = c.GetSiteURLHeader() + } } } - if action == model.OAUTH_ACTION_MOBILE { - return - } - w.Header().Set("Content-Type", "text/html; charset=utf-8") - http.Redirect(w, r, redirectUrl, http.StatusTemporaryRedirect) + http.Redirect(w, r, redirectURL, http.StatusTemporaryRedirect) } func loginWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) { @@ -345,7 +359,12 @@ func loginWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) { } loginHint := r.URL.Query().Get("login_hint") - redirectTo := r.URL.Query().Get("redirect_to") + redirectURL := r.URL.Query().Get("redirect_to") + + if redirectURL != "" && !utils.IsValidWebAuthRedirectURL(c.App.Config(), redirectURL) { + c.Err = model.NewAppError("loginWithOAuth", "api.invalid_redirect_url", nil, "", http.StatusBadRequest) + return + } teamId, err := c.App.GetTeamIdFromQuery(r.URL.Query()) if err != nil { @@ -353,7 +372,7 @@ func loginWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) { return } - authUrl, err := c.App.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_LOGIN, redirectTo, loginHint, false) + authUrl, err := c.App.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_LOGIN, redirectURL, loginHint, false) if err != nil { c.Err = err return @@ -368,13 +387,21 @@ func mobileLoginWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) { return } + redirectURL := r.URL.Query().Get("redirect_to") + + if redirectURL != "" && !utils.IsValidMobileAuthRedirectURL(c.App.Config(), redirectURL) { + err := model.NewAppError("mobileLoginWithOAuth", "api.invalid_custom_url_scheme", nil, "", http.StatusBadRequest) + utils.RenderMobileError(c.App.Config(), w, err, redirectURL) + return + } + teamId, err := c.App.GetTeamIdFromQuery(r.URL.Query()) if err != nil { c.Err = err return } - authUrl, err := c.App.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_MOBILE, "", "", true) + authUrl, err := c.App.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_MOBILE, redirectURL, "", true) if err != nil { c.Err = err return diff --git a/web/saml.go b/web/saml.go index 90b7f9a050..5f80835715 100644 --- a/web/saml.go +++ b/web/saml.go @@ -12,6 +12,7 @@ import ( "github.com/mattermost/mattermost-server/v5/audit" "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" + "github.com/mattermost/mattermost-server/v5/utils" ) func (w *Web) InitSaml() { @@ -34,7 +35,7 @@ func loginWithSaml(c *Context, w http.ResponseWriter, r *http.Request) { } action := r.URL.Query().Get("action") isMobile := action == model.OAUTH_ACTION_MOBILE - redirectTo := r.URL.Query().Get("redirect_to") + redirectURL := r.URL.Query().Get("redirect_to") relayProps := map[string]string{} relayState := "" @@ -46,8 +47,13 @@ func loginWithSaml(c *Context, w http.ResponseWriter, r *http.Request) { } } - if redirectTo != "" { - relayProps["redirect_to"] = redirectTo + if redirectURL != "" { + if isMobile && !utils.IsValidMobileAuthRedirectURL(c.App.Config(), redirectURL) { + invalidSchemeErr := model.NewAppError("loginWithOAuth", "api.invalid_custom_url_scheme", nil, "", http.StatusBadRequest) + utils.RenderMobileError(c.App.Config(), w, invalidSchemeErr, redirectURL) + return + } + relayProps["redirect_to"] = redirectURL } relayProps[model.USER_AUTH_SERVICE_IS_MOBILE] = strconv.FormatBool(isMobile) @@ -96,22 +102,39 @@ func completeSaml(c *Context, w http.ResponseWriter, r *http.Request) { action := relayProps["action"] auditRec.AddMeta("action", action) - user, err := samlInterface.DoLogin(encodedXML, relayProps) - if err != nil { - c.LogAudit("fail") - if action == model.OAUTH_ACTION_MOBILE { + isMobile := action == model.OAUTH_ACTION_MOBILE + redirectURL := "" + hasRedirectURL := false + if val, ok := relayProps["redirect_to"]; ok { + redirectURL = val + hasRedirectURL = len(val) > 0 + } + + handleError := func(err *model.AppError) { + if isMobile { err.Translate(c.App.T) - w.Write([]byte(err.ToJson())) + if hasRedirectURL { + utils.RenderMobileError(c.App.Config(), w, err, redirectURL) + } else { + w.Write([]byte(err.ToJson())) + } } else { c.Err = err c.Err.StatusCode = http.StatusFound } + } + + user, err := samlInterface.DoLogin(encodedXML, relayProps) + if err != nil { + c.LogAudit("fail") + mlog.Error(err.Error()) + handleError(err) return } if err = c.App.CheckUserAllAuthenticationCriteria(user, ""); err != nil { - c.Err = err - c.Err.StatusCode = http.StatusFound + mlog.Error(err.Error()) + handleError(err) return } @@ -143,13 +166,10 @@ func completeSaml(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddMeta("obtained_user_id", user.Id) c.LogAuditWithUserId(user.Id, "obtained user") - isMobile, parseErr := strconv.ParseBool(relayProps[model.USER_AUTH_SERVICE_IS_MOBILE]) - if parseErr != nil { - mlog.Warn("Error parsing boolean property from relay props", mlog.Err(parseErr)) - } err = c.App.DoLogin(w, r, user, "", isMobile, false, true) if err != nil { - c.Err = err + mlog.Error(err.Error()) + handleError(err) return } @@ -158,12 +178,23 @@ func completeSaml(c *Context, w http.ResponseWriter, r *http.Request) { c.App.AttachSessionCookies(w, r) - if val, ok := relayProps["redirect_to"]; ok { - http.Redirect(w, r, c.GetSiteURLHeader()+val, http.StatusFound) + if hasRedirectURL { + if isMobile { + // Mobile clients with redirect url support + redirectURL = utils.AppendQueryParamsToURL(redirectURL, map[string]string{ + model.SESSION_COOKIE_TOKEN: c.App.Session().Token, + model.SESSION_COOKIE_CSRF: c.App.Session().GetCSRF(), + }) + utils.RenderMobileAuthComplete(w, redirectURL) + } else { + redirectURL = c.GetSiteURLHeader() + redirectURL + http.Redirect(w, r, redirectURL, http.StatusFound) + } return } switch action { + // Mobile clients with web view implementation case model.OAUTH_ACTION_MOBILE: ReturnStatusOK(w) case model.OAUTH_ACTION_EMAIL_TO_SSO: