diff --git a/store/localcachelayer/role_layer_test.go b/store/localcachelayer/role_layer_test.go index 60af018dc1..7843a7c4b1 100644 --- a/store/localcachelayer/role_layer_test.go +++ b/store/localcachelayer/role_layer_test.go @@ -14,7 +14,7 @@ import ( ) func TestRoleStore(t *testing.T) { - StoreTest(t, storetest.TestRoleStore) + StoreTestWithSqlSupplier(t, storetest.TestRoleStore) } func TestRoleStoreCache(t *testing.T) { diff --git a/store/sqlstore/role_store.go b/store/sqlstore/role_store.go index 9999a061e6..0de3c23d6f 100644 --- a/store/sqlstore/role_store.go +++ b/store/sqlstore/role_store.go @@ -251,10 +251,10 @@ func (s *SqlRoleStore) PermanentDeleteAll() *model.AppError { func (s *SqlRoleStore) channelHigherScopedPermissionsQuery(roleNames []string) string { sqlTmpl := ` SELECT - RoleSchemes.DefaultChannelGuestRole AS GuestRoleName, + '' AS GuestRoleName, RoleSchemes.DefaultChannelUserRole AS UserRoleName, RoleSchemes.DefaultChannelAdminRole AS AdminRoleName, - GuestRoles.Permissions AS HigherScopedGuestPermissions, + '' AS HigherScopedGuestPermissions, UserRoles.Permissions AS HigherScopedUserPermissions, AdminRoles.Permissions AS HigherScopedAdminPermissions FROM @@ -262,14 +262,32 @@ func (s *SqlRoleStore) channelHigherScopedPermissionsQuery(roleNames []string) s JOIN Channels ON Channels.SchemeId = RoleSchemes.Id JOIN Teams ON Teams.Id = Channels.TeamId JOIN Schemes ON Schemes.Id = Teams.SchemeId - JOIN Roles AS GuestRoles ON GuestRoles.Name = Schemes.DefaultChannelGuestRole - JOIN Roles AS UserRoles ON UserRoles.Name = Schemes.DefaultChannelUserRole - JOIN Roles AS AdminRoles ON AdminRoles.Name = Schemes.DefaultChannelAdminRole + RIGHT JOIN Roles AS UserRoles ON UserRoles.Name = Schemes.DefaultChannelUserRole + RIGHT JOIN Roles AS AdminRoles ON AdminRoles.Name = Schemes.DefaultChannelAdminRole + WHERE + RoleSchemes.DefaultChannelUserRole IN ('%[1]s') + OR RoleSchemes.DefaultChannelAdminRole IN ('%[1]s') + + UNION + + SELECT + RoleSchemes.DefaultChannelGuestRole AS GuestRoleName, + '' AS UserRoleName, + '' AS AdminRoleName, + GuestRoles.Permissions AS HigherScopedGuestPermissions, + '' AS HigherScopedUserPermissions, + '' AS HigherScopedAdminPermissions + FROM + Schemes AS RoleSchemes + JOIN Channels ON Channels.SchemeId = RoleSchemes.Id + JOIN Teams ON Teams.Id = Channels.TeamId + JOIN Schemes ON Schemes.Id = Teams.SchemeId + RIGHT JOIN Roles AS GuestRoles ON GuestRoles.Name = Schemes.DefaultChannelGuestRole WHERE RoleSchemes.DefaultChannelGuestRole IN ('%[1]s') - OR RoleSchemes.DefaultChannelUserRole IN ('%[1]s') - OR RoleSchemes.DefaultChannelAdminRole IN ('%[1]s') + UNION + SELECT Schemes.DefaultChannelGuestRole AS GuestRoleName, Schemes.DefaultChannelUserRole AS UserRoleName, diff --git a/store/sqlstore/role_store_test.go b/store/sqlstore/role_store_test.go index a5aa823ab0..ad882618cc 100644 --- a/store/sqlstore/role_store_test.go +++ b/store/sqlstore/role_store_test.go @@ -10,5 +10,5 @@ import ( ) func TestRoleStore(t *testing.T) { - StoreTest(t, storetest.TestRoleStore) + StoreTestWithSqlSupplier(t, storetest.TestRoleStore) } diff --git a/store/storetest/role_store.go b/store/storetest/role_store.go index df6bbc202b..83ce7d462f 100644 --- a/store/storetest/role_store.go +++ b/store/storetest/role_store.go @@ -4,6 +4,7 @@ package storetest import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -13,7 +14,7 @@ import ( "github.com/mattermost/mattermost-server/v5/store" ) -func TestRoleStore(t *testing.T, ss store.Store) { +func TestRoleStore(t *testing.T, ss store.Store, s SqlSupplier) { t.Run("Save", func(t *testing.T) { testRoleStoreSave(t, ss) }) t.Run("Get", func(t *testing.T) { testRoleStoreGet(t, ss) }) t.Run("GetAll", func(t *testing.T) { testRoleStoreGetAll(t, ss) }) @@ -22,6 +23,7 @@ func TestRoleStore(t *testing.T, ss store.Store) { t.Run("Delete", func(t *testing.T) { testRoleStoreDelete(t, ss) }) t.Run("PermanentDeleteAll", func(t *testing.T) { testRoleStorePermanentDeleteAll(t, ss) }) t.Run("LowerScopedChannelSchemeRoles_AllChannelSchemeRoles", func(t *testing.T) { testRoleStoreLowerScopedChannelSchemeRoles(t, ss) }) + t.Run("ChannelHigherScopedPermissionsBlankTeamSchemeChannelGuest", func(t *testing.T) { testRoleStoreChannelHigherScopedPermissionsBlankTeamSchemeChannelGuest(t, ss, s) }) } func testRoleStoreSave(t *testing.T, ss store.Store) { @@ -513,3 +515,83 @@ func testRoleStoreLowerScopedChannelSchemeRoles(t *testing.T, ss store.Store) { }) }) } + +func testRoleStoreChannelHigherScopedPermissionsBlankTeamSchemeChannelGuest(t *testing.T, ss store.Store, s SqlSupplier) { + teamScheme := &model.Scheme{ + DisplayName: model.NewId(), + Name: model.NewId(), + Description: model.NewId(), + Scope: model.SCHEME_SCOPE_TEAM, + } + teamScheme, err := ss.Scheme().Save(teamScheme) + require.Nil(t, err) + defer ss.Scheme().Delete(teamScheme.Id) + + channelScheme := &model.Scheme{ + DisplayName: model.NewId(), + Name: model.NewId(), + Description: model.NewId(), + Scope: model.SCHEME_SCOPE_CHANNEL, + } + channelScheme, err = ss.Scheme().Save(channelScheme) + require.Nil(t, err) + defer ss.Scheme().Delete(channelScheme.Id) + + team := &model.Team{ + DisplayName: "Name", + Name: "zz" + model.NewId(), + Email: MakeEmail(), + Type: model.TEAM_OPEN, + SchemeId: &teamScheme.Id, + } + team, err = ss.Team().Save(team) + require.Nil(t, err) + defer ss.Team().PermanentDelete(team.Id) + + channel := &model.Channel{ + TeamId: team.Id, + DisplayName: "Display " + model.NewId(), + Name: "zz" + model.NewId() + "b", + Type: model.CHANNEL_OPEN, + SchemeId: &channelScheme.Id, + } + channel, err = ss.Channel().Save(channel, -1) + require.Nil(t, err) + defer ss.Channel().Delete(channel.Id, 0) + + channelSchemeUserRole, err := ss.Role().GetByName(channelScheme.DefaultChannelUserRole) + require.Nil(t, err) + channelSchemeUserRole.Permissions = []string{} + _, err = ss.Role().Save(channelSchemeUserRole) + require.Nil(t, err) + + teamSchemeUserRole, err := ss.Role().GetByName(teamScheme.DefaultChannelUserRole) + require.Nil(t, err) + teamSchemeUserRole.Permissions = []string{model.PERMISSION_UPLOAD_FILE.Id} + _, err = ss.Role().Save(teamSchemeUserRole) + require.Nil(t, err) + + // get the channel scheme user role again and ensure that it has the permission inherited from the team + // scheme user role + roleMapBefore, err := ss.Role().ChannelHigherScopedPermissions([]string{channelSchemeUserRole.Name}) + require.Nil(t, err) + + // blank-out the guest role to simulate an old team scheme, ensure it's blank + result, sqlErr := s.GetMaster().Exec(fmt.Sprintf("UPDATE Schemes SET DefaultChannelGuestRole = '' WHERE Id = '%s'", teamScheme.Id)) + require.Nil(t, sqlErr) + rows, serr := result.RowsAffected() + require.Nil(t, serr) + require.Equal(t, int64(1), rows) + teamScheme, err = ss.Scheme().Get(teamScheme.Id) + require.Nil(t, err) + require.Equal(t, "", teamScheme.DefaultChannelGuestRole) + + // trigger a cache clear + _, err = ss.Role().Save(channelSchemeUserRole) + require.Nil(t, err) + + roleMapAfter, err := ss.Role().ChannelHigherScopedPermissions([]string{channelSchemeUserRole.Name}) + require.Nil(t, err) + + require.Equal(t, len(roleMapBefore), len(roleMapAfter)) +}