Lack of Invalidation of Legacy Remote Cluster Invite Tokens After Confirmation

@@ -850,10 +850,13 @@ func (a *App) AuthorizeOAuthUser(c request.CTX, w http.ResponseWriter, r *http.R
 		return nil, stateProps, nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest).Wrap(cookieErr)
 	}
 
-	expectedTokenExtra := generateOAuthStateTokenExtra(stateEmail, stateAction, cookie.Value)
-	if expectedTokenExtra != expectedToken.Extra {
-		err := errors.New("Extra token value does not match token generated from state")
-		return nil, stateProps, nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest).Wrap(err)
+	tokenEmail, tokenAction, tokenCookie, parseErr := parseOAuthStateTokenExtra(expectedToken.Extra)
+	if parseErr != nil {
+		return nil, stateProps, nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest).Wrap(parseErr)
+	}
+
+	if tokenEmail != stateEmail || tokenAction != stateAction || tokenCookie != cookie.Value {
+		return nil, stateProps, nil, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.invalid_state.app_error", nil, "", http.StatusBadRequest).Wrap(errors.New("invalid state token"))
 	}
 
 	appErr = a.DeleteToken(expectedToken)
@@ -1037,3 +1040,18 @@ func (a *App) SwitchOAuthToEmail(c request.CTX, email, password, requesterId str
 func generateOAuthStateTokenExtra(email, action, cookie string) string {
 	return email + ":" + action + ":" + cookie
 }
+
+// parseOAuthStateTokenExtra parses a token extra string in the format "email:action:cookie".
+// Returns an error if the token does not contain exactly 3 colon-separated parts.
+func parseOAuthStateTokenExtra(tokenExtra string) (email, action, cookie string, err error) {
+	parts := strings.Split(tokenExtra, ":")
+	if len(parts) != 3 {
+		return "", "", "", fmt.Errorf("invalid token format: expected exactly 3 parts separated by ':', got %d", len(parts))
+	}
+
+	email = parts[0]
+	action = parts[1]
+	cookie = parts[2]
+
+	return email, action, cookie, nil
+}

@@ -534,6 +534,7 @@ func TestAuthorizeOAuthUser(t *testing.T) {
 				recorder := httptest.ResponseRecorder{}
 				body, receivedStateProps, _, err := th.App.AuthorizeOAuthUser(th.Context, &recorder, request, model.ServiceGitlab, "", state, "")
 
+				require.Nil(t, err)
 				require.NotNil(t, body)
 				bodyBytes, bodyErr := io.ReadAll(body)
 				require.NoError(t, bodyErr)
@@ -695,3 +696,190 @@ func TestDeactivatedUserOAuthApp(t *testing.T) {
 	require.Equal(t, http.StatusBadRequest, appErr.StatusCode)
 	assert.Equal(t, "api.oauth.get_access_token.expired_code.app_error", appErr.Id)
 }
+
+func TestParseOAuthStateTokenExtra(t *testing.T) {
+	t.Run("valid token with normal values", func(t *testing.T) {
+		email, action, cookie, err := parseOAuthStateTokenExtra("user@example.com:email_to_sso:randomcookie123")
+		require.NoError(t, err)
+		assert.Equal(t, "user@example.com", email)
+		assert.Equal(t, "email_to_sso", action)
+		assert.Equal(t, "randomcookie123", cookie)
+	})
+
+	t.Run("valid token with empty email and action", func(t *testing.T) {
+		email, action, cookie, err := parseOAuthStateTokenExtra("::randomcookie123")
+		require.NoError(t, err)
+		assert.Equal(t, "", email)
+		assert.Equal(t, "", action)
+		assert.Equal(t, "randomcookie123", cookie)
+	})
+
+	t.Run("token with too many colons", func(t *testing.T) {
+		_, _, _, err := parseOAuthStateTokenExtra("user@example.com:action:value:extra")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "expected exactly 3 parts")
+		assert.Contains(t, err.Error(), "got 4")
+	})
+
+	t.Run("token with too few colons", func(t *testing.T) {
+		_, _, _, err := parseOAuthStateTokenExtra("user@example.com:email_to_sso")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "expected exactly 3 parts")
+		assert.Contains(t, err.Error(), "got 2")
+	})
+
+	t.Run("token with no colons", func(t *testing.T) {
+		_, _, _, err := parseOAuthStateTokenExtra("invalidtoken")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "expected exactly 3 parts")
+		assert.Contains(t, err.Error(), "got 1")
+	})
+
+	t.Run("empty token string", func(t *testing.T) {
+		_, _, _, err := parseOAuthStateTokenExtra("")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "expected exactly 3 parts")
+	})
+}
+
+func TestAuthorizeOAuthUser_InvalidToken(t *testing.T) {
+	mainHelper.Parallel(t)
+	th := Setup(t)
+	defer th.TearDown()
+
+	mockProvider := &mocks.OAuthProvider{}
+	einterfaces.RegisterOAuthProvider(model.ServiceOpenid, mockProvider)
+
+	service := model.ServiceOpenid
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.EnableOAuthServiceProvider = true
+		cfg.OpenIdSettings.Enable = model.NewPointer(true)
+		cfg.OpenIdSettings.Id = model.NewPointer("test-client-id")
+		cfg.OpenIdSettings.Secret = model.NewPointer("test-secret")
+		cfg.OpenIdSettings.Scope = model.NewPointer(OpenIDScope)
+	})
+
+	mockProvider.On("GetSSOSettings", mock.Anything, mock.Anything, service).Return(&model.SSOSettings{
+		Enable: model.NewPointer(true),
+		Id:     model.NewPointer("test-client-id"),
+		Secret: model.NewPointer("test-secret"),
+	}, nil)
+
+	t.Run("rejects token with extra delimiters in email field", func(t *testing.T) {
+		cookieValue := model.NewId()
+
+		invalidEmail := "user@example.com:action"
+		action := "email_to_sso"
+
+		tokenExtra := generateOAuthStateTokenExtra(invalidEmail, action, cookieValue)
+		token, err := th.App.CreateOAuthStateToken(tokenExtra)
+		require.Nil(t, err)
+
+		stateProps := map[string]string{
+			"token":  token.Token,
+			"email":  "user@example.com",
+			"action": action,
+		}
+		state := base64.StdEncoding.EncodeToString([]byte(model.MapToJSON(stateProps)))
+
+		w := httptest.NewRecorder()
+		r := httptest.NewRequest("GET", "/", nil)
+		r.AddCookie(&http.Cookie{
+			Name:  CookieOAuth,
+			Value: "action:" + cookieValue,
+		})
+
+		_, _, _, appErr := th.App.AuthorizeOAuthUser(th.Context, w, r, service, "auth-code", state, "http://localhost/callback")
+
+		require.NotNil(t, appErr)
+		assert.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+		assert.Equal(t, "api.user.authorize_oauth_user.invalid_state.app_error", appErr.Id)
+	})
+
+	t.Run("rejects token with mismatched email", func(t *testing.T) {
+		cookieValue := model.NewId()
+		action := "email_to_sso"
+
+		tokenExtra := generateOAuthStateTokenExtra("token@example.com", action, cookieValue)
+		token, err := th.App.CreateOAuthStateToken(tokenExtra)
+		require.Nil(t, err)
+
+		stateProps := map[string]string{
+			"token":  token.Token,
+			"email":  "state@example.com",
+			"action": action,
+		}
+		state := base64.StdEncoding.EncodeToString([]byte(model.MapToJSON(stateProps)))
+
+		w := httptest.NewRecorder()
+		r := httptest.NewRequest("GET", "/", nil)
+		r.AddCookie(&http.Cookie{
+			Name:  CookieOAuth,
+			Value: cookieValue,
+		})
+
+		_, _, _, appErr := th.App.AuthorizeOAuthUser(th.Context, w, r, service, "auth-code", state, "http://localhost/callback")
+
+		require.NotNil(t, appErr)
+		assert.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+		assert.Equal(t, "api.user.authorize_oauth_user.invalid_state.app_error", appErr.Id)
+	})
+
+	t.Run("rejects token with mismatched action", func(t *testing.T) {
+		cookieValue := model.NewId()
+		email := "user@example.com"
+
+		tokenExtra := generateOAuthStateTokenExtra(email, "email_to_sso", cookieValue)
+		token, err := th.App.CreateOAuthStateToken(tokenExtra)
+		require.Nil(t, err)
+
+		stateProps := map[string]string{
+			"token":  token.Token,
+			"email":  email,
+			"action": "sso_to_email",
+		}
+		state := base64.StdEncoding.EncodeToString([]byte(model.MapToJSON(stateProps)))
+
+		w := httptest.NewRecorder()
+		r := httptest.NewRequest("GET", "/", nil)
+		r.AddCookie(&http.Cookie{
+			Name:  CookieOAuth,
+			Value: cookieValue,
+		})
+
+		_, _, _, appErr := th.App.AuthorizeOAuthUser(th.Context, w, r, service, "auth-code", state, "http://localhost/callback")
+
+		require.NotNil(t, appErr)
+		assert.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+		assert.Equal(t, "api.user.authorize_oauth_user.invalid_state.app_error", appErr.Id)
+	})
+
+	t.Run("rejects token with mismatched cookie", func(t *testing.T) {
+		email := "user@example.com"
+		action := "email_to_sso"
+
+		tokenExtra := generateOAuthStateTokenExtra(email, action, "token-cookie-value")
+		token, err := th.App.CreateOAuthStateToken(tokenExtra)
+		require.Nil(t, err)
+
+		stateProps := map[string]string{
+			"token":  token.Token,
+			"email":  email,
+			"action": action,
+		}
+		state := base64.StdEncoding.EncodeToString([]byte(model.MapToJSON(stateProps)))
+
+		w := httptest.NewRecorder()
+		r := httptest.NewRequest("GET", "/", nil)
+		r.AddCookie(&http.Cookie{
+			Name:  CookieOAuth,
+			Value: "different-cookie-value",
+		})
+
+		_, _, _, appErr := th.App.AuthorizeOAuthUser(th.Context, w, r, service, "auth-code", state, "http://localhost/callback")
+
+		require.NotNil(t, appErr)
+		assert.Equal(t, http.StatusBadRequest, appErr.StatusCode)
+		assert.Equal(t, "api.user.authorize_oauth_user.invalid_state.app_error", appErr.Id)
+	})
+}

@@ -2427,7 +2427,7 @@ func getGroupMessageMembersCommonTeams(c *Context, w http.ResponseWriter, r *htt
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(teams); err != nil {
+	if err := json.NewEncoder(w).Encode(c.App.SanitizeTeams(*c.AppContext.Session(), teams)); err != nil {
 		c.Logger.Warn("Error while writing response from getGroupMessageMembersCommonTeams", mlog.Err(err))
 	}
 }

@@ -2863,69 +2863,61 @@ func TestIsCRTEnabledForUser(t *testing.T) {
 
 func TestGetGroupMessageMembersCommonTeams(t *testing.T) {
 	mainHelper.Parallel(t)
-	th := SetupWithStoreMock(t)
+	th := Setup(t).InitBasic()
 	defer th.TearDown()
 
-	mockStore := th.App.Srv().Store().(*mocks.Store)
-
-	mockChannelStore := mocks.ChannelStore{}
-	mockStore.On("Channel").Return(&mockChannelStore)
-	mockChannelStore.On("Get", "gm_channel_id", true).Return(&model.Channel{Type: model.ChannelTypeGroup}, nil)
-
-	mockTeamStore := mocks.TeamStore{}
-	mockStore.On("Team").Return(&mockTeamStore)
-
-	th.App.Srv().Store().Team()
-
-	mockTeamStore.On("GetCommonTeamIDsForMultipleUsers", []string{"user_id_1", "user_id_2"}).Return([]string{"team_id_1", "team_id_2", "team_id_3"}, nil).Times(1)
-	mockTeamStore.On("GetMany", []string{"team_id_1", "team_id_2", "team_id_3"}).Return(
-		[]*model.Team{
-			{DisplayName: "Team 1"},
-			{DisplayName: "Team 2"},
-			{DisplayName: "Team 3"},
-		},
-		nil,
-	)
+	teamsToCreate := 2
+	usersToCreate := 4 // at least 3 users to create a GM channel, last user is not in any team
+	teams := make([]string, 0, teamsToCreate)
+	for i := 0; i < cap(teams); i++ {
+		team := th.CreateTeam()
+		defer func(team *model.Team) {
+			appErr := th.App.PermanentDeleteTeam(th.Context, team)
+			require.Nil(t, appErr)
+		}(team)
+		teams = append(teams, team.Id)
+	}
 
-	mockUserStore := mocks.UserStore{}
-	mockStore.On("User").Return(&mockUserStore)
-	options := &model.UserGetOptions{
-		PerPage:     model.ChannelGroupMaxUsers,
-		Page:        0,
-		InChannelId: "gm_channel_id",
-		Inactive:    false,
-		Active:      true,
+	users := make([]string, 0, usersToCreate)
+	for i := 0; i < cap(users); i++ {
+		user := th.CreateUser()
+		defer func(user *model.User) {
+			appErr := th.App.PermanentDeleteUser(th.Context, user)
+			require.Nil(t, appErr)
+		}(user)
+		users = append(users, user.Id)
 	}
-	mockUserStore.On("GetProfilesInChannel", options).Return([]*model.User{
-		{
-			Id: "user_id_1",
-		},
-		{
-			Id: "user_id_2",
-		},
-	}, nil)
 
-	var err error
-	th.App.ch.srv.teamService, err = teams.New(teams.ServiceConfig{
-		TeamStore:    &mockTeamStore,
-		ChannelStore: &mockChannelStore,
-		GroupStore:   &mocks.GroupStore{},
-		Users:        th.App.ch.srv.userService,
-		WebHub:       th.App.ch.srv.platform,
-		ConfigFn:     th.App.ch.srv.platform.Config,
-		LicenseFn:    th.App.ch.srv.License,
-	})
-	require.NoError(t, err)
+	for _, teamId := range teams {
+		// add first 3 users to each team, last user is not in any team
+		for i := range 3 {
+			_, _, appErr := th.App.AddUserToTeam(th.Context, teamId, users[i], "")
+			require.Nil(t, appErr)
+		}
+	}
 
-	commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, "gm_channel_id")
+	// create GM channel with first 3 users who share common teams
+	gmChannel, appErr := th.App.createGroupChannel(th.Context, users[:3], users[0])
 	require.Nil(t, appErr)
-	require.Equal(t, 3, len(commonTeams))
+	require.NotNil(t, gmChannel)
 
-	// case of no common teams
-	mockTeamStore.On("GetCommonTeamIDsForMultipleUsers", []string{"user_id_1", "user_id_2"}).Return([]string{}, nil)
-	commonTeams, appErr = th.App.GetGroupMessageMembersCommonTeams(th.Context, "gm_channel_id")
+	// normally you can't create a GM channel with users that don't share any teams, but we do it here to test the edge case
+	// create GM channel with last 3 users, where last member is not in any team
+	otherGMChannel, appErr := th.App.createGroupChannel(th.Context, users[1:], users[0])
 	require.Nil(t, appErr)
-	require.Equal(t, 0, len(commonTeams))
+	require.NotNil(t, otherGMChannel)
+
+	t.Run("Get teams for GM channel", func(t *testing.T) {
+		commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, gmChannel.Id)
+		require.Nil(t, appErr)
+		require.Equal(t, 2, len(commonTeams))
+	})
+
+	t.Run("No common teams", func(t *testing.T) {
+		commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, otherGMChannel.Id)
+		require.Nil(t, appErr)
+		require.Equal(t, 0, len(commonTeams))
+	})
 }
 
 func TestConvertGroupMessageToChannel(t *testing.T) {

@@ -2467,7 +2467,7 @@ func getGroupMessageMembersCommonTeams(c *Context, w http.ResponseWriter, r *htt
 		return
 	}
 
-	if err := json.NewEncoder(w).Encode(teams); err != nil {
+	if err := json.NewEncoder(w).Encode(c.App.SanitizeTeams(*c.AppContext.Session(), teams)); err != nil {
 		c.Logger.Warn("Error while writing response from getGroupMessageMembersCommonTeams", mlog.Err(err))
 	}
 }

@@ -2863,69 +2863,61 @@ func TestIsCRTEnabledForUser(t *testing.T) {
 
 func TestGetGroupMessageMembersCommonTeams(t *testing.T) {
 	mainHelper.Parallel(t)
-	th := SetupWithStoreMock(t)
+	th := Setup(t).InitBasic()
 	defer th.TearDown()
 
-	mockStore := th.App.Srv().Store().(*mocks.Store)
-
-	mockChannelStore := mocks.ChannelStore{}
-	mockStore.On("Channel").Return(&mockChannelStore)
-	mockChannelStore.On("Get", "gm_channel_id", true).Return(&model.Channel{Type: model.ChannelTypeGroup}, nil)
-
-	mockTeamStore := mocks.TeamStore{}
-	mockStore.On("Team").Return(&mockTeamStore)
-
-	th.App.Srv().Store().Team()
-
-	mockTeamStore.On("GetCommonTeamIDsForMultipleUsers", []string{"user_id_1", "user_id_2"}).Return([]string{"team_id_1", "team_id_2", "team_id_3"}, nil).Times(1)
-	mockTeamStore.On("GetMany", []string{"team_id_1", "team_id_2", "team_id_3"}).Return(
-		[]*model.Team{
-			{DisplayName: "Team 1"},
-			{DisplayName: "Team 2"},
-			{DisplayName: "Team 3"},
-		},
-		nil,
-	)
+	teamsToCreate := 2
+	usersToCreate := 4 // at least 3 users to create a GM channel, last user is not in any team
+	teams := make([]string, 0, teamsToCreate)
+	for i := 0; i < cap(teams); i++ {
+		team := th.CreateTeam()
+		defer func(team *model.Team) {
+			appErr := th.App.PermanentDeleteTeam(th.Context, team)
+			require.Nil(t, appErr)
+		}(team)
+		teams = append(teams, team.Id)
+	}
 
-	mockUserStore := mocks.UserStore{}
-	mockStore.On("User").Return(&mockUserStore)
-	options := &model.UserGetOptions{
-		PerPage:     model.ChannelGroupMaxUsers,
-		Page:        0,
-		InChannelId: "gm_channel_id",
-		Inactive:    false,
-		Active:      true,
+	users := make([]string, 0, usersToCreate)
+	for i := 0; i < cap(users); i++ {
+		user := th.CreateUser()
+		defer func(user *model.User) {
+			appErr := th.App.PermanentDeleteUser(th.Context, user)
+			require.Nil(t, appErr)
+		}(user)
+		users = append(users, user.Id)
 	}
-	mockUserStore.On("GetProfilesInChannel", options).Return([]*model.User{
-		{
-			Id: "user_id_1",
-		},
-		{
-			Id: "user_id_2",
-		},
-	}, nil)
 
-	var err error
-	th.App.ch.srv.teamService, err = teams.New(teams.ServiceConfig{
-		TeamStore:    &mockTeamStore,
-		ChannelStore: &mockChannelStore,
-		GroupStore:   &mocks.GroupStore{},
-		Users:        th.App.ch.srv.userService,
-		WebHub:       th.App.ch.srv.platform,
-		ConfigFn:     th.App.ch.srv.platform.Config,
-		LicenseFn:    th.App.ch.srv.License,
-	})
-	require.NoError(t, err)
+	for _, teamId := range teams {
+		// add first 3 users to each team, last user is not in any team
+		for i := range 3 {
+			_, _, appErr := th.App.AddUserToTeam(th.Context, teamId, users[i], "")
+			require.Nil(t, appErr)
+		}
+	}
 
-	commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, "gm_channel_id")
+	// create GM channel with first 3 users who share common teams
+	gmChannel, appErr := th.App.createGroupChannel(th.Context, users[:3], users[0])
 	require.Nil(t, appErr)
-	require.Equal(t, 3, len(commonTeams))
+	require.NotNil(t, gmChannel)
 
-	// case of no common teams
-	mockTeamStore.On("GetCommonTeamIDsForMultipleUsers", []string{"user_id_1", "user_id_2"}).Return([]string{}, nil)
-	commonTeams, appErr = th.App.GetGroupMessageMembersCommonTeams(th.Context, "gm_channel_id")
+	// normally you can't create a GM channel with users that don't share any teams, but we do it here to test the edge case
+	// create GM channel with last 3 users, where last member is not in any team
+	otherGMChannel, appErr := th.App.createGroupChannel(th.Context, users[1:], users[0])
 	require.Nil(t, appErr)
-	require.Equal(t, 0, len(commonTeams))
+	require.NotNil(t, otherGMChannel)
+
+	t.Run("Get teams for GM channel", func(t *testing.T) {
+		commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, gmChannel.Id)
+		require.Nil(t, appErr)
+		require.Equal(t, 2, len(commonTeams))
+	})
+
+	t.Run("No common teams", func(t *testing.T) {
+		commonTeams, appErr := th.App.GetGroupMessageMembersCommonTeams(th.Context, otherGMChannel.Id)
+		require.Nil(t, appErr)
+		require.Equal(t, 0, len(commonTeams))
+	})
 }
 
 func TestConvertGroupMessageToChannel(t *testing.T) {