Alerting: Add manage permissions UI logic for Contact Points (#92885)

* Add showPolicies prop

* Add manage permissions component for easier reuse within alerting

* Add method for checking whether to show access control within alerting

* Remove accidental console.log from main

* Tweak styling for contact point width and add manage permissions drawer

* Improve typing for access control type response

* Add basic test for manage permissions on contact points list

* Only show manage permissions if grafana AM and alertingApiServer is enabled

* Update i18n

* Add test utils for turning features on and back off

* Add access control handlers

* Update tests with new util

* Pass AM in and add tests

* Receiver OSS resource permissions

There is a complication that is not fully addressed: Viewer defaults to read:*
and Editor defaults to read+write+delete:*

This is different to other resource permissions where non-admin are not granted
any global permissions and instead access is handled solely by resource-specific
permissions that are populated on create and removed on delete.

This allows them to easily remove permission to view or edit a single resource
from basic roles.

The reason this is tricky here is that we have multiple APIs that can
create/delete receivers: config api, provisioning api, and k8s receivers api.
Config api in particular is not well-equipped to determine when creates/deletes
are happening and thus ensuring that the proper resource-specific permissions
are created/deleted is finicky.

We would also have to create a migration to populate resource-specific
permissions for all current receivers. This migration would need to be reset so
it can run again if the flag is disabled.

* Add access control permissions

* Pass in contact point ID to receivers form

* Temporarily remove access control check for contact points

* Include access control metadata in k8s receiver List & Get

GET: Always included.
LIST: Included by adding a label selector with value `grafana.com/accessControl`

* Include new permissions for contact points navbar

* Fix receiver creator fixed role to not give global read

* Include in-use metadata in k8s receiver List & Get

GET: Always included.
LIST: Included by adding a label selector with value `grafana.com/inUse`

* Add receiver creator permission to receiver writer

* Add receiver creator permission to navbar

* Always allow listing receivers, don't return 403

* Remove receiver read precondition from receiver create

Otherwise, Creator role will not be able to create their first receiver

* Update routes permissions

* Add further support for RBAC in contact points

* Update routes permissions

* Update contact points header logic

* Back out test feature toggle refactor

Not working atm, not sure why

* Tidy up imports

* Update mock permissions

* Revert more test changes

* Update i18n

* Sync inuse metadata pr

* Add back canAdmin permissions after main merge

* Split out check for policies navtree item

* Tidy up utils and imports and fix rules in use

* Fix contact point tests and act warnings

* Add missing ReceiverPermissionAdmin after merge conflict

* Move contact points permissions

* Only show contact points filter when permissions are correct

* Move to constants

* Fallback to empty array and remove labelSelectors (not needed)

* Allow `toAbility` to take multiple actions

* Show builtin alertmanager if contact points permission

* Add empty state and hide templates if missing permissions

* Translations

* Tidy up mock data

* Fix tests and templates permission

* Update message for unused contact points

* Don't return 403 when user lists receivers and has access to none

* Fix receiver create not adding empty uid permissions

* Move SetDefaultPermissions to ReceiverPermissionService

* Have SetDefaultPermissions use uid from string

Fixes circular dependency

* Add FakeReceiverPermissionsService and fix test wiring

* Implement resource permission handling in provisioning API and renames

Create: Sets to default permissions
Delete: Removes permissions
Update: If receiver name is modified and the new name doesn't exist, it copies
the permissions from the old receiver to the newly created one. If old receiver
is now empty, it removes the old permissions as well.

* Split contact point permissions checks for read/modify

* Generalise getting annotation values from k8s entities

* Proxy RouteDeleteAlertingConfig through MultiOrgAlertmanager

* Cleanup permissions on config api reset and restore

* Cleanup permissions on config api POST

note this is still not available with feature flag enabled

* Gate the permission manager behind FF until initial migration is added

* Sync changes from config api PR

* Switch to named export

* Revert unnecessary changes

* Revert Filter auth change and implement in k8s api only

* Don't allow new scoped permissions to give access without FF

Prevents complications around mixed support for the scoped permissions causing
oddities in the UI.

* Fix integration tests to account for list permission change

* Move to `permissions` file

* Add additional tests for contact points

* Fix redirect for viewer on edit page

* Combine alerting test utils and move to new file location

* Allow new permissions to access provisioning export paths with FF

* Always allow exporting if its grafana flavoured

* Fix logic for showing auto generated policies

* Fix delete logic for contact point only referenced by a rule

* Suppress warning message when renaming a contact point

* Clear team and role perm cache on receiver rename

Prevents temporarily broken UI permissions after rename when a user's source of
elevated permissions comes from a cached team or basic role permission.

* Debug log failed cache clear on CopyPermissions

---------

Co-authored-by: Matt Jacobson <matthew.jacobson@grafana.com>
This commit is contained in:
Tom Ratcliffe
2024-09-27 19:56:32 +01:00
committed by GitHub
co-authored by Matt Jacobson
parent 86faeae6d2
commit fc51ec70ba
39 changed files with 809 additions and 108 deletions
@@ -53,9 +53,7 @@ func Authorize(ctx context.Context, ac AccessControlService, attr authorizer.Att
return deny(err)
}
case "list":
if err := ac.AuthorizeReadSome(ctx, user); err != nil { // Preconditions, further checks are done downstream.
return deny(err)
}
return authorizer.DecisionAllow, "", nil // Always allow listing, receivers are filtered downstream.
case "create":
if err := ac.AuthorizeCreate(ctx, user); err != nil {
return deny(err)
@@ -2,9 +2,10 @@ package receiver
import (
"context"
"errors"
"fmt"
"k8s.io/apimachinery/pkg/api/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/internalversion"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
@@ -14,6 +15,7 @@ import (
notifications "github.com/grafana/grafana/pkg/apis/alerting_notifications/v0alpha1"
grafanaRest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/services/apiserver/endpoints/request"
alertingac "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol"
"github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions"
ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/services/ngalert/notifier/legacy_storage"
@@ -88,7 +90,14 @@ func (s *legacyStorage) List(ctx context.Context, opts *internalversion.ListOpti
res, err := s.service.GetReceivers(ctx, q, user)
if err != nil {
return nil, err
// This API should not be returning a forbidden error when the user does not have access to any resources.
// This can be true for a contact point creator role, for example.
// This should eventually be changed downstream in the auth logic but provisioning API currently relies on this
// behaviour to return useful forbidden errors when exporting decrypted receivers.
if !errors.Is(err, alertingac.ErrAuthorizationBase) {
return nil, err
}
res = nil
}
accesses, err := s.metadata.AccessControlMetadata(ctx, user, res...)
@@ -112,7 +121,7 @@ func (s *legacyStorage) Get(ctx context.Context, uid string, _ *metav1.GetOption
name, err := legacy_storage.UidToName(uid)
if err != nil {
return nil, errors.NewNotFound(resourceInfo.GroupResource(), uid)
return nil, apierrors.NewNotFound(resourceInfo.GroupResource(), uid)
}
q := ngmodels.GetReceiverQuery{
OrgID: info.OrgID,
@@ -172,7 +181,7 @@ func (s *legacyStorage) Create(ctx context.Context,
return nil, fmt.Errorf("expected receiver but got %s", obj.GetObjectKind().GroupVersionKind())
}
if p.ObjectMeta.Name != "" { // TODO remove when metadata.name can be defined by user
return nil, errors.NewBadRequest("object's metadata.name should be empty")
return nil, apierrors.NewBadRequest("object's metadata.name should be empty")
}
model, _, err := convertToDomainModel(p)
if err != nil {
@@ -271,5 +280,5 @@ func (s *legacyStorage) Delete(ctx context.Context, uid string, deleteValidation
}
func (s *legacyStorage) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *internalversion.ListOptions) (runtime.Object, error) {
return nil, errors.NewMethodNotSupported(resourceInfo.GroupResource(), "deleteCollection")
return nil, apierrors.NewMethodNotSupported(resourceInfo.GroupResource(), "deleteCollection")
}
@@ -133,7 +133,14 @@ func (r ReceiverPermissionsService) CopyPermissions(ctx context.Context, orgID i
// Clear permission cache for the user who updated the receiver, so that new permissions are fetched for their next call
// Required for cases when caller wants to immediately interact with the newly updated object
if user != nil && user.IsIdentityType(claims.TypeUser) {
r.ac.ClearUserPermissionCache(user)
// A more comprehensive means of clearing the user's permissions cache than ClearUserPermissionCache.
// It also clears the cache for basic roles and teams, which is required for the user to not have temporarily
// broken UI permissions when their source of elevated permissions comes from a cached team or basic role
// permission.
_, err = r.ac.GetUserPermissions(ctx, user, accesscontrol.Options{ReloadCache: true})
if err != nil {
r.log.Debug("Failed to clear user permissions cache", "error", err)
}
}
return countCustomPermissions(setPermissionCommands), nil
+22 -1
View File
@@ -417,11 +417,32 @@ func (s *ServiceImpl) buildAlertNavLinks(c *contextmodel.ReqContext) *navtree.Na
})
}
if hasAccess(ac.EvalAny(ac.EvalPermission(ac.ActionAlertingNotificationsRead), ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead))) {
contactPointsPerms := []ac.Evaluator{
ac.EvalPermission(ac.ActionAlertingNotificationsRead),
ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead),
}
// With the new alerting API, we have other permissions to consider. We don't want to consider these with the old
// alerting API to maintain backwards compatibility.
if s.features.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingApiServer) {
contactPointsPerms = append(contactPointsPerms,
ac.EvalPermission(ac.ActionAlertingReceiversRead),
ac.EvalPermission(ac.ActionAlertingReceiversReadSecrets),
ac.EvalPermission(ac.ActionAlertingReceiversCreate),
)
}
if hasAccess(ac.EvalAny(contactPointsPerms...)) {
alertChildNavs = append(alertChildNavs, &navtree.NavLink{
Text: "Contact points", SubTitle: "Choose how to notify your contact points when an alert instance fires", Id: "receivers", Url: s.cfg.AppSubURL + "/alerting/notifications",
Icon: "comment-alt-share",
})
}
if hasAccess(ac.EvalAny(
ac.EvalPermission(ac.ActionAlertingNotificationsRead),
ac.EvalPermission(ac.ActionAlertingNotificationsExternalRead),
)) {
alertChildNavs = append(alertChildNavs, &navtree.NavLink{Text: "Notification policies", SubTitle: "Determine how alerts are routed to contact points", Id: "am-routes", Url: s.cfg.AppSubURL + "/alerting/routes", Icon: "sitemap"})
}
+18 -2
View File
@@ -8,6 +8,7 @@ import (
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/datasources"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/web"
)
@@ -242,9 +243,8 @@ func (api *API) authorize(method, path string) web.Handler {
http.MethodGet + "/api/v1/ngalert/alertmanagers":
return middleware.ReqOrgAdmin
// Grafana-only Provisioning Read Paths
// Grafana-only Provisioning Export Paths for everything except contact points.
case http.MethodGet + "/api/v1/provisioning/policies/export",
http.MethodGet + "/api/v1/provisioning/contact-points/export",
http.MethodGet + "/api/v1/provisioning/mute-timings/export",
http.MethodGet + "/api/v1/provisioning/mute-timings/{name}/export":
eval = ac.EvalAny(
@@ -254,6 +254,22 @@ func (api *API) authorize(method, path string) web.Handler {
ac.EvalPermission(ac.ActionAlertingProvisioningReadSecrets), // organization scope
)
// Grafana-only Provisioning Export Paths for contact points.
case http.MethodGet + "/api/v1/provisioning/contact-points/export":
perms := []ac.Evaluator{
ac.EvalPermission(ac.ActionAlertingNotificationsRead), // organization scope
ac.EvalPermission(ac.ActionAlertingProvisioningRead), // organization scope
ac.EvalPermission(ac.ActionAlertingNotificationsProvisioningRead), // organization scope
ac.EvalPermission(ac.ActionAlertingProvisioningReadSecrets), // organization scope
}
if api.FeatureManager.IsEnabledGlobally(featuremgmt.FlagAlertingApiServer) {
perms = append(perms,
ac.EvalPermission(ac.ActionAlertingReceiversRead),
ac.EvalPermission(ac.ActionAlertingReceiversReadSecrets),
)
}
eval = ac.EvalAny(perms...)
case http.MethodGet + "/api/v1/provisioning/alert-rules",
http.MethodGet + "/api/v1/provisioning/alert-rules/export":
eval = ac.EvalAny(
@@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/featuremgmt"
)
func TestAuthorize(t *testing.T) {
@@ -43,7 +44,7 @@ func TestAuthorize(t *testing.T) {
require.Len(t, paths, 59)
ac := acmock.New()
api := &API{AccessControl: ac}
api := &API{AccessControl: ac, FeatureManager: featuremgmt.WithFeatures()}
t.Run("should not panic on known routes", func(t *testing.T) {
for path, methods := range paths {
@@ -396,9 +396,10 @@ func TestIntegrationResourcePermissions(t *testing.T) {
assert.Equalf(t, expectedGetWithMetadata, got, "Expected %v but got %v", expectedGetWithMetadata, got)
})
} else {
t.Run("should be forbidden to list receivers", func(t *testing.T) {
_, err := client.List(ctx, v1.ListOptions{})
require.Truef(t, errors.IsForbidden(err), "should get Forbidden error but got %s", err)
t.Run("list receivers should be empty", func(t *testing.T) {
list, err := client.List(ctx, v1.ListOptions{})
require.NoError(t, err)
require.Emptyf(t, list.Items, "Expected no receivers but got %v", list.Items)
})
t.Run("should be forbidden to read receiver by name", func(t *testing.T) {
@@ -640,9 +641,10 @@ func TestIntegrationAccessControl(t *testing.T) {
})
})
} else {
t.Run("should be forbidden to list receivers", func(t *testing.T) {
_, err := client.List(ctx, v1.ListOptions{})
require.Truef(t, errors.IsForbidden(err), "should get Forbidden error but got %s", err)
t.Run("list receivers should be empty", func(t *testing.T) {
list, err := client.List(ctx, v1.ListOptions{})
require.NoError(t, err)
require.Emptyf(t, list.Items, "Expected no receivers but got %v", list.Items)
})
t.Run("should be forbidden to read receiver by name", func(t *testing.T) {