From 9de8283077c39963051f0b839ea7ce312c7a5fcb Mon Sep 17 00:00:00 2001 From: Mario de Frutos Dieguez Date: Thu, 30 Jan 2020 09:25:52 +0100 Subject: [PATCH] MM-22067: Check guest email allowance separately (#13776) --- app/team.go | 23 +++++++++----- app/team_test.go | 61 +++++++++++++++++++++++++++++++++++ app/user_test.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 7 deletions(-) diff --git a/app/team.go b/app/team.go index 47ce221cea..60596536e3 100644 --- a/app/team.go +++ b/app/team.go @@ -65,10 +65,8 @@ func (a *App) normalizeDomains(domains string) []string { return strings.Fields(strings.TrimSpace(strings.ToLower(strings.Replace(strings.Replace(domains, "@", " ", -1), ",", " ", -1)))) } -func (a *App) isTeamEmailAddressAllowed(email string, allowedDomains string) bool { - email = strings.ToLower(email) - // First check per team allowedDomains, then app wide restrictions - for _, restriction := range []string{allowedDomains, *a.Config().TeamSettings.RestrictCreationToDomains} { +func (a *App) isEmailAddressAllowed(email string, allowedDomains []string) bool { + for _, restriction := range allowedDomains { domains := a.normalizeDomains(restriction) if len(domains) <= 0 { continue @@ -93,7 +91,16 @@ func (a *App) isTeamEmailAllowed(user *model.User, team *model.Team) bool { return true } email := strings.ToLower(user.Email) - return a.isTeamEmailAddressAllowed(email, team.AllowedDomains) + allowedDomains := a.getAllowedDomains(user, team) + return a.isEmailAddressAllowed(email, allowedDomains) +} + +func (a *App) getAllowedDomains(user *model.User, team *model.Team) []string { + if user.IsGuest() { + return []string{*a.Config().GuestAccountsSettings.RestrictCreationToDomains} + } + // First check per team allowedDomains, then app wide restrictions + return []string{team.AllowedDomains, *a.Config().TeamSettings.RestrictCreationToDomains} } func (a *App) UpdateTeam(team *model.Team) (*model.Team, *model.AppError) { @@ -1086,6 +1093,7 @@ func (a *App) InviteNewUsersToTeamGracefully(emailList []string, teamId, senderI if err != nil { return nil, err } + allowedDomains := a.getAllowedDomains(user, team) var inviteListWithErrors []*model.EmailInviteWithError var goodEmails []string for _, email := range emailList { @@ -1093,7 +1101,7 @@ func (a *App) InviteNewUsersToTeamGracefully(emailList []string, teamId, senderI Email: email, Error: nil, } - if !a.isTeamEmailAddressAllowed(email, team.AllowedDomains) { + if !a.isEmailAddressAllowed(email, allowedDomains) { invite.Error = model.NewAppError("InviteNewUsersToTeam", "api.team.invite_members.invalid_email.app_error", map[string]interface{}{"Addresses": email}, "", http.StatusBadRequest) } else { goodEmails = append(goodEmails, email) @@ -1207,10 +1215,11 @@ func (a *App) InviteNewUsersToTeam(emailList []string, teamId, senderId string) return err } + allowedDomains := a.getAllowedDomains(user, team) var invalidEmailList []string for _, email := range emailList { - if !a.isTeamEmailAddressAllowed(email, team.AllowedDomains) { + if !a.isEmailAddressAllowed(email, allowedDomains) { invalidEmailList = append(invalidEmailList, email) } } diff --git a/app/team_test.go b/app/team_test.go index dac5da2496..f7d2de833e 100644 --- a/app/team_test.go +++ b/app/team_test.go @@ -269,6 +269,67 @@ func TestAddUserToTeamByToken(t *testing.T) { assert.NotNil(t, err) }) + t.Run("invalid add a guest user with a non-granted email domain", func(t *testing.T) { + restrictedDomain := *th.App.Config().GuestAccountsSettings.RestrictCreationToDomains + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.GuestAccountsSettings.RestrictCreationToDomains = &restrictedDomain }) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.RestrictCreationToDomains = "restricted.com" }) + token := model.NewToken( + TOKEN_TYPE_GUEST_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "channels": th.BasicChannel.Id}), + ) + require.Nil(t, th.App.Srv.Store.Token().Save(token)) + _, err := th.App.AddUserToTeamByToken(rguest.Id, token.Token) + require.NotNil(t, err) + assert.Equal(t, "api.team.join_user_to_team.allowed_domains.app_error", err.Id) + }) + + t.Run("add a guest user with a granted email domain", func(t *testing.T) { + restrictedDomain := *th.App.Config().GuestAccountsSettings.RestrictCreationToDomains + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.GuestAccountsSettings.RestrictCreationToDomains = &restrictedDomain }) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.RestrictCreationToDomains = "restricted.com" }) + token := model.NewToken( + TOKEN_TYPE_GUEST_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "channels": th.BasicChannel.Id}), + ) + guestEmail := rguest.Email + rguest.Email = "test@restricted.com" + _, err := th.App.Srv.Store.User().Update(rguest, false) + require.Nil(t, err) + require.Nil(t, th.App.Srv.Store.Token().Save(token)) + _, err = th.App.AddUserToTeamByToken(rguest.Id, token.Token) + require.Nil(t, err) + rguest.Email = guestEmail + _, err = th.App.Srv.Store.User().Update(rguest, false) + require.Nil(t, err) + }) + + t.Run("add a guest user even though there are team and system domain restrictions", func(t *testing.T) { + th.BasicTeam.AllowedDomains = "restricted-team.com" + _, err := th.Server.Store.Team().Update(th.BasicTeam) + require.Nil(t, err) + restrictedDomain := *th.App.Config().TeamSettings.RestrictCreationToDomains + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { cfg.TeamSettings.RestrictCreationToDomains = &restrictedDomain }) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.RestrictCreationToDomains = "restricted.com" }) + token := model.NewToken( + TOKEN_TYPE_GUEST_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "channels": th.BasicChannel.Id}), + ) + _, err = th.App.Srv.Store.User().Update(rguest, false) + require.Nil(t, err) + require.Nil(t, th.App.Srv.Store.Token().Save(token)) + _, err = th.App.AddUserToTeamByToken(rguest.Id, token.Token) + require.Nil(t, err) + th.BasicTeam.AllowedDomains = "" + _, err = th.Server.Store.Team().Update(th.BasicTeam) + require.Nil(t, err) + }) + t.Run("valid request from guest invite", func(t *testing.T) { token := model.NewToken( TOKEN_TYPE_GUEST_INVITATION, diff --git a/app/user_test.go b/app/user_test.go index e8a1236315..5fc8acac22 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -653,6 +653,88 @@ func TestCreateUserWithToken(t *testing.T) { require.Len(t, *members, 1) assert.Equal(t, (*members)[0].ChannelId, th.BasicChannel.Id) }) + + t.Run("create guest having email domain restrictions", func(t *testing.T) { + enableGuestDomainRestricions := *th.App.Config().GuestAccountsSettings.RestrictCreationToDomains + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.GuestAccountsSettings.RestrictCreationToDomains = &enableGuestDomainRestricions + }) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.RestrictCreationToDomains = "restricted.com" }) + forbiddenInvitationEmail := model.NewId() + "other-email@test.com" + grantedInvitationEmail := model.NewId() + "other-email@restricted.com" + forbiddenDomainToken := model.NewToken( + TOKEN_TYPE_GUEST_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": forbiddenInvitationEmail, "channels": th.BasicChannel.Id}), + ) + grantedDomainToken := model.NewToken( + TOKEN_TYPE_GUEST_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": grantedInvitationEmail, "channels": th.BasicChannel.Id}), + ) + require.Nil(t, th.App.Srv.Store.Token().Save(forbiddenDomainToken)) + require.Nil(t, th.App.Srv.Store.Token().Save(grantedDomainToken)) + guest := model.User{ + Email: strings.ToLower(model.NewId()) + "+test@example.com", + Nickname: "Darth Vader", + Username: "vader" + model.NewId(), + Password: "passwd1", + AuthService: "", + } + newGuest, err := th.App.CreateUserWithToken(&guest, forbiddenDomainToken) + require.NotNil(t, err) + require.Nil(t, newGuest) + assert.Equal(t, "api.user.create_user.accepted_domain.app_error", err.Id) + + newGuest, err = th.App.CreateUserWithToken(&guest, grantedDomainToken) + require.Nil(t, err) + assert.True(t, newGuest.IsGuest()) + require.Equal(t, grantedInvitationEmail, newGuest.Email) + _, err = th.App.Srv.Store.Token().GetByToken(grantedDomainToken.Token) + require.NotNil(t, err) + + members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, newGuest.Id) + require.Nil(t, err) + require.Len(t, *members, 1) + assert.Equal(t, (*members)[0].ChannelId, th.BasicChannel.Id) + }) + + t.Run("create guest having team and system email domain restrictions", func(t *testing.T) { + th.BasicTeam.AllowedDomains = "restricted-team.com" + _, err := th.App.UpdateTeam(th.BasicTeam) + require.Nil(t, err, "Should update the team") + enableGuestDomainRestricions := *th.App.Config().TeamSettings.RestrictCreationToDomains + defer func() { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.TeamSettings.RestrictCreationToDomains = &enableGuestDomainRestricions + }) + }() + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.RestrictCreationToDomains = "restricted.com" }) + invitationEmail := model.NewId() + "other-email@test.com" + token := model.NewToken( + TOKEN_TYPE_GUEST_INVITATION, + model.MapToJson(map[string]string{"teamId": th.BasicTeam.Id, "email": invitationEmail, "channels": th.BasicChannel.Id}), + ) + require.Nil(t, th.App.Srv.Store.Token().Save(token)) + guest := model.User{ + Email: strings.ToLower(model.NewId()) + "+test@example.com", + Nickname: "Darth Vader", + Username: "vader" + model.NewId(), + Password: "passwd1", + AuthService: "", + } + newGuest, err := th.App.CreateUserWithToken(&guest, token) + require.Nil(t, err) + assert.True(t, newGuest.IsGuest()) + assert.Equal(t, invitationEmail, newGuest.Email, "The user email must be the invitation one") + _, err = th.App.Srv.Store.Token().GetByToken(token.Token) + require.NotNil(t, err) + + members, err := th.App.GetChannelMembersForUser(th.BasicTeam.Id, newGuest.Id) + require.Nil(t, err) + require.Len(t, *members, 1) + assert.Equal(t, (*members)[0].ChannelId, th.BasicChannel.Id) + }) } func TestPermanentDeleteUser(t *testing.T) {