MM-60480: Do not invalidate channel member cache on login (#28143)

On user login, we were aggressively clearing the channel member
cache when it wasn't necessary. There is no channel membership
info that is changing on a user login.

Additionally, we fix some more issues with pointer passing
to the cache. It's a known problem that our value passing
style isn't consistent when we fetch items from cache.
Sometimes we pass pointer-to-pointer, sometimes it's just a
pointer. This is also the reason for https://github.com/mattermost/mattermost/pull/27830.

We fix 2 such cases where we passed pointer-to-pointer
but didn't handle the special case inside the cache.
This time, we actually fix it in the app layer instead
of the cache layer and also add a test for good measure.

https://mattermost.atlassian.net/browse/MM-60480

```release-note
NONE
```

Co-authored-by: Mattermost Build <build@mattermost.com>

server/channels/app/authentication.go

@@ -64,13 +64,13 @@ func (a *App) CheckPasswordAndAllCriteria(rctx request.CTX, user *model.User, pa
 		return err
 	}
 
+	defer a.Srv().Store().User().InvalidateProfileCacheForUser(user.Id)
+
 	if err := users.CheckUserPassword(user, password); err != nil {
 		if passErr := a.Srv().Store().User().UpdateFailedPasswordAttempts(user.Id, user.FailedAttempts+1); passErr != nil {
 			return model.NewAppError("CheckPasswordAndAllCriteria", "app.user.update_failed_pwd_attempts.app_error", nil, "", http.StatusInternalServerError).Wrap(passErr)
 		}
 
-		a.InvalidateCacheForUser(user.Id)
-
 		var invErr *users.ErrInvalidPassword
 		switch {
 		case errors.As(err, &invErr):
@@ -89,8 +89,6 @@ func (a *App) CheckPasswordAndAllCriteria(rctx request.CTX, user *model.User, pa
 			}
 		}
 
-		a.InvalidateCacheForUser(user.Id)
-
 		return err
 	}
 
@@ -98,8 +96,6 @@ func (a *App) CheckPasswordAndAllCriteria(rctx request.CTX, user *model.User, pa
 		return model.NewAppError("CheckPasswordAndAllCriteria", "app.user.update_failed_pwd_attempts.app_error", nil, "", http.StatusInternalServerError).Wrap(passErr)
 	}
 
-	a.InvalidateCacheForUser(user.Id)
-
 	if err := a.CheckUserPostflightAuthenticationCriteria(rctx, user); err != nil {
 		return err
 	}

server/channels/app/platform/session.go

@@ -65,7 +65,9 @@ func (ps *PlatformService) ClearUserSessionCacheLocal(userID string) {
 
 		toPass := make([]any, 0, len(keys))
 		for i := 0; i < len(keys); i++ {
-			var session *model.Session
+			// This always needs to be a pointer to a value.
+			// Otherwise the msp unmarshaler will fail to work.
+			var session model.Session
 			toPass = append(toPass, &session)
 		}
 
@@ -77,7 +79,7 @@ func (ps *PlatformService) ClearUserSessionCacheLocal(userID string) {
 				}
 				continue
 			}
-			gotSession := *(toPass[i].(**model.Session))
+			gotSession := toPass[i].(*model.Session)
 			if gotSession == nil {
 				ps.logger.Warn("Found nil session in ClearUserSessionCacheLocal. This is not expected")
 				continue

server/channels/store/localcachelayer/channel_layer.go

@@ -291,7 +291,7 @@ func (s LocalCacheChannelStore) GetAllChannelMembersForUser(ctx request.CTX, use
 		cache_key += "_deleted"
 	}
 	if allowFromCache {
-		ids := make(map[string]string)
+		var ids model.StringMap
 		if err := s.rootStore.doStandardReadCache(s.rootStore.channelMembersForUserCache, cache_key, &ids); err == nil {
 			return ids, nil
 		}

server/channels/store/localcachelayer/channel_layer_test.go

@@ -10,9 +10,12 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/mattermost/mattermost/server/public/plugin/plugintest/mock"
 	"github.com/mattermost/mattermost/server/public/shared/mlog"
+	"github.com/mattermost/mattermost/server/public/shared/request"
 	"github.com/mattermost/mattermost/server/v8/channels/store/storetest"
 	"github.com/mattermost/mattermost/server/v8/channels/store/storetest/mocks"
+	cmocks "github.com/mattermost/mattermost/server/v8/platform/services/cache/mocks"
 )
 
 func TestChannelStore(t *testing.T) {
@@ -417,3 +420,20 @@ func TestChannelStoreGetByNamesCache(t *testing.T) {
 		mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetByNames", 2)
 	})
 }
+
+func TestChannelStoreGetAllChannelMembersForUser(t *testing.T) {
+	logger := mlog.CreateConsoleTestLogger(t)
+
+	mockStore := getMockStore(t)
+	mockCacheProvider := getMockCacheProvider()
+	cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider, logger)
+	require.NoError(t, err)
+
+	cmock := cmocks.NewCache(t)
+	cmock.On("Get", "u1", mock.AnythingOfType("*model.StringMap")).Return(nil)
+
+	cachedStore.channel.rootStore.channelMembersForUserCache = cmock
+
+	_, err = cachedStore.Channel().GetAllChannelMembersForUser(request.TestContext(t), "u1", true, false)
+	require.NoError(t, err)
+}