From 3928535456f9fcb025ed86edeb4d258f2c524150 Mon Sep 17 00:00:00 2001 From: Corey Hulen Date: Wed, 11 May 2016 11:04:30 -0700 Subject: [PATCH 01/10] PLT-2905 fixing upgrade of SSO accounts (#2962) * PLT-2905 fixing upgrade of SSO accounts * Fixing multiple Auths mapped to different emails --- api/admin_test.go | 3 ++- api/authentication.go | 6 +++--- api/oauth.go | 7 +++++-- api/user.go | 15 ++++++++------- api/user_test.go | 6 ++++-- i18n/en.json | 2 +- mattermost.go | 35 +++++++++++++++++++++++++++++----- model/gitlab/gitlab.go | 2 +- model/user.go | 27 ++++++++++++++++++-------- store/sql_user_store.go | 37 ++++++++++++++++++++++++------------ store/sql_user_store_test.go | 34 +++++++++++++++++++++------------ store/store.go | 4 ++-- 12 files changed, 122 insertions(+), 56 deletions(-) diff --git a/api/admin_test.go b/api/admin_test.go index 933c3d59c3..f3d3ec4ed5 100644 --- a/api/admin_test.go +++ b/api/admin_test.go @@ -457,7 +457,8 @@ func TestAdminResetPassword(t *testing.T) { t.Fatal("Should have errored - password too short") } - user2 := &model.User{Email: strings.ToLower(model.NewId()) + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", AuthData: "1", AuthService: "random"} + authData := model.NewId() + user2 := &model.User{Email: strings.ToLower(model.NewId()) + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", AuthData: &authData, AuthService: "random"} user2 = Client.Must(Client.CreateUser(user2, "")).Data.(*model.User) LinkUserToTeam(user2, team) store.Must(Srv.Store.User().VerifyEmail(user2.Id)) diff --git a/api/authentication.go b/api/authentication.go index 10ed578e1d..9243947add 100644 --- a/api/authentication.go +++ b/api/authentication.go @@ -39,17 +39,17 @@ func checkUserPassword(user *model.User, password string) *model.AppError { } } -func checkLdapUserPasswordAndAllCriteria(ldapId, password, mfaToken string) (*model.User, *model.AppError) { +func checkLdapUserPasswordAndAllCriteria(ldapId *string, password string, mfaToken string) (*model.User, *model.AppError) { ldapInterface := einterfaces.GetLdapInterface() - if ldapInterface == nil { + if ldapInterface == nil || ldapId == nil { err := model.NewLocAppError("doLdapAuthentication", "api.user.login_ldap.not_available.app_error", nil, "") err.StatusCode = http.StatusNotImplemented return nil, err } var user *model.User - if ldapUser, err := ldapInterface.DoLogin(ldapId, password); err != nil { + if ldapUser, err := ldapInterface.DoLogin(*ldapId, password); err != nil { err.StatusCode = http.StatusUnauthorized return nil, err } else { diff --git a/api/oauth.go b/api/oauth.go index 0375f4e6f8..37ca5ce0a4 100644 --- a/api/oauth.go +++ b/api/oauth.go @@ -600,8 +600,11 @@ func CompleteSwitchWithOAuth(c *Context, w http.ResponseWriter, r *http.Request, return } else { ssoUser := provider.GetUserFromJson(userData) - authData = ssoUser.AuthData ssoEmail = ssoUser.Email + + if ssoUser.AuthData != nil { + authData = *ssoUser.AuthData + } } if len(authData) == 0 { @@ -628,7 +631,7 @@ func CompleteSwitchWithOAuth(c *Context, w http.ResponseWriter, r *http.Request, return } - if result := <-Srv.Store.User().UpdateAuthData(user.Id, service, authData, ssoEmail); result.Err != nil { + if result := <-Srv.Store.User().UpdateAuthData(user.Id, service, &authData, ssoEmail); result.Err != nil { c.Err = result.Err return } diff --git a/api/user.go b/api/user.go index c53a643c71..4b9c3a3c84 100644 --- a/api/user.go +++ b/api/user.go @@ -535,7 +535,7 @@ func LoginByOAuth(c *Context, w http.ResponseWriter, r *http.Request, service st } var user *model.User - if result := <-Srv.Store.User().GetByAuth(authData, service); result.Err != nil { + if result := <-Srv.Store.User().GetByAuth(&authData, service); result.Err != nil { if result.Err.Id == store.MISSING_AUTH_ACCOUNT_ERROR { return CreateOAuthUser(c, w, r, service, bytes.NewReader(buf.Bytes()), "") } @@ -1289,7 +1289,8 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) { } rusers[0].Password = "" - rusers[0].AuthData = "" + rusers[0].AuthData = new(string) + *rusers[0].AuthData = "" w.Write([]byte(rusers[0].ToJson())) } } @@ -1337,7 +1338,7 @@ func updatePassword(c *Context, w http.ResponseWriter, r *http.Request) { user := result.Data.(*model.User) - if user.AuthData != "" { + if user.AuthData != nil && *user.AuthData != "" { c.LogAudit("failed - tried to update user password who was logged in through oauth") c.Err = model.NewLocAppError("updatePassword", "api.user.update_password.oauth.app_error", nil, "auth_service="+user.AuthService) c.Err.StatusCode = http.StatusBadRequest @@ -1653,7 +1654,7 @@ func sendPasswordReset(c *Context, w http.ResponseWriter, r *http.Request) { user = result.Data.(*model.User) } - if len(user.AuthData) != 0 { + if user.AuthData != nil && len(*user.AuthData) != 0 { c.Err = model.NewLocAppError("sendPasswordReset", "api.user.send_password_reset.sso.app_error", nil, "userId="+user.Id) return } @@ -1749,7 +1750,7 @@ func ResetPassword(c *Context, userId, newPassword string) *model.AppError { user = result.Data.(*model.User) } - if len(user.AuthData) != 0 && !c.IsSystemAdmin() { + if user.AuthData != nil && len(*user.AuthData) != 0 && !c.IsSystemAdmin() { return model.NewLocAppError("ResetPassword", "api.user.reset_password.sso.app_error", nil, "userId="+user.Id) } @@ -2148,13 +2149,13 @@ func ldapToEmail(c *Context, w http.ResponseWriter, r *http.Request) { } ldapInterface := einterfaces.GetLdapInterface() - if ldapInterface == nil { + if ldapInterface == nil || user.AuthData == nil { c.Err = model.NewLocAppError("ldapToEmail", "api.user.ldap_to_email.not_available.app_error", nil, "") c.Err.StatusCode = http.StatusNotImplemented return } - if err := ldapInterface.CheckPassword(user.AuthData, ldapPassword); err != nil { + if err := ldapInterface.CheckPassword(*user.AuthData, ldapPassword); err != nil { c.LogAuditWithUserId(user.Id, "fail - ldap authentication failed") c.Err = err return diff --git a/api/user_test.go b/api/user_test.go index 9dd57dc209..c34d32c11f 100644 --- a/api/user_test.go +++ b/api/user_test.go @@ -1109,7 +1109,8 @@ func TestSendPasswordReset(t *testing.T) { t.Fatal("Should have errored - bad email") } - user2 := &model.User{Email: strings.ToLower(model.NewId()) + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", AuthData: "1", AuthService: "random"} + authData := model.NewId() + user2 := &model.User{Email: strings.ToLower(model.NewId()) + "success+test@simulator.amazonses.com", Nickname: "Corey Hulen", AuthData: &authData, AuthService: "random"} user2 = Client.Must(Client.CreateUser(user2, "")).Data.(*model.User) LinkUserToTeam(user2, team) store.Must(Srv.Store.User().VerifyEmail(user2.Id)) @@ -1178,7 +1179,8 @@ func TestResetPassword(t *testing.T) { recovery = result.Data.(*model.PasswordRecovery) } - if result := <-Srv.Store.User().UpdateAuthData(user.Id, "random", "1", ""); result.Err != nil { + authData := model.NewId() + if result := <-Srv.Store.User().UpdateAuthData(user.Id, "random", &authData, ""); result.Err != nil { t.Fatal(result.Err) } diff --git a/i18n/en.json b/i18n/en.json index 432279c23d..cfa8eaf563 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1337,7 +1337,7 @@ }, { "id": "api.templates.upgrade_30_body.info", - "translation": "

YOUR DUPLICATE ACCOUNTS HAVE BEEN UPDATED

Your Mattermost server is being upgraded to Version 3.0, which lets you use a single account across multiple teams.

You are receiving this email because the upgrade process has detected your account had the same email or username as other accounts on the server.

The following updates have been made:

{{if .EmailChanged }}- The duplicate email of an account on the `/{{.TeamName}}` team was changed to `{{.Email}}`. If you use email and password to login, you can use this new email address for login.

{{end}}{{if .UsernameChanged }}- The duplicate username of an account on the team site `/{{.TeamName}}` has been changed to `{{.Username}}` to avoid confusion with other accounts.

{{end}} RECOMMENDED ACTION:

It is recommended that you login to your teams used by your duplicate accounts and add your primary account to the team and any public channels and private groups which you wish to continue using.

This gives your primary account access to all public channel and private group history. You can continue to access the direct message history of your duplicate accounts by logging in with their credentials.

FOR MORE INFORMATION:

For more information on the upgrade to Mattermost 3.0 please see: http://www.mattermost.org/upgrading-to-mattermost-3-0/

" + "translation": "

YOUR DUPLICATE ACCOUNTS HAVE BEEN UPDATED

Your Mattermost server is being upgraded to Version 3.0, which lets you use a single account across multiple teams.

You are receiving this email because the upgrade process has detected your account had the same email or username as other accounts on the server.

The following updates have been made:

{{if .EmailChanged }}- The duplicate email of an account on the `/{{.TeamName}}` team was changed to `{{.Email}}`. You will need to use email and password to login, you can use this new email address for login.

{{end}}{{if .UsernameChanged }}- The duplicate username of an account on the team site `/{{.TeamName}}` has been changed to `{{.Username}}` to avoid confusion with other accounts.

{{end}} RECOMMENDED ACTION:

It is recommended that you login to your teams used by your duplicate accounts and add your primary account to the team and any public channels and private groups which you wish to continue using.

This gives your primary account access to all public channel and private group history. You can continue to access the direct message history of your duplicate accounts by logging in with their credentials.

FOR MORE INFORMATION:

For more information on the upgrade to Mattermost 3.0 please see: http://www.mattermost.org/upgrading-to-mattermost-3-0/

" }, { "id": "api.templates.upgrade_30_subject.info", diff --git a/mattermost.go b/mattermost.go index be9b08a95a..6fe285d58a 100644 --- a/mattermost.go +++ b/mattermost.go @@ -373,14 +373,15 @@ func cmdUpdateDb30() { uniqueEmails := make(map[string]bool) uniqueUsernames := make(map[string]bool) - primaryUsers := convertTeamTo30(team.Name, team, uniqueEmails, uniqueUsernames) + uniqueAuths := make(map[string]bool) + primaryUsers := convertTeamTo30(team.Name, team, uniqueEmails, uniqueUsernames, uniqueAuths) l4g.Info("Upgraded %v users", len(primaryUsers)) for _, otherTeam := range teams { if otherTeam.Id != team.Id { l4g.Info("Upgrading team %v", otherTeam.Name) - users := convertTeamTo30(team.Name, otherTeam, uniqueEmails, uniqueUsernames) + users := convertTeamTo30(team.Name, otherTeam, uniqueEmails, uniqueUsernames, uniqueAuths) l4g.Info("Upgraded %v users", len(users)) } @@ -400,6 +401,18 @@ func cmdUpdateDb30() { flushLogAndExit(1) } + if _, err := store.GetMaster().Exec(` + UPDATE Users + SET + AuthData = NULL + WHERE + AuthData = '' + `, + ); err != nil { + l4g.Error("Failed to update AuthData types details=%v", err) + flushLogAndExit(1) + } + extraLength := store.GetMaxLengthOfColumnIfExists("Audits", "ExtraInfo") if len(extraLength) > 0 && extraLength != "1024" { store.AlterColumnTypeIfExists("Audits", "ExtraInfo", "VARCHAR(1024)", "VARCHAR(1024)") @@ -424,6 +437,7 @@ func cmdUpdateDb30() { store.RemoveIndexIfExists("idx_users_team_id", "Users") store.CreateUniqueIndexIfNotExists("idx_users_email_unique", "Users", "Email") store.CreateUniqueIndexIfNotExists("idx_users_username_unique", "Users", "Username") + store.CreateUniqueIndexIfNotExists("idx_users_authdata_unique", "Users", "AuthData") store.RemoveColumnIfExists("Teams", "AllowTeamListing") store.RemoveColumnIfExists("Users", "TeamId") } @@ -448,12 +462,13 @@ type UserForUpgrade struct { Email string Roles string TeamId string + AuthData *string } -func convertTeamTo30(primaryTeamName string, team *TeamForUpgrade, uniqueEmails map[string]bool, uniqueUsernames map[string]bool) []*UserForUpgrade { +func convertTeamTo30(primaryTeamName string, team *TeamForUpgrade, uniqueEmails map[string]bool, uniqueUsernames map[string]bool, uniqueAuths map[string]bool) []*UserForUpgrade { store := api.Srv.Store.(*store.SqlStore) var users []*UserForUpgrade - if _, err := store.GetMaster().Select(&users, "SELECT Users.Id, Users.Username, Users.Email, Users.Roles, Users.TeamId FROM Users WHERE Users.TeamId = :TeamId", map[string]interface{}{"TeamId": team.Id}); err != nil { + if _, err := store.GetMaster().Select(&users, "SELECT Users.Id, Users.Username, Users.Email, Users.Roles, Users.TeamId, Users.AuthData FROM Users WHERE Users.TeamId = :TeamId", map[string]interface{}{"TeamId": team.Id}); err != nil { l4g.Error("Failed to load profiles for team details=%v", err) flushLogAndExit(1) } @@ -530,13 +545,19 @@ func convertTeamTo30(primaryTeamName string, team *TeamForUpgrade, uniqueEmails } } + if user.AuthData != nil && *user.AuthData != "" && uniqueAuths[*user.AuthData] { + shouldUpdateUser = true + } + if shouldUpdateUser { if _, err := store.GetMaster().Exec(` UPDATE Users SET Email = :Email, Username = :Username, - Roles = :Roles + Roles = :Roles, + AuthService = '', + AuthData = NULL WHERE Id = :Id `, @@ -590,6 +611,10 @@ func convertTeamTo30(primaryTeamName string, team *TeamForUpgrade, uniqueEmails uniqueEmails[user.Email] = true uniqueUsernames[user.Username] = true + + if user.AuthData != nil && *user.AuthData != "" { + uniqueAuths[*user.AuthData] = true + } } return users diff --git a/model/gitlab/gitlab.go b/model/gitlab/gitlab.go index d6071f99fa..fc70dd93f1 100644 --- a/model/gitlab/gitlab.go +++ b/model/gitlab/gitlab.go @@ -47,7 +47,7 @@ func userFromGitLabUser(glu *GitLabUser) *model.User { } strings.TrimSpace(user.Email) user.Email = glu.Email - user.AuthData = strconv.FormatInt(glu.Id, 10) + *user.AuthData = strconv.FormatInt(glu.Id, 10) user.AuthService = model.USER_AUTH_SERVICE_GITLAB return user diff --git a/model/user.go b/model/user.go index 7563de8ae0..15c281401b 100644 --- a/model/user.go +++ b/model/user.go @@ -37,7 +37,7 @@ type User struct { DeleteAt int64 `json:"delete_at"` Username string `json:"username"` Password string `json:"password,omitempty"` - AuthData string `json:"auth_data,omitempty"` + AuthData *string `json:"auth_data,omitempty"` AuthService string `json:"auth_service"` Email string `json:"email"` EmailVerified bool `json:"email_verified,omitempty"` @@ -99,15 +99,15 @@ func (u *User) IsValid() *AppError { return NewLocAppError("User.IsValid", "model.user.is_valid.pwd.app_error", nil, "user_id="+u.Id) } - if len(u.AuthData) > 128 { + if u.AuthData != nil && len(*u.AuthData) > 128 { return NewLocAppError("User.IsValid", "model.user.is_valid.auth_data.app_error", nil, "user_id="+u.Id) } - if len(u.AuthData) > 0 && len(u.AuthService) == 0 { + if u.AuthData != nil && len(*u.AuthData) > 0 && len(u.AuthService) == 0 { return NewLocAppError("User.IsValid", "model.user.is_valid.auth_data_type.app_error", nil, "user_id="+u.Id) } - if len(u.Password) > 0 && len(u.AuthData) > 0 { + if len(u.Password) > 0 && u.AuthData != nil && len(*u.AuthData) > 0 { return NewLocAppError("User.IsValid", "model.user.is_valid.auth_data_pwd.app_error", nil, "user_id="+u.Id) } @@ -130,6 +130,10 @@ func (u *User) PreSave() { u.Username = NewId() } + if u.AuthData != nil && *u.AuthData == "" { + u.AuthData = nil + } + u.Username = strings.ToLower(u.Username) u.Email = strings.ToLower(u.Email) u.Locale = strings.ToLower(u.Locale) @@ -165,6 +169,10 @@ func (u *User) PreUpdate() { u.Locale = strings.ToLower(u.Locale) u.UpdateAt = GetMillis() + if u.AuthData != nil && *u.AuthData == "" { + u.AuthData = nil + } + if u.NotifyProps == nil || len(u.NotifyProps) == 0 { u.SetDefaultNotifications() } else if _, ok := u.NotifyProps["mention_keys"]; ok { @@ -237,7 +245,8 @@ func (u *User) IsAway() bool { // Remove any private data from the user object func (u *User) Sanitize(options map[string]bool) { u.Password = "" - u.AuthData = "" + u.AuthData = new(string) + *u.AuthData = "" u.MfaSecret = "" if len(options) != 0 && !options["email"] { @@ -255,7 +264,8 @@ func (u *User) Sanitize(options map[string]bool) { func (u *User) ClearNonProfileFields() { u.UpdateAt = 0 u.Password = "" - u.AuthData = "" + u.AuthData = new(string) + *u.AuthData = "" u.AuthService = "" u.MfaActive = false u.MfaSecret = "" @@ -376,7 +386,8 @@ func (u *User) IsLDAPUser() bool { func (u *User) PreExport() { u.Password = "" - u.AuthData = "" + u.AuthData = new(string) + *u.AuthData = "" u.LastActivityAt = 0 u.LastPingAt = 0 u.LastPasswordUpdate = 0 @@ -429,7 +440,7 @@ func HashPassword(password string) string { // ComparePassword compares the hash func ComparePassword(hash string, password string) bool { - if len(password) == 0 { + if len(password) == 0 || len(hash) == 0 { return false } diff --git a/store/sql_user_store.go b/store/sql_user_store.go index 974081a64e..080d8d1288 100644 --- a/store/sql_user_store.go +++ b/store/sql_user_store.go @@ -29,7 +29,7 @@ func NewSqlUserStore(sqlStore *SqlStore) UserStore { table.ColMap("Id").SetMaxSize(26) table.ColMap("Username").SetMaxSize(64).SetUnique(true) table.ColMap("Password").SetMaxSize(128) - table.ColMap("AuthData").SetMaxSize(128) + table.ColMap("AuthData").SetMaxSize(128).SetUnique(true) table.ColMap("AuthService").SetMaxSize(32) table.ColMap("Email").SetMaxSize(128).SetUnique(true) table.ColMap("Nickname").SetMaxSize(64) @@ -265,7 +265,7 @@ func (us SqlUserStore) UpdatePassword(userId, hashedPassword string) StoreChanne updateAt := model.GetMillis() - if _, err := us.GetMaster().Exec("UPDATE Users SET Password = :Password, LastPasswordUpdate = :LastPasswordUpdate, UpdateAt = :UpdateAt, AuthData = '', AuthService = '', EmailVerified = true, FailedAttempts = 0 WHERE Id = :UserId", map[string]interface{}{"Password": hashedPassword, "LastPasswordUpdate": updateAt, "UpdateAt": updateAt, "UserId": userId}); err != nil { + if _, err := us.GetMaster().Exec("UPDATE Users SET Password = :Password, LastPasswordUpdate = :LastPasswordUpdate, UpdateAt = :UpdateAt, AuthData = NULL, AuthService = '', EmailVerified = true, FailedAttempts = 0 WHERE Id = :UserId", map[string]interface{}{"Password": hashedPassword, "LastPasswordUpdate": updateAt, "UpdateAt": updateAt, "UserId": userId}); err != nil { result.Err = model.NewLocAppError("SqlUserStore.UpdatePassword", "store.sql_user.update_password.app_error", nil, "id="+userId+", "+err.Error()) } else { result.Data = userId @@ -297,7 +297,7 @@ func (us SqlUserStore) UpdateFailedPasswordAttempts(userId string, attempts int) return storeChannel } -func (us SqlUserStore) UpdateAuthData(userId, service, authData, email string) StoreChannel { +func (us SqlUserStore) UpdateAuthData(userId string, service string, authData *string, email string) StoreChannel { storeChannel := make(StoreChannel) @@ -513,7 +513,8 @@ func (us SqlUserStore) GetAllProfiles() StoreChannel { for _, u := range users { u.Password = "" - u.AuthData = "" + u.AuthData = new(string) + *u.AuthData = "" userMap[u.Id] = u } @@ -564,7 +565,8 @@ func (us SqlUserStore) GetProfiles(teamId string) StoreChannel { for _, u := range users { u.Password = "" - u.AuthData = "" + u.AuthData = new(string) + *u.AuthData = "" userMap[u.Id] = u } @@ -623,7 +625,8 @@ func (us SqlUserStore) GetDirectProfiles(userId string) StoreChannel { for _, u := range users { u.Password = "" - u.AuthData = "" + u.AuthData = new(string) + *u.AuthData = "" userMap[u.Id] = u } @@ -665,7 +668,8 @@ func (us SqlUserStore) GetProfileByIds(userIds []string) StoreChannel { for _, u := range users { u.Password = "" - u.AuthData = "" + u.AuthData = new(string) + *u.AuthData = "" userMap[u.Id] = u } @@ -696,7 +700,8 @@ func (us SqlUserStore) GetSystemAdminProfiles() StoreChannel { for _, u := range users { u.Password = "" - u.AuthData = "" + u.AuthData = new(string) + *u.AuthData = "" userMap[u.Id] = u } @@ -734,20 +739,27 @@ func (us SqlUserStore) GetByEmail(email string) StoreChannel { return storeChannel } -func (us SqlUserStore) GetByAuth(authData string, authService string) StoreChannel { +func (us SqlUserStore) GetByAuth(authData *string, authService string) StoreChannel { storeChannel := make(StoreChannel) go func() { result := StoreResult{} + if authData == nil || *authData == "" { + result.Err = model.NewLocAppError("SqlUserStore.GetByAuth", MISSING_AUTH_ACCOUNT_ERROR, nil, "authData='', authService="+authService) + storeChannel <- result + close(storeChannel) + return + } + user := model.User{} if err := us.GetReplica().SelectOne(&user, "SELECT * FROM Users WHERE AuthData = :AuthData AND AuthService = :AuthService", map[string]interface{}{"AuthData": authData, "AuthService": authService}); err != nil { if err == sql.ErrNoRows { - result.Err = model.NewLocAppError("SqlUserStore.GetByAuth", MISSING_AUTH_ACCOUNT_ERROR, nil, "authData="+authData+", authService="+authService+", "+err.Error()) + result.Err = model.NewLocAppError("SqlUserStore.GetByAuth", MISSING_AUTH_ACCOUNT_ERROR, nil, "authData="+*authData+", authService="+authService+", "+err.Error()) } else { - result.Err = model.NewLocAppError("SqlUserStore.GetByAuth", "store.sql_user.get_by_auth.other.app_error", nil, "authData="+authData+", authService="+authService+", "+err.Error()) + result.Err = model.NewLocAppError("SqlUserStore.GetByAuth", "store.sql_user.get_by_auth.other.app_error", nil, "authData="+*authData+", authService="+authService+", "+err.Error()) } } @@ -857,7 +869,8 @@ func (us SqlUserStore) GetForExport(teamId string) StoreChannel { } else { for _, u := range users { u.Password = "" - u.AuthData = "" + u.AuthData = new(string) + *u.AuthData = "" } result.Data = users diff --git a/store/sql_user_store_test.go b/store/sql_user_store_test.go index b48da55f5d..5c33ea0f1a 100644 --- a/store/sql_user_store_test.go +++ b/store/sql_user_store_test.go @@ -458,9 +458,11 @@ func TestUserStoreGetByAuthData(t *testing.T) { teamId := model.NewId() + auth := "123" + model.NewId() + u1 := &model.User{} u1.Email = model.NewId() - u1.AuthData = "123" + model.NewId() + u1.AuthData = &auth u1.AuthService = "service" Must(store.User().Save(u1)) Must(store.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u1.Id})) @@ -469,7 +471,8 @@ func TestUserStoreGetByAuthData(t *testing.T) { t.Fatal(err) } - if err := (<-store.User().GetByAuth("", "")).Err; err == nil { + rauth := "" + if err := (<-store.User().GetByAuth(&rauth, "")).Err; err == nil { t.Fatal("Should have failed because of missing auth data") } } @@ -497,19 +500,23 @@ func TestUserStoreGetByUsername(t *testing.T) { func TestUserStoreGetForLogin(t *testing.T) { Setup() + auth := model.NewId() + u1 := &model.User{ Email: model.NewId(), Username: model.NewId(), AuthService: model.USER_AUTH_SERVICE_GITLAB, - AuthData: model.NewId(), + AuthData: &auth, } Must(store.User().Save(u1)) + auth2 := model.NewId() + u2 := &model.User{ Email: model.NewId(), Username: model.NewId(), AuthService: model.USER_AUTH_SERVICE_LDAP, - AuthData: model.NewId(), + AuthData: &auth2, } Must(store.User().Save(u2)) @@ -525,14 +532,14 @@ func TestUserStoreGetForLogin(t *testing.T) { t.Fatal("Should have gotten user1 by email") } - if result := <-store.User().GetForLogin(u2.AuthData, true, true, true); result.Err != nil { + if result := <-store.User().GetForLogin(*u2.AuthData, true, true, true); result.Err != nil { t.Fatal("Should have gotten user by LDAP AuthData", result.Err) } else if result.Data.(*model.User).Id != u2.Id { t.Fatal("Should have gotten user2 by LDAP AuthData") } // prevent getting user by AuthData when they're not an LDAP user - if result := <-store.User().GetForLogin(u1.AuthData, true, true, true); result.Err == nil { + if result := <-store.User().GetForLogin(*u1.AuthData, true, true, true); result.Err == nil { t.Fatal("Should not have gotten user by non-LDAP AuthData") } @@ -545,23 +552,26 @@ func TestUserStoreGetForLogin(t *testing.T) { t.Fatal("Should have failed to get user1 by email") } - if result := <-store.User().GetForLogin(u2.AuthData, true, true, false); result.Err == nil { + if result := <-store.User().GetForLogin(*u2.AuthData, true, true, false); result.Err == nil { t.Fatal("Should have failed to get user3 by 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: model.NewId(), + AuthData: &auth3, } Must(store.User().Save(u3)) + u4 := &model.User{ Email: model.NewId(), Username: model.NewId(), AuthService: model.USER_AUTH_SERVICE_LDAP, - AuthData: u3.Username, + AuthData: &u3.Username, } Must(store.User().Save(u4)) @@ -620,9 +630,9 @@ func TestUserStoreUpdateAuthData(t *testing.T) { Must(store.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: u1.Id})) service := "someservice" - authData := "1" + authData := model.NewId() - if err := (<-store.User().UpdateAuthData(u1.Id, service, authData, "")).Err; err != nil { + if err := (<-store.User().UpdateAuthData(u1.Id, service, &authData, "")).Err; err != nil { t.Fatal(err) } @@ -633,7 +643,7 @@ func TestUserStoreUpdateAuthData(t *testing.T) { if user.AuthService != service { t.Fatal("AuthService was not updated correctly") } - if user.AuthData != authData { + if *user.AuthData != authData { t.Fatal("AuthData was not updated correctly") } if user.Password != "" { diff --git a/store/store.go b/store/store.go index 7801f78f99..37aafdd4a6 100644 --- a/store/store.go +++ b/store/store.go @@ -126,7 +126,7 @@ type UserStore interface { UpdateLastActivityAt(userId string, time int64) StoreChannel UpdateUserAndSessionActivity(userId string, sessionId string, time int64) StoreChannel UpdatePassword(userId, newPassword string) StoreChannel - UpdateAuthData(userId, service, authData, email string) StoreChannel + UpdateAuthData(userId string, service string, authData *string, email string) StoreChannel UpdateMfaSecret(userId, secret string) StoreChannel UpdateMfaActive(userId string, active bool) StoreChannel Get(id string) StoreChannel @@ -136,7 +136,7 @@ type UserStore interface { GetDirectProfiles(userId string) StoreChannel GetProfileByIds(userId []string) StoreChannel GetByEmail(email string) StoreChannel - GetByAuth(authData string, authService string) StoreChannel + GetByAuth(authData *string, authService string) StoreChannel GetByUsername(username string) StoreChannel GetForLogin(loginId string, allowSignInWithUsername, allowSignInWithEmail, ldapEnabled bool) StoreChannel VerifyEmail(userId string) StoreChannel From 7babccfa511995708ac3eab1efeb5f2bfcf7455e Mon Sep 17 00:00:00 2001 From: it33 Date: Wed, 11 May 2016 11:08:38 -0700 Subject: [PATCH 02/10] Propose changing "Other teams" to "Outside this team" (#2969) * Update sidebar.jsx * Added "Outside this team" --- webapp/components/sidebar.jsx | 2 +- webapp/i18n/en.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/webapp/components/sidebar.jsx b/webapp/components/sidebar.jsx index 3bfe473937..a03e3e215c 100644 --- a/webapp/components/sidebar.jsx +++ b/webapp/components/sidebar.jsx @@ -514,7 +514,7 @@ export default class Sidebar extends React.Component {
); diff --git a/webapp/i18n/en.json b/webapp/i18n/en.json index b622c9ab10..cfa4d9e25b 100644 --- a/webapp/i18n/en.json +++ b/webapp/i18n/en.json @@ -1130,7 +1130,7 @@ "sidebar.direct": "Direct Messages", "sidebar.more": "More", "sidebar.moreElips": "More...", - "sidebar.otherMembers": "Other teams", + "sidebar.otherMembers": "Outside this team", "sidebar.pg": "Private Groups", "sidebar.removeList": "Remove from list", "sidebar.tutorialScreen1": "

Channels

Channels organize conversations across different topics. They’re open to everyone on your team. To send private communications use Direct Messages for a single person or Private Groups for multiple people.

", From 04f7273461a130e93f19af935df673016ab1dbc7 Mon Sep 17 00:00:00 2001 From: Asaad Mahmood Date: Thu, 12 May 2016 00:30:00 +0500 Subject: [PATCH 03/10] Improving compliance stuff (#2963) * Improving compliance stuff * Fixing padding on mobile --- webapp/components/admin_console/audits.jsx | 27 +-- .../admin_console/compliance_reports.jsx | 199 +++++++++--------- webapp/i18n/en.json | 18 +- webapp/sass/responsive/_mobile.scss | 9 + webapp/sass/routes/_admin-console.scss | 11 - webapp/sass/routes/_compliance.scss | 38 ++++ webapp/sass/routes/_module.scss | 1 + 7 files changed, 171 insertions(+), 132 deletions(-) create mode 100644 webapp/sass/routes/_compliance.scss diff --git a/webapp/components/admin_console/audits.jsx b/webapp/components/admin_console/audits.jsx index 1f94de7da3..cb500f29c0 100644 --- a/webapp/components/admin_console/audits.jsx +++ b/webapp/components/admin_console/audits.jsx @@ -75,24 +75,25 @@ export default class Audits extends React.Component {
-
+

+

- -
+
{content}
diff --git a/webapp/components/admin_console/compliance_reports.jsx b/webapp/components/admin_console/compliance_reports.jsx index 04b2c4deb0..79b0d2210c 100644 --- a/webapp/components/admin_console/compliance_reports.jsx +++ b/webapp/components/admin_console/compliance_reports.jsx @@ -266,7 +266,7 @@ export default class ComplianceReports extends React.Component { } return ( -
+

- - - - - - - - - - - - - -
- - -
- - - - - - - - - - - - - -
- {serverError} -
+
+
+ + +
+
+ + +
+
+ + +
+
+
+
+ + +
+
+ + +
+
+
-
+ {serverError} +
+ +
+
{content}
diff --git a/webapp/i18n/en.json b/webapp/i18n/en.json index cfa4d9e25b..7e46903db4 100644 --- a/webapp/i18n/en.json +++ b/webapp/i18n/en.json @@ -75,8 +75,8 @@ "add_outgoing_webhook.triggerWOrds": "Trigger Words (One Per Line)", "add_outgoing_webhook.triggerWords": "Trigger Words (One Per Line)", "add_outgoing_webhook.triggerWordsOrChannelRequired": "A valid channel or a list of trigger words is required", - "admin.audits.reload": "Reload", - "admin.audits.title": "User Activity", + "admin.audits.reload": "Reload User Activity Logs", + "admin.audits.title": "User Activity Logs", "admin.compliance.directoryDescription": "Directory to which compliance reports are written. If blank, will be set to ./data/.", "admin.compliance.directoryExample": "Ex \"./data/\"", "admin.compliance.directoryTitle": "Compliance Directory Location:", @@ -91,18 +91,18 @@ "admin.compliance.title": "Compliance Settings", "admin.compliance.true": "true", "admin.compliance_reports.desc": "Job Name:", - "admin.compliance_reports.desc_placeholder": "Ex \"Audit 445 for HR\"", + "admin.compliance_reports.desc_placeholder": "E.g. \"Audit 445 for HR\"", "admin.compliance_reports.emails": "Emails:", - "admin.compliance_reports.emails_placeholder": "Ex \"bill@example.com, bob@example.com\"", + "admin.compliance_reports.emails_placeholder": "E.g. \"bill@example.com, bob@example.com\"", "admin.compliance_reports.from": "From:", - "admin.compliance_reports.from_placeholder": "Ex \"2016-03-11\"", + "admin.compliance_reports.from_placeholder": "E.g. \"2016-03-11\"", "admin.compliance_reports.keywords": "Keywords:", - "admin.compliance_reports.keywords_placeholder": "Ex \"shorting stock\"", - "admin.compliance_reports.reload": "Reload", - "admin.compliance_reports.run": "Run", + "admin.compliance_reports.keywords_placeholder": "E.g. \"shorting stock\"", + "admin.compliance_reports.reload": "Reload Completed Compliance Reports", + "admin.compliance_reports.run": "Run Compliance Report", "admin.compliance_reports.title": "Compliance Reports", "admin.compliance_reports.to": "To:", - "admin.compliance_reports.to_placeholder": "Ex \"2016-03-15\"", + "admin.compliance_reports.to_placeholder": "E.g. \"2016-03-15\"", "admin.compliance_table.desc": "Description", "admin.compliance_table.download": "Download", "admin.compliance_table.params": "Params", diff --git a/webapp/sass/responsive/_mobile.scss b/webapp/sass/responsive/_mobile.scss index 3a4cd3b893..cc3d7a4b9f 100644 --- a/webapp/sass/responsive/_mobile.scss +++ b/webapp/sass/responsive/_mobile.scss @@ -11,6 +11,15 @@ } } + .compliance-panel, + .audit-panel { + .row { + > .form-group { + padding-left: 15px; + } + } + } + .user-popover { pointer-events: none; } diff --git a/webapp/sass/routes/_admin-console.scss b/webapp/sass/routes/_admin-console.scss index 65fefdb331..0f47e7529e 100644 --- a/webapp/sass/routes/_admin-console.scss +++ b/webapp/sass/routes/_admin-console.scss @@ -160,17 +160,6 @@ width: 100%; } - .compliance__panel, - .audit__panel { - background-color: $white; - border: 1px solid $border-gray; - height: 70vh; - margin-top: 10px; - overflow: auto; - padding: 5px; - width: 100%; - } - .app__content { color: #333; diff --git a/webapp/sass/routes/_compliance.scss b/webapp/sass/routes/_compliance.scss new file mode 100644 index 0000000000..57eb538c67 --- /dev/null +++ b/webapp/sass/routes/_compliance.scss @@ -0,0 +1,38 @@ +@charset 'UTF-8'; + +.compliance-panel__table, +.audit-panel__table { + background-color: $white; + border: 1px solid $border-gray; + margin-top: 10px; + max-height: 70vh; + min-height: 100px; + overflow: auto; + padding: 5px; + width: 100%; +} + +.compliance-panel, +.audit-panel { + .row { + > .form-group { + padding-left: 0; + + &:first-child { + padding-left: 15px; + } + } + + label { + font-weight: 600; + } + } + + .fa-refresh { + margin-right: 5px; + } +} + +.compliance-panel { + margin-bottom: 3em; +} diff --git a/webapp/sass/routes/_module.scss b/webapp/sass/routes/_module.scss index 4f3f6f9cd4..11b815007f 100644 --- a/webapp/sass/routes/_module.scss +++ b/webapp/sass/routes/_module.scss @@ -4,6 +4,7 @@ @import 'activity-log'; @import 'admin-console'; @import 'backstage'; +@import 'compliance'; @import 'docs'; @import 'error-page'; @import 'loading'; From 0fa51b764d87372f7ba0112282558714e60a7664 Mon Sep 17 00:00:00 2001 From: enahum Date: Thu, 12 May 2016 09:31:08 -0300 Subject: [PATCH 04/10] Add missing locs (#2971) --- webapp/components/sidebar.jsx | 2 +- webapp/i18n/es.json | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/webapp/components/sidebar.jsx b/webapp/components/sidebar.jsx index a03e3e215c..a4d85f4ff1 100644 --- a/webapp/components/sidebar.jsx +++ b/webapp/components/sidebar.jsx @@ -514,7 +514,7 @@ export default class Sidebar extends React.Component {
); diff --git a/webapp/i18n/es.json b/webapp/i18n/es.json index ce29d234bd..7aa3071f6b 100644 --- a/webapp/i18n/es.json +++ b/webapp/i18n/es.json @@ -1130,7 +1130,7 @@ "sidebar.direct": "Mensajes Directos", "sidebar.more": "Más", "sidebar.moreElips": "Más...", - "sidebar.otherMembers": "Otros equipos", + "sidebar.otherMembers": "Fuera de este equipo", "sidebar.pg": "Grupos Privados", "sidebar.removeList": "Remover de la lista", "sidebar.tutorialScreen1": "

Canales

Canales organizan las conversaciones en diferentes tópicos. Son abiertos para cualquier persona de tu equipo. Para enviar comunicaciones privadas con una sola persona utiliza Mensajes Directos o con multiples personas utilizando Grupos Privados.

", @@ -1341,7 +1341,9 @@ "user.settings.general.title": "Configuración General", "user.settings.general.uploadImage": "Pinchar 'Editar' para subir una imagen.", "user.settings.general.username": "Nombre de usuario", + "user.settings.general.usernameInfo": "Escoge algo que sea fácil de reconocer y recordar para tus compañeros.", "user.settings.general.usernameReserved": "Este nombre de usuario está reservado, por favor escoge otro", + "user.settings.general.usernameRestrictions": "El nombre de usuario debe comenzar con una letra y debe contener entre {min} y {max} caracteres en minúscula creado con numeros, letras y los símbolos '.', '-', y '_'.", "user.settings.general.validEmail": "Por favor ingresa una dirección de correo electrónico válida", "user.settings.general.validImage": "Sólo pueden ser utilizadas imágenes JPG o PNG en el perfil", "user.settings.import_theme.cancel": "Cancelar", From 82e6cf785c13443f3496941c3ef884506420c735 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Thu, 12 May 2016 09:49:40 -0400 Subject: [PATCH 05/10] Fix incorrect password error with MFA enabled (#2977) --- webapp/components/login/login.jsx | 1 + 1 file changed, 1 insertion(+) diff --git a/webapp/components/login/login.jsx b/webapp/components/login/login.jsx index 0a12ed6453..60d25a2caa 100644 --- a/webapp/components/login/login.jsx +++ b/webapp/components/login/login.jsx @@ -98,6 +98,7 @@ export default class Login extends React.Component { }); } else if (err.id === 'api.user.check_user_password.invalid.app_error' || err.id === 'ent.ldap.do_login.invalid_password.app_error') { this.setState({ + showMfa: false, serverError: ( Date: Thu, 12 May 2016 11:30:53 -0400 Subject: [PATCH 06/10] PLT-2927/PLT-2924 Fixing issues with integration lists (#2974) * Changed IntegrationStore to store integrations by team * Fixed regenerating a command's token not causing the UI to update * Re-added IntegrationStore.hasReceived methods --- webapp/client/client.jsx | 2 +- .../backstage/installed_commands.jsx | 15 +- .../backstage/installed_incoming_webhooks.jsx | 15 +- .../backstage/installed_outgoing_webhooks.jsx | 15 +- webapp/stores/integration_store.jsx | 142 ++++++++++-------- webapp/utils/async_client.jsx | 6 + 6 files changed, 116 insertions(+), 79 deletions(-) diff --git a/webapp/client/client.jsx b/webapp/client/client.jsx index 12cf21f5c7..3ee7b1de99 100644 --- a/webapp/client/client.jsx +++ b/webapp/client/client.jsx @@ -1203,7 +1203,7 @@ export default class Client { end(this.handleResponse.bind(this, 'listTeamCommands', success, error)); } - regenCommandToken = (commandId, suggest, success, error) => { + regenCommandToken = (commandId, success, error) => { request. post(`${this.getCommandsRoute()}/regen_token`). set(this.defaultHeaders). diff --git a/webapp/components/backstage/installed_commands.jsx b/webapp/components/backstage/installed_commands.jsx index 71373e077b..df1f566879 100644 --- a/webapp/components/backstage/installed_commands.jsx +++ b/webapp/components/backstage/installed_commands.jsx @@ -5,6 +5,7 @@ import React from 'react'; import * as AsyncClient from 'utils/async_client.jsx'; import IntegrationStore from 'stores/integration_store.jsx'; +import TeamStore from 'stores/team_store.jsx'; import * as Utils from 'utils/utils.jsx'; import {FormattedMessage} from 'react-intl'; @@ -20,16 +21,18 @@ export default class InstalledCommands extends React.Component { this.regenCommandToken = this.regenCommandToken.bind(this); this.deleteCommand = this.deleteCommand.bind(this); + const teamId = TeamStore.getCurrentId(); + this.state = { - commands: IntegrationStore.getCommands(), - loading: !IntegrationStore.hasReceivedCommands() + commands: IntegrationStore.getCommands(teamId), + loading: !IntegrationStore.hasReceivedCommands(teamId) }; } componentDidMount() { IntegrationStore.addChangeListener(this.handleIntegrationChange); - if (window.mm_config.EnableCommands === 'true' && this.state.loading) { + if (window.mm_config.EnableCommands === 'true') { AsyncClient.listTeamCommands(); } } @@ -39,9 +42,11 @@ export default class InstalledCommands extends React.Component { } handleIntegrationChange() { + const teamId = TeamStore.getCurrentId(); + this.setState({ - commands: IntegrationStore.getCommands(), - loading: !IntegrationStore.hasReceivedCommands() + commands: IntegrationStore.getCommands(teamId), + loading: !IntegrationStore.hasReceivedCommands(teamId) }); } diff --git a/webapp/components/backstage/installed_incoming_webhooks.jsx b/webapp/components/backstage/installed_incoming_webhooks.jsx index 389f65919e..0a38a6ab56 100644 --- a/webapp/components/backstage/installed_incoming_webhooks.jsx +++ b/webapp/components/backstage/installed_incoming_webhooks.jsx @@ -5,6 +5,7 @@ import React from 'react'; import * as AsyncClient from 'utils/async_client.jsx'; import IntegrationStore from 'stores/integration_store.jsx'; +import TeamStore from 'stores/team_store.jsx'; import * as Utils from 'utils/utils.jsx'; import {FormattedMessage} from 'react-intl'; @@ -19,16 +20,18 @@ export default class InstalledIncomingWebhooks extends React.Component { this.deleteIncomingWebhook = this.deleteIncomingWebhook.bind(this); + const teamId = TeamStore.getCurrentId(); + this.state = { - incomingWebhooks: IntegrationStore.getIncomingWebhooks(), - loading: !IntegrationStore.hasReceivedIncomingWebhooks() + incomingWebhooks: IntegrationStore.getIncomingWebhooks(teamId), + loading: !IntegrationStore.hasReceivedIncomingWebhooks(teamId) }; } componentDidMount() { IntegrationStore.addChangeListener(this.handleIntegrationChange); - if (window.mm_config.EnableIncomingWebhooks === 'true' && this.state.loading) { + if (window.mm_config.EnableIncomingWebhooks === 'true') { AsyncClient.listIncomingHooks(); } } @@ -38,9 +41,11 @@ export default class InstalledIncomingWebhooks extends React.Component { } handleIntegrationChange() { + const teamId = TeamStore.getCurrentId(); + this.setState({ - incomingWebhooks: IntegrationStore.getIncomingWebhooks(), - loading: !IntegrationStore.hasReceivedIncomingWebhooks() + incomingWebhooks: IntegrationStore.getIncomingWebhooks(teamId), + loading: !IntegrationStore.hasReceivedIncomingWebhooks(teamId) }); } diff --git a/webapp/components/backstage/installed_outgoing_webhooks.jsx b/webapp/components/backstage/installed_outgoing_webhooks.jsx index e0817fda8d..b79bc35304 100644 --- a/webapp/components/backstage/installed_outgoing_webhooks.jsx +++ b/webapp/components/backstage/installed_outgoing_webhooks.jsx @@ -5,6 +5,7 @@ import React from 'react'; import * as AsyncClient from 'utils/async_client.jsx'; import IntegrationStore from 'stores/integration_store.jsx'; +import TeamStore from 'stores/team_store.jsx'; import * as Utils from 'utils/utils.jsx'; import {FormattedMessage} from 'react-intl'; @@ -20,16 +21,18 @@ export default class InstalledOutgoingWebhooks extends React.Component { this.regenOutgoingWebhookToken = this.regenOutgoingWebhookToken.bind(this); this.deleteOutgoingWebhook = this.deleteOutgoingWebhook.bind(this); + const teamId = TeamStore.getCurrentId(); + this.state = { - outgoingWebhooks: IntegrationStore.getOutgoingWebhooks(), - loading: !IntegrationStore.hasReceivedOutgoingWebhooks() + outgoingWebhooks: IntegrationStore.getOutgoingWebhooks(teamId), + loading: !IntegrationStore.hasReceivedOutgoingWebhooks(teamId) }; } componentDidMount() { IntegrationStore.addChangeListener(this.handleIntegrationChange); - if (window.mm_config.EnableOutgoingWebhooks === 'true' && this.state.loading) { + if (window.mm_config.EnableOutgoingWebhooks === 'true') { AsyncClient.listOutgoingHooks(); } } @@ -39,9 +42,11 @@ export default class InstalledOutgoingWebhooks extends React.Component { } handleIntegrationChange() { + const teamId = TeamStore.getCurrentId(); + this.setState({ - outgoingWebhooks: IntegrationStore.getOutgoingWebhooks(), - loading: !IntegrationStore.hasReceivedOutgoingWebhooks() + outgoingWebhooks: IntegrationStore.getOutgoingWebhooks(teamId), + loading: !IntegrationStore.hasReceivedOutgoingWebhooks(teamId) }); } diff --git a/webapp/stores/integration_store.jsx b/webapp/stores/integration_store.jsx index 12cbc3407e..454e6290bd 100644 --- a/webapp/stores/integration_store.jsx +++ b/webapp/stores/integration_store.jsx @@ -15,14 +15,11 @@ class IntegrationStore extends EventEmitter { this.dispatchToken = AppDispatcher.register(this.handleEventPayload.bind(this)); - this.incomingWebhooks = []; - this.receivedIncomingWebhooks = false; + this.incomingWebhooks = new Map(); - this.outgoingWebhooks = []; - this.receivedOutgoingWebhooks = false; + this.outgoingWebhooks = new Map(); - this.commands = []; - this.receivedCommands = false; + this.commands = new Map(); } addChangeListener(callback) { @@ -37,100 +34,119 @@ class IntegrationStore extends EventEmitter { this.emit(CHANGE_EVENT); } - hasReceivedIncomingWebhooks() { - return this.receivedIncomingWebhooks; + hasReceivedIncomingWebhooks(teamId) { + return this.incomingWebhooks.has(teamId); } - getIncomingWebhooks() { - return this.incomingWebhooks; + getIncomingWebhooks(teamId) { + return this.incomingWebhooks.get(teamId) || []; } - setIncomingWebhooks(incomingWebhooks) { - this.incomingWebhooks = incomingWebhooks; - this.receivedIncomingWebhooks = true; + setIncomingWebhooks(teamId, incomingWebhooks) { + this.incomingWebhooks.set(teamId, incomingWebhooks); } addIncomingWebhook(incomingWebhook) { - this.incomingWebhooks.push(incomingWebhook); + const teamId = incomingWebhook.team_id; + const incomingWebhooks = this.getIncomingWebhooks(teamId); + + incomingWebhooks.push(incomingWebhook); + + this.setIncomingWebhooks(teamId, incomingWebhooks); } - removeIncomingWebhook(id) { - for (let i = 0; i < this.incomingWebhooks.length; i++) { - if (this.incomingWebhooks[i].id === id) { - this.incomingWebhooks.splice(i, 1); - break; - } - } + removeIncomingWebhook(teamId, id) { + let incomingWebhooks = this.getIncomingWebhooks(teamId); + + incomingWebhooks = incomingWebhooks.filter((incomingWebhook) => incomingWebhook.id !== id); + + this.setIncomingWebhooks(teamId, incomingWebhooks); } - hasReceivedOutgoingWebhooks() { - return this.receivedOutgoingWebhooks; + hasReceivedOutgoingWebhooks(teamId) { + return this.outgoingWebhooks.has(teamId); } - getOutgoingWebhooks() { - return this.outgoingWebhooks; + getOutgoingWebhooks(teamId) { + return this.outgoingWebhooks.get(teamId) || []; } - setOutgoingWebhooks(outgoingWebhooks) { - this.outgoingWebhooks = outgoingWebhooks; - this.receivedOutgoingWebhooks = true; + setOutgoingWebhooks(teamId, outgoingWebhooks) { + this.outgoingWebhooks.set(teamId, outgoingWebhooks); } addOutgoingWebhook(outgoingWebhook) { - this.outgoingWebhooks.push(outgoingWebhook); + const teamId = outgoingWebhook.team_id; + const outgoingWebhooks = this.getOutgoingWebhooks(teamId); + + outgoingWebhooks.push(outgoingWebhook); + + this.setOutgoingWebhooks(teamId, outgoingWebhooks); } updateOutgoingWebhook(outgoingWebhook) { - for (let i = 0; i < this.outgoingWebhooks.length; i++) { - if (this.outgoingWebhooks[i].id === outgoingWebhook.id) { - this.outgoingWebhooks[i] = outgoingWebhook; + const teamId = outgoingWebhook.team_id; + const outgoingWebhooks = this.getOutgoingWebhooks(teamId); + + for (let i = 0; i < outgoingWebhooks.length; i++) { + if (outgoingWebhooks[i].id === outgoingWebhook.id) { + outgoingWebhooks[i] = outgoingWebhook; break; } } + + this.setOutgoingWebhooks(teamId, outgoingWebhooks); } - removeOutgoingWebhook(id) { - for (let i = 0; i < this.outgoingWebhooks.length; i++) { - if (this.outgoingWebhooks[i].id === id) { - this.outgoingWebhooks.splice(i, 1); - break; - } - } + removeOutgoingWebhook(teamId, id) { + let outgoingWebhooks = this.getOutgoingWebhooks(teamId); + + outgoingWebhooks = outgoingWebhooks.filter((outgoingWebhook) => outgoingWebhook.id !== id); + + this.setOutgoingWebhooks(teamId, outgoingWebhooks); } - hasReceivedCommands() { - return this.receivedCommands; + hasReceivedCommands(teamId) { + return this.commands.has(teamId); } - getCommands() { - return this.commands; + getCommands(teamId) { + return this.commands.get(teamId) || []; } - setCommands(commands) { - this.commands = commands; - this.receivedCommands = true; + setCommands(teamId, commands) { + this.commands.set(teamId, commands); } addCommand(command) { - this.commands.push(command); + const teamId = command.team_id; + const commands = this.getCommands(teamId); + + commands.push(command); + + this.setCommands(teamId, commands); } updateCommand(command) { - for (let i = 0; i < this.commands.length; i++) { - if (this.commands[i].id === command.id) { - this.commands[i] = command; + const teamId = command.team_id; + const commands = this.getCommands(teamId); + + for (let i = 0; i < commands.length; i++) { + if (commands[i].id === command.id) { + commands[i] = command; break; } } + + this.setCommands(teamId, commands); } - removeCommand(id) { - for (let i = 0; i < this.commands.length; i++) { - if (this.commands[i].id === id) { - this.commands.splice(i, 1); - break; - } - } + removeCommand(teamId, id) { + let commands = this.getCommands(teamId); + + commands = commands.filter((command) => command.id !== id); + + this.setCommands(teamId, commands); } handleEventPayload(payload) { @@ -138,7 +154,7 @@ class IntegrationStore extends EventEmitter { switch (action.type) { case ActionTypes.RECEIVED_INCOMING_WEBHOOKS: - this.setIncomingWebhooks(action.incomingWebhooks); + this.setIncomingWebhooks(action.teamId, action.incomingWebhooks); this.emitChange(); break; case ActionTypes.RECEIVED_INCOMING_WEBHOOK: @@ -146,11 +162,11 @@ class IntegrationStore extends EventEmitter { this.emitChange(); break; case ActionTypes.REMOVED_INCOMING_WEBHOOK: - this.removeIncomingWebhook(action.id); + this.removeIncomingWebhook(action.teamId, action.id); this.emitChange(); break; case ActionTypes.RECEIVED_OUTGOING_WEBHOOKS: - this.setOutgoingWebhooks(action.outgoingWebhooks); + this.setOutgoingWebhooks(action.teamId, action.outgoingWebhooks); this.emitChange(); break; case ActionTypes.RECEIVED_OUTGOING_WEBHOOK: @@ -162,11 +178,11 @@ class IntegrationStore extends EventEmitter { this.emitChange(); break; case ActionTypes.REMOVED_OUTGOING_WEBHOOK: - this.removeOutgoingWebhook(action.id); + this.removeOutgoingWebhook(action.teamId, action.id); this.emitChange(); break; case ActionTypes.RECEIVED_COMMANDS: - this.setCommands(action.commands); + this.setCommands(action.teamId, action.commands); this.emitChange(); break; case ActionTypes.RECEIVED_COMMAND: @@ -178,7 +194,7 @@ class IntegrationStore extends EventEmitter { this.emitChange(); break; case ActionTypes.REMOVED_COMMAND: - this.removeCommand(action.id); + this.removeCommand(action.teamId, action.id); this.emitChange(); break; } diff --git a/webapp/utils/async_client.jsx b/webapp/utils/async_client.jsx index a562964b14..6535c024d1 100644 --- a/webapp/utils/async_client.jsx +++ b/webapp/utils/async_client.jsx @@ -1145,6 +1145,7 @@ export function listIncomingHooks() { AppDispatcher.handleServerAction({ type: ActionTypes.RECEIVED_INCOMING_WEBHOOKS, + teamId: Client.teamId, incomingWebhooks: data }); }, @@ -1168,6 +1169,7 @@ export function listOutgoingHooks() { AppDispatcher.handleServerAction({ type: ActionTypes.RECEIVED_OUTGOING_WEBHOOKS, + teamId: Client.teamId, outgoingWebhooks: data }); }, @@ -1230,6 +1232,7 @@ export function deleteIncomingHook(id) { () => { AppDispatcher.handleServerAction({ type: ActionTypes.REMOVED_INCOMING_WEBHOOK, + teamId: Client.teamId, id }); }, @@ -1245,6 +1248,7 @@ export function deleteOutgoingHook(id) { () => { AppDispatcher.handleServerAction({ type: ActionTypes.REMOVED_OUTGOING_WEBHOOK, + teamId: Client.teamId, id }); }, @@ -1282,6 +1286,7 @@ export function listTeamCommands() { AppDispatcher.handleServerAction({ type: ActionTypes.RECEIVED_COMMANDS, + teamId: Client.teamId, commands: data }); }, @@ -1321,6 +1326,7 @@ export function deleteCommand(id) { () => { AppDispatcher.handleServerAction({ type: ActionTypes.REMOVED_COMMAND, + teamId: Client.teamId, id }); }, From 97450762dbb8323756d0f52cc7b59b86d0319b97 Mon Sep 17 00:00:00 2001 From: thoemy Date: Thu, 12 May 2016 14:30:44 +0200 Subject: [PATCH 07/10] Fix parsing attachment field links into markdown (#2958) (#2959) Field contents are stored in 'value' not 'text'. --- api/post.go | 8 ++++---- tests/test-slack-attachments.json | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/api/post.go b/api/post.go index ac499e615e..734cb71481 100644 --- a/api/post.go +++ b/api/post.go @@ -200,10 +200,10 @@ func CreateWebhookPost(c *Context, channelId, text, overrideUsername, overrideIc // parse attachment field links into Markdown format for j, fInt := range fields { field := fInt.(map[string]interface{}) - if _, ok := field["text"]; ok { - fText := field["text"].(string) - fText = linkWithTextRegex.ReplaceAllString(fText, "[${2}](${1})") - field["text"] = fText + if _, ok := field["value"]; ok { + fValue := field["value"].(string) + fValue = linkWithTextRegex.ReplaceAllString(fValue, "[${2}](${1})") + field["value"] = fValue fields[j] = field } } diff --git a/tests/test-slack-attachments.json b/tests/test-slack-attachments.json index 1c499b4ca8..3787eb1437 100644 --- a/tests/test-slack-attachments.json +++ b/tests/test-slack-attachments.json @@ -39,6 +39,11 @@ "short": true, "title": "Short 2", "value": "Another one" + }, + { + "short": true, + "title": "Field with link", + "value": "" } ], "mrkdwn_in": [ From 04dfa2a9eb50d60b2d4c2533f465799966b363a6 Mon Sep 17 00:00:00 2001 From: thoemy Date: Thu, 12 May 2016 13:44:44 +0200 Subject: [PATCH 08/10] Improve incoming webhook slack compatibility (#2972) (#2973) By checking for form urlencoded content instead of JSON, requests without or with a wrong Content-Type header and a JSON body are correctly parsed. --- api/webhook.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/webhook.go b/api/webhook.go index ea628e39c6..a4367026ff 100644 --- a/api/webhook.go +++ b/api/webhook.go @@ -358,10 +358,10 @@ func incomingWebhook(c *Context, w http.ResponseWriter, r *http.Request) { var parsedRequest *model.IncomingWebhookRequest contentType := r.Header.Get("Content-Type") - if strings.Split(contentType, "; ")[0] == "application/json" { - parsedRequest = model.IncomingWebhookRequestFromJson(r.Body) - } else { + if strings.Split(contentType, "; ")[0] == "application/x-www-form-urlencoded" { parsedRequest = model.IncomingWebhookRequestFromJson(strings.NewReader(r.FormValue("payload"))) + } else { + parsedRequest = model.IncomingWebhookRequestFromJson(r.Body) } if parsedRequest == nil { From 6631f28d92d68e4e39848038f7f263f8588aa2ac Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Thu, 12 May 2016 12:06:26 -0400 Subject: [PATCH 09/10] Improved handling of edge case where an LDAP user shares a username with a non-LDAP user (#2980) --- api/user.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/api/user.go b/api/user.go index 4b9c3a3c84..9e93ae7794 100644 --- a/api/user.go +++ b/api/user.go @@ -494,8 +494,11 @@ func getUserForLogin(loginId string, onlyLdap bool) (*model.User, *model.AppErro *utils.Cfg.EmailSettings.EnableSignInWithUsername && !onlyLdap, *utils.Cfg.EmailSettings.EnableSignInWithEmail && !onlyLdap, ldapAvailable, - ); result.Err != nil { - + ); 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 From 9a701b7e5b31ca803a2d92f0b3b1d7cc68bf3c37 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Thu, 12 May 2016 12:06:36 -0400 Subject: [PATCH 10/10] Stopped supressing LDAP user filtered error on clientside (#2981) --- webapp/components/login/login.jsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/webapp/components/login/login.jsx b/webapp/components/login/login.jsx index 60d25a2caa..8ae3fdd4d4 100644 --- a/webapp/components/login/login.jsx +++ b/webapp/components/login/login.jsx @@ -85,8 +85,7 @@ export default class Login extends React.Component { browserHistory.push('/should_verify_email?&email=' + encodeURIComponent(loginId)); return; } else if (err.id === 'store.sql_user.get_for_login.app_error' || - err.id === 'ent.ldap.do_login.user_not_registered.app_error' || - err.id === 'ent.ldap.do_login.user_filtered.app_error') { + err.id === 'ent.ldap.do_login.user_not_registered.app_error') { this.setState({ showMfa: false, serverError: (