Alerting: separate out silence auth service preconditions checks (#87998)

* Alerting: separate out silence auth service preconditions checks Will be useful for subsequent PR that adds metadata to silence response * Add silence read wildcard scope to precondition for read all silences
2025-01-16 19:52:33 -06:00 · 2024-05-23 12:34:42 -04:00 · 2024-05-23 12:34:42 -04:00 · bc5d077b30
commit bc5d077b30
parent 33db776c91
2 changed files with 240 additions and 94 deletions
--- a/pkg/services/ngalert/accesscontrol/silences.go
+++ b/pkg/services/ngalert/accesscontrol/silences.go
@ -23,7 +23,7 @@ const (

 var (
 	// asserts full read-only access to silences
-	readAllSilencesEvaluator = ac.EvalPermission(instancesRead)
+	readAllSilencesEvaluator = ac.EvalAny(ac.EvalPermission(instancesRead), ac.EvalPermission(silenceRead, dashboards.ScopeFoldersProvider.GetResourceAllScope()))
 	// shortcut assertion that to check if user can read silences
 	readSomeSilenceEvaluator = ac.EvalAny(ac.EvalPermission(instancesRead), ac.EvalPermission(silenceRead))
 	// asserts whether user has read access to rules in a specific folder
@ -97,157 +97,219 @@ func NewSilenceService(ac ac.AccessControl, store RuleUIDToNamespaceStore) *Sile
 	}
 }

+// silenceWithFolder is a helper struct that holds a silence and its associated rule and folder UIDs.
+type silenceWithFolder struct {
+	*models.Silence
+	ruleUID   *string
+	folderUID string
+}
+
 // FilterByAccess filters the given list of silences based on the access control permissions of the user.
 // Global silence (one that is not attached to a particular rule) is considered available to all users.
 // For silences that are not attached to a rule, are checked against authorization.
 // This method is more preferred when many silences need to be checked.
 func (s SilenceService) FilterByAccess(ctx context.Context, user identity.Requester, silences ...*models.Silence) ([]*models.Silence, error) {
-	canAll, err := s.HasAccess(ctx, user, readAllSilencesEvaluator)
-	if err != nil || canAll { // return early if user can either read all silences or there is an error
-		return silences, err
-	}
-	canSome, err := s.HasAccess(ctx, user, readSomeSilenceEvaluator)
-	if err != nil || !canSome {
+	canAll, err := s.authorizeReadSilencePreConditions(ctx, user)
+	if err != nil {
 		return nil, err
 	}
-	result := make([]*models.Silence, 0, len(silences))
-	silencesByRuleUID := make(map[string][]*models.Silence, len(silences))
-	for _, silence := range silences {
-		ruleUID := silence.GetRuleUID()
-		if ruleUID == nil { // if this is a general silence
-			result = append(result, silence)
-			continue
-		}
-		key := *ruleUID
-		silencesByRuleUID[key] = append(silencesByRuleUID[key], silence)
+	if canAll {
+		return silences, nil
 	}
-	if len(silencesByRuleUID) == 0 { // if only general silences are provided no need in other checks
-		return result, nil
-	}
-	namespacesByRuleUID, err := s.store.GetNamespacesByRuleUID(ctx, user.GetOrgID(), maps.Keys(silencesByRuleUID)...)
+
+	silencesWithFolders, err := s.withFolders(ctx, user.GetOrgID(), silences...)
 	if err != nil {
 		return nil, err
 	}

+	result := make([]*models.Silence, 0, len(silences))
 	namespacesByAccess := make(map[string]bool) // caches results of permissions check for each namespace to avoid repeated checks for the same folder
-	for ruleUID, silence := range silencesByRuleUID {
-		ns, ok := namespacesByRuleUID[ruleUID]
-		if !ok { // this means that there is no rule with such UID.
-			continue
-		}
-		hasAccess, ok := namespacesByAccess[ns]
+	for _, silWithFolder := range silencesWithFolders {
+		hasAccess, ok := namespacesByAccess[silWithFolder.folderUID]
 		if !ok {
-			hasAccess, err = s.HasAccess(ctx, user, readRuleSilenceEvaluator(ns))
-			if err != nil {
-				return nil, err
+			hasAccess = s.authorizeReadSilence(ctx, user, silWithFolder) == nil
+
+			// Cache non-empty namespaces to avoid repeated checks for the same folder.
+			if silWithFolder.folderUID != "" {
+				namespacesByAccess[silWithFolder.folderUID] = hasAccess
 			}
-			namespacesByAccess[ns] = hasAccess
 		}
 		if hasAccess {
-			result = append(result, silence...)
+			result = append(result, silWithFolder.Silence)
 		}
 	}
 	return result, nil
 }

-// AuthorizeReadSilence checks if user has access to read a silence
+// AuthorizeReadSilence checks if user has access to read a silence.
 func (s SilenceService) AuthorizeReadSilence(ctx context.Context, user identity.Requester, silence *models.Silence) error {
-	canAll, err := s.HasAccess(ctx, user, readAllSilencesEvaluator)
+	canAll, err := s.authorizeReadSilencePreConditions(ctx, user)
 	if canAll || err != nil { // return early if user can either read all silences or there is error
 		return err
 	}

-	can, err := s.HasAccess(ctx, user, readSomeSilenceEvaluator)
-	if err != nil {
-		return err
-	}
-	if !can { // User does not have silence permissions at all.
-		return NewAuthorizationErrorWithPermissions("read any silences", readSomeSilenceEvaluator)
-	}
-	ruleUID := silence.GetRuleUID()
-	if ruleUID == nil {
-		return nil // no rule UID means that this is a general silence and at this point the user can read them
-	}
-
-	// otherwise resolve rule key to the action's scope
-	folderUID, err := s.ruleUIDToFolderUID(ctx, user.GetOrgID(), *ruleUID)
-	if err != nil {
+	silWithFolder, err := s.withFolders(ctx, user.GetOrgID(), silence)
+	if err != nil || len(silWithFolder) != 1 {
 		return fmt.Errorf("resolve rule UID to folder UID: %w", err)
 	}
-	if folderUID == "" { // if we did not find folder by rule UID then it does not exist.
-		return NewAuthorizationErrorGeneric(fmt.Sprintf("read silence for rule %s", *ruleUID))
+
+	return s.authorizeReadSilence(ctx, user, silWithFolder[0])
+}
+
+// authorizeReadSilencePreConditions checks necessary preconditions for reading silences. Returns true if user can
+// read all silences. Returns error if user does not have access to read any silences.
+func (s SilenceService) authorizeReadSilencePreConditions(ctx context.Context, user identity.Requester) (bool, error) {
+	canAll, err := s.HasAccess(ctx, user, readAllSilencesEvaluator)
+	if canAll || err != nil { // return early if user can either read all silences or there is error
+		return canAll, err
 	}
-	return s.HasAccessOrError(ctx, user, readRuleSilenceEvaluator(folderUID), func() string {
+
+	can, err := s.HasAccess(ctx, user, readSomeSilenceEvaluator)
+	if err != nil {
+		return false, err
+	}
+	if !can { // User does not have silence permissions at all.
+		return false, NewAuthorizationErrorWithPermissions("read any silences", readSomeSilenceEvaluator)
+	}
+	return false, nil
+}
+
+// authorizeReadSilence checks if user has access to read a silence given precondition checks have passed.
+func (s SilenceService) authorizeReadSilence(ctx context.Context, user identity.Requester, silence *silenceWithFolder) error {
+	if silence.ruleUID == nil {
+		return nil // No rule metadata means that this is a general silence and at this point the user can read them
+	}
+
+	if silence.folderUID == "" { // if we did not find folder by rule UID then it does not exist.
+		return NewAuthorizationErrorGeneric(fmt.Sprintf("read silence for rule %s", *silence.ruleUID))
+	}
+	return s.HasAccessOrError(ctx, user, readRuleSilenceEvaluator(silence.folderUID), func() string {
 		return "read silence"
 	})
 }

 // AuthorizeCreateSilence checks if user has access to create a silence. Returns ErrAuthorizationBase if user is not authorized
 func (s SilenceService) AuthorizeCreateSilence(ctx context.Context, user identity.Requester, silence *models.Silence) error {
+	canAny, err := s.authorizeCreateSilencePreConditions(ctx, user)
+	if canAny || err != nil { // return early if user can either create any silence or there is an error
+		return err
+	}
+
+	silWithFolder, err := s.withFolders(ctx, user.GetOrgID(), silence)
+	if err != nil || len(silWithFolder) != 1 {
+		return fmt.Errorf("resolve rule UID to folder UID: %w", err)
+	}
+
+	return s.authorizeCreateSilence(ctx, user, silWithFolder[0])
+}
+
+// authorizeCreateSilencePreConditions checks necessary preconditions for creating silences. Returns true if user can
+// create any silence. Returns error if user does not have access to create any silences at all.
+func (s SilenceService) authorizeCreateSilencePreConditions(ctx context.Context, user identity.Requester) (bool, error) {
 	canAny, err := s.HasAccess(ctx, user, createAnySilenceEvaluator)
 	if err != nil || canAny {
 		// return early if user can either create any silence or there is an error
-		return err
+		return canAny, err
 	}
-	ruleUID := silence.GetRuleUID()
-	if ruleUID == nil {
+
+	// pre-check whether a user has at least some basic permissions before hit the store
+	if err := s.HasAccessOrError(ctx, user, createSomeRuleSilenceEvaluator, func() string { return "create any silences" }); err != nil {
+		return false, err
+	}
+
+	return false, nil
+}
+
+// authorizeCreateSilence checks if user has access to create a silence given precondition checks have passed.
+func (s SilenceService) authorizeCreateSilence(ctx context.Context, user identity.Requester, silence *silenceWithFolder) error {
+	if silence.ruleUID == nil {
 		return s.HasAccessOrError(ctx, user, createGeneralSilenceEvaluator, func() string {
 			return "create a general silence"
 		})
 	}
-	// pre-check whether a user has at least some basic permissions before hit the store
-	if err := s.HasAccessOrError(ctx, user, createSomeRuleSilenceEvaluator, func() string { return "create any silences" }); err != nil {
-		return err
+
+	if silence.folderUID == "" { // if we did not find folder by rule UID then it does not exist.
+		return NewAuthorizationErrorGeneric(fmt.Sprintf("create silence for rule %s", *silence.ruleUID))
 	}
-	folderUID, err := s.ruleUIDToFolderUID(ctx, user.GetOrgID(), *ruleUID)
-	if err != nil {
-		return fmt.Errorf("resolve rule UID to folder UID: %w", err)
-	}
-	if folderUID == "" { // if we did not find folder by rule UID then it does not exist.
-		return NewAuthorizationErrorGeneric(fmt.Sprintf("create silence for rule %s", *ruleUID))
-	}
-	return s.HasAccessOrError(ctx, user, createRuleSilenceEvaluator(folderUID), func() string {
-		return fmt.Sprintf("create silence for rule %s", *ruleUID)
+	return s.HasAccessOrError(ctx, user, createRuleSilenceEvaluator(silence.folderUID), func() string {
+		return fmt.Sprintf("create silence for rule %s", *silence.ruleUID)
 	})
 }

 // AuthorizeUpdateSilence checks if user has access to update\expire a silence. Returns ErrAuthorizationBase if user is not authorized
 func (s SilenceService) AuthorizeUpdateSilence(ctx context.Context, user identity.Requester, silence *models.Silence) error {
+	canAny, err := s.authorizeUpdateSilencePreConditions(ctx, user)
+	if canAny || err != nil { // return early if user can either update any silence or there is an error
+		return err
+	}
+
+	silWithFolder, err := s.withFolders(ctx, user.GetOrgID(), silence)
+	if err != nil || len(silWithFolder) != 1 {
+		return fmt.Errorf("resolve rule UID to folder UID: %w", err)
+	}
+
+	return s.authorizeUpdateSilence(ctx, user, silWithFolder[0])
+}
+
+// authorizeUpdateSilencePreConditions checks necessary preconditions for updating silences. Returns true if user can
+// update any silence. Returns error if user does not have access to update any silences at all.
+func (s SilenceService) authorizeUpdateSilencePreConditions(ctx context.Context, user identity.Requester) (bool, error) {
 	canAny, err := s.HasAccess(ctx, user, updateAnySilenceEvaluator)
 	if err != nil || canAny {
 		// return early if user can either update any silence or there is an error
-		return err
+		return canAny, err
 	}
-	ruleUID := silence.GetRuleUID()
-	if ruleUID == nil {
+
+	// pre-check whether a user has at least some basic permissions before hit the store
+	if err := s.HasAccessOrError(ctx, user, updateSomeRuleSilenceEvaluator, func() string { return "update some silences" }); err != nil {
+		return false, err
+	}
+
+	return false, nil
+}
+
+// authorizeUpdateSilence checks if user has access to update a silence given precondition checks have passed.
+func (s SilenceService) authorizeUpdateSilence(ctx context.Context, user identity.Requester, silence *silenceWithFolder) error {
+	if silence.ruleUID == nil {
 		return s.HasAccessOrError(ctx, user, updateGeneralSilenceEvaluator, func() string {
 			return "update a general silence"
 		})
 	}
-	// pre-check whether a user has at least some basic permissions before hit the store
-	if err := s.HasAccessOrError(ctx, user, updateSomeRuleSilenceEvaluator, func() string { return "update any silences" }); err != nil {
-		return err
+
+	if silence.folderUID == "" { // if we did not find folder by rule UID then it does not exist.
+		return NewAuthorizationErrorGeneric(fmt.Sprintf("create update for rule %s", *silence.ruleUID))
 	}
-	folderUID, err := s.ruleUIDToFolderUID(ctx, user.GetOrgID(), *ruleUID)
-	if err != nil {
-		return fmt.Errorf("resolve rule UID to folder UID: %w", err)
-	}
-	if folderUID == "" { // if we did not find folder by rule UID then it does not exist.
-		return NewAuthorizationErrorGeneric(fmt.Sprintf("update silence for rule %s", *ruleUID))
-	}
-	return s.HasAccessOrError(ctx, user, updateRuleSilenceEvaluator(folderUID), func() string {
-		return fmt.Sprintf("update silence for rule %s", *ruleUID)
+	return s.HasAccessOrError(ctx, user, updateRuleSilenceEvaluator(silence.folderUID), func() string {
+		return fmt.Sprintf("update silence for rule %s", *silence.ruleUID)
 	})
 }

-func (s SilenceService) ruleUIDToFolderUID(ctx context.Context, orgID int64, ruleUID string) (string, error) {
-	namespaces, err := s.store.GetNamespacesByRuleUID(ctx, orgID, ruleUID)
+// withFolders resolves rule UIDs to folder UIDs for rule-specific silences and returns a list of silenceWithFolder
+// that includes rule information, if available.
+func (s SilenceService) withFolders(ctx context.Context, orgID int64, silences ...*models.Silence) ([]*silenceWithFolder, error) {
+	result := make([]*silenceWithFolder, 0, len(silences))
+	ruleUIDs := make(map[string]struct{})
+	for _, silence := range silences {
+		silWithFolder := silenceWithFolder{Silence: silence, ruleUID: silence.GetRuleUID()}
+		if silWithFolder.ruleUID != nil {
+			ruleUIDs[*silWithFolder.ruleUID] = struct{}{}
+		}
+		result = append(result, &silWithFolder)
+	}
+
+	if len(ruleUIDs) == 0 {
+		return result, nil
+	}
+
+	namespaceByRuleUID, err := s.store.GetNamespacesByRuleUID(ctx, orgID, maps.Keys(ruleUIDs)...)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
-	uid, ok := namespaces[ruleUID]
-	if !ok {
-		return "", nil
+
+	for _, silWithFolder := range result {
+		if silWithFolder.ruleUID != nil {
+			silWithFolder.folderUID = namespaceByRuleUID[*silWithFolder.ruleUID]
+		}
 	}
-	return uid, nil
+	return result, nil
 }
--- a/pkg/services/ngalert/accesscontrol/silences_test.go
+++ b/pkg/services/ngalert/accesscontrol/silences_test.go
@ -41,12 +41,14 @@ func TestFilterByAccess(t *testing.T) {
 		name             string
 		user             identity.Requester
 		expected         []*models.Silence
+		expectedErr      error
 		expectedDbAccess bool
 	}{
 		{
-			name:             "no silence access, empty list",
+			name:             "no silence access, cannot read",
 			user:             newUser(),
 			expected:         []*models.Silence{},
+			expectedErr:      ErrAuthorizationBase,
 			expectedDbAccess: false,
 		},
 		{
@ -60,6 +62,17 @@ func TestFilterByAccess(t *testing.T) {
 			},
 			expectedDbAccess: false,
 		},
+		{
+			name: "silence wildcard should get all",
+			user: newUser(ac.Permission{Action: silenceRead, Scope: dashboards.ScopeFoldersProvider.GetResourceAllScope()}),
+			expected: []*models.Silence{
+				global,
+				ruleSilence1,
+				ruleSilence2,
+				notFoundRule,
+			},
+			expectedDbAccess: false,
+		},
 		{
 			name: "silence reader should get global + folder",
 			user: newUser(ac.Permission{Action: silenceRead, Scope: folder1Scope}),
@ -69,6 +82,14 @@ func TestFilterByAccess(t *testing.T) {
 			},
 			expectedDbAccess: true,
 		},
+		{
+			name: "silence reader with no accessible rule silences, global only",
+			user: newUser(ac.Permission{Action: silenceRead, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("unknown-folder")}),
+			expected: []*models.Silence{
+				global,
+			},
+			expectedDbAccess: true,
+		},
 	}
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
@ -83,8 +104,12 @@ func TestFilterByAccess(t *testing.T) {

 			actual, err := svc.FilterByAccess(context.Background(), testCase.user, silences...)

-			require.NoError(t, err)
-			require.ElementsMatch(t, testCase.expected, actual)
+			if testCase.expectedErr != nil {
+				assert.ErrorIs(t, err, testCase.expectedErr)
+			} else {
+				assert.NoError(t, err)
+				require.ElementsMatch(t, testCase.expected, actual)
+			}

 			if testCase.expectedDbAccess {
 				require.Equal(t, store.Calls, 1)
@ -126,6 +151,13 @@ func TestAuthorizeReadSilence(t *testing.T) {
 			expectedErr:      nil,
 			expectedDbAccess: false,
 		},
+		{
+			name:             "silence wildcard reader can read any silence",
+			user:             newUser(ac.Permission{Action: silenceRead, Scope: dashboards.ScopeFoldersProvider.GetResourceAllScope()}),
+			silence:          []*models.Silence{global, ruleSilence1, notFoundRule},
+			expectedErr:      nil,
+			expectedDbAccess: false,
+		},
 		{
 			name:             "silence reader can read global",
 			user:             newUser(ac.Permission{Action: silenceRead, Scope: folder1Scope}),
@ -176,9 +208,9 @@ func TestAuthorizeReadSilence(t *testing.T) {
 						assert.NoError(t, err)
 					}
 					if testCase.expectedDbAccess {
-						require.Equal(t, store.Calls, 1)
+						require.Equal(t, 1, store.Calls)
 					} else {
-						require.Equal(t, store.Calls, 0)
+						require.Equal(t, 0, store.Calls)
 					}
 				})
 			}
@ -224,6 +256,11 @@ func TestAuthorizeCreateSilence(t *testing.T) {
 			user:        newUser(ac.Permission{Action: instancesRead}),
 			expectedErr: ErrAuthorizationBase,
 		},
+		{
+			name:        "no create access, silence wildcard reader",
+			user:        newUser(ac.Permission{Action: silenceRead, Scope: dashboards.ScopeFoldersProvider.GetResourceAllScope()}),
+			expectedErr: ErrAuthorizationBase,
+		},
 		{
 			name:        "no create access, silence reader",
 			user:        newUser(ac.Permission{Action: silenceRead, Scope: folder1Scope}, ac.Permission{Action: silenceRead, Scope: folder2Scope}),
@ -244,6 +281,11 @@ func TestAuthorizeCreateSilence(t *testing.T) {
 			user:        newUser(ac.Permission{Action: instancesCreate}, ac.Permission{Action: instancesRead}),
 			expectedErr: nil,
 		},
+		{
+			name:        "silence wildcard read + instance create can do everything",
+			user:        newUser(ac.Permission{Action: instancesCreate}, ac.Permission{Action: silenceRead, Scope: dashboards.ScopeFoldersProvider.GetResourceAllScope()}),
+			expectedErr: nil,
+		},
 		{
 			name: "instance read + silence create",
 			user: newUser(ac.Permission{Action: silenceCreate, Scope: folder1Scope}, ac.Permission{Action: instancesRead}),
@ -276,6 +318,22 @@ func TestAuthorizeCreateSilence(t *testing.T) {
 			expectedErr:      ErrAuthorizationBase,
 			expectedDbAccess: true,
 		},
+		{
+			name: "silence read + silence wildcard create",
+			user: newUser(ac.Permission{Action: silenceRead, Scope: folder1Scope}, ac.Permission{Action: silenceCreate, Scope: dashboards.ScopeFoldersProvider.GetResourceAllScope()}),
+			overrides: map[*models.Silence]override{
+				global: {
+					expectedErr:      ErrAuthorizationBase,
+					expectedDbAccess: false,
+				},
+				ruleSilence1: {
+					expectedErr:      nil,
+					expectedDbAccess: true,
+				},
+			},
+			expectedErr:      ErrAuthorizationBase,
+			expectedDbAccess: true,
+		},
 		{
 			name: "silence read + create",
 			user: newUser(ac.Permission{Action: silenceRead, Scope: folder1Scope}, ac.Permission{Action: silenceCreate, Scope: folder1Scope}),
@ -369,6 +427,11 @@ func TestAuthorizeUpdateSilence(t *testing.T) {
 			user:        newUser(ac.Permission{Action: instancesRead}),
 			expectedErr: ErrAuthorizationBase,
 		},
+		{
+			name:        "no write access, silence wildcard reader",
+			user:        newUser(ac.Permission{Action: silenceRead, Scope: dashboards.ScopeFoldersProvider.GetResourceAllScope()}),
+			expectedErr: ErrAuthorizationBase,
+		},
 		{
 			name:        "no write access, silence reader",
 			user:        newUser(ac.Permission{Action: silenceRead, Scope: folder1Scope}, ac.Permission{Action: silenceRead, Scope: folder2Scope}),
@ -389,6 +452,11 @@ func TestAuthorizeUpdateSilence(t *testing.T) {
 			user:        newUser(ac.Permission{Action: instancesWrite}, ac.Permission{Action: instancesRead}),
 			expectedErr: nil,
 		},
+		{
+			name:        "silence wildcard read + instance write can do everything",
+			user:        newUser(ac.Permission{Action: instancesWrite}, ac.Permission{Action: silenceRead, Scope: dashboards.ScopeFoldersProvider.GetResourceAllScope()}),
+			expectedErr: nil,
+		},
 		{
 			name: "instance read + silence write",
 			user: newUser(ac.Permission{Action: silenceWrite, Scope: folder1Scope}, ac.Permission{Action: instancesRead}),
@ -421,6 +489,22 @@ func TestAuthorizeUpdateSilence(t *testing.T) {
 			expectedErr:      ErrAuthorizationBase,
 			expectedDbAccess: true,
 		},
+		{
+			name: "silence read + silence wildcard write",
+			user: newUser(ac.Permission{Action: silenceRead, Scope: folder1Scope}, ac.Permission{Action: silenceWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceAllScope()}),
+			overrides: map[*models.Silence]override{
+				global: {
+					expectedErr:      ErrAuthorizationBase,
+					expectedDbAccess: false,
+				},
+				ruleSilence1: {
+					expectedErr:      nil,
+					expectedDbAccess: true,
+				},
+			},
+			expectedErr:      ErrAuthorizationBase,
+			expectedDbAccess: true,
+		},
 		{
 			name: "silence read + write",
 			user: newUser(ac.Permission{Action: silenceRead, Scope: folder1Scope}, ac.Permission{Action: silenceWrite, Scope: folder1Scope}),