From 2936dc87d074e6d83147c9e6cf4ae8bac4e4af8d Mon Sep 17 00:00:00 2001 From: Daniel Schalla Date: Thu, 2 Aug 2018 00:16:04 +0200 Subject: CSRF Token Implementation for Plugins (#9192) deleted test config fix test config Dont wipe the session token for plugins Simplified Tokens; Generate CSRF for other sessions Remove CSRF from Access Token; Remove Getter/Setter from Context fix removed setter remove getcsrf helper from plugin api enforce csrf only for cookie auth --- app/login.go | 2 +- app/oauth.go | 1 + app/plugin_api.go | 10 ++++++++++ app/plugin_requests.go | 30 +++++++++++++++++++++++++++--- model/session.go | 14 ++++++++++++++ model/session_test.go | 15 +++++++++++++++ plugin/api.go | 3 +++ plugin/client_rpc_generated.go | 29 +++++++++++++++++++++++++++++ plugin/context.go | 1 + plugin/plugintest/api.go | 25 +++++++++++++++++++++++++ 10 files changed, 126 insertions(+), 4 deletions(-) diff --git a/app/login.go b/app/login.go index 0d22f2635..4897ae171 100644 --- a/app/login.go +++ b/app/login.go @@ -126,7 +126,7 @@ func (a *App) GetUserForLogin(id, loginId string) (*model.User, *model.AppError) func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string) (*model.Session, *model.AppError) { session := &model.Session{UserId: user.Id, Roles: user.GetRawRoles(), DeviceId: deviceId, IsOAuth: false} - + session.GenerateCSRF() maxAge := *a.Config().ServiceSettings.SessionLengthWebInDays * 60 * 60 * 24 if len(deviceId) > 0 { diff --git a/app/oauth.go b/app/oauth.go index 60ea39255..a0123c0e9 100644 --- a/app/oauth.go +++ b/app/oauth.go @@ -334,6 +334,7 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, c func (a *App) newSession(appName string, user *model.User) (*model.Session, *model.AppError) { // set new token an session session := &model.Session{UserId: user.Id, Roles: user.Roles, IsOAuth: true} + session.GenerateCSRF() session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthSSOInDays) session.AddProp(model.SESSION_PROP_PLATFORM, appName) session.AddProp(model.SESSION_PROP_OS, "OAuth2") diff --git a/app/plugin_api.go b/app/plugin_api.go index 66f17bdfb..31bfd401d 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -65,6 +65,16 @@ func (api *PluginAPI) UnregisterCommand(teamId, trigger string) error { return nil } +func (api *PluginAPI) GetSession(sessionId string) (*model.Session, *model.AppError) { + session, err := api.app.GetSessionById(sessionId) + + if err != nil { + return nil, err + } + + return session, nil +} + func (api *PluginAPI) GetConfig() *model.Config { return api.app.GetConfig() } diff --git a/app/plugin_requests.go b/app/plugin_requests.go index 2f9ac1476..ec6091017 100644 --- a/app/plugin_requests.go +++ b/app/plugin_requests.go @@ -8,11 +8,13 @@ import ( "path" "strings" + "bytes" "github.com/gorilla/mux" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" "github.com/mattermost/mattermost-server/utils" + "io/ioutil" ) func (a *App) ServePluginRequest(w http.ResponseWriter, r *http.Request) { @@ -38,22 +40,44 @@ func (a *App) ServePluginRequest(w http.ResponseWriter, r *http.Request) { func (a *App) servePluginRequest(w http.ResponseWriter, r *http.Request, handler func(*plugin.Context, http.ResponseWriter, *http.Request)) { token := "" + context := &plugin.Context{} + cookieAuth := false authHeader := r.Header.Get(model.HEADER_AUTH) if strings.HasPrefix(strings.ToUpper(authHeader), model.HEADER_BEARER+" ") { token = authHeader[len(model.HEADER_BEARER)+1:] } else if strings.HasPrefix(strings.ToLower(authHeader), model.HEADER_TOKEN+" ") { token = authHeader[len(model.HEADER_TOKEN)+1:] - } else if cookie, _ := r.Cookie(model.SESSION_COOKIE_TOKEN); cookie != nil && (r.Method == "GET" || r.Header.Get(model.HEADER_REQUESTED_WITH) == model.HEADER_REQUESTED_WITH_XML) { + } else if cookie, _ := r.Cookie(model.SESSION_COOKIE_TOKEN); cookie != nil { token = cookie.Value + cookieAuth = true } else { token = r.URL.Query().Get("access_token") } r.Header.Del("Mattermost-User-Id") if token != "" { - if session, err := a.GetSession(token); session != nil && err == nil { + session, err := a.GetSession(token) + csrfCheckPassed := true + + if err == nil && cookieAuth && r.Method != "GET" && r.Header.Get(model.HEADER_REQUESTED_WITH) != model.HEADER_REQUESTED_WITH_XML { + bodyBytes, _ := ioutil.ReadAll(r.Body) + r.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes)) + r.ParseForm() + sentToken := r.FormValue("csrf") + expectedToken := session.GetCSRF() + + if sentToken != expectedToken { + csrfCheckPassed = false + } + + // Set Request Body again, since otherwise form values aren't accessible in plugin handler + r.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes)) + } + + if session != nil && err == nil && csrfCheckPassed { r.Header.Set("Mattermost-User-Id", session.UserId) + context.SessionId = session.Id } } @@ -76,5 +100,5 @@ func (a *App) servePluginRequest(w http.ResponseWriter, r *http.Request, handler r.URL.RawQuery = newQuery.Encode() r.URL.Path = strings.TrimPrefix(r.URL.Path, path.Join(subpath, "plugins", params["plugin_id"])) - handler(&plugin.Context{}, w, r) + handler(context, w, r) } diff --git a/model/session.go b/model/session.go index 7c6bbe06d..d59e9b183 100644 --- a/model/session.go +++ b/model/session.go @@ -135,6 +135,20 @@ func (me *Session) GetUserRoles() []string { return strings.Fields(me.Roles) } +func (me *Session) GenerateCSRF() string { + token := NewId() + me.AddProp("csrf", token) + return token +} + +func (me *Session) GetCSRF() string { + if me.Props == nil { + return "" + } + + return me.Props["csrf"] +} + func SessionsToJson(o []*Session) string { if b, err := json.Marshal(o); err != nil { return "[]" diff --git a/model/session_test.go b/model/session_test.go index 5f4a4730d..bf32d2f09 100644 --- a/model/session_test.go +++ b/model/session_test.go @@ -63,3 +63,18 @@ func TestSessionJson(t *testing.T) { session.SetExpireInDays(10) } + +func TestSessionCSRF(t *testing.T) { + s := Session{} + token := s.GetCSRF() + assert.Empty(t, token) + + token = s.GenerateCSRF() + assert.NotEmpty(t, token) + + token2 := s.GetCSRF() + assert.NotEmpty(t, token2) + assert.Equal(t, token, token2) +} + + diff --git a/plugin/api.go b/plugin/api.go index 0b413d4d1..d0ad178ca 100644 --- a/plugin/api.go +++ b/plugin/api.go @@ -25,6 +25,9 @@ type API interface { // UnregisterCommand unregisters a command previously registered via RegisterCommand. UnregisterCommand(teamId, trigger string) error + // GetSession returns the session object for the Session ID + GetSession(sessionId string) (*model.Session, *model.AppError) + // GetConfig fetches the currently persisted config GetConfig() *model.Config diff --git a/plugin/client_rpc_generated.go b/plugin/client_rpc_generated.go index 98b906186..b43b93c5b 100644 --- a/plugin/client_rpc_generated.go +++ b/plugin/client_rpc_generated.go @@ -558,6 +558,35 @@ func (s *apiRPCServer) UnregisterCommand(args *Z_UnregisterCommandArgs, returns return nil } +type Z_GetSessionArgs struct { + A string +} + +type Z_GetSessionReturns struct { + A *model.Session + B *model.AppError +} + +func (g *apiRPCClient) GetSession(sessionId string) (*model.Session, *model.AppError) { + _args := &Z_GetSessionArgs{sessionId} + _returns := &Z_GetSessionReturns{} + if err := g.client.Call("Plugin.GetSession", _args, _returns); err != nil { + log.Printf("RPC call to GetSession API failed: %s", err.Error()) + } + return _returns.A, _returns.B +} + +func (s *apiRPCServer) GetSession(args *Z_GetSessionArgs, returns *Z_GetSessionReturns) error { + if hook, ok := s.impl.(interface { + GetSession(sessionId string) (*model.Session, *model.AppError) + }); ok { + returns.A, returns.B = hook.GetSession(args.A) + } else { + return fmt.Errorf("API GetSession called but not implemented.") + } + return nil +} + type Z_GetConfigArgs struct { } diff --git a/plugin/context.go b/plugin/context.go index 60d01bbe4..ec3f8b65e 100644 --- a/plugin/context.go +++ b/plugin/context.go @@ -7,4 +7,5 @@ package plugin // // It is currently a placeholder while the implementation details are sorted out. type Context struct { + SessionId string } diff --git a/plugin/plugintest/api.go b/plugin/plugintest/api.go index 70e90df4b..35a83ca9c 100644 --- a/plugin/plugintest/api.go +++ b/plugin/plugintest/api.go @@ -499,6 +499,31 @@ func (_m *API) GetPublicChannelsForTeam(teamId string, offset int, limit int) (* return r0, r1 } +// GetSession provides a mock function with given fields: sessionId +func (_m *API) GetSession(sessionId string) (*model.Session, *model.AppError) { + ret := _m.Called(sessionId) + + var r0 *model.Session + if rf, ok := ret.Get(0).(func(string) *model.Session); ok { + r0 = rf(sessionId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Session) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string) *model.AppError); ok { + r1 = rf(sessionId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // GetTeam provides a mock function with given fields: teamId func (_m *API) GetTeam(teamId string) (*model.Team, *model.AppError) { ret := _m.Called(teamId) -- cgit v1.2.3-1-g7c22