fix guest user import when guest user doesn't have any memberships

This PR fixes an issue where a guest user without channel or team memberships
is not being imported and the importer will then throw an error and abort.

Key changes:
1. Updated validateGuestRoles function to allow system guests without any teams/channels
2. Improved error handling with specific error messages for different validation failures
3. Updated tests to reflect new behavior allowing system guests with no team memberships
4. Added new i18n error messages for better user feedback

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

server/channels/app/imports/import_validators.go

@@ -267,8 +267,8 @@ func ValidateUserImportData(data *UserImportData) *model.AppError {
 		return model.NewAppError("BulkImport", "app.import.validate_user_import_data.roles_invalid.error", nil, "", http.StatusBadRequest)
 	}
 
-	if !isValidGuestRoles(*data) {
-		return model.NewAppError("BulkImport", "app.import.validate_user_import_data.guest_roles_conflict.error", nil, "", http.StatusBadRequest)
+	if err := validateGuestRoles(*data); err != nil {
+		return err
 	}
 
 	if data.NotifyProps != nil {
@@ -758,49 +758,77 @@ func isValidEmailBatchingInterval(emailInterval string) bool {
 		emailInterval == model.PreferenceEmailIntervalHour
 }
 
-// isValidGuestRoles checks if the user has both guest roles in the same team or channel.
-// at this point we assume that the user has a valid role scheme.
-func isValidGuestRoles(data UserImportData) bool {
+// validateGuestRoles checks if the user has guest roles consistently across system, team, and channel levels.
+// At this point, we assume that the user has a valid role scheme.
+func validateGuestRoles(data UserImportData) *model.AppError {
 	if data.Roles == nil {
-		return true
+		return nil
 	}
 	isSystemGuest := model.IsInRole(*data.Roles, model.SystemGuestRoleId)
 
-	var isTeamGuest, isChannelGuest bool
-	if data.Teams != nil {
-		// counters for guest roles for teams and channels
-		// we expect the total count of guest roles to be equal to the total count of teams and channels
-		var gtc, ctc int
-		for _, team := range *data.Teams {
-			if team.Roles != nil && model.IsInRole(*team.Roles, model.TeamGuestRoleId) {
-				gtc++
-			}
+	// If user has no teams, they can still be a system guest without issue
+	if data.Teams == nil || len(*data.Teams) == 0 {
+		return nil
+	}
+
+	var isTeamGuest, isChannelGuest bool
+	var hasChannels bool // hasChannels indicates if the user has any channels within their teams
+
+	var teamGuestCount, channelGuestCount int
+	var totalTeams, totalChannels int
+
+	totalTeams = len(*data.Teams)
+	for _, team := range *data.Teams {
+		if team.Roles != nil && model.IsInRole(*team.Roles, model.TeamGuestRoleId) {
+			teamGuestCount++
+		}
+
+		if len(model.SafeDereference(team.Channels)) > 0 {
+			hasChannels = true
+			totalChannels += len(*team.Channels)
 
-			if team.Channels == nil || len(*team.Channels) == 0 {
-				continue
-			}
 			for _, channel := range *team.Channels {
 				if channel.Roles != nil && model.IsInRole(*channel.Roles, model.ChannelGuestRoleId) {
-					ctc++
+					channelGuestCount++
 				}
 			}
-
-			if ctc == len(*team.Channels) {
-				isChannelGuest = true
-			}
-		}
-		if gtc == len(*data.Teams) {
-			isTeamGuest = true
 		}
 	}
 
-	// basically we want to be sure if the user either fully guest in all 3 places or not at all
-	// (a | b | c) & !(a & b & c) -> 3-way XOR?
-	if (isSystemGuest || isTeamGuest || isChannelGuest) && !(isSystemGuest && isTeamGuest && isChannelGuest) {
-		return false
+	// Set flags based on whether all available teams/channels have guest roles
+	if totalTeams > 0 && teamGuestCount == totalTeams {
+		isTeamGuest = true
 	}
 
-	return true
+	if hasChannels && channelGuestCount == totalChannels {
+		isChannelGuest = true
+	}
+
+	// If the user is a system guest, they must have consistent guest roles in any teams/channels they belong to
+	if isSystemGuest {
+		// If they have teams, they must be a team guest in all teams
+		if totalTeams > 0 && !isTeamGuest {
+			return model.NewAppError("BulkImport", "app.import.validate_user_import_data.system_guest_missing_team_guest_roles.error", map[string]any{"TeamGuestCount": teamGuestCount, "TotalTeams": totalTeams}, "", http.StatusBadRequest)
+		}
+
+		// If they have channels, they must be a channel guest in all channels
+		if hasChannels && !isChannelGuest {
+			return model.NewAppError("BulkImport", "app.import.validate_user_import_data.system_guest_missing_channel_guest_roles.error", map[string]any{"ChannelGuestCount": channelGuestCount, "TotalChannels": totalChannels}, "", http.StatusBadRequest)
+		}
+
+		return nil
+	}
+
+	// If not a system guest, ensure consistency in the other direction
+	// If they're a team or channel guest, they must be a system guest
+	if (isTeamGuest || isChannelGuest) && !isSystemGuest {
+		if isTeamGuest {
+			return model.NewAppError("BulkImport", "app.import.validate_user_import_data.team_guest_missing_system_guest_role.error", nil, "", http.StatusBadRequest)
+		}
+		return model.NewAppError("BulkImport", "app.import.validate_user_import_data.channel_guest_missing_system_guest_role.error", nil, "", http.StatusBadRequest)
+	}
+
+	return nil
 }
 
 // ValidateAttachmentPathForImport joins 'path' to 'basePath' (defaulting to "." if empty) and ensures

server/channels/app/imports/import_validators_test.go

@@ -1601,17 +1601,16 @@ func checkNoError(t *testing.T, err *model.AppError) {
 	require.Nil(t, err, "Unexpected Error: %v", err)
 }
 
-func TestIsValidGuestRoles(t *testing.T) {
+func TestValidateGuestRoles(t *testing.T) {
 	testCases := []struct {
-		name     string
-		input    UserImportData
-		expected bool
+		name        string
+		input       UserImportData
+		expectError bool
 	}{
 		{
 			name: "Valid case: User is a guest in all places",
 			input: UserImportData{
-				Username: model.NewPointer("guest1"),
-				Roles:    model.NewPointer(model.SystemGuestRoleId),
+				Roles: model.NewPointer(model.SystemGuestRoleId),
 				Teams: &[]UserTeamImportData{
 					{
 						Roles: model.NewPointer(model.TeamGuestRoleId),
@@ -1621,13 +1620,12 @@ func TestIsValidGuestRoles(t *testing.T) {
 					},
 				},
 			},
-			expected: true,
+			expectError: false,
 		},
 		{
 			name: "Invalid case: User is a guest in a team but not in another team",
 			input: UserImportData{
-				Username: model.NewPointer("mixeduser1"),
-				Roles:    model.NewPointer(model.SystemGuestRoleId),
+				Roles: model.NewPointer(model.SystemGuestRoleId),
 				Teams: &[]UserTeamImportData{
 					{
 						Roles: model.NewPointer(model.TeamGuestRoleId),
@@ -1643,13 +1641,12 @@ func TestIsValidGuestRoles(t *testing.T) {
 					},
 				},
 			},
-			expected: false,
+			expectError: true,
 		},
 		{
 			name: "Invalid case: User is a guest in a team but not in another team and has no channel membership",
 			input: UserImportData{
-				Username: model.NewPointer("mixeduser2"),
-				Roles:    model.NewPointer(model.SystemGuestRoleId),
+				Roles: model.NewPointer(model.SystemGuestRoleId),
 				Teams: &[]UserTeamImportData{
 					{
 						Roles: model.NewPointer(model.TeamGuestRoleId),
@@ -1663,21 +1660,27 @@ func TestIsValidGuestRoles(t *testing.T) {
 					},
 				},
 			},
-			expected: false,
+			expectError: true,
 		},
 		{
-			name: "Invalid case: User is system guest but not guest in team and channel",
+			name: "Valid case: User is system guest with no teams",
 			input: UserImportData{
-				Username: model.NewPointer("systemguestonly"),
-				Roles:    model.NewPointer(model.SystemGuestRoleId),
+				Roles: model.NewPointer(model.SystemGuestRoleId),
 			},
-			expected: false,
+			expectError: false,
+		},
+		{
+			name: "Valid case: User is system guest with empty teams array",
+			input: UserImportData{
+				Roles: model.NewPointer(model.SystemGuestRoleId),
+				Teams: &[]UserTeamImportData{},
+			},
+			expectError: false,
 		},
 		{
 			name: "Invalid case: User has mixed roles",
 			input: UserImportData{
-				Username: model.NewPointer("mixeduser3"),
-				Roles:    model.NewPointer(model.SystemGuestRoleId),
+				Roles: model.NewPointer(model.SystemGuestRoleId),
 				Teams: &[]UserTeamImportData{
 					{
 						Roles: model.NewPointer(model.TeamUserRoleId),
@@ -1687,20 +1690,42 @@ func TestIsValidGuestRoles(t *testing.T) {
 					},
 				},
 			},
-			expected: false,
+			expectError: true,
 		},
 		{
-			name: "Valid case: User does not have any role defined in any place",
+			name: "Valid case: User is system guest with team guest role but no channels",
 			input: UserImportData{
-				Username: model.NewPointer("noroleuser"),
+				Roles: model.NewPointer(model.SystemGuestRoleId),
+				Teams: &[]UserTeamImportData{
+					{
+						Roles:    model.NewPointer(model.TeamGuestRoleId),
+						Channels: &[]UserChannelImportData{},
+					},
+				},
 			},
-			expected: true,
+			expectError: false,
+		},
+		{
+			name: "Valid case: User is system guest with team guest role and nil channels",
+			input: UserImportData{
+				Roles: model.NewPointer(model.SystemGuestRoleId),
+				Teams: &[]UserTeamImportData{
+					{
+						Roles: model.NewPointer(model.TeamGuestRoleId),
+					},
+				},
+			},
+			expectError: false,
+		},
+		{
+			name:        "Valid case: User does not have any role defined in any place",
+			input:       UserImportData{},
+			expectError: false,
 		},
 		{
 			name: "Valid case: User is not a guest in any place",
 			input: UserImportData{
-				Username: model.NewPointer("normaluser"),
-				Roles:    model.NewPointer(model.SystemUserRoleId),
+				Roles: model.NewPointer(model.SystemUserRoleId),
 				Teams: &[]UserTeamImportData{
 					{
 						Roles: model.NewPointer(model.TeamAdminRoleId),
@@ -1710,81 +1735,18 @@ func TestIsValidGuestRoles(t *testing.T) {
 					},
 				},
 			},
-			expected: true,
-		},
-		{
-			name: "Valid case: User with team but nil channels array",
-			input: UserImportData{
-				Username: model.NewPointer("nilchannelsuser"),
-				Roles:    model.NewPointer(model.SystemUserRoleId),
-				Teams: &[]UserTeamImportData{
-					{
-						Roles:    model.NewPointer(model.TeamUserRoleId),
-						Channels: nil,
-					},
-				},
-			},
-			expected: true,
-		},
-		{
-			name: "Invalid case: User is guest in channels but not in system or team",
-			input: UserImportData{
-				Username: model.NewPointer("testuser3"),
-				Roles:    model.NewPointer(model.SystemUserRoleId),
-				Teams: &[]UserTeamImportData{
-					{
-						Roles: model.NewPointer(model.TeamUserRoleId),
-						Channels: &[]UserChannelImportData{
-							{Roles: model.NewPointer(model.ChannelGuestRoleId)},
-						},
-					},
-				},
-			},
-			expected: false,
-		},
-		{
-			name: "Invalid case: User is system guest and team guest but has no channels",
-			input: UserImportData{
-				Username: model.NewPointer("testuser4"),
-				Roles:    model.NewPointer(model.SystemGuestRoleId),
-				Teams: &[]UserTeamImportData{
-					{
-						Roles:    model.NewPointer(model.TeamGuestRoleId),
-						Channels: &[]UserChannelImportData{},
-					},
-				},
-			},
-			expected: false,
-		},
-		{
-			name: "Valid case: User is guest in all places with multiple teams and channels",
-			input: UserImportData{
-				Username: model.NewPointer("testuser5"),
-				Roles:    model.NewPointer(model.SystemGuestRoleId),
-				Teams: &[]UserTeamImportData{
-					{
-						Roles: model.NewPointer(model.TeamGuestRoleId),
-						Channels: &[]UserChannelImportData{
-							{Roles: model.NewPointer(model.ChannelGuestRoleId)},
-							{Roles: model.NewPointer(model.ChannelGuestRoleId)},
-						},
-					},
-					{
-						Roles: model.NewPointer(model.TeamGuestRoleId),
-						Channels: &[]UserChannelImportData{
-							{Roles: model.NewPointer(model.ChannelGuestRoleId)},
-						},
-					},
-				},
-			},
-			expected: true,
+			expectError: false,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			result := isValidGuestRoles(tc.input)
-			assert.Equal(t, tc.expected, result, tc.name)
+			err := validateGuestRoles(tc.input)
+			if tc.expectError {
+				assert.NotNil(t, err, tc.name)
+			} else {
+				assert.Nil(t, err, tc.name)
+			}
 		})
 	}
 }

server/i18n/en.json

@@ -6486,6 +6486,22 @@
     "id": "app.import.validate_user_import_data.guest_roles_conflict.error",
     "translation": "User roles are not consistent with guest status."
   },
+  {
+    "id": "app.import.validate_user_import_data.system_guest_missing_team_guest_roles.error",
+    "translation": "User is a system guest but does not have team guest roles in all teams. Team guest count: {{.TeamGuestCount}}, Total teams: {{.TotalTeams}}."
+  },
+  {
+    "id": "app.import.validate_user_import_data.system_guest_missing_channel_guest_roles.error",
+    "translation": "User is a system guest but does not have channel guest roles in all channels. Channel guest count: {{.ChannelGuestCount}}, Total channels: {{.TotalChannels}}."
+  },
+  {
+    "id": "app.import.validate_user_import_data.team_guest_missing_system_guest_role.error",
+    "translation": "User has team guest roles but is not a system guest."
+  },
+  {
+    "id": "app.import.validate_user_import_data.channel_guest_missing_system_guest_role.error",
+    "translation": "User has channel guest roles but is not a system guest."
+  },
   {
     "id": "app.import.validate_user_import_data.invalid_image_path.error",
     "translation": "User profile image path is invalid: \"{{.Path}}\""