From 4a1e8f3d98a8d09098497b8683350523257a877d Mon Sep 17 00:00:00 2001
From: Gabriel MABILLE <gamab@users.noreply.github.com>
Date: Fri, 2 Feb 2024 11:12:00 +0100
Subject: [PATCH] RBAC: Reject plugin registrations without a name (#81719)

* RBAC: Reject plugin registrations without a name

* Lint'
---
 pkg/services/accesscontrol/errors.go          | 10 +++++++++
 .../accesscontrol/pluginutils/utils.go        |  3 +++
 .../accesscontrol/pluginutils/utils_test.go   | 21 +++++++++++++------
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/pkg/services/accesscontrol/errors.go b/pkg/services/accesscontrol/errors.go
index 73394a67f51..175f5015329 100644
--- a/pkg/services/accesscontrol/errors.go
+++ b/pkg/services/accesscontrol/errors.go
@@ -21,6 +21,16 @@ func (e *ErrorInvalidRole) Error() string {
 	return "role is invalid"
 }
 
+type ErrorRoleNameMissing struct{}
+
+func (e *ErrorRoleNameMissing) Error() string {
+	return "role has been defined without a name"
+}
+
+func (e *ErrorRoleNameMissing) Unwrap() error {
+	return &ErrorInvalidRole{}
+}
+
 type ErrorRolePrefixMissing struct {
 	Role     string
 	Prefixes []string
diff --git a/pkg/services/accesscontrol/pluginutils/utils.go b/pkg/services/accesscontrol/pluginutils/utils.go
index 362e927afb9..ffbde39295f 100644
--- a/pkg/services/accesscontrol/pluginutils/utils.go
+++ b/pkg/services/accesscontrol/pluginutils/utils.go
@@ -34,6 +34,9 @@ func ValidatePluginRole(pluginID string, role ac.RoleDTO) error {
 	if pluginID == "" {
 		return ac.ErrPluginIDRequired
 	}
+	if role.DisplayName == "" {
+		return &ac.ErrorRoleNameMissing{}
+	}
 	if !strings.HasPrefix(role.Name, ac.PluginRolePrefix+pluginID+":") {
 		return &ac.ErrorRolePrefixMissing{Role: role.Name, Prefixes: []string{ac.PluginRolePrefix + pluginID + ":"}}
 	}
diff --git a/pkg/services/accesscontrol/pluginutils/utils_test.go b/pkg/services/accesscontrol/pluginutils/utils_test.go
index c6432a355ab..38ae6f4765c 100644
--- a/pkg/services/accesscontrol/pluginutils/utils_test.go
+++ b/pkg/services/accesscontrol/pluginutils/utils_test.go
@@ -85,34 +85,41 @@ func TestValidatePluginRole(t *testing.T) {
 		role     ac.RoleDTO
 		wantErr  error
 	}{
+		{
+			name:     "empty display name",
+			pluginID: "test-app",
+			role:     ac.RoleDTO{DisplayName: ""},
+			wantErr:  &ac.ErrorInvalidRole{},
+		},
 		{
 			name:     "empty",
 			pluginID: "",
-			role:     ac.RoleDTO{Name: "plugins::"},
+			role:     ac.RoleDTO{Name: "plugins::reader", DisplayName: "Reader"},
 			wantErr:  ac.ErrPluginIDRequired,
 		},
 		{
 			name:     "invalid name",
 			pluginID: "test-app",
-			role:     ac.RoleDTO{Name: "test-app:reader"},
+			role:     ac.RoleDTO{Name: "test-app:reader", DisplayName: "Reader"},
 			wantErr:  &ac.ErrorInvalidRole{},
 		},
 		{
 			name:     "invalid id in name",
 			pluginID: "test-app",
-			role:     ac.RoleDTO{Name: "plugins:test-app2:reader"},
+			role:     ac.RoleDTO{Name: "plugins:test-app2:reader", DisplayName: "Reader"},
 			wantErr:  &ac.ErrorInvalidRole{},
 		},
 		{
 			name:     "valid name",
 			pluginID: "test-app",
-			role:     ac.RoleDTO{Name: "plugins:test-app:reader"},
+			role:     ac.RoleDTO{Name: "plugins:test-app:reader", DisplayName: "Reader"},
 		},
 		{
 			name:     "invalid permission",
 			pluginID: "test-app",
 			role: ac.RoleDTO{
 				Name:        "plugins:test-app:reader",
+				DisplayName: "Reader",
 				Permissions: []ac.Permission{{Action: "invalidtest-app:read"}},
 			},
 			wantErr: &ac.ErrorInvalidRole{},
@@ -121,7 +128,8 @@ func TestValidatePluginRole(t *testing.T) {
 			name:     "valid permissions",
 			pluginID: "test-app",
 			role: ac.RoleDTO{
-				Name: "plugins:test-app:reader",
+				Name:        "plugins:test-app:reader",
+				DisplayName: "Reader",
 				Permissions: []ac.Permission{
 					{Action: "plugins.app:access", Scope: "plugins:id:test-app"},
 					{Action: "test-app:read"},
@@ -133,7 +141,8 @@ func TestValidatePluginRole(t *testing.T) {
 			name:     "invalid permission targets other plugin",
 			pluginID: "test-app",
 			role: ac.RoleDTO{
-				Name: "plugins:test-app:reader",
+				Name:        "plugins:test-app:reader",
+				DisplayName: "Reader",
 				Permissions: []ac.Permission{
 					{Action: "plugins.app:access", Scope: "plugins:id:other-app"},
 				},