Minor fix

8 files changed, 114 insertions, 20 deletions

diff --git a/api/oauth.go b/api/oauth.go
index 84d30ee61..a239e889b 100644
--- a/api/oauth.go
+++ b/api/oauth.go
@@ -157,7 +157,7 @@ func loginWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if authUrl, err := app.GetOAuthLoginEndpoint(service, teamId, model.OAUTH_ACTION_LOGIN, redirectTo, loginHint); err != nil {
+	if authUrl, err := app.GetOAuthLoginEndpoint(w, r, service, teamId, model.OAUTH_ACTION_LOGIN, redirectTo, loginHint); err != nil {
 		c.Err = err
 		return
 	} else {
@@ -180,7 +180,7 @@ func signupWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if authUrl, err := app.GetOAuthSignupEndpoint(service, teamId); err != nil {
+	if authUrl, err := app.GetOAuthSignupEndpoint(w, r, service, teamId); err != nil {
 		c.Err = err
 		return
 	} else {
diff --git a/api/user.go b/api/user.go
index eb249cb39..0b2fbfba8 100644
--- a/api/user.go
+++ b/api/user.go
@@ -866,7 +866,7 @@ func emailToOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	link, err := app.SwitchEmailToOAuth(email, password, mfaToken, service)
+	link, err := app.SwitchEmailToOAuth(w, r, email, password, mfaToken, service)
 	if err != nil {
 		c.Err = err
 		return
diff --git a/api4/oauth.go b/api4/oauth.go
index 402651b92..d00b4a666 100644
--- a/api4/oauth.go
+++ b/api4/oauth.go
@@ -400,7 +400,7 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	uri := c.GetSiteURLHeader() + "/signup/" + service + "/complete"
 
-	body, teamId, props, err := app.AuthorizeOAuthUser(service, code, state, uri)
+	body, teamId, props, err := app.AuthorizeOAuthUser(w, r, service, code, state, uri)
 	if err != nil {
 		c.Err = err
 		return
@@ -455,7 +455,7 @@ func loginWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if authUrl, err := app.GetOAuthLoginEndpoint(c.Params.Service, teamId, model.OAUTH_ACTION_LOGIN, redirectTo, loginHint); err != nil {
+	if authUrl, err := app.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_LOGIN, redirectTo, loginHint); err != nil {
 		c.Err = err
 		return
 	} else {
@@ -475,7 +475,7 @@ func mobileLoginWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if authUrl, err := app.GetOAuthLoginEndpoint(c.Params.Service, teamId, model.OAUTH_ACTION_MOBILE, "", ""); err != nil {
+	if authUrl, err := app.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_MOBILE, "", ""); err != nil {
 		c.Err = err
 		return
 	} else {
@@ -500,7 +500,7 @@ func signupWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if authUrl, err := app.GetOAuthSignupEndpoint(c.Params.Service, teamId); err != nil {
+	if authUrl, err := app.GetOAuthSignupEndpoint(w, r, c.Params.Service, teamId); err != nil {
 		c.Err = err
 		return
 	} else {
diff --git a/api4/user.go b/api4/user.go
index 04faf13c4..f13c33f0b 100644
--- a/api4/user.go
+++ b/api4/user.go
@@ -1056,7 +1056,7 @@ func switchAccountType(c *Context, w http.ResponseWriter, r *http.Request) {
 	var err *model.AppError
 
 	if switchRequest.EmailToOAuth() {
-		link, err = app.SwitchEmailToOAuth(switchRequest.Email, switchRequest.Password, switchRequest.MfaCode, switchRequest.NewService)
+		link, err = app.SwitchEmailToOAuth(w, r, switchRequest.Email, switchRequest.Password, switchRequest.MfaCode, switchRequest.NewService)
 	} else if switchRequest.OAuthToEmail() {
 		c.SessionRequired()
 		if c.Err != nil {
diff --git a/app/oauth.go b/app/oauth.go
index a2edae8be..4bc84272b 100644
--- a/app/oauth.go
+++ b/app/oauth.go
@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
+	"time"
 
 	l4g "github.com/alecthomas/log4go"
 	"github.com/mattermost/platform/einterfaces"
@@ -20,6 +21,11 @@ import (
 	"github.com/mattermost/platform/utils"
 )
 
+const (
+	OAUTH_COOKIE_MAX_AGE_SECONDS = 30 * 60 // 30 minutes
+	COOKIE_OAUTH                 = "MMOAUTH"
+)
+
 func CreateOAuthApp(app *model.OAuthApp) (*model.OAuthApp, *model.AppError) {
 	if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider {
 		return nil, model.NewAppError("CreateOAuthApp", "api.oauth.register_oauth_app.turn_off.app_error", nil, "", http.StatusNotImplemented)
@@ -289,7 +295,7 @@ func newSessionUpdateToken(appName string, accessData *model.AccessData, user *m
 	return accessRsp, nil
 }
 
-func GetOAuthLoginEndpoint(service, teamId, action, redirectTo, loginHint string) (string, *model.AppError) {
+func GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, service, teamId, action, redirectTo, loginHint string) (string, *model.AppError) {
 	stateProps := map[string]string{}
 	stateProps["action"] = action
 	if len(teamId) != 0 {
@@ -300,21 +306,21 @@ func GetOAuthLoginEndpoint(service, teamId, action, redirectTo, loginHint string
 		stateProps["redirect_to"] = redirectTo
 	}
 
-	if authUrl, err := GetAuthorizationCode(service, stateProps, loginHint); err != nil {
+	if authUrl, err := GetAuthorizationCode(w, r, service, stateProps, loginHint); err != nil {
 		return "", err
 	} else {
 		return authUrl, nil
 	}
 }
 
-func GetOAuthSignupEndpoint(service, teamId string) (string, *model.AppError) {
+func GetOAuthSignupEndpoint(w http.ResponseWriter, r *http.Request, service, teamId string) (string, *model.AppError) {
 	stateProps := map[string]string{}
 	stateProps["action"] = model.OAUTH_ACTION_SIGNUP
 	if len(teamId) != 0 {
 		stateProps["team_id"] = teamId
 	}
 
-	if authUrl, err := GetAuthorizationCode(service, stateProps, ""); err != nil {
+	if authUrl, err := GetAuthorizationCode(w, r, service, stateProps, ""); err != nil {
 		return "", err
 	} else {
 		return authUrl, nil
@@ -519,17 +525,69 @@ func CompleteSwitchWithOAuth(service string, userData io.ReadCloser, email strin
 	return user, nil
 }
 
-func GetAuthorizationCode(service string, props map[string]string, loginHint string) (string, *model.AppError) {
+func CreateOAuthStateToken(extra string) (*model.Token, *model.AppError) {
+	token := model.NewToken(model.TOKEN_TYPE_OAUTH, extra)
+
+	if result := <-Srv.Store.Token().Save(token); result.Err != nil {
+		return nil, result.Err
+	}
+
+	return token, nil
+}
+
+func GetOAuthStateToken(token string) (*model.Token, *model.AppError) {
+	if result := <-Srv.Store.Token().GetByToken(token); result.Err != nil {
+		return nil, model.NewAppError("GetOAuthStateToken", "api.oauth.invalid_state_token.app_error", nil, result.Err.Error(), http.StatusBadRequest)
+	} else {
+		token := result.Data.(*model.Token)
+		if token.Type != model.TOKEN_TYPE_OAUTH {
+			return nil, model.NewAppError("GetOAuthStateToken", "api.oauth.invalid_state_token.app_error", nil, "", http.StatusBadRequest)
+		}
+
+		return token, nil
+	}
+}
+
+func generateOAuthStateTokenExtra(email, action, cookie string) string {
+	return email + ":" + action + ":" + cookie
+}
+
+func GetAuthorizationCode(w http.ResponseWriter, r *http.Request, service string, props map[string]string, loginHint string) (string, *model.AppError) {
 	sso := utils.Cfg.GetSSOService(service)
 	if sso != nil && !sso.Enable {
 		return "", model.NewAppError("GetAuthorizationCode", "api.user.get_authorization_code.unsupported.app_error", nil, "service="+service, http.StatusNotImplemented)
 	}
 
+	secure := false
+	if GetProtocol(r) == "https" {
+		secure = true
+	}
+
+	cookieValue := model.NewId()
+	expiresAt := time.Unix(model.GetMillis()/1000+int64(OAUTH_COOKIE_MAX_AGE_SECONDS), 0)
+	oauthCookie := &http.Cookie{
+		Name:     COOKIE_OAUTH,
+		Value:    cookieValue,
+		Path:     "/",
+		MaxAge:   OAUTH_COOKIE_MAX_AGE_SECONDS,
+		Expires:  expiresAt,
+		HttpOnly: true,
+		Secure:   secure,
+	}
+
+	http.SetCookie(w, oauthCookie)
+
 	clientId := sso.Id
 	endpoint := sso.AuthEndpoint
 	scope := sso.Scope
 
-	props["hash"] = utils.HashSha256(clientId)
+	tokenExtra := generateOAuthStateTokenExtra(props["email"], props["action"], cookieValue)
+	stateToken, err := CreateOAuthStateToken(tokenExtra)
+	if err != nil {
+		return "", err
+	}
+
+	props["token"] = stateToken.Token
 	state := b64.StdEncoding.EncodeToString([]byte(model.MapToJson(props)))
 
 	redirectUri := utils.GetSiteURL() + "/signup/" + service + "/complete"
@@ -547,7 +605,7 @@ func GetAuthorizationCode(service string, props map[string]string, loginHint str
 	return authUrl, nil
 }
 
-func AuthorizeOAuthUser(service, code, state, redirectUri string) (io.ReadCloser, string, map[string]string, *model.AppError) {
+func AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service, code, state, redirectUri string) (io.ReadCloser, string, map[string]string, *model.AppError) {
 	sso := utils.Cfg.GetSSOService(service)
 	if sso == nil || !sso.Enable {
 		return nil, "", nil, model.NewLocAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.unsupported.app_error", nil, "service="+service)
@@ -562,10 +620,41 @@ func AuthorizeOAuthUser(service, code, state, redirectUri string) (io.ReadCloser
 
 	stateProps := model.MapFromJson(strings.NewReader(stateStr))
 
-	if stateProps["hash"] != utils.HashSha256(sso.Id) {
-		return nil, "", nil, model.NewLocAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "")
+	expectedToken, err := GetOAuthStateToken(stateProps["token"])
+	if err != nil {
+		return nil, "", nil, err
+	}
+
+	stateEmail := stateProps["email"]
+	stateAction := stateProps["action"]
+	if stateAction == model.OAUTH_ACTION_EMAIL_TO_SSO && stateEmail == "" {
+		return nil, "", nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest)
 	}
 
+	cookieValue := ""
+	if cookie, err := r.Cookie(COOKIE_OAUTH); err != nil {
+		return nil, "", nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest)
+	} else {
+		cookieValue = cookie.Value
+	}
+
+	expectedTokenExtra := generateOAuthStateTokenExtra(stateEmail, stateAction, cookieValue)
+	if expectedTokenExtra != expectedToken.Extra {
+		return nil, "", nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest)
+	}
+
+	DeleteToken(expectedToken)
+
+	cookie := &http.Cookie{
+		Name:     COOKIE_OAUTH,
+		Value:    "",
+		Path:     "/",
+		MaxAge:   -1,
+		HttpOnly: true,
+	}
+
+	http.SetCookie(w, cookie)
+
 	teamId := stateProps["team_id"]
 
 	p := url.Values{}
@@ -617,7 +706,7 @@ func AuthorizeOAuthUser(service, code, state, redirectUri string) (io.ReadCloser
 
 }
 
-func SwitchEmailToOAuth(email, password, code, service string) (string, *model.AppError) {
+func SwitchEmailToOAuth(w http.ResponseWriter, r *http.Request, email, password, code, service string) (string, *model.AppError) {
 	var user *model.User
 	var err *model.AppError
 	if user, err = GetUserByEmail(email); err != nil {
@@ -635,7 +724,7 @@ func SwitchEmailToOAuth(email, password, code, service string) (string, *model.A
 	if service == model.USER_AUTH_SERVICE_SAML {
 		return utils.GetSiteURL() + "/login/sso/saml?action=" + model.OAUTH_ACTION_EMAIL_TO_SSO + "&email=" + email, nil
 	} else {
-		if authUrl, err := GetAuthorizationCode(service, stateProps, ""); err != nil {
+		if authUrl, err := GetAuthorizationCode(w, r, service, stateProps, ""); err != nil {
 			return "", err
 		} else {
 			return authUrl, nil
diff --git a/i18n/en.json b/i18n/en.json
index 7d23a13c1..6784bcd75 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -2700,6 +2700,10 @@
     "translation": "You are deleting %v that is a system administrator.  You may need to set another account as the system administrator using the command line tools."
   },
   {
+    "id": "api.oauth.invalid_state_token.app_error",
+    "translation": "Invalid state token"
+  },
+  {
     "id": "api.user.reset_password.invalid_link.app_error",
     "translation": "The reset password link does not appear to be valid"
   },
diff --git a/model/token.go b/model/token.go
index 6666e112b..a4d10c7f8 100644
--- a/model/token.go
+++ b/model/token.go
@@ -8,6 +8,7 @@ import "net/http"
 const (
 	TOKEN_SIZE            = 64
 	MAX_TOKEN_EXIPRY_TIME = 1000 * 60 * 60 * 24 // 24 hour
+	TOKEN_TYPE_OAUTH      = "oauth"
 )
 
 type Token struct {
diff --git a/webapp/yarn.lock b/webapp/yarn.lock
index 66774deb5..dde0b98d3 100644
--- a/webapp/yarn.lock
+++ b/webapp/yarn.lock
@@ -5043,7 +5043,7 @@ math-expression-evaluator@^1.2.14:
 
 mattermost-redux@mattermost/mattermost-redux#webapp-master:
   version "0.0.1"
-  resolved "https://codeload.github.com/mattermost/mattermost-redux/tar.gz/dd48556075c8be41aa5ac4a0165bbe830d496875"
+  resolved "https://codeload.github.com/mattermost/mattermost-redux/tar.gz/b4bab66d36f10ace06bcd3243d68807bfbca9c48"
   dependencies:
     deep-equal "1.0.1"
     harmony-reflect "1.5.1"