Skip to content

Commit d6c468c

Browse files
Jguerkalleep
andauthored
Auth: Add empty role definition (grafana#64694)
* Allow setting role as None Co-authored-by: gamab <[email protected]> Seeking for places where role.None would be used Co-authored-by: Jguer <[email protected]> Adding None role to the frontend Co-authored-by: Jguer <[email protected]> unify org role declaration and remove from add permission fix backend test fix backend lint * remove role none from frontend * Simplify checks Co-authored-by: Kalle Persson <[email protected]> * nits --------- Co-authored-by: Kalle Persson <[email protected]>
1 parent b6fbf30 commit d6c468c

File tree

12 files changed

+103
-39
lines changed

12 files changed

+103
-39
lines changed

packages/grafana-data/src/types/orgs.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ export interface UserOrgDTO {
55
}
66

77
export enum OrgRole {
8-
Admin = 'Admin',
9-
Editor = 'Editor',
8+
None = 'None',
109
Viewer = 'Viewer',
10+
Editor = 'Editor',
11+
Admin = 'Admin',
1112
}

pkg/login/social/azuread_oauth.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ func (s *SocialAzureAD) extractRoleAndAdmin(claims *azureClaims) (org.RoleType,
192192
return s.defaultRole(false), false
193193
}
194194

195-
roleOrder := []org.RoleType{RoleGrafanaAdmin, org.RoleAdmin, org.RoleEditor, org.RoleViewer}
195+
roleOrder := []org.RoleType{RoleGrafanaAdmin, org.RoleAdmin, org.RoleEditor,
196+
org.RoleViewer, org.RoleNone}
196197
for _, role := range roleOrder {
197198
if found := hasRole(claims.Roles, role); found {
198199
if role == RoleGrafanaAdmin {

pkg/middleware/auth.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import (
2020

2121
type AuthOptions struct {
2222
ReqGrafanaAdmin bool
23-
ReqSignedIn bool
2423
ReqNoAnonynmous bool
24+
ReqSignedIn bool
2525
}
2626

2727
func accessForbidden(c *contextmodel.ReqContext) {

pkg/models/roletype/role_type.go

+35-24
Original file line numberDiff line numberDiff line change
@@ -9,47 +9,58 @@ import (
99
type RoleType string
1010

1111
const (
12+
RoleNone RoleType = "None"
1213
RoleViewer RoleType = "Viewer"
1314
RoleEditor RoleType = "Editor"
1415
RoleAdmin RoleType = "Admin"
1516
)
1617

17-
func (r RoleType) IsValid() bool {
18-
return r == RoleViewer || r == RoleAdmin || r == RoleEditor
18+
var rolePrecedence = map[RoleType]int{
19+
RoleNone: 10,
20+
RoleViewer: 20,
21+
RoleEditor: 30,
22+
RoleAdmin: 40,
1923
}
2024

21-
func (r RoleType) Includes(other RoleType) bool {
22-
if r == RoleAdmin {
23-
return true
24-
}
25+
// Needed to keep stable order
26+
var roleOrder = [...]RoleType{
27+
RoleNone,
28+
RoleViewer,
29+
RoleEditor,
30+
RoleAdmin,
31+
}
2532

26-
if r == RoleEditor {
27-
return other != RoleAdmin
28-
}
33+
func (r RoleType) IsValid() bool {
34+
_, ok := rolePrecedence[r]
35+
return ok
36+
}
2937

30-
return r == other
38+
func (r RoleType) Includes(other RoleType) bool {
39+
return rolePrecedence[r] >= rolePrecedence[other]
3140
}
3241

3342
func (r RoleType) Children() []RoleType {
34-
switch r {
35-
case RoleAdmin:
36-
return []RoleType{RoleEditor, RoleViewer}
37-
case RoleEditor:
38-
return []RoleType{RoleViewer}
39-
default:
40-
return nil
43+
children := make([]RoleType, 0, 3)
44+
45+
for _, role := range roleOrder {
46+
if rolePrecedence[r] > rolePrecedence[role] {
47+
children = append(children, role)
48+
}
4149
}
50+
51+
return children
4252
}
4353

4454
func (r RoleType) Parents() []RoleType {
45-
switch r {
46-
case RoleEditor:
47-
return []RoleType{RoleAdmin}
48-
case RoleViewer:
49-
return []RoleType{RoleEditor, RoleAdmin}
50-
default:
51-
return nil
55+
parents := make([]RoleType, 0, 3)
56+
57+
for _, role := range roleOrder {
58+
if rolePrecedence[r] < rolePrecedence[role] {
59+
parents = append(parents, role)
60+
}
5261
}
62+
63+
return parents
5364
}
5465

5566
func (r *RoleType) UnmarshalText(data []byte) error {

pkg/services/accesscontrol/errors.go

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
var (
99
ErrFixedRolePrefixMissing = errors.New("fixed role should be prefixed with '" + FixedRolePrefix + "'")
1010
ErrInvalidBuiltinRole = errors.New("built-in role is not valid")
11+
ErrNoneRoleAssignment = errors.New("none role cannot receive permissions")
1112
ErrInvalidScope = errors.New("invalid scope")
1213
ErrResolverNotFound = errors.New("no resolver found")
1314
ErrPluginIDRequired = errors.New("plugin ID is required")

pkg/services/accesscontrol/roles.go

+14
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,9 @@ func ValidateFixedRole(role RoleDTO) error {
264264
// ValidateBuiltInRoles errors when a built-in role does not match expected pattern
265265
func ValidateBuiltInRoles(builtInRoles []string) error {
266266
for _, br := range builtInRoles {
267+
if org.RoleType(br) == org.RoleNone {
268+
return ErrNoneRoleAssignment
269+
}
267270
if !org.RoleType(br).IsValid() && br != RoleGrafanaAdmin {
268271
return fmt.Errorf("'%s' %w", br, ErrInvalidBuiltinRole)
269272
}
@@ -327,6 +330,17 @@ func BuildBasicRoleDefinitions() map[string]*RoleDTO {
327330
Permissions: []Permission{},
328331
Hidden: true,
329332
},
333+
string(org.RoleNone): {
334+
Name: BasicRolePrefix + "none",
335+
UID: BasicRoleUIDPrefix + "none",
336+
OrgID: GlobalOrgID,
337+
Version: 1,
338+
DisplayName: string(org.RoleNone),
339+
Description: "None role",
340+
Group: "Basic",
341+
Permissions: []Permission{},
342+
Hidden: true,
343+
},
330344
RoleGrafanaAdmin: {
331345
Name: BasicRolePrefix + "grafana_admin",
332346
UID: BasicRoleUIDPrefix + "grafana_admin",

pkg/services/org/model.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,10 @@ type OrgUser struct {
4747
type RoleType = roletype.RoleType
4848

4949
const (
50-
RoleViewer RoleType = "Viewer"
51-
RoleEditor RoleType = "Editor"
52-
RoleAdmin RoleType = "Admin"
50+
RoleNone RoleType = roletype.RoleNone
51+
RoleViewer RoleType = roletype.RoleViewer
52+
RoleEditor RoleType = roletype.RoleEditor
53+
RoleAdmin RoleType = roletype.RoleAdmin
5354
)
5455

5556
type CreateOrgCommand struct {

pkg/services/serviceaccounts/database/store_test.go

+34
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,40 @@ func TestStore_CreateServiceAccount(t *testing.T) {
7777
})
7878
}
7979

80+
func TestStore_CreateServiceAccountRoleNone(t *testing.T) {
81+
_, store := setupTestDatabase(t)
82+
orgQuery := &org.CreateOrgCommand{Name: orgimpl.MainOrgName}
83+
orgResult, err := store.orgService.CreateWithMember(context.Background(), orgQuery)
84+
require.NoError(t, err)
85+
86+
serviceAccountName := "new Service Account"
87+
serviceAccountOrgId := orgResult.ID
88+
serviceAccountRole := org.RoleNone
89+
saForm := serviceaccounts.CreateServiceAccountForm{
90+
Name: serviceAccountName,
91+
Role: &serviceAccountRole,
92+
IsDisabled: nil,
93+
}
94+
95+
saDTO, err := store.CreateServiceAccount(context.Background(), serviceAccountOrgId, &saForm)
96+
require.NoError(t, err)
97+
assert.Equal(t, "sa-new-service-account", saDTO.Login)
98+
assert.Equal(t, serviceAccountName, saDTO.Name)
99+
assert.Equal(t, 0, int(saDTO.Tokens))
100+
101+
retrieved, err := store.RetrieveServiceAccount(context.Background(), serviceAccountOrgId, saDTO.Id)
102+
require.NoError(t, err)
103+
assert.Equal(t, "sa-new-service-account", retrieved.Login)
104+
assert.Equal(t, serviceAccountName, retrieved.Name)
105+
assert.Equal(t, serviceAccountOrgId, retrieved.OrgId)
106+
assert.Equal(t, string(serviceAccountRole), retrieved.Role)
107+
108+
retrievedId, err := store.RetrieveServiceAccountIdByName(context.Background(), serviceAccountOrgId, serviceAccountName)
109+
require.NoError(t, err)
110+
assert.Equal(t, saDTO.Id, retrievedId)
111+
assert.Equal(t, saDTO.Role, string(org.RoleNone))
112+
}
113+
80114
func TestStore_DeleteServiceAccount(t *testing.T) {
81115
cases := []struct {
82116
desc string

pkg/services/sqlstore/migrations/ualert/permissions.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,14 @@ import (
1717
type roleType string
1818

1919
const (
20+
RoleNone roleType = "None"
2021
RoleViewer roleType = "Viewer"
2122
RoleEditor roleType = "Editor"
2223
RoleAdmin roleType = "Admin"
2324
)
2425

2526
func (r roleType) IsValid() bool {
26-
return r == RoleViewer || r == RoleAdmin || r == RoleEditor
27+
return r == RoleViewer || r == RoleAdmin || r == RoleEditor || r == RoleNone
2728
}
2829

2930
type permissionType int

public/app/core/components/AccessControl/AddPermission.tsx

+3-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ export const AddPermission = ({
8787
{target === PermissionTarget.BuiltInRole && (
8888
<Select
8989
aria-label={'Built-in role picker'}
90-
options={Object.values(OrgRole).map((r) => ({ value: r, label: r }))}
90+
options={Object.values(OrgRole)
91+
.filter((r) => r !== OrgRole.None)
92+
.map((r) => ({ value: r, label: r }))}
9193
onChange={(r) => setBuiltinRole(r.value || '')}
9294
width="auto"
9395
/>

public/app/core/components/RolePicker/RolePickerMenu.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ interface RolesCollectionEntry {
2929
roles: Role[];
3030
}
3131

32-
const BasicRoles = Object.values(OrgRole);
32+
const BasicRoles = Object.values(OrgRole).filter((r) => r !== OrgRole.None);
3333
const BasicRoleOption: Array<SelectableValue<OrgRole>> = BasicRoles.map((r) => ({
3434
label: r,
3535
value: r,

public/app/types/acl.ts

+3-5
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
1+
import { OrgRole } from '@grafana/data';
2+
13
export enum TeamPermissionLevel {
24
Admin = 4,
35
Editor = 2,
46
Member = 0,
57
Viewer = 1,
68
}
79

8-
export enum OrgRole {
9-
Viewer = 'Viewer',
10-
Editor = 'Editor',
11-
Admin = 'Admin',
12-
}
10+
export { OrgRole as OrgRole };
1311

1412
export interface DashboardAclDTO {
1513
id?: number;

0 commit comments

Comments
 (0)