diff options
-rw-r--r-- | api/oauth.go | 4 | ||||
-rw-r--r-- | api/user.go | 2 | ||||
-rw-r--r-- | api4/oauth.go | 8 | ||||
-rw-r--r-- | api4/user.go | 2 | ||||
-rw-r--r-- | app/oauth.go | 111 | ||||
-rw-r--r-- | i18n/en.json | 4 | ||||
-rw-r--r-- | model/token.go | 1 | ||||
-rw-r--r-- | webapp/yarn.lock | 2 |
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" |