diff options
55 files changed, 289 insertions, 213 deletions
@@ -275,6 +275,10 @@ store-mocks: ## Creates mock files. go get github.com/vektra/mockery/... $(GOPATH)/bin/mockery -dir store -all -output store/storetest/mocks -note 'Regenerate this file using `make store-mocks`.' +ldap-mocks: ## Creates mock files for ldap. + go get github.com/vektra/mockery/... + GOPATH=$(shell go env GOPATH) $(shell go env GOPATH)/bin/mockery -dir enterprise/ldap -all -output enterprise/ldap/mocks -note 'Regenerate this file using `make ldap-mocks`.' + update-jira-plugin: ## Updates Jira plugin. go get github.com/mattermost/go-bindata/... curl -s https://api.github.com/repos/mattermost/mattermost-plugin-jira/releases/latest | grep browser_download_url | grep darwin-amd64 | cut -d '"' -f 4 | wget -qi - -O plugin.tar.gz diff --git a/api/emoji_test.go b/api/emoji_test.go index 69922a54a..108c416e1 100644 --- a/api/emoji_test.go +++ b/api/emoji_test.go @@ -272,7 +272,7 @@ func TestDeleteEmoji(t *testing.T) { func createTestEmoji(t *testing.T, a *app.App, emoji *model.Emoji, imageData []byte) *model.Emoji { emoji = store.Must(a.Srv.Store.Emoji().Save(emoji)).(*model.Emoji) - if err := a.WriteFile(imageData, "emoji/"+emoji.Id+"/image"); err != nil { + if _, err := a.WriteFile(bytes.NewReader(imageData), "emoji/"+emoji.Id+"/image"); err != nil { store.Must(a.Srv.Store.Emoji().Delete(emoji.Id, time.Now().Unix())) t.Fatalf("failed to write image: %v", err.Error()) } diff --git a/api/user.go b/api/user.go index 15fd4c7ea..5931eac1e 100644 --- a/api/user.go +++ b/api/user.go @@ -107,7 +107,7 @@ func login(c *Context, w http.ResponseWriter, r *http.Request) { ldapOnly := props["ldap_only"] == "true" c.LogAudit("attempt - user_id=" + id + " login_id=" + loginId) - user, err := c.App.AuthenticateUserForLogin(id, loginId, password, mfaToken, deviceId, ldapOnly) + user, err := c.App.AuthenticateUserForLogin(id, loginId, password, mfaToken, ldapOnly) if err != nil { c.LogAudit("failure - user_id=" + id + " login_id=" + loginId) c.Err = err @@ -1072,7 +1072,7 @@ func checkMfa(c *Context, w http.ResponseWriter, r *http.Request) { } rdata := map[string]string{} - if user, err := c.App.GetUserForLogin(loginId, false); err != nil { + if user, err := c.App.GetUserForLogin("", loginId); err != nil { rdata["mfa_required"] = "false" } else { rdata["mfa_required"] = strconv.FormatBool(user.MfaActive) diff --git a/api4/user.go b/api4/user.go index 897c49ad1..2a539a551 100644 --- a/api4/user.go +++ b/api4/user.go @@ -771,7 +771,7 @@ func checkUserMfa(c *Context, w http.ResponseWriter, r *http.Request) { return } - if user, err := c.App.GetUserForLogin(loginId, false); err == nil { + if user, err := c.App.GetUserForLogin("", loginId); err == nil { resp["mfa_required"] = user.MfaActive } @@ -943,7 +943,7 @@ func login(c *Context, w http.ResponseWriter, r *http.Request) { ldapOnly := props["ldap_only"] == "true" c.LogAuditWithUserId(id, "attempt - login_id="+loginId) - user, err := c.App.AuthenticateUserForLogin(id, loginId, password, mfaToken, deviceId, ldapOnly) + user, err := c.App.AuthenticateUserForLogin(id, loginId, password, mfaToken, ldapOnly) if err != nil { c.LogAuditWithUserId(id, "failure - login_id="+loginId) c.Err = err @@ -1167,7 +1167,7 @@ func sendVerificationEmail(c *Context, w http.ResponseWriter, r *http.Request) { return } - user, err := c.App.GetUserForLogin(email, false) + user, err := c.App.GetUserForLogin("", email) if err != nil { // Don't want to leak whether the email is valid or not ReturnStatusOK(w) @@ -1205,7 +1205,7 @@ func switchAccountType(c *Context, w http.ResponseWriter, r *http.Request) { link, err = c.App.SwitchOAuthToEmail(switchRequest.Email, switchRequest.NewPassword, c.Session.UserId) } else if switchRequest.EmailToLdap() { - link, err = c.App.SwitchEmailToLdap(switchRequest.Email, switchRequest.Password, switchRequest.MfaCode, switchRequest.LdapId, switchRequest.NewPassword) + link, err = c.App.SwitchEmailToLdap(switchRequest.Email, switchRequest.Password, switchRequest.MfaCode, switchRequest.LdapLoginId, switchRequest.NewPassword) } else if switchRequest.LdapToEmail() { link, err = c.App.SwitchLdapToEmail(switchRequest.Password, switchRequest.MfaCode, switchRequest.Email, switchRequest.NewPassword) } else { diff --git a/app/emoji.go b/app/emoji.go index 2f957fbcc..b07331e65 100644 --- a/app/emoji.go +++ b/app/emoji.go @@ -98,7 +98,7 @@ func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *mode if err := gif.EncodeAll(newbuf, resized_gif); err != nil { return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.gif_encode_error", nil, "", http.StatusBadRequest) } - if err := a.WriteFile(newbuf.Bytes(), getEmojiImagePath(id)); err != nil { + if _, err := a.WriteFile(newbuf, getEmojiImagePath(id)); err != nil { return err } } @@ -110,14 +110,15 @@ func (a *App) UploadEmojiImage(id string, imageData *multipart.FileHeader) *mode if err := png.Encode(newbuf, resized_image); err != nil { return model.NewAppError("uploadEmojiImage", "api.emoji.upload.large_image.encode_error", nil, "", http.StatusBadRequest) } - if err := a.WriteFile(newbuf.Bytes(), getEmojiImagePath(id)); err != nil { + if _, err := a.WriteFile(newbuf, getEmojiImagePath(id)); err != nil { return err } } } } - return a.WriteFile(buf.Bytes(), getEmojiImagePath(id)) + _, appErr := a.WriteFile(buf, getEmojiImagePath(id)) + return appErr } func (a *App) DeleteEmoji(emoji *model.Emoji) *model.AppError { diff --git a/app/file.go b/app/file.go index 87e1986a2..cb8d54cb1 100644 --- a/app/file.go +++ b/app/file.go @@ -78,12 +78,13 @@ func (a *App) MoveFile(oldPath, newPath string) *model.AppError { return backend.MoveFile(oldPath, newPath) } -func (a *App) WriteFile(f []byte, path string) *model.AppError { +func (a *App) WriteFile(fr io.Reader, path string) (int64, *model.AppError) { backend, err := a.FileBackend() if err != nil { - return err + return 0, err } - return backend.WriteFile(f, path) + + return backend.WriteFile(fr, path) } func (a *App) RemoveFile(path string) *model.AppError { @@ -414,7 +415,7 @@ func (a *App) DoUploadFile(now time.Time, rawTeamId string, rawChannelId string, info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb.jpg" } - if err := a.WriteFile(data, info.Path); err != nil { + if _, err := a.WriteFile(bytes.NewReader(data), info.Path); err != nil { return nil, err } @@ -531,7 +532,7 @@ func (a *App) generateThumbnailImage(img image.Image, thumbnailPath string, widt return } - if err := a.WriteFile(buf.Bytes(), thumbnailPath); err != nil { + if _, err := a.WriteFile(buf, thumbnailPath); err != nil { mlog.Error(fmt.Sprintf("Unable to upload thumbnail path=%v err=%v", thumbnailPath, err)) return } @@ -553,7 +554,7 @@ func (a *App) generatePreviewImage(img image.Image, previewPath string, width in return } - if err := a.WriteFile(buf.Bytes(), previewPath); err != nil { + if _, err := a.WriteFile(buf, previewPath); err != nil { mlog.Error(fmt.Sprintf("Unable to upload preview err=%v", err), mlog.String("path", previewPath)) return } diff --git a/app/ldap.go b/app/ldap.go index 22c3b746b..544905b70 100644 --- a/app/ldap.go +++ b/app/ldap.go @@ -40,7 +40,7 @@ func (a *App) TestLdap() *model.AppError { return nil } -func (a *App) SwitchEmailToLdap(email, password, code, ldapId, ldapPassword string) (string, *model.AppError) { +func (a *App) SwitchEmailToLdap(email, password, code, ldapLoginId, ldapPassword string) (string, *model.AppError) { if a.License() != nil && !*a.Config().ServiceSettings.ExperimentalEnableAuthenticationTransfer { return "", model.NewAppError("emailToLdap", "api.user.email_to_ldap.not_available.app_error", nil, "", http.StatusForbidden) } @@ -63,7 +63,7 @@ func (a *App) SwitchEmailToLdap(email, password, code, ldapId, ldapPassword stri return "", model.NewAppError("SwitchEmailToLdap", "api.user.email_to_ldap.not_available.app_error", nil, "", http.StatusNotImplemented) } - if err := ldapInterface.SwitchToLdap(user.Id, ldapId, ldapPassword); err != nil { + if err := ldapInterface.SwitchToLdap(user.Id, ldapLoginId, ldapPassword); err != nil { return "", err } @@ -95,7 +95,7 @@ func (a *App) SwitchLdapToEmail(ldapPassword, code, email, newPassword string) ( return "", model.NewAppError("SwitchLdapToEmail", "api.user.ldap_to_email.not_available.app_error", nil, "", http.StatusNotImplemented) } - if err := ldapInterface.CheckPassword(*user.AuthData, ldapPassword); err != nil { + if err := ldapInterface.CheckPasswordAuthData(*user.AuthData, ldapPassword); err != nil { return "", err } diff --git a/app/login.go b/app/login.go index 43b022749..529e4cb21 100644 --- a/app/login.go +++ b/app/login.go @@ -11,47 +11,69 @@ import ( "github.com/avct/uasurfer" "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" ) -func (a *App) AuthenticateUserForLogin(id, loginId, password, mfaToken, deviceId string, ldapOnly bool) (*model.User, *model.AppError) { +func (a *App) AuthenticateUserForLogin(id, loginId, password, mfaToken string, ldapOnly bool) (user *model.User, err *model.AppError) { + // Do statistics + defer func() { + if a.Metrics != nil { + if user == nil || err != nil { + a.Metrics.IncrementLoginFail() + } else { + a.Metrics.IncrementLogin() + } + } + }() + if len(password) == 0 { err := model.NewAppError("AuthenticateUserForLogin", "api.user.login.blank_pwd.app_error", nil, "", http.StatusBadRequest) return nil, err } - var user *model.User - var err *model.AppError + // Get the MM user we are trying to login + if user, err = a.GetUserForLogin(id, loginId); err != nil { + return nil, err + } + + // and then authenticate them + if user, err = a.authenticateUser(user, password, mfaToken); err != nil { + return nil, err + } + + return user, nil +} + +func (a *App) GetUserForLogin(id, loginId string) (*model.User, *model.AppError) { + enableUsername := *a.Config().EmailSettings.EnableSignInWithUsername + enableEmail := *a.Config().EmailSettings.EnableSignInWithEmail + // If we are given a userID then fail if we can't find a user with that ID if len(id) != 0 { - if user, err = a.GetUser(id); err != nil { - err.StatusCode = http.StatusBadRequest - if a.Metrics != nil { - a.Metrics.IncrementLoginFail() + if user, err := a.GetUser(id); err != nil { + if err.Id != store.MISSING_ACCOUNT_ERROR { + err.StatusCode = http.StatusInternalServerError + return nil, err + } else { + err.StatusCode = http.StatusBadRequest + return nil, err } - return nil, err - } - } else { - if user, err = a.GetUserForLogin(loginId, ldapOnly); err != nil { - if a.Metrics != nil { - a.Metrics.IncrementLoginFail() - } - return nil, err + } else { + return user, nil } } - // and then authenticate them - if user, err = a.authenticateUser(user, password, mfaToken); err != nil { - if a.Metrics != nil { - a.Metrics.IncrementLoginFail() - } - return nil, err + // Try to get the user by username/email + if result := <-a.Srv.Store.User().GetForLogin(loginId, enableUsername, enableEmail); result.Err == nil { + return result.Data.(*model.User), nil } - if a.Metrics != nil { - a.Metrics.IncrementLogin() + // Try to get the user with LDAP + if user, err := a.Ldap.GetUser(loginId); err == nil { + return user, nil } - return user, nil + return nil, model.NewAppError("GetUserForLogin", "store.sql_user.get_for_login.app_error", nil, "", http.StatusBadRequest) } func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string) (*model.Session, *model.AppError) { diff --git a/app/team.go b/app/team.go index d8ebbab2a..2833e2eed 100644 --- a/app/team.go +++ b/app/team.go @@ -1057,7 +1057,7 @@ func (a *App) SetTeamIconFromFile(teamId string, file multipart.File) *model.App path := "teams/" + teamId + "/teamIcon.png" - if err := a.WriteFile(buf.Bytes(), path); err != nil { + if _, err := a.WriteFile(buf, path); err != nil { return model.NewAppError("SetTeamIcon", "api.team.set_team_icon.write_file.app_error", nil, "", http.StatusInternalServerError) } diff --git a/app/user.go b/app/user.go index fd8b6b377..2ee410684 100644 --- a/app/user.go +++ b/app/user.go @@ -382,38 +382,6 @@ func (a *App) GetUserByAuth(authData *string, authService string) (*model.User, } } -func (a *App) GetUserForLogin(loginId string, onlyLdap bool) (*model.User, *model.AppError) { - license := a.License() - ldapAvailable := *a.Config().LdapSettings.Enable && a.Ldap != nil && license != nil && *license.Features.LDAP - - if result := <-a.Srv.Store.User().GetForLogin( - loginId, - *a.Config().EmailSettings.EnableSignInWithUsername && !onlyLdap, - *a.Config().EmailSettings.EnableSignInWithEmail && !onlyLdap, - ldapAvailable, - ); result.Err != nil && result.Err.Id == "store.sql_user.get_for_login.multiple_users" { - // don't fall back to LDAP in this case since we already know there's an LDAP user, but that it shouldn't work - result.Err.StatusCode = http.StatusBadRequest - return nil, result.Err - } else if result.Err != nil { - if !ldapAvailable { - // failed to find user and no LDAP server to fall back on - result.Err.StatusCode = http.StatusBadRequest - return nil, result.Err - } - - // fall back to LDAP server to see if we can find a user - if ldapUser, ldapErr := a.Ldap.GetUser(loginId); ldapErr != nil { - ldapErr.StatusCode = http.StatusBadRequest - return nil, ldapErr - } else { - return ldapUser, nil - } - } else { - return result.Data.(*model.User), nil - } -} - func (a *App) GetUsers(offset int, limit int) ([]*model.User, *model.AppError) { if result := <-a.Srv.Store.User().GetAllProfiles(offset, limit); result.Err != nil { return nil, result.Err @@ -786,7 +754,7 @@ func (a *App) GetProfileImage(user *model.User) ([]byte, bool, *model.AppError) } if user.LastPictureUpdate == 0 { - if err := a.WriteFile(img, path); err != nil { + if _, err := a.WriteFile(bytes.NewReader(img), path); err != nil { return nil, false, err } } @@ -842,7 +810,7 @@ func (a *App) SetProfileImageFromFile(userId string, file multipart.File) *model path := "users/" + userId + "/profile.png" - if err := a.WriteFile(buf.Bytes(), path); err != nil { + if _, err := a.WriteFile(buf, path); err != nil { return model.NewAppError("SetProfileImage", "api.user.upload_profile_user.upload_profile.app_error", nil, "", http.StatusInternalServerError) } diff --git a/cmd/commands/ldap.go b/cmd/commands/ldap.go index 0c79ce32b..03c366213 100644 --- a/cmd/commands/ldap.go +++ b/cmd/commands/ldap.go @@ -22,9 +22,19 @@ var LdapSyncCmd = &cobra.Command{ RunE: ldapSyncCmdF, } +var LdapIdMigrate = &cobra.Command{ + Use: "idmigrate", + Short: "Migrate LDAP IdAttribute to new value", + Long: "Migrate LDAP IdAttribute to new value. Run this utility then change the IdAttribute to the new value.", + Example: " ldap idmigrate objectGUID", + Args: cobra.ExactArgs(1), + RunE: ldapIdMigrateCmdF, +} + func init() { LdapCmd.AddCommand( LdapSyncCmd, + LdapIdMigrate, ) cmd.RootCmd.AddCommand(LdapCmd) } @@ -47,3 +57,22 @@ func ldapSyncCmdF(command *cobra.Command, args []string) error { return nil } + +func ldapIdMigrateCmdF(command *cobra.Command, args []string) error { + a, err := cmd.InitDBCommandContextCobra(command) + if err != nil { + return err + } + defer a.Shutdown() + + toAttribute := args[0] + if ldapI := a.Ldap; ldapI != nil { + if err := ldapI.MigrateIDAttribute(toAttribute); err != nil { + cmd.CommandPrintErrorln("ERROR: AD/LDAP IdAttribute migration failed! Error: " + err.Error()) + } else { + cmd.CommandPrettyPrintln("SUCCESS: AD/LDAP IdAttribute migration complete. You can now change your IdAttribute to: " + toAttribute) + } + } + + return nil +} diff --git a/config/default.json b/config/default.json index 8bf06dc8b..80a694e38 100644 --- a/config/default.json +++ b/config/default.json @@ -263,6 +263,7 @@ "NicknameAttribute": "", "IdAttribute": "", "PositionAttribute": "", + "LoginIdAttribute": "", "SyncIntervalMinutes": 60, "SkipCertificateVerification": false, "QueryTimeout": 60, diff --git a/einterfaces/ldap.go b/einterfaces/ldap.go index 26326b174..31e8b7cf8 100644 --- a/einterfaces/ldap.go +++ b/einterfaces/ldap.go @@ -4,8 +4,6 @@ package einterfaces import ( - "github.com/go-ldap/ldap" - "github.com/mattermost/mattermost-server/model" ) @@ -14,12 +12,11 @@ type LdapInterface interface { GetUser(id string) (*model.User, *model.AppError) GetUserAttributes(id string, attributes []string) (map[string]string, *model.AppError) CheckPassword(id string, password string) *model.AppError + CheckPasswordAuthData(authData string, password string) *model.AppError SwitchToLdap(userId, ldapId, ldapPassword string) *model.AppError ValidateFilter(filter string) *model.AppError StartSynchronizeJob(waitForJobToFinish bool) (*model.Job, *model.AppError) RunTest() *model.AppError GetAllLdapUsers() ([]*model.User, *model.AppError) - UserFromLdapUser(ldapUser *ldap.Entry) *model.User - UserHasUpdateFromLdap(existingUser *model.User, currentLdapUser *model.User) bool - UpdateLocalLdapUser(existingUser *model.User, currentLdapUser *model.User) *model.User + MigrateIDAttribute(toAttribute string) error } diff --git a/i18n/en.json b/i18n/en.json index c993b0411..58a950921 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -5027,6 +5027,10 @@ "translation": "AD/LDAP field \"ID Attribute\" is required." }, { + "id": "model.config.is_valid.ldap_login_id", + "translation": "AD/LDAP field \"Login ID Attribute\" is required." + }, + { "id": "model.config.is_valid.ldap_lastname", "translation": "AD/LDAP field \"Last Name Attribute\" is required." }, diff --git a/mlog/log.go b/mlog/log.go index ad537a11d..28f7408c4 100644 --- a/mlog/log.go +++ b/mlog/log.go @@ -29,6 +29,7 @@ type Field = zapcore.Field var Int64 = zap.Int64 var Int = zap.Int var String = zap.String +var Any = zap.Any var Err = zap.Error type LoggerConfiguration struct { diff --git a/model/config.go b/model/config.go index 5074b7637..07cd9d977 100644 --- a/model/config.go +++ b/model/config.go @@ -1144,6 +1144,7 @@ type LdapSettings struct { NicknameAttribute *string IdAttribute *string PositionAttribute *string + LoginIdAttribute *string // Synchronization SyncIntervalMinutes *int @@ -1227,6 +1228,12 @@ func (s *LdapSettings) SetDefaults() { s.PositionAttribute = NewString(LDAP_SETTINGS_DEFAULT_POSITION_ATTRIBUTE) } + // For those upgrading to the version when LoginIdAttribute was added + // they need IdAttribute == LoginIdAttribute not to break + if s.LoginIdAttribute == nil { + s.LoginIdAttribute = s.IdAttribute + } + if s.SyncIntervalMinutes == nil { s.SyncIntervalMinutes = NewInt(60) } @@ -2074,6 +2081,10 @@ func (ls *LdapSettings) isValid() *AppError { if *ls.IdAttribute == "" { return NewAppError("Config.IsValid", "model.config.is_valid.ldap_id", nil, "", http.StatusBadRequest) } + + if *ls.LoginIdAttribute == "" { + return NewAppError("Config.IsValid", "model.config.is_valid.ldap_login_id", nil, "", http.StatusBadRequest) + } } return nil diff --git a/model/switch_request.go b/model/switch_request.go index e153c92f4..2a522f492 100644 --- a/model/switch_request.go +++ b/model/switch_request.go @@ -15,7 +15,7 @@ type SwitchRequest struct { Password string `json:"password"` NewPassword string `json:"new_password"` MfaCode string `json:"mfa_code"` - LdapId string `json:"ldap_id"` + LdapLoginId string `json:"ldap_id"` } func (o *SwitchRequest) ToJson() string { diff --git a/model/websocket_client.go b/model/websocket_client.go index 4ff4f617b..788dbee20 100644 --- a/model/websocket_client.go +++ b/model/websocket_client.go @@ -6,24 +6,28 @@ package model import ( "encoding/json" "net/http" + "time" "github.com/gorilla/websocket" ) const ( - SOCKET_MAX_MESSAGE_SIZE_KB = 8 * 1024 // 8KB + SOCKET_MAX_MESSAGE_SIZE_KB = 8 * 1024 // 8KB + PING_TIMEOUT_BUFFER_SECONDS = 5 ) type WebSocketClient struct { - Url string // The location of the server like "ws://localhost:8065" - ApiUrl string // The api location of the server like "ws://localhost:8065/api/v3" - ConnectUrl string // The websocket URL to connect to like "ws://localhost:8065/api/v3/path/to/websocket" - Conn *websocket.Conn // The WebSocket connection - AuthToken string // The token used to open the WebSocket - Sequence int64 // The ever-incrementing sequence attached to each WebSocket action - EventChannel chan *WebSocketEvent - ResponseChannel chan *WebSocketResponse - ListenError *AppError + Url string // The location of the server like "ws://localhost:8065" + ApiUrl string // The api location of the server like "ws://localhost:8065/api/v3" + ConnectUrl string // The websocket URL to connect to like "ws://localhost:8065/api/v3/path/to/websocket" + Conn *websocket.Conn // The WebSocket connection + AuthToken string // The token used to open the WebSocket + Sequence int64 // The ever-incrementing sequence attached to each WebSocket action + PingTimeoutChannel chan bool // The channel used to signal ping timeouts + EventChannel chan *WebSocketEvent + ResponseChannel chan *WebSocketResponse + ListenError *AppError + pingTimeoutTimer *time.Timer } // NewWebSocketClient constructs a new WebSocket client with convenience @@ -47,11 +51,15 @@ func NewWebSocketClientWithDialer(dialer *websocket.Dialer, url, authToken strin conn, authToken, 1, + make(chan bool, 1), make(chan *WebSocketEvent, 100), make(chan *WebSocketResponse, 100), nil, + nil, } + client.configurePingHandling() + client.SendMessage(WEBSOCKET_AUTHENTICATION_CHALLENGE, map[string]interface{}{"token": authToken}) return client, nil @@ -78,11 +86,15 @@ func NewWebSocketClient4WithDialer(dialer *websocket.Dialer, url, authToken stri conn, authToken, 1, + make(chan bool, 1), make(chan *WebSocketEvent, 100), make(chan *WebSocketResponse, 100), nil, + nil, } + client.configurePingHandling() + client.SendMessage(WEBSOCKET_AUTHENTICATION_CHALLENGE, map[string]interface{}{"token": authToken}) return client, nil @@ -99,6 +111,8 @@ func (wsc *WebSocketClient) ConnectWithDialer(dialer *websocket.Dialer) *AppErro return NewAppError("Connect", "model.websocket_client.connect_fail.app_error", nil, err.Error(), http.StatusInternalServerError) } + wsc.configurePingHandling() + wsc.EventChannel = make(chan *WebSocketEvent, 100) wsc.ResponseChannel = make(chan *WebSocketResponse, 100) @@ -181,3 +195,24 @@ func (wsc *WebSocketClient) GetStatusesByIds(userIds []string) { } wsc.SendMessage("get_statuses_by_ids", data) } + +func (wsc *WebSocketClient) configurePingHandling() { + wsc.Conn.SetPingHandler(wsc.pingHandler) + wsc.pingTimeoutTimer = time.NewTimer(time.Second * (60 + PING_TIMEOUT_BUFFER_SECONDS)) + go wsc.pingWatchdog() +} + +func (wsc *WebSocketClient) pingHandler(appData string) error { + if !wsc.pingTimeoutTimer.Stop() { + <-wsc.pingTimeoutTimer.C + } + + wsc.pingTimeoutTimer.Reset(time.Second * (60 + PING_TIMEOUT_BUFFER_SECONDS)) + wsc.Conn.WriteMessage(websocket.PongMessage, []byte{}) + return nil +} + +func (wsc *WebSocketClient) pingWatchdog() { + <-wsc.pingTimeoutTimer.C + wsc.PingTimeoutChannel <- true +} diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index f4ed3e400..a695e4aa8 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -819,13 +819,12 @@ func (us SqlUserStore) GetByUsername(username string) store.StoreChannel { }) } -func (us SqlUserStore) GetForLogin(loginId string, allowSignInWithUsername, allowSignInWithEmail, ldapEnabled bool) store.StoreChannel { +func (us SqlUserStore) GetForLogin(loginId string, allowSignInWithUsername, allowSignInWithEmail bool) store.StoreChannel { return store.Do(func(result *store.StoreResult) { params := map[string]interface{}{ "LoginId": loginId, "AllowSignInWithUsername": allowSignInWithUsername, "AllowSignInWithEmail": allowSignInWithEmail, - "LdapEnabled": ldapEnabled, } users := []*model.User{} @@ -837,8 +836,7 @@ func (us SqlUserStore) GetForLogin(loginId string, allowSignInWithUsername, allo Users WHERE (:AllowSignInWithUsername AND Username = :LoginId) - OR (:AllowSignInWithEmail AND Email = :LoginId) - OR (:LdapEnabled AND AuthService = '`+model.USER_AUTH_SERVICE_LDAP+`' AND AuthData = :LoginId)`, + OR (:AllowSignInWithEmail AND Email = :LoginId)`, params); err != nil { result.Err = model.NewAppError("SqlUserStore.GetForLogin", "store.sql_user.get_for_login.app_error", nil, err.Error(), http.StatusInternalServerError) } else if len(users) == 1 { diff --git a/store/store.go b/store/store.go index dd149fbe9..2e85c0a68 100644 --- a/store/store.go +++ b/store/store.go @@ -231,7 +231,7 @@ type UserStore interface { GetByAuth(authData *string, authService string) StoreChannel GetAllUsingAuthService(authService string) StoreChannel GetByUsername(username string) StoreChannel - GetForLogin(loginId string, allowSignInWithUsername, allowSignInWithEmail, ldapEnabled bool) StoreChannel + GetForLogin(loginId string, allowSignInWithUsername, allowSignInWithEmail bool) StoreChannel VerifyEmail(userId string) StoreChannel GetEtagForAllProfiles() StoreChannel GetEtagForProfiles(teamId string) StoreChannel diff --git a/store/storetest/mocks/AuditStore.go b/store/storetest/mocks/AuditStore.go index df84545bd..d1ee9082e 100644 --- a/store/storetest/mocks/AuditStore.go +++ b/store/storetest/mocks/AuditStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/ChannelMemberHistoryStore.go b/store/storetest/mocks/ChannelMemberHistoryStore.go index 16155b982..ae8d024d1 100644 --- a/store/storetest/mocks/ChannelMemberHistoryStore.go +++ b/store/storetest/mocks/ChannelMemberHistoryStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 912dbf29c..ecc8b8768 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/ClusterDiscoveryStore.go b/store/storetest/mocks/ClusterDiscoveryStore.go index 997dcf03f..4010006d8 100644 --- a/store/storetest/mocks/ClusterDiscoveryStore.go +++ b/store/storetest/mocks/ClusterDiscoveryStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/CommandStore.go b/store/storetest/mocks/CommandStore.go index de4bc4e34..798bbee4d 100644 --- a/store/storetest/mocks/CommandStore.go +++ b/store/storetest/mocks/CommandStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/CommandWebhookStore.go b/store/storetest/mocks/CommandWebhookStore.go index cede8cdd2..5129388ae 100644 --- a/store/storetest/mocks/CommandWebhookStore.go +++ b/store/storetest/mocks/CommandWebhookStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/ComplianceStore.go b/store/storetest/mocks/ComplianceStore.go index fb828cd4b..dd75941b3 100644 --- a/store/storetest/mocks/ComplianceStore.go +++ b/store/storetest/mocks/ComplianceStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/EmojiStore.go b/store/storetest/mocks/EmojiStore.go index 9871c98aa..b1f0a3217 100644 --- a/store/storetest/mocks/EmojiStore.go +++ b/store/storetest/mocks/EmojiStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/FileInfoStore.go b/store/storetest/mocks/FileInfoStore.go index 4dddf0bd7..67f922146 100644 --- a/store/storetest/mocks/FileInfoStore.go +++ b/store/storetest/mocks/FileInfoStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/JobStore.go b/store/storetest/mocks/JobStore.go index d3558212e..a78a3f94e 100644 --- a/store/storetest/mocks/JobStore.go +++ b/store/storetest/mocks/JobStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/LayeredStoreDatabaseLayer.go b/store/storetest/mocks/LayeredStoreDatabaseLayer.go index a505b6434..e6b8bafb1 100644 --- a/store/storetest/mocks/LayeredStoreDatabaseLayer.go +++ b/store/storetest/mocks/LayeredStoreDatabaseLayer.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/LayeredStoreSupplier.go b/store/storetest/mocks/LayeredStoreSupplier.go index 18dbe3af1..ef6fd99e2 100644 --- a/store/storetest/mocks/LayeredStoreSupplier.go +++ b/store/storetest/mocks/LayeredStoreSupplier.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/LicenseStore.go b/store/storetest/mocks/LicenseStore.go index 5c65425ea..f00ebba78 100644 --- a/store/storetest/mocks/LicenseStore.go +++ b/store/storetest/mocks/LicenseStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/OAuthStore.go b/store/storetest/mocks/OAuthStore.go index fb49d715d..a39570b6c 100644 --- a/store/storetest/mocks/OAuthStore.go +++ b/store/storetest/mocks/OAuthStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/PluginStore.go b/store/storetest/mocks/PluginStore.go index 920b0f63c..b6f161a86 100644 --- a/store/storetest/mocks/PluginStore.go +++ b/store/storetest/mocks/PluginStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/PostStore.go b/store/storetest/mocks/PostStore.go index bdfbb3321..130bfafd7 100644 --- a/store/storetest/mocks/PostStore.go +++ b/store/storetest/mocks/PostStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/PreferenceStore.go b/store/storetest/mocks/PreferenceStore.go index 5ad56914a..f53ae06d5 100644 --- a/store/storetest/mocks/PreferenceStore.go +++ b/store/storetest/mocks/PreferenceStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/ReactionStore.go b/store/storetest/mocks/ReactionStore.go index ce09f3f76..b3e81a83b 100644 --- a/store/storetest/mocks/ReactionStore.go +++ b/store/storetest/mocks/ReactionStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/RoleStore.go b/store/storetest/mocks/RoleStore.go index c1b14d5dc..95e1914e0 100644 --- a/store/storetest/mocks/RoleStore.go +++ b/store/storetest/mocks/RoleStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/SessionStore.go b/store/storetest/mocks/SessionStore.go index 70b2bd945..819ae948d 100644 --- a/store/storetest/mocks/SessionStore.go +++ b/store/storetest/mocks/SessionStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/SqlStore.go b/store/storetest/mocks/SqlStore.go index 021baa7d3..6d2c7ec15 100644 --- a/store/storetest/mocks/SqlStore.go +++ b/store/storetest/mocks/SqlStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/StatusStore.go b/store/storetest/mocks/StatusStore.go index 4acb90bdd..68ccdd4ec 100644 --- a/store/storetest/mocks/StatusStore.go +++ b/store/storetest/mocks/StatusStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/Store.go b/store/storetest/mocks/Store.go index dd1967cd5..5af15d125 100644 --- a/store/storetest/mocks/Store.go +++ b/store/storetest/mocks/Store.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/SystemStore.go b/store/storetest/mocks/SystemStore.go index b31e4646d..e36396fe1 100644 --- a/store/storetest/mocks/SystemStore.go +++ b/store/storetest/mocks/SystemStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/TeamStore.go b/store/storetest/mocks/TeamStore.go index 42303ff26..51a968784 100644 --- a/store/storetest/mocks/TeamStore.go +++ b/store/storetest/mocks/TeamStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/TokenStore.go b/store/storetest/mocks/TokenStore.go index b4baacf03..657aeca49 100644 --- a/store/storetest/mocks/TokenStore.go +++ b/store/storetest/mocks/TokenStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/UserAccessTokenStore.go b/store/storetest/mocks/UserAccessTokenStore.go index c5ef0fefe..fd98a8a99 100644 --- a/store/storetest/mocks/UserAccessTokenStore.go +++ b/store/storetest/mocks/UserAccessTokenStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index 369a29e7a..347dd2065 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. @@ -258,13 +258,13 @@ func (_m *UserStore) GetEtagForProfilesNotInTeam(teamId string) store.StoreChann return r0 } -// GetForLogin provides a mock function with given fields: loginId, allowSignInWithUsername, allowSignInWithEmail, ldapEnabled -func (_m *UserStore) GetForLogin(loginId string, allowSignInWithUsername bool, allowSignInWithEmail bool, ldapEnabled bool) store.StoreChannel { - ret := _m.Called(loginId, allowSignInWithUsername, allowSignInWithEmail, ldapEnabled) +// GetForLogin provides a mock function with given fields: loginId, allowSignInWithUsername, allowSignInWithEmail +func (_m *UserStore) GetForLogin(loginId string, allowSignInWithUsername bool, allowSignInWithEmail bool) store.StoreChannel { + ret := _m.Called(loginId, allowSignInWithUsername, allowSignInWithEmail) var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string, bool, bool, bool) store.StoreChannel); ok { - r0 = rf(loginId, allowSignInWithUsername, allowSignInWithEmail, ldapEnabled) + if rf, ok := ret.Get(0).(func(string, bool, bool) store.StoreChannel); ok { + r0 = rf(loginId, allowSignInWithUsername, allowSignInWithEmail) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(store.StoreChannel) diff --git a/store/storetest/mocks/WebhookStore.go b/store/storetest/mocks/WebhookStore.go index bf5b636eb..a0b2b0bee 100644 --- a/store/storetest/mocks/WebhookStore.go +++ b/store/storetest/mocks/WebhookStore.go @@ -1,4 +1,4 @@ -// Code generated by mockery v1.0.0 +// Code generated by mockery v1.0.0. DO NOT EDIT. // Regenerate this file using `make store-mocks`. diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 2fd7d4190..66f54df39 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -1091,64 +1091,26 @@ func testUserStoreGetForLogin(t *testing.T, ss store.Store) { } store.Must(ss.User().Save(u2)) - if result := <-ss.User().GetForLogin(u1.Username, true, true, true); result.Err != nil { + if result := <-ss.User().GetForLogin(u1.Username, true, true); result.Err != nil { t.Fatal("Should have gotten user by username", result.Err) } else if result.Data.(*model.User).Id != u1.Id { t.Fatal("Should have gotten user1 by username") } - if result := <-ss.User().GetForLogin(u1.Email, true, true, true); result.Err != nil { + if result := <-ss.User().GetForLogin(u1.Email, true, true); result.Err != nil { t.Fatal("Should have gotten user by email", result.Err) } else if result.Data.(*model.User).Id != u1.Id { t.Fatal("Should have gotten user1 by email") } - if result := <-ss.User().GetForLogin(*u2.AuthData, true, true, true); result.Err != nil { - t.Fatal("Should have gotten user by AD/LDAP AuthData", result.Err) - } else if result.Data.(*model.User).Id != u2.Id { - t.Fatal("Should have gotten user2 by AD/LDAP AuthData") - } - - // prevent getting user by AuthData when they're not an LDAP user - if result := <-ss.User().GetForLogin(*u1.AuthData, true, true, true); result.Err == nil { - t.Fatal("Should not have gotten user by non-AD/LDAP AuthData") - } - // prevent getting user when different login methods are disabled - if result := <-ss.User().GetForLogin(u1.Username, false, true, true); result.Err == nil { + if result := <-ss.User().GetForLogin(u1.Username, false, true); result.Err == nil { t.Fatal("Should have failed to get user1 by username") } - if result := <-ss.User().GetForLogin(u1.Email, true, false, true); result.Err == nil { + if result := <-ss.User().GetForLogin(u1.Email, true, false); result.Err == nil { t.Fatal("Should have failed to get user1 by email") } - - if result := <-ss.User().GetForLogin(*u2.AuthData, true, true, false); result.Err == nil { - t.Fatal("Should have failed to get user3 by AD/LDAP AuthData") - } - - auth3 := model.NewId() - - // test a special case where two users will have conflicting login information so we throw a special error - u3 := &model.User{ - Email: model.NewId(), - Username: model.NewId(), - AuthService: model.USER_AUTH_SERVICE_LDAP, - AuthData: &auth3, - } - store.Must(ss.User().Save(u3)) - - u4 := &model.User{ - Email: model.NewId(), - Username: model.NewId(), - AuthService: model.USER_AUTH_SERVICE_LDAP, - AuthData: &u3.Username, - } - store.Must(ss.User().Save(u4)) - - if err := (<-ss.User().GetForLogin(u3.Username, true, true, true)).Err; err == nil { - t.Fatal("Should have failed to get users with conflicting login information") - } } func testUserStoreUpdatePassword(t *testing.T, ss store.Store) { diff --git a/utils/file_backend.go b/utils/file_backend.go index 42af7f604..0da0bc3b8 100644 --- a/utils/file_backend.go +++ b/utils/file_backend.go @@ -4,6 +4,7 @@ package utils import ( + "io" "net/http" "github.com/mattermost/mattermost-server/model" @@ -15,7 +16,7 @@ type FileBackend interface { ReadFile(path string) ([]byte, *model.AppError) CopyFile(oldPath, newPath string) *model.AppError MoveFile(oldPath, newPath string) *model.AppError - WriteFile(f []byte, path string) *model.AppError + WriteFile(fr io.Reader, path string) (int64, *model.AppError) RemoveFile(path string) *model.AppError ListDirectory(path string) (*[]string, *model.AppError) diff --git a/utils/file_backend_local.go b/utils/file_backend_local.go index f85ace55a..37bca7987 100644 --- a/utils/file_backend_local.go +++ b/utils/file_backend_local.go @@ -4,6 +4,8 @@ package utils import ( + "bytes" + "io" "io/ioutil" "net/http" "os" @@ -22,8 +24,8 @@ type LocalFileBackend struct { } func (b *LocalFileBackend) TestConnection() *model.AppError { - f := []byte("testingwrite") - if err := writeFileLocally(f, filepath.Join(b.directory, TEST_FILE_PATH)); err != nil { + f := bytes.NewReader([]byte("testingwrite")) + if _, err := writeFileLocally(f, filepath.Join(b.directory, TEST_FILE_PATH)); err != nil { return model.NewAppError("TestFileConnection", "Don't have permissions to write to local path specified or other error.", nil, err.Error(), http.StatusInternalServerError) } os.Remove(filepath.Join(b.directory, TEST_FILE_PATH)) @@ -58,21 +60,25 @@ func (b *LocalFileBackend) MoveFile(oldPath, newPath string) *model.AppError { return nil } -func (b *LocalFileBackend) WriteFile(f []byte, path string) *model.AppError { - return writeFileLocally(f, filepath.Join(b.directory, path)) +func (b *LocalFileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppError) { + return writeFileLocally(fr, filepath.Join(b.directory, path)) } -func writeFileLocally(f []byte, path string) *model.AppError { +func writeFileLocally(fr io.Reader, path string) (int64, *model.AppError) { if err := os.MkdirAll(filepath.Dir(path), 0774); err != nil { directory, _ := filepath.Abs(filepath.Dir(path)) - return model.NewAppError("WriteFile", "api.file.write_file_locally.create_dir.app_error", nil, "directory="+directory+", err="+err.Error(), http.StatusInternalServerError) + return 0, model.NewAppError("WriteFile", "api.file.write_file_locally.create_dir.app_error", nil, "directory="+directory+", err="+err.Error(), http.StatusInternalServerError) } - - if err := ioutil.WriteFile(path, f, 0644); err != nil { - return model.NewAppError("WriteFile", "api.file.write_file_locally.writing.app_error", nil, err.Error(), http.StatusInternalServerError) + fw, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644) + if err != nil { + return 0, model.NewAppError("WriteFile", "api.file.write_file_locally.writing.app_error", nil, err.Error(), http.StatusInternalServerError) } - - return nil + defer fw.Close() + written, err := io.Copy(fw, fr) + if err != nil { + return written, model.NewAppError("WriteFile", "api.file.write_file_locally.writing.app_error", nil, err.Error(), http.StatusInternalServerError) + } + return written, nil } func (b *LocalFileBackend) RemoveFile(path string) *model.AppError { diff --git a/utils/file_backend_s3.go b/utils/file_backend_s3.go index 87ed3dd16..1772f09ea 100644 --- a/utils/file_backend_s3.go +++ b/utils/file_backend_s3.go @@ -4,7 +4,7 @@ package utils import ( - "bytes" + "io" "io/ioutil" "net/http" "os" @@ -136,10 +136,10 @@ func (b *S3FileBackend) MoveFile(oldPath, newPath string) *model.AppError { return nil } -func (b *S3FileBackend) WriteFile(f []byte, path string) *model.AppError { +func (b *S3FileBackend) WriteFile(fr io.Reader, path string) (int64, *model.AppError) { s3Clnt, err := b.s3New() if err != nil { - return model.NewAppError("WriteFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError) + return 0, model.NewAppError("WriteFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError) } var contentType string @@ -150,12 +150,12 @@ func (b *S3FileBackend) WriteFile(f []byte, path string) *model.AppError { } options := s3PutOptions(b.encrypt, contentType) - - if _, err = s3Clnt.PutObject(b.bucket, path, bytes.NewReader(f), -1, options); err != nil { - return model.NewAppError("WriteFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError) + written, err := s3Clnt.PutObject(b.bucket, path, fr, -1, options) + if err != nil { + return written, model.NewAppError("WriteFile", "api.file.write_file.s3.app_error", nil, err.Error(), http.StatusInternalServerError) } - return nil + return written, nil } func (b *S3FileBackend) RemoveFile(path string) *model.AppError { diff --git a/utils/file_backend_test.go b/utils/file_backend_test.go index 047e9df62..7f8265d73 100644 --- a/utils/file_backend_test.go +++ b/utils/file_backend_test.go @@ -4,6 +4,7 @@ package utils import ( + "bytes" "fmt" "io/ioutil" "os" @@ -95,7 +96,9 @@ func (s *FileBackendTestSuite) TestReadWriteFile() { b := []byte("test") path := "tests/" + model.NewId() - s.Nil(s.backend.WriteFile(b, path)) + written, err := s.backend.WriteFile(bytes.NewReader(b), path) + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path) read, err := s.backend.ReadFile(path) @@ -109,7 +112,9 @@ func (s *FileBackendTestSuite) TestReadWriteFileImage() { b := []byte("testimage") path := "tests/" + model.NewId() + ".png" - s.Nil(s.backend.WriteFile(b, path)) + written, err := s.backend.WriteFile(bytes.NewReader(b), path) + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path) read, err := s.backend.ReadFile(path) @@ -124,8 +129,9 @@ func (s *FileBackendTestSuite) TestCopyFile() { path1 := "tests/" + model.NewId() path2 := "tests/" + model.NewId() - err := s.backend.WriteFile(b, path1) + written, err := s.backend.WriteFile(bytes.NewReader(b), path1) s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path1) err = s.backend.CopyFile(path1, path2) @@ -144,8 +150,9 @@ func (s *FileBackendTestSuite) TestCopyFileToDirectoryThatDoesntExist() { path1 := "tests/" + model.NewId() path2 := "tests/newdirectory/" + model.NewId() - err := s.backend.WriteFile(b, path1) + written, err := s.backend.WriteFile(bytes.NewReader(b), path1) s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path1) err = s.backend.CopyFile(path1, path2) @@ -164,13 +171,15 @@ func (s *FileBackendTestSuite) TestMoveFile() { path1 := "tests/" + model.NewId() path2 := "tests/" + model.NewId() - s.Nil(s.backend.WriteFile(b, path1)) + written, err := s.backend.WriteFile(bytes.NewReader(b), path1) + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path1) s.Nil(s.backend.MoveFile(path1, path2)) defer s.backend.RemoveFile(path2) - _, err := s.backend.ReadFile(path1) + _, err = s.backend.ReadFile(path1) s.Error(err) _, err = s.backend.ReadFile(path2) @@ -181,15 +190,26 @@ func (s *FileBackendTestSuite) TestRemoveFile() { b := []byte("test") path := "tests/" + model.NewId() - s.Nil(s.backend.WriteFile(b, path)) + written, err := s.backend.WriteFile(bytes.NewReader(b), path) + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") s.Nil(s.backend.RemoveFile(path)) - _, err := s.backend.ReadFile(path) + _, err = s.backend.ReadFile(path) s.Error(err) - s.Nil(s.backend.WriteFile(b, "tests2/foo")) - s.Nil(s.backend.WriteFile(b, "tests2/bar")) - s.Nil(s.backend.WriteFile(b, "tests2/asdf")) + written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/foo") + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") + + written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/bar") + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") + + written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/asdf") + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") + s.Nil(s.backend.RemoveDirectory("tests2")) } @@ -198,9 +218,14 @@ func (s *FileBackendTestSuite) TestListDirectory() { path1 := "19700101/" + model.NewId() path2 := "19800101/" + model.NewId() - s.Nil(s.backend.WriteFile(b, path1)) + written, err := s.backend.WriteFile(bytes.NewReader(b), path1) + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path1) - s.Nil(s.backend.WriteFile(b, path2)) + + written, err = s.backend.WriteFile(bytes.NewReader(b), path2) + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path2) paths, err := s.backend.ListDirectory("") @@ -222,13 +247,21 @@ func (s *FileBackendTestSuite) TestListDirectory() { func (s *FileBackendTestSuite) TestRemoveDirectory() { b := []byte("test") - s.Nil(s.backend.WriteFile(b, "tests2/foo")) - s.Nil(s.backend.WriteFile(b, "tests2/bar")) - s.Nil(s.backend.WriteFile(b, "tests2/aaa")) + written, err := s.backend.WriteFile(bytes.NewReader(b), "tests2/foo") + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") + + written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/bar") + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") + + written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/aaa") + s.Nil(err) + s.EqualValues(len(b), written, "expected given number of bytes to have been written") s.Nil(s.backend.RemoveDirectory("tests2")) - _, err := s.backend.ReadFile("tests2/foo") + _, err = s.backend.ReadFile("tests2/foo") s.Error(err) _, err = s.backend.ReadFile("tests2/bar") s.Error(err) diff --git a/utils/mail_test.go b/utils/mail_test.go index 6bd8e7044..22a50df5f 100644 --- a/utils/mail_test.go +++ b/utils/mail_test.go @@ -150,8 +150,10 @@ func TestSendMailUsingConfig(t *testing.T) { filePath2 := fmt.Sprintf("test2/%s", fileName) fileContents1 := []byte("hello world") fileContents2 := []byte("foo bar") - assert.Nil(t, fileBackend.WriteFile(fileContents1, filePath1)) - assert.Nil(t, fileBackend.WriteFile(fileContents2, filePath2)) + _, err := fileBackend.WriteFile(bytes.NewReader(fileContents1), filePath1) + assert.Nil(t, err) + _, err := fileBackend.WriteFile(bytes.NewReader(fileContents2), filePath2) + assert.Nil(t, err) defer fileBackend.RemoveFile(filePath1) defer fileBackend.RemoveFile(filePath2) |