summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDaniel Schalla <daniel@schalla.me>2018-08-02 00:16:04 +0200
committerChristopher Speller <crspeller@gmail.com>2018-08-01 15:16:04 -0700
commit2936dc87d074e6d83147c9e6cf4ae8bac4e4af8d (patch)
tree2e843f8fdf8382b13fe0a902e7b6183f1f4475bd
parent90e84d76efa775cdf7c54363218bf6817cd1bf33 (diff)
downloadchat-2936dc87d074e6d83147c9e6cf4ae8bac4e4af8d.tar.gz
chat-2936dc87d074e6d83147c9e6cf4ae8bac4e4af8d.tar.bz2
chat-2936dc87d074e6d83147c9e6cf4ae8bac4e4af8d.zip
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
-rw-r--r--app/login.go2
-rw-r--r--app/oauth.go1
-rw-r--r--app/plugin_api.go10
-rw-r--r--app/plugin_requests.go30
-rw-r--r--model/session.go14
-rw-r--r--model/session_test.go15
-rw-r--r--plugin/api.go3
-rw-r--r--plugin/client_rpc_generated.go29
-rw-r--r--plugin/context.go1
-rw-r--r--plugin/plugintest/api.go25
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)