From 4cdf2391390c0ebdcc82d9087743a46d47da27d2 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 20 Nov 2020 18:07:31 +0530 Subject: [PATCH] MM-29107: Fix race in LDAP login (#16239) * MM-29107: Fix race in LDAP login We remove the goroutine to make things synchronous. This removes the race and makes things more reliable. It is already tested by TestLogin in ldap_test.go in -race mode. https://mattermost.atlassian.net/browse/MM-29107 ```release-note NONE ``` * Address review comments Co-authored-by: Mattermod --- app/login.go | 4 +++- einterfaces/ldap.go | 2 +- einterfaces/mocks/LdapInterface.go | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/login.go b/app/login.go index 2cbfcaed70..2ee7a6289a 100644 --- a/app/login.go +++ b/app/login.go @@ -217,8 +217,10 @@ func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, a.SetSession(session) if a.Srv().License() != nil && *a.Srv().License().Features.LDAP && a.Ldap() != nil { + userVal := *user + sessionVal := *session a.Srv().Go(func() { - a.Ldap().UpdateProfilePictureIfNecessary(*user, session) + a.Ldap().UpdateProfilePictureIfNecessary(userVal, sessionVal) }) } diff --git a/einterfaces/ldap.go b/einterfaces/ldap.go index 3784251a08..fb91792eec 100644 --- a/einterfaces/ldap.go +++ b/einterfaces/ldap.go @@ -21,6 +21,6 @@ type LdapInterface interface { GetGroup(groupUID string) (*model.Group, *model.AppError) GetAllGroupsPage(page int, perPage int, opts model.LdapGroupSearchOpts) ([]*model.Group, int, *model.AppError) FirstLoginSync(user *model.User, userAuthService, userAuthData, email string) *model.AppError - UpdateProfilePictureIfNecessary(model.User, *model.Session) + UpdateProfilePictureIfNecessary(model.User, model.Session) GetADLdapIdFromSAMLId(authData string) string } diff --git a/einterfaces/mocks/LdapInterface.go b/einterfaces/mocks/LdapInterface.go index 4389ce3776..a2d2c3103c 100644 --- a/einterfaces/mocks/LdapInterface.go +++ b/einterfaces/mocks/LdapInterface.go @@ -305,6 +305,6 @@ func (_m *LdapInterface) SwitchToLdap(userId string, ldapId string, ldapPassword } // UpdateProfilePictureIfNecessary provides a mock function with given fields: _a0, _a1 -func (_m *LdapInterface) UpdateProfilePictureIfNecessary(_a0 model.User, _a1 *model.Session) { +func (_m *LdapInterface) UpdateProfilePictureIfNecessary(_a0 model.User, _a1 model.Session) { _m.Called(_a0, _a1) }