After OAUTH, SAML auth completion, Redirect to App custom url scheme with token data as query params. (#16447)

* Added redirection after Auth complete * Fixed gofmt * Handling error while parsing the url, added util function to check for a valid mobile redirect url * Added check for custom scheme url validation * Added test to verify custom schele url * Added mobile message screens * Translation strings for mobile screens * Added mobile specific screens for success and error * Added error logs and changed variable name for consistency with oauth.go * i18n fix * Reusing assigned variable instead of map * Added AppCustomUrlScheme property * Code refactor and removed dependency from cookies to build the final url * Changed util function * Updated util test * Fixed go lint * Code refactor and unsused code removed * Code refactor * simplified boolean checks * Added support of whitelist of appCustomURLSchemes * Changed i18 en * Fixed validating redirecturl for web Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
2025-02-25 18:55:24 -06:00 · 2021-01-19 21:16:22 +05:30
parent 911d1f070e
commit 3da6f270ec
7 changed files with 267 additions and 55 deletions
--- 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."
--- 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 {
--- 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, `<a href="`+template.HTMLEscapeString(destination)+`" style="color: #c0c0c0;">...</a>`)
 	fmt.Fprintln(w, `</body></html>`)
 }
+
+func RenderMobileAuthComplete(w http.ResponseWriter, redirectUrl string) {
+	RenderMobileMessage(w, `
+		<div class="icon text-success" style="font-size: 4em">
+			<i class="fa fa-check-circle" title="Success Icon"></i>
+		</div>
+		<h2> `+T("api.oauth.auth_complete")+` </h2>
+		<p id="redirecting-message"> `+T("api.oauth.redirecting_back")+` </p>
+		<p id="close-tab-message" style="display: none"> `+T("api.oauth.close_browser")+` </p>
+		<noscript><meta http-equiv="refresh" content="2; url=`+template.HTMLEscapeString(redirectUrl)+`"></noscript>
+		<script>
+			window.onload = function() {
+				setTimeout(function() {
+					document.getElementById('redirecting-message').style.display = 'none';
+					document.getElementById('close-tab-message').style.display = 'block';
+					window.location='`+template.HTMLEscapeString(template.JSEscapeString(redirectUrl))+`';
+				}, 2000);
+			}
+		</script>
+	`)
+}
+
+func RenderMobileError(config *model.Config, w http.ResponseWriter, err *model.AppError, redirectURL string) {
+	RenderMobileMessage(w, `
+		<div class="icon" style="color: #ccc; font-size: 4em">
+			<span class="fa fa-warning"></span>
+		</div>
+		<h2> `+T("error")+` </h2>
+		<p> `+err.Message+` </p>
+		<a href="`+redirectURL+`">
+			`+T("api.back_to_app", map[string]interface{}{"SiteName": config.TeamSettings.SiteName})+`
+		</a>
+	`)
+}
+
+func RenderMobileMessage(w http.ResponseWriter, message string) {
+	w.Header().Set("Content-Type", "text/html")
+	fmt.Fprintln(w, `
+		<!DOCTYPE html>
+		<html>
+			<head>
+				<meta charset="utf-8">
+				<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0, user-scalable=yes, viewport-fit=cover">
+				<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.css" integrity="sha512-5A8nwdMOWrSz20fDsjczgUidUBR8liPYU+WymTZP1lmY9G6Oc7HlZv156XqnsgNUzTyMefFTcsFH/tnJE/+xBg==" crossorigin="anonymous" />
+				<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" integrity="sha384-BVYiiSIFeK1dGmJRAkycuHAHRg32OmUcww7on3RYdg4Va+PmSTsz/K68vbdEjh4u" crossorigin="anonymous" />
+				<style>
+					.message-container {
+						color: #555;
+						display: table-cell;
+						padding: 5em 0;
+						text-align: left;
+						vertical-align: top;
+					}
+				</style>
+			</head>
+			<body>
+				<div class="container-fluid">
+					<div class="message-container">
+						`+message+`
+					</div>
+				</div>
+			</body>
+		</html>
+	`)
+}
--- 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
+}
--- 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)
+}
--- 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
--- 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: