Slack import bypasses email verification for team access controls
Description
Mattermost versions 10.10.x <= 10.10.2, 10.5.x <= 10.5.10, 10.11.x <= 10.11.2 fail to validate email ownership during Slack import process which allows attackers to create verified user accounts with arbitrary email domains via malicious Slack import data to bypass email-based team access restrictions
Affected packages
Versions sourced from the GitHub Security Advisory.
| Package | Affected versions | Patched versions |
|---|---|---|
github.com/mattermost/mattermost/server/v8Go | < 8.0.0-20250822083415-01b95392a450 | 8.0.0-20250822083415-01b95392a450 |
github.com/mattermost/mattermost-serverGo | >= 10.10.0, < 10.10.3 | 10.10.3 |
github.com/mattermost/mattermost-serverGo | >= 10.5.0, < 10.5.11 | 10.5.11 |
github.com/mattermost/mattermost-serverGo | >= 10.11.0, < 10.11.3 | 10.11.3 |
Affected products
1- Range: 10.10.0
Patches
3ef896a4ea60cMm 64925 - prevent slack import email auto validation for non admin users (#33609) (#33779)
3 files changed · +343 −6
server/channels/app/slack.go+18 −1 modified@@ -49,7 +49,24 @@ func (a *App) SlackImport(c request.CTX, fileData multipart.File, fileSize int64 }, } - importer := slackimport.New(a.Srv().Store(), actions, a.Config()) + // Determine if this is an Admin import: + // mattermost cmd imports (no session) are treated as admin imports since only server admins can run them + // Web imports (include mmctl calls) check the actual user's role + isAdminImport := false + + if c.Session() == nil { + // no session means it's being run directly on the server and only + // server admins can run CLI commands, so treat as admin import + isAdminImport = true + c.Logger().Info("Slack import initiated via CLI, treating as admin import") + } else if c.Session().UserId != "" { + // Web API + mmctl import - check if the user is a system admin + if user, err := a.GetUser(c.Session().UserId); err == nil { + isAdminImport = user.IsSystemAdmin() + } + } + + importer := slackimport.NewWithAdminFlag(a.Srv().Store(), actions, a.Config(), isAdminImport) return importer.SlackImport(c, fileData, fileSize, teamID) }
server/platform/services/slackimport/slackimport.go+24 −5 modified@@ -100,9 +100,10 @@ type Actions struct { // SlackImporter is a service that allows to import slack dumps into mattermost type SlackImporter struct { - store store.Store - actions Actions - config *model.Config + store store.Store + actions Actions + config *model.Config + isAdminImport bool } // New creates a new SlackImporter service instance. It receive a store, a set of actions and the current config. @@ -115,6 +116,17 @@ func New(store store.Store, actions Actions, config *model.Config) *SlackImporte } } +// NewWithAdminFlag creates a new SlackImporter service instance with information about whether this is an admin import. +// This allows for enhanced security controls based on the importing user's role. +func NewWithAdminFlag(store store.Store, actions Actions, config *model.Config, isAdminImport bool) *SlackImporter { + return &SlackImporter{ + store: store, + actions: actions, + config: config, + isAdminImport: isAdminImport, + } +} + func (si *SlackImporter) SlackImport(rctx request.CTX, fileData multipart.File, fileSize int64, teamID string) (*model.AppError, *bytes.Buffer) { // Create log file log := bytes.NewBufferString(i18n.T("api.slackimport.slack_import.log")) @@ -718,8 +730,15 @@ func (si *SlackImporter) oldImportUser(rctx request.CTX, team *model.Team, user return nil } - if _, err := si.store.User().VerifyEmail(ruser.Id, ruser.Email); err != nil { - rctx.Logger().Warn("Failed to set email verified.", mlog.Err(err)) + // Only system admins can automatically verify emails during import + if si.isAdminImport { + if _, err := si.store.User().VerifyEmail(ruser.Id, ruser.Email); err != nil { + rctx.Logger().Warn("Failed to set email verified for admin import.", mlog.Err(err)) + } + } else { + // Non-admin users: emails remain unverified + rctx.Logger().Debug("Email verification skipped for non-admin import.", + mlog.String("user_email", ruser.Email)) } if _, err := si.actions.JoinUserToTeam(team, user, ""); err != nil {
server/platform/services/slackimport/slackimport_test.go+301 −0 modified@@ -13,6 +13,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" @@ -459,3 +460,303 @@ func TestSlackUploadFile(t *testing.T) { require.False(t, ok) }) } + +func TestOldImportUserEmailVerificationIsNotAutomatic(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Must remain false after import + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + importer := New(store, actions, config) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "SECURITY: VerifyEmail should NOT be called - this prevents domain bypass vulnerability") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify the user was saved with unverified email (VerifyEmail should not have been called) + userStore.AssertCalled(t, "Save", mock.AnythingOfType("*request.Context"), mock.MatchedBy(func(u *model.User) bool { + return u.Email == "testuser@restricted-domain.com" && !u.EmailVerified + })) +} + +// TestSlackImportEnhancedSecurityAdminCanVerifyEmails tests that system admins can automatically verify emails +func TestSlackImportEnhancedSecurityAdminCanVerifyEmails(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it SHOULD be called for admin imports) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Will be verified by admin import + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Pass true to indicate this is an admin import + importer := NewWithAdminFlag(store, actions, config, true) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.True(t, verifyEmailCalled, "ADMIN IMPORT: VerifyEmail SHOULD be called for system admin imports") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was called with correct parameters + userStore.AssertCalled(t, "VerifyEmail", "test-user-id", "testuser@restricted-domain.com") +} + +// TestSlackImportEnhancedSecurityNonAdminCannotVerifyEmails tests that non-admin users cannot automatically verify emails +func TestSlackImportEnhancedSecurityNonAdminCannotVerifyEmails(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called for non-admin imports) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Should remain false for non-admin import + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Pass false to indicate this is NOT an admin import + importer := NewWithAdminFlag(store, actions, config, false) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "NON-ADMIN IMPORT: VerifyEmail should NOT be called for non-admin imports") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was NOT called + userStore.AssertNotCalled(t, "VerifyEmail") +} + +// TestSlackImportEnhancedSecurityNoImportingUser tests behavior when no importing user is provided +func TestSlackImportEnhancedSecurityNoImportingUser(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called when no importing user) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Should remain false when no importing user + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Pass false to indicate no admin privileges (default secure behavior) + importer := NewWithAdminFlag(store, actions, config, false) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "NO IMPORTING USER: VerifyEmail should NOT be called when no importing user is provided") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was NOT called + userStore.AssertNotCalled(t, "VerifyEmail") +} + +// TestSlackImportEnhancedSecurityBackwardsCompatibility tests that the old New() constructor still works +func TestSlackImportEnhancedSecurityBackwardsCompatibility(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called with old constructor) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Should remain false with old constructor + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Use the old constructor (backwards compatibility) + importer := New(store, actions, config) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "BACKWARDS COMPATIBILITY: VerifyEmail should NOT be called with old constructor") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was NOT called + userStore.AssertNotCalled(t, "VerifyEmail") +}
0d6e8fa2e468Mm 64925 - prevent slack import email auto validation for non admin users (#33609) (#33780)
3 files changed · +343 −6
server/channels/app/slack.go+18 −1 modified@@ -49,7 +49,24 @@ func (a *App) SlackImport(c request.CTX, fileData multipart.File, fileSize int64 }, } - importer := slackimport.New(a.Srv().Store(), actions, a.Config()) + // Determine if this is an Admin import: + // mattermost cmd imports (no session) are treated as admin imports since only server admins can run them + // Web imports (include mmctl calls) check the actual user's role + isAdminImport := false + + if c.Session() == nil { + // no session means it's being run directly on the server and only + // server admins can run CLI commands, so treat as admin import + isAdminImport = true + c.Logger().Info("Slack import initiated via CLI, treating as admin import") + } else if c.Session().UserId != "" { + // Web API + mmctl import - check if the user is a system admin + if user, err := a.GetUser(c.Session().UserId); err == nil { + isAdminImport = user.IsSystemAdmin() + } + } + + importer := slackimport.NewWithAdminFlag(a.Srv().Store(), actions, a.Config(), isAdminImport) return importer.SlackImport(c, fileData, fileSize, teamID) }
server/platform/services/slackimport/slackimport.go+24 −5 modified@@ -100,9 +100,10 @@ type Actions struct { // SlackImporter is a service that allows to import slack dumps into mattermost type SlackImporter struct { - store store.Store - actions Actions - config *model.Config + store store.Store + actions Actions + config *model.Config + isAdminImport bool } // New creates a new SlackImporter service instance. It receive a store, a set of actions and the current config. @@ -115,6 +116,17 @@ func New(store store.Store, actions Actions, config *model.Config) *SlackImporte } } +// NewWithAdminFlag creates a new SlackImporter service instance with information about whether this is an admin import. +// This allows for enhanced security controls based on the importing user's role. +func NewWithAdminFlag(store store.Store, actions Actions, config *model.Config, isAdminImport bool) *SlackImporter { + return &SlackImporter{ + store: store, + actions: actions, + config: config, + isAdminImport: isAdminImport, + } +} + func (si *SlackImporter) SlackImport(rctx request.CTX, fileData multipart.File, fileSize int64, teamID string) (*model.AppError, *bytes.Buffer) { // Create log file log := bytes.NewBufferString(i18n.T("api.slackimport.slack_import.log")) @@ -718,8 +730,15 @@ func (si *SlackImporter) oldImportUser(rctx request.CTX, team *model.Team, user return nil } - if _, err := si.store.User().VerifyEmail(ruser.Id, ruser.Email); err != nil { - rctx.Logger().Warn("Failed to set email verified.", mlog.Err(err)) + // Only system admins can automatically verify emails during import + if si.isAdminImport { + if _, err := si.store.User().VerifyEmail(ruser.Id, ruser.Email); err != nil { + rctx.Logger().Warn("Failed to set email verified for admin import.", mlog.Err(err)) + } + } else { + // Non-admin users: emails remain unverified + rctx.Logger().Debug("Email verification skipped for non-admin import.", + mlog.String("user_email", ruser.Email)) } if _, err := si.actions.JoinUserToTeam(team, user, ""); err != nil {
server/platform/services/slackimport/slackimport_test.go+301 −0 modified@@ -13,6 +13,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" @@ -459,3 +460,303 @@ func TestSlackUploadFile(t *testing.T) { require.False(t, ok) }) } + +func TestOldImportUserEmailVerificationIsNotAutomatic(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Must remain false after import + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + importer := New(store, actions, config) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "SECURITY: VerifyEmail should NOT be called - this prevents domain bypass vulnerability") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify the user was saved with unverified email (VerifyEmail should not have been called) + userStore.AssertCalled(t, "Save", mock.AnythingOfType("*request.Context"), mock.MatchedBy(func(u *model.User) bool { + return u.Email == "testuser@restricted-domain.com" && !u.EmailVerified + })) +} + +// TestSlackImportEnhancedSecurityAdminCanVerifyEmails tests that system admins can automatically verify emails +func TestSlackImportEnhancedSecurityAdminCanVerifyEmails(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it SHOULD be called for admin imports) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Will be verified by admin import + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Pass true to indicate this is an admin import + importer := NewWithAdminFlag(store, actions, config, true) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.True(t, verifyEmailCalled, "ADMIN IMPORT: VerifyEmail SHOULD be called for system admin imports") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was called with correct parameters + userStore.AssertCalled(t, "VerifyEmail", "test-user-id", "testuser@restricted-domain.com") +} + +// TestSlackImportEnhancedSecurityNonAdminCannotVerifyEmails tests that non-admin users cannot automatically verify emails +func TestSlackImportEnhancedSecurityNonAdminCannotVerifyEmails(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called for non-admin imports) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Should remain false for non-admin import + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Pass false to indicate this is NOT an admin import + importer := NewWithAdminFlag(store, actions, config, false) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "NON-ADMIN IMPORT: VerifyEmail should NOT be called for non-admin imports") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was NOT called + userStore.AssertNotCalled(t, "VerifyEmail") +} + +// TestSlackImportEnhancedSecurityNoImportingUser tests behavior when no importing user is provided +func TestSlackImportEnhancedSecurityNoImportingUser(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called when no importing user) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Should remain false when no importing user + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Pass false to indicate no admin privileges (default secure behavior) + importer := NewWithAdminFlag(store, actions, config, false) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "NO IMPORTING USER: VerifyEmail should NOT be called when no importing user is provided") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was NOT called + userStore.AssertNotCalled(t, "VerifyEmail") +} + +// TestSlackImportEnhancedSecurityBackwardsCompatibility tests that the old New() constructor still works +func TestSlackImportEnhancedSecurityBackwardsCompatibility(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called with old constructor) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Should remain false with old constructor + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Use the old constructor (backwards compatibility) + importer := New(store, actions, config) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "BACKWARDS COMPATIBILITY: VerifyEmail should NOT be called with old constructor") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was NOT called + userStore.AssertNotCalled(t, "VerifyEmail") +}
01b95392a450Mm 64925 - prevent slack import email auto validation for non admin users (#33609) (#33781)
3 files changed · +343 −6
server/channels/app/slack.go+18 −1 modified@@ -49,7 +49,24 @@ func (a *App) SlackImport(c request.CTX, fileData multipart.File, fileSize int64 }, } - importer := slackimport.New(a.Srv().Store(), actions, a.Config()) + // Determine if this is an Admin import: + // mattermost cmd imports (no session) are treated as admin imports since only server admins can run them + // Web imports (include mmctl calls) check the actual user's role + isAdminImport := false + + if c.Session() == nil { + // no session means it's being run directly on the server and only + // server admins can run CLI commands, so treat as admin import + isAdminImport = true + c.Logger().Info("Slack import initiated via CLI, treating as admin import") + } else if c.Session().UserId != "" { + // Web API + mmctl import - check if the user is a system admin + if user, err := a.GetUser(c.Session().UserId); err == nil { + isAdminImport = user.IsSystemAdmin() + } + } + + importer := slackimport.NewWithAdminFlag(a.Srv().Store(), actions, a.Config(), isAdminImport) return importer.SlackImport(c, fileData, fileSize, teamID) }
server/platform/services/slackimport/slackimport.go+24 −5 modified@@ -100,9 +100,10 @@ type Actions struct { // SlackImporter is a service that allows to import slack dumps into mattermost type SlackImporter struct { - store store.Store - actions Actions - config *model.Config + store store.Store + actions Actions + config *model.Config + isAdminImport bool } // New creates a new SlackImporter service instance. It receive a store, a set of actions and the current config. @@ -115,6 +116,17 @@ func New(store store.Store, actions Actions, config *model.Config) *SlackImporte } } +// NewWithAdminFlag creates a new SlackImporter service instance with information about whether this is an admin import. +// This allows for enhanced security controls based on the importing user's role. +func NewWithAdminFlag(store store.Store, actions Actions, config *model.Config, isAdminImport bool) *SlackImporter { + return &SlackImporter{ + store: store, + actions: actions, + config: config, + isAdminImport: isAdminImport, + } +} + func (si *SlackImporter) SlackImport(rctx request.CTX, fileData multipart.File, fileSize int64, teamID string) (*model.AppError, *bytes.Buffer) { // Create log file log := bytes.NewBufferString(i18n.T("api.slackimport.slack_import.log")) @@ -716,8 +728,15 @@ func (si *SlackImporter) oldImportUser(rctx request.CTX, team *model.Team, user return nil } - if _, err := si.store.User().VerifyEmail(ruser.Id, ruser.Email); err != nil { - rctx.Logger().Warn("Failed to set email verified.", mlog.Err(err)) + // Only system admins can automatically verify emails during import + if si.isAdminImport { + if _, err := si.store.User().VerifyEmail(ruser.Id, ruser.Email); err != nil { + rctx.Logger().Warn("Failed to set email verified for admin import.", mlog.Err(err)) + } + } else { + // Non-admin users: emails remain unverified + rctx.Logger().Debug("Email verification skipped for non-admin import.", + mlog.String("user_email", ruser.Email)) } if _, err := si.actions.JoinUserToTeam(team, user, ""); err != nil {
server/platform/services/slackimport/slackimport_test.go+301 −0 modified@@ -13,6 +13,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" @@ -459,3 +460,303 @@ func TestSlackUploadFile(t *testing.T) { require.False(t, ok) }) } + +func TestOldImportUserEmailVerificationIsNotAutomatic(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Must remain false after import + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + importer := New(store, actions, config) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "SECURITY: VerifyEmail should NOT be called - this prevents domain bypass vulnerability") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify the user was saved with unverified email (VerifyEmail should not have been called) + userStore.AssertCalled(t, "Save", mock.AnythingOfType("*request.Context"), mock.MatchedBy(func(u *model.User) bool { + return u.Email == "testuser@restricted-domain.com" && !u.EmailVerified + })) +} + +// TestSlackImportEnhancedSecurityAdminCanVerifyEmails tests that system admins can automatically verify emails +func TestSlackImportEnhancedSecurityAdminCanVerifyEmails(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it SHOULD be called for admin imports) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Will be verified by admin import + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Pass true to indicate this is an admin import + importer := NewWithAdminFlag(store, actions, config, true) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.True(t, verifyEmailCalled, "ADMIN IMPORT: VerifyEmail SHOULD be called for system admin imports") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was called with correct parameters + userStore.AssertCalled(t, "VerifyEmail", "test-user-id", "testuser@restricted-domain.com") +} + +// TestSlackImportEnhancedSecurityNonAdminCannotVerifyEmails tests that non-admin users cannot automatically verify emails +func TestSlackImportEnhancedSecurityNonAdminCannotVerifyEmails(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called for non-admin imports) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Should remain false for non-admin import + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Pass false to indicate this is NOT an admin import + importer := NewWithAdminFlag(store, actions, config, false) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "NON-ADMIN IMPORT: VerifyEmail should NOT be called for non-admin imports") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was NOT called + userStore.AssertNotCalled(t, "VerifyEmail") +} + +// TestSlackImportEnhancedSecurityNoImportingUser tests behavior when no importing user is provided +func TestSlackImportEnhancedSecurityNoImportingUser(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called when no importing user) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Should remain false when no importing user + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Pass false to indicate no admin privileges (default secure behavior) + importer := NewWithAdminFlag(store, actions, config, false) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "NO IMPORTING USER: VerifyEmail should NOT be called when no importing user is provided") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was NOT called + userStore.AssertNotCalled(t, "VerifyEmail") +} + +// TestSlackImportEnhancedSecurityBackwardsCompatibility tests that the old New() constructor still works +func TestSlackImportEnhancedSecurityBackwardsCompatibility(t *testing.T) { + rctx := request.TestContext(t) + + store := &mocks.Store{} + userStore := &mocks.UserStore{} + store.On("User").Return(userStore) + + // Track if VerifyEmail is called (it should NOT be called with old constructor) + verifyEmailCalled := false + userStore.On("VerifyEmail", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return("user-id", nil).Run(func(args mock.Arguments) { + verifyEmailCalled = true + }) + + savedUser := &model.User{ + Id: "test-user-id", + Username: "testuser", + Email: "testuser@restricted-domain.com", + EmailVerified: false, // Should remain false with old constructor + Roles: model.SystemUserRoleId, + } + userStore.On("Save", mock.AnythingOfType("*request.Context"), mock.AnythingOfType("*model.User")).Return(savedUser, nil) + + joinTeamCalled := false + actions := Actions{ + JoinUserToTeam: func(team *model.Team, user *model.User, userRequestorId string) (*model.TeamMember, *model.AppError) { + joinTeamCalled = true + return &model.TeamMember{}, nil + }, + } + + config := &model.Config{} + config.SetDefaults() + + // Use the old constructor (backwards compatibility) + importer := New(store, actions, config) + + team := &model.Team{ + Id: "test-team-id", + Name: "test-team", + } + + user := &model.User{ + Username: "testuser", + Email: "testuser@restricted-domain.com", + FirstName: "Test", + LastName: "User", + } + + result := importer.oldImportUser(rctx, team, user) + + require.NotNil(t, result, "User import should succeed") + assert.Equal(t, "test-user-id", result.Id, "Should return the saved user") + assert.False(t, verifyEmailCalled, "BACKWARDS COMPATIBILITY: VerifyEmail should NOT be called with old constructor") + assert.True(t, joinTeamCalled, "User should still be joined to the team") + + // Verify VerifyEmail was NOT called + userStore.AssertNotCalled(t, "VerifyEmail") +}
Vulnerability mechanics
Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.
References
6- github.com/advisories/GHSA-3q4q-wqm6-hvf3ghsaADVISORY
- nvd.nist.gov/vuln/detail/CVE-2025-41410ghsaADVISORY
- github.com/mattermost/mattermost/commit/01b95392a450676407475596d1c041a047067329ghsaWEB
- github.com/mattermost/mattermost/commit/0d6e8fa2e4681a172a136db18001104a57f9c28eghsaWEB
- github.com/mattermost/mattermost/commit/ef896a4ea60cacbe03124106e1f42e5c25276427ghsaWEB
- mattermost.com/security-updatesghsaWEB
News mentions
0No linked articles in our index yet.