From 53c74fa2f56a4b1b029be8a71a3ec882d88b86e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Thu, 14 Mar 2019 14:24:13 +0100 Subject: [PATCH] teams: refactor so that you can only delete teams if you are team admin --- pkg/models/team.go | 13 ++++++----- pkg/services/sqlstore/team.go | 19 +++++++++++++--- public/app/features/teams/TeamList.tsx | 7 ++++-- .../app/features/teams/__mocks__/teamMocks.ts | 2 ++ .../__snapshots__/TeamList.test.tsx.snap | 7 ++++++ public/app/features/teams/state/navModel.ts | 3 ++- public/app/features/teams/state/selectors.ts | 22 +++++++++++++++---- public/app/types/teams.ts | 3 +++ 8 files changed, 60 insertions(+), 16 deletions(-) diff --git a/pkg/models/team.go b/pkg/models/team.go index 5b659331601..bc8cbba8100 100644 --- a/pkg/models/team.go +++ b/pkg/models/team.go @@ -73,12 +73,13 @@ type SearchTeamsQuery struct { } type TeamDTO struct { - Id int64 `json:"id"` - OrgId int64 `json:"orgId"` - Name string `json:"name"` - Email string `json:"email"` - AvatarUrl string `json:"avatarUrl"` - MemberCount int64 `json:"memberCount"` + Id int64 `json:"id"` + OrgId int64 `json:"orgId"` + Name string `json:"name"` + Email string `json:"email"` + AvatarUrl string `json:"avatarUrl"` + MemberCount int64 `json:"memberCount"` + Permission PermissionType `json:"permission"` } type SearchTeamQueryResult struct { diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index b561f2e00f6..03fd2df78fc 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -23,13 +23,25 @@ func init() { bus.AddHandler("sql", GetTeamMembers) } +func getTeamSearchSqlBase() string { + return `SELECT + team.id as id, + team.org_id, + team.name as name, + team.email as email, + (SELECT COUNT(*) from team_member where team_member.team_id = team.id) as member_count, + team_member.permission + FROM team as team + INNER JOIN team_member on team.id = team_member.team_id AND team_member.user_id = ? ` +} + func getTeamSelectSqlBase() string { return `SELECT team.id as id, team.org_id, team.name as name, team.email as email, - (SELECT COUNT(*) from team_member where team_member.team_id = team.id) as member_count + (SELECT COUNT(*) from team_member where team_member.team_id = team.id) as member_count FROM team as team ` } @@ -146,10 +158,11 @@ func SearchTeams(query *m.SearchTeamsQuery) error { var sql bytes.Buffer params := make([]interface{}, 0) - sql.WriteString(getTeamSelectSqlBase()) if query.UserIdFilter > 0 { - sql.WriteString(`INNER JOIN team_member on team.id = team_member.team_id AND team_member.user_id = ?`) + sql.WriteString(getTeamSearchSqlBase()) params = append(params, query.UserIdFilter) + } else { + sql.WriteString(getTeamSelectSqlBase()) } sql.WriteString(` WHERE team.org_id = ?`) diff --git a/public/app/features/teams/TeamList.tsx b/public/app/features/teams/TeamList.tsx index f603994b578..e9d51785d72 100644 --- a/public/app/features/teams/TeamList.tsx +++ b/public/app/features/teams/TeamList.tsx @@ -6,7 +6,7 @@ import { DeleteButton } from '@grafana/ui'; import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA'; import { NavModel, Team, OrgRole } from 'app/types'; import { loadTeams, deleteTeam, setSearchQuery } from './state/actions'; -import { getSearchQuery, getTeams, getTeamsCount } from './state/selectors'; +import { getSearchQuery, getTeams, getTeamsCount, isPermissionTeamAdmin } from './state/selectors'; import { getNavModel } from 'app/core/selectors/navModel'; import { FilterInput } from 'app/core/components/FilterInput/FilterInput'; import { config } from 'app/core/config'; @@ -43,7 +43,10 @@ export class TeamList extends PureComponent { }; renderTeam(team: Team) { + const { editorsCanAdmin, signedInUser } = this.props; + const permission = team.permission; const teamUrl = `org/teams/edit/${team.id}`; + const canDelete = isPermissionTeamAdmin({ permission, editorsCanAdmin, signedInUser }); return ( @@ -62,7 +65,7 @@ export class TeamList extends PureComponent { {team.memberCount} - this.deleteTeam(team)} /> + this.deleteTeam(team)} disabled={!canDelete} /> ); diff --git a/public/app/features/teams/__mocks__/teamMocks.ts b/public/app/features/teams/__mocks__/teamMocks.ts index f38f8f2b144..abaa5ef555f 100644 --- a/public/app/features/teams/__mocks__/teamMocks.ts +++ b/public/app/features/teams/__mocks__/teamMocks.ts @@ -9,6 +9,7 @@ export const getMultipleMockTeams = (numberOfTeams: number): Team[] => { avatarUrl: 'some/url/', email: `test-${i}@test.com`, memberCount: i, + permission: TeamPermissionLevel.Member, }); } @@ -22,6 +23,7 @@ export const getMockTeam = (): Team => { avatarUrl: 'some/url/', email: 'test@test.com', memberCount: 1, + permission: TeamPermissionLevel.Member, }; }; diff --git a/public/app/features/teams/__snapshots__/TeamList.test.tsx.snap b/public/app/features/teams/__snapshots__/TeamList.test.tsx.snap index d4dd2170bae..430466559c5 100644 --- a/public/app/features/teams/__snapshots__/TeamList.test.tsx.snap +++ b/public/app/features/teams/__snapshots__/TeamList.test.tsx.snap @@ -133,6 +133,7 @@ exports[`Render should render teams table 1`] = ` className="text-right" > @@ -183,6 +184,7 @@ exports[`Render should render teams table 1`] = ` className="text-right" > @@ -233,6 +235,7 @@ exports[`Render should render teams table 1`] = ` className="text-right" > @@ -283,6 +286,7 @@ exports[`Render should render teams table 1`] = ` className="text-right" > @@ -333,6 +337,7 @@ exports[`Render should render teams table 1`] = ` className="text-right" > @@ -458,6 +463,7 @@ exports[`Render when feature toggle editorsCanAdmin is turned on and signedin us className="text-right" > @@ -583,6 +589,7 @@ exports[`Render when feature toggle editorsCanAdmin is turned on and signedin us className="text-right" > diff --git a/public/app/features/teams/state/navModel.ts b/public/app/features/teams/state/navModel.ts index 2fd5a68e680..aeb6b85f91e 100644 --- a/public/app/features/teams/state/navModel.ts +++ b/public/app/features/teams/state/navModel.ts @@ -1,4 +1,4 @@ -import { Team, NavModelItem, NavModel } from 'app/types'; +import { Team, NavModelItem, NavModel, TeamPermissionLevel } from 'app/types'; import config from 'app/core/config'; export function buildNavModel(team: Team): NavModelItem { @@ -47,6 +47,7 @@ export function getTeamLoadingNav(pageName: string): NavModel { name: 'Loading', email: 'loading', memberCount: 0, + permission: TeamPermissionLevel.Member, }); let node: NavModelItem; diff --git a/public/app/features/teams/state/selectors.ts b/public/app/features/teams/state/selectors.ts index d8b8220bb44..e770abfc093 100644 --- a/public/app/features/teams/state/selectors.ts +++ b/public/app/features/teams/state/selectors.ts @@ -37,10 +37,24 @@ export interface Config { } export const isSignedInUserTeamAdmin = (config: Config): boolean => { - const userInMembers = config.members.find(m => m.userId === config.signedInUser.id); - const isAdmin = config.signedInUser.isGrafanaAdmin || config.signedInUser.orgRole === OrgRole.Admin; - const userIsTeamAdmin = userInMembers && userInMembers.permission === TeamPermissionLevel.Admin; + const { members, signedInUser, editorsCanAdmin } = config; + const userInMembers = members.find(m => m.userId === signedInUser.id); + const permission = userInMembers ? userInMembers.permission : TeamPermissionLevel.Member; + + return isPermissionTeamAdmin({ permission, signedInUser, editorsCanAdmin }); +}; + +export interface PermissionConfig { + permission: TeamPermissionLevel; + editorsCanAdmin: boolean; + signedInUser: User; +} + +export const isPermissionTeamAdmin = (config: PermissionConfig): boolean => { + const { permission, signedInUser, editorsCanAdmin } = config; + const isAdmin = signedInUser.isGrafanaAdmin || signedInUser.orgRole === OrgRole.Admin; + const userIsTeamAdmin = permission === TeamPermissionLevel.Admin; const isSignedInUserTeamAdmin = isAdmin || userIsTeamAdmin; - return isSignedInUserTeamAdmin || !config.editorsCanAdmin; + return isSignedInUserTeamAdmin || !editorsCanAdmin; }; diff --git a/public/app/types/teams.ts b/public/app/types/teams.ts index ef804e437d4..707ff97b738 100644 --- a/public/app/types/teams.ts +++ b/public/app/types/teams.ts @@ -1,9 +1,12 @@ +import { TeamPermissionLevel } from './acl'; + export interface Team { id: number; name: string; avatarUrl: string; email: string; memberCount: number; + permission: TeamPermissionLevel; } export interface TeamMember {