From 3ffff632becdb389da1ade4c60e82aa903445991 Mon Sep 17 00:00:00 2001
From: Gabriel MABILLE <gamab@users.noreply.github.com>
Date: Wed, 17 May 2023 16:28:14 +0200
Subject: [PATCH] RBAC: Refine validation of external services permissions
 (#68633)

* RBAC: Refine validation of external services permissions

* Forgot to log the ext-id
---
 pkg/services/accesscontrol/models.go      | 14 +++++++++
 pkg/services/accesscontrol/models_test.go | 38 ++++++++++++++++++++---
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go
index aadd63aea7f..e16c59749e3 100644
--- a/pkg/services/accesscontrol/models.go
+++ b/pkg/services/accesscontrol/models.go
@@ -271,9 +271,23 @@ func (cmd *SaveExternalServiceRoleCommand) Validate() error {
 		return fmt.Errorf("invalid org id %d for global role %t", cmd.OrgID, cmd.Global)
 	}
 
+	// Check and deduplicate permissions
 	if cmd.Permissions == nil || len(cmd.Permissions) == 0 {
 		return errors.New("no permissions provided")
 	}
+	dedupMap := map[Permission]bool{}
+	dedup := make([]Permission, 0, len(cmd.Permissions))
+	for i := range cmd.Permissions {
+		if len(cmd.Permissions[i].Action) == 0 {
+			return fmt.Errorf("external service %v requests a permission with no Action", cmd.ExternalServiceID)
+		}
+		if dedupMap[cmd.Permissions[i]] {
+			continue
+		}
+		dedupMap[cmd.Permissions[i]] = true
+		dedup = append(dedup, cmd.Permissions[i])
+	}
+	cmd.Permissions = dedup
 
 	if cmd.ServiceAccountID <= 0 {
 		return fmt.Errorf("invalid service account id %d", cmd.ServiceAccountID)
diff --git a/pkg/services/accesscontrol/models_test.go b/pkg/services/accesscontrol/models_test.go
index a8503442f38..6990cdc2b7f 100644
--- a/pkg/services/accesscontrol/models_test.go
+++ b/pkg/services/accesscontrol/models_test.go
@@ -8,10 +8,11 @@ import (
 
 func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) {
 	tests := []struct {
-		name    string
-		cmd     SaveExternalServiceRoleCommand
-		wantID  string
-		wantErr bool
+		name            string
+		cmd             SaveExternalServiceRoleCommand
+		wantID          string
+		wantPermissions []Permission
+		wantErr         bool
 	}{
 		{
 			name: "invalid global statement",
@@ -64,6 +65,32 @@ func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) {
 			wantErr: false,
 			wantID:  "thisis-a-very-strange-app-name",
 		},
+		{
+			name: "invalid empty Action",
+			cmd: SaveExternalServiceRoleCommand{
+				OrgID:             1,
+				ExternalServiceID: "app 1",
+				ServiceAccountID:  2,
+				Permissions:       []Permission{{Action: "", Scope: "users:id:1"}},
+			},
+			wantID:  "app-1",
+			wantErr: true,
+		},
+		{
+			name: "permission deduplication",
+			cmd: SaveExternalServiceRoleCommand{
+				OrgID:             1,
+				ExternalServiceID: "app 1",
+				ServiceAccountID:  2,
+				Permissions: []Permission{
+					{Action: "users:read", Scope: "users:id:1"},
+					{Action: "users:read", Scope: "users:id:1"},
+				},
+			},
+			wantErr:         false,
+			wantID:          "app-1",
+			wantPermissions: []Permission{{Action: "users:read", Scope: "users:id:1"}},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -75,6 +102,9 @@ func TestSaveExternalServiceRoleCommand_Validate(t *testing.T) {
 			require.NoError(t, err)
 
 			require.Equal(t, tt.wantID, tt.cmd.ExternalServiceID)
+			if tt.wantPermissions != nil {
+				require.ElementsMatch(t, tt.wantPermissions, tt.cmd.Permissions)
+			}
 		})
 	}
 }