From b5acc5b8d668d60a9765ed418ed367089510036d Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Thu, 11 Jun 2026 19:00:32 +0300 Subject: [PATCH 1/9] PMM-15138 Fix alerting permissions --- managed/services/grafana/auth_server.go | 57 +++++++++++++++----- managed/services/grafana/auth_server_test.go | 25 +++++++++ 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index fa850f2c12..e4487abae4 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -63,7 +63,11 @@ var rules = map[string]role{ "/qan.v1.CollectorService.": viewer, "/qan.v1.QANService.": viewer, + // Alerting: viewers may list templates (GET /v1/alerting/templates); + // creating rules requires editor. Template writes (create/update/delete) + // share read paths, so they are method-qualified in methodRules. "/v1/alerting": viewer, + "/v1/alerting/rules": editor, "/v1/advisors": editor, "/v1/advisors/checks:": editor, "/v1/advisors/failedServices": editor, @@ -126,6 +130,19 @@ var rules = map[string]role{ // "/" is a special case in this code } +// methodRules maps "METHOD original-URL-prefix" to the minimal required role. +// Entries take precedence over rules and let operations that share a path but +// use different HTTP methods require different roles. +var methodRules = map[string]role{ + // Alerting template writes must require editor. CreateTemplate (POST) + // shares its path with ListTemplates (GET, viewer in rules); Update (PUT) + // and Delete (DELETE) live under it. Qualify by method so viewers keep + // read access while writes are denied. + http.MethodPost + " /v1/alerting/templates": editor, + http.MethodPut + " /v1/alerting/templates/": editor, + http.MethodDelete + " /v1/alerting/templates/": editor, +} + var lbacPrefixes = []string{ "/graph/api/datasources/uid", "/graph/api/ds/query", @@ -331,7 +348,7 @@ func (s *AuthServer) maybeAddLBACFilters(ctx context.Context, rw http.ResponseWr jsonFilters, err := json.Marshal(filters) if err != nil { - return errors.WithStack(err) + return fmt.Errorf("failed to marshal LBAC filters: %w", err) } rw.Header().Set(lbacHeaderName, base64.StdEncoding.EncodeToString(jsonFilters)) @@ -454,6 +471,28 @@ func nextPrefix(path string) string { return path[:i+1] } +// resolveRule returns the minimal role required for the given HTTP method and +// cleaned path, together with the matched rule prefix. It walks path prefixes +// from longest to shortest; a method-specific rule (keyed by "METHOD prefix") +// takes precedence over a path-only rule at the same prefix, so read and write +// operations that share a path can require different roles. The bool is false +// when no rule matches, in which case the caller falls back to grafanaAdmin. +func resolveRule(method, cleanedPath string) (role, string, bool) { + prefix := cleanedPath + for { + if r, ok := methodRules[method+" "+prefix]; ok { + return r, prefix, true + } + if r, ok := rules[prefix]; ok { + return r, prefix, true + } + if prefix == "/" { + return none, prefix, false + } + prefix = nextPrefix(prefix) + } +} + func isLocalAgentConnection(req *http.Request) bool { ip := strings.Split(req.RemoteAddr, ":")[0] // pmmAgent := req.Header.Get("Pmm-Agent-Id") @@ -482,20 +521,11 @@ func (s *AuthServer) authenticate(ctx context.Context, req *http.Request, l *log } } - // find the longest prefix present in rules - prefix := cleanedPath - for prefix != "/" { - if _, ok := rules[prefix]; ok { - break - } - prefix = nextPrefix(prefix) - } - - // fallback to Grafana admin if there is no explicit rule - minRole, ok := rules[prefix] + minRole, prefix, ok := resolveRule(req.Method, cleanedPath) if ok { l = l.WithField("prefix", prefix) } else { + // fallback to Grafana admin if there is no explicit rule l.Warn("No explicit rule, falling back to Grafana admin.") minRole = grafanaAdmin } @@ -596,7 +626,8 @@ func (s *AuthServer) retrieveRole(ctx context.Context, hash string, authHeaders authUser, err := s.c.getAuthUser(ctx, authHeaders, l) if err != nil { l.Warnf("%s", err) - if cErr, ok := errors.Cause(err).(*clientError); ok { //nolint:errorlint + var cErr *clientError + if errors.As(err, &cErr) { code := codes.Internal if cErr.Code == 401 || cErr.Code == 403 { code = codes.Unauthenticated diff --git a/managed/services/grafana/auth_server_test.go b/managed/services/grafana/auth_server_test.go index a8c20a36cd..430a3410a5 100644 --- a/managed/services/grafana/auth_server_test.go +++ b/managed/services/grafana/auth_server_test.go @@ -62,6 +62,31 @@ func TestNextPrefix(t *testing.T) { } } +func TestResolveRule(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + method string + path string + wantRole role + }{ + // Alerting: only listing templates is viewable; writes need editor. + {http.MethodGet, "/v1/alerting/templates", viewer}, // ListTemplates + {http.MethodPost, "/v1/alerting/templates", editor}, // CreateTemplate + {http.MethodPut, "/v1/alerting/templates/foo", editor}, // UpdateTemplate + {http.MethodDelete, "/v1/alerting/templates/foo", editor}, // DeleteTemplate + {http.MethodPost, "/v1/alerting/rules", editor}, // CreateRule + } { + t.Run(fmt.Sprintf("%s %s", tc.method, tc.path), func(t *testing.T) { + t.Parallel() + + got, _, ok := resolveRule(tc.method, tc.path) + require.True(t, ok) + assert.Equal(t, tc.wantRole, got) + }) + } +} + func TestAuthServerAuthenticate(t *testing.T) { t.Parallel() From 548ddd3571c7cbd267988a72ee215cce2f353fd4 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Thu, 11 Jun 2026 19:15:41 +0300 Subject: [PATCH 2/9] PMM-15138 refactor the use of native errors pkg --- managed/services/grafana/auth_server.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index e4487abae4..4bdb565c14 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -20,6 +20,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "net/http" "net/http/httputil" @@ -31,7 +32,6 @@ import ( "unicode/utf8" "github.com/lib/pq" - "github.com/pkg/errors" "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "gopkg.in/reform.v1" @@ -434,10 +434,10 @@ func extractOriginalRequest(req *http.Request) error { return errors.New("empty X-Original-Uri") } if origURI[0] != '/' { - return errors.Errorf("unexpected X-Original-Uri: %q", origURI) + return fmt.Errorf("unexpected X-Original-Uri: %s", origURI) } if !utf8.ValidString(origURI) { - return errors.Errorf("invalid X-Original-Uri: %q", origURI) + return fmt.Errorf("invalid X-Original-Uri: %s", origURI) } req.Method = origMethod From 2eaf3c67839ee05af4109efe1b5e27a410721c64 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Fri, 12 Jun 2026 00:34:43 +0300 Subject: [PATCH 3/9] PMM-15138 Do not use quotes in the logs --- managed/services/grafana/auth_server.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index 4bdb565c14..ec90236885 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -531,7 +531,7 @@ func (s *AuthServer) authenticate(ctx context.Context, req *http.Request, l *log } if minRole == none { - l.Debugf("Minimal required role is %q, granting access without checking Grafana.", minRole) + l.Debugf("Minimal required role is %s, granting access without checking Grafana.", minRole) return nil, nil } @@ -564,11 +564,11 @@ func (s *AuthServer) authenticate(ctx context.Context, req *http.Request, l *log } if minRole <= user.role { - l.Debugf("Minimal required role is %q, granting access.", minRole) + l.Debugf("Minimal required role is %s, granting access.", minRole) return user, nil } - l.Warnf("Minimal required role is %q.", minRole) + l.Warnf("Minimal required role is %s, denying access.", minRole) return nil, &authError{code: codes.PermissionDenied, message: "Access denied."} } From c0336c57f033d52ef64d9d9165bc15f21a37a692 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Fri, 12 Jun 2026 14:51:19 +0300 Subject: [PATCH 4/9] PMM-15138 Remove fallback warning --- managed/services/grafana/auth_server.go | 1 - 1 file changed, 1 deletion(-) diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index ec90236885..35ebac3719 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -525,7 +525,6 @@ func (s *AuthServer) authenticate(ctx context.Context, req *http.Request, l *log if ok { l = l.WithField("prefix", prefix) } else { - // fallback to Grafana admin if there is no explicit rule l.Warn("No explicit rule, falling back to Grafana admin.") minRole = grafanaAdmin } From 02ba68cdea49b44b294dbab5c4ac3aabfae38fae Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Fri, 12 Jun 2026 15:19:39 +0300 Subject: [PATCH 5/9] Update managed/services/grafana/auth_server.go Co-authored-by: Maxim Kondratenko --- managed/services/grafana/auth_server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index 35ebac3719..776dcfc59b 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -625,8 +625,8 @@ func (s *AuthServer) retrieveRole(ctx context.Context, hash string, authHeaders authUser, err := s.c.getAuthUser(ctx, authHeaders, l) if err != nil { l.Warnf("%s", err) - var cErr *clientError - if errors.As(err, &cErr) { + cErr ,ok := errors.AsType[*clientError](err) + if ok { code := codes.Internal if cErr.Code == 401 || cErr.Code == 403 { code = codes.Unauthenticated From 7826d07aefb2d6030256dda79e5d7d0cdfdd39cd Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Fri, 12 Jun 2026 16:29:43 +0300 Subject: [PATCH 6/9] PMM-15138 Resolve minRole in a single place --- managed/services/grafana/auth_server.go | 24 ++++++++------------ managed/services/grafana/auth_server_test.go | 5 ++-- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index 776dcfc59b..b6ba7cb94e 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -475,19 +475,20 @@ func nextPrefix(path string) string { // cleaned path, together with the matched rule prefix. It walks path prefixes // from longest to shortest; a method-specific rule (keyed by "METHOD prefix") // takes precedence over a path-only rule at the same prefix, so read and write -// operations that share a path can require different roles. The bool is false -// when no rule matches, in which case the caller falls back to grafanaAdmin. -func resolveRule(method, cleanedPath string) (role, string, bool) { +// operations that share a path can require different roles. When no rule +// matches it logs a warning and falls back to grafanaAdmin. +func resolveRule(method, cleanedPath string, l *logrus.Entry) (role, string) { prefix := cleanedPath for { if r, ok := methodRules[method+" "+prefix]; ok { - return r, prefix, true + return r, prefix } if r, ok := rules[prefix]; ok { - return r, prefix, true + return r, prefix } if prefix == "/" { - return none, prefix, false + l.Warn("No explicit rule, falling back to Grafana admin.") + return grafanaAdmin, prefix } prefix = nextPrefix(prefix) } @@ -521,13 +522,8 @@ func (s *AuthServer) authenticate(ctx context.Context, req *http.Request, l *log } } - minRole, prefix, ok := resolveRule(req.Method, cleanedPath) - if ok { - l = l.WithField("prefix", prefix) - } else { - l.Warn("No explicit rule, falling back to Grafana admin.") - minRole = grafanaAdmin - } + minRole, prefix := resolveRule(req.Method, cleanedPath, l) + l = l.WithField("prefix", prefix) if minRole == none { l.Debugf("Minimal required role is %s, granting access without checking Grafana.", minRole) @@ -625,7 +621,7 @@ func (s *AuthServer) retrieveRole(ctx context.Context, hash string, authHeaders authUser, err := s.c.getAuthUser(ctx, authHeaders, l) if err != nil { l.Warnf("%s", err) - cErr ,ok := errors.AsType[*clientError](err) + cErr, ok := errors.AsType[*clientError](err) if ok { code := codes.Internal if cErr.Code == 401 || cErr.Code == 403 { diff --git a/managed/services/grafana/auth_server_test.go b/managed/services/grafana/auth_server_test.go index 430a3410a5..e9451c4be0 100644 --- a/managed/services/grafana/auth_server_test.go +++ b/managed/services/grafana/auth_server_test.go @@ -76,12 +76,13 @@ func TestResolveRule(t *testing.T) { {http.MethodPut, "/v1/alerting/templates/foo", editor}, // UpdateTemplate {http.MethodDelete, "/v1/alerting/templates/foo", editor}, // DeleteTemplate {http.MethodPost, "/v1/alerting/rules", editor}, // CreateRule + // No matching rule falls back to grafanaAdmin. + {http.MethodGet, "/v1/unknown", grafanaAdmin}, } { t.Run(fmt.Sprintf("%s %s", tc.method, tc.path), func(t *testing.T) { t.Parallel() - got, _, ok := resolveRule(tc.method, tc.path) - require.True(t, ok) + got, _ := resolveRule(tc.method, tc.path, logrus.WithField("test", t.Name())) assert.Equal(t, tc.wantRole, got) }) } From 33712fd44ab6925024cf54540018cb62e01a8a03 Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Sat, 13 Jun 2026 00:53:35 +0300 Subject: [PATCH 7/9] PMM-15138 Return 403 for authorization denials The nginx auth layer returned 401 for permission denials, which triggers the error_page 401 re-run against /auth_request as a GET. For method-specific rules (e.g. POST /v1/alerting/templates) the GET re-run is allowed, so the client received a misleading 200 even though the write was blocked. Map PermissionDenied to 403 so nginx denies outright via error_page 403, which serves a static body without re-running. Unauthenticated stays 401. --- api-tests/server/auth_test.go | 10 ++-- .../ansible/roles/nginx/files/conf.d/pmm.conf | 14 +++++- managed/services/grafana/auth_server.go | 47 ++++++++++--------- 3 files changed, 42 insertions(+), 29 deletions(-) diff --git a/api-tests/server/auth_test.go b/api-tests/server/auth_test.go index 984a55cc88..5f7ae1c253 100644 --- a/api-tests/server/auth_test.go +++ b/api-tests/server/auth_test.go @@ -217,9 +217,9 @@ func TestBasicAuthPermissions(t *testing.T) { userCase []userCase }{ {name: "settings", url: "/v1/server/settings", method: "GET", userCase: []userCase{ - {userType: "default", login: none, statusCode: 401}, - {userType: "viewer", login: viewer, statusCode: 401}, - {userType: "editor", login: editor, statusCode: 401}, + {userType: "default", login: none, statusCode: 403}, + {userType: "viewer", login: viewer, statusCode: 403}, + {userType: "editor", login: editor, statusCode: 403}, {userType: "admin", login: admin, statusCode: 200}, }}, } @@ -365,8 +365,8 @@ func TestServiceAccountPermissions(t *testing.T) { }{ {name: "settings", url: "/v1/server/settings", method: "GET", userCase: []userCase{ {userType: "default", statusCode: 401}, - {userType: "viewer", serviceToken: viewerToken, statusCode: 401}, - {userType: "editor", serviceToken: editorToken, statusCode: 401}, + {userType: "viewer", serviceToken: viewerToken, statusCode: 403}, + {userType: "editor", serviceToken: editorToken, statusCode: 403}, {userType: "admin", serviceToken: adminToken, statusCode: 200}, }}, } diff --git a/build/ansible/roles/nginx/files/conf.d/pmm.conf b/build/ansible/roles/nginx/files/conf.d/pmm.conf index 952cc5a689..7061eb56e3 100644 --- a/build/ansible/roles/nginx/files/conf.d/pmm.conf +++ b/build/ansible/roles/nginx/files/conf.d/pmm.conf @@ -99,9 +99,12 @@ # nginx completely ignores auth_request subrequest response body. # We use that directive to send the same request to the same location as a normal request # to get a response body or redirect and return it to the client. - # auth_request supports only 401 and 403 statuses; 401 is reserved for this configration, - # and 403 is used for normal pmm-managed API errors. + # 401 is re-run through /auth_request (below) to produce a response body. 403 + # (insufficient role) is served a static body by @access_denied without re-running: + # the re-run is always a GET, which would wrongly pass method-specific rules. + # proxy_intercept_errors is off, so backend API 403s pass through unchanged. error_page 401 = /auth_request; + error_page 403 = @access_denied; # Internal location for authentication via pmm-managed/Grafana. # First, nginx sends request there to authenticate it. If it is not authenticated by pmm-managed/Grafana, @@ -126,6 +129,13 @@ proxy_set_header X-Original-Method $request_method; } + # Static body matching pmm-managed's PermissionDenied response (see error_page 403 above). + location @access_denied { + auth_request off; + default_type application/json; + return 403 '{"code":7,"error":"Access denied.","message":"Access denied."}'; + } + # PMM UI location /pmm-ui { # Will redirect on FE to login page if user is not authenticated diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index b6ba7cb94e..dd30bb0d2b 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -63,8 +63,7 @@ var rules = map[string]role{ "/qan.v1.CollectorService.": viewer, "/qan.v1.QANService.": viewer, - // Alerting: viewers may list templates (GET /v1/alerting/templates); - // creating rules requires editor. Template writes (create/update/delete) + // Viewers may list templates; creating rules needs editor. Template writes // share read paths, so they are method-qualified in methodRules. "/v1/alerting": viewer, "/v1/alerting/rules": editor, @@ -130,14 +129,11 @@ var rules = map[string]role{ // "/" is a special case in this code } -// methodRules maps "METHOD original-URL-prefix" to the minimal required role. -// Entries take precedence over rules and let operations that share a path but -// use different HTTP methods require different roles. +// methodRules maps "METHOD url-prefix" to the minimal role. Entries take precedence +// over rules, letting operations on a shared path differ by HTTP method. var methodRules = map[string]role{ - // Alerting template writes must require editor. CreateTemplate (POST) - // shares its path with ListTemplates (GET, viewer in rules); Update (PUT) - // and Delete (DELETE) live under it. Qualify by method so viewers keep - // read access while writes are denied. + // Template writes need editor; they share paths with the viewer-readable + // list (POST) or sit under it (PUT/DELETE), so they're qualified by method. http.MethodPost + " /v1/alerting/templates": editor, http.MethodPut + " /v1/alerting/templates/": editor, http.MethodDelete + " /v1/alerting/templates/": editor, @@ -271,7 +267,7 @@ func (s *AuthServer) ServeHTTP(rw http.ResponseWriter, req *http.Request) { "error": authErr.message, "message": authErr.message, //nolint:goconst } - s.returnError(rw, m, l) + s.returnError(rw, httpStatusForAuthError(authErr.code), m, l) return } @@ -290,18 +286,27 @@ func (s *AuthServer) ServeHTTP(rw http.ResponseWriter, req *http.Request) { } l.Errorf("Failed to add VMProxy filters: %s", errF) - s.returnError(rw, m, l) + s.returnError(rw, authenticationErrorCode, m, l) return } } -func (s *AuthServer) returnError(rw http.ResponseWriter, msg map[string]any, l *logrus.Entry) { - // nginx completely ignores auth_request subrequest response body. - // We respond with 401 (authenticationErrorCode); our nginx configuration then sends - // the same request as a normal request to the same location and returns response body to the client. +// httpStatusForAuthError maps an authError code to the HTTP status nginx receives. +// PermissionDenied uses 403 so nginx denies outright; the 401 re-run is a GET and would +// wrongly pass method-specific rules. Authentication and internal errors stay 401. +func httpStatusForAuthError(code codes.Code) int { + if code == codes.PermissionDenied { + return http.StatusForbidden + } + return authenticationErrorCode +} + +func (s *AuthServer) returnError(rw http.ResponseWriter, status int, msg map[string]any, l *logrus.Entry) { + // nginx ignores the auth_request subrequest body: on 401 it re-runs the request to + // /auth_request to fetch this body; on 403 it serves a static body via error_page 403. rw.Header().Set("Content-Type", "application/json") - rw.WriteHeader(authenticationErrorCode) + rw.WriteHeader(status) err := json.NewEncoder(rw).Encode(msg) if err != nil { l.Warnf("%s", err) @@ -471,12 +476,10 @@ func nextPrefix(path string) string { return path[:i+1] } -// resolveRule returns the minimal role required for the given HTTP method and -// cleaned path, together with the matched rule prefix. It walks path prefixes -// from longest to shortest; a method-specific rule (keyed by "METHOD prefix") -// takes precedence over a path-only rule at the same prefix, so read and write -// operations that share a path can require different roles. When no rule -// matches it logs a warning and falls back to grafanaAdmin. +// resolveRule returns the minimal role for the given method and path, plus the matched +// prefix. It walks prefixes longest-to-shortest; a method-specific rule ("METHOD prefix") +// beats a path-only rule at the same prefix, so read and write on a shared path can differ. +// With no match it logs a warning and falls back to grafanaAdmin. func resolveRule(method, cleanedPath string, l *logrus.Entry) (role, string) { prefix := cleanedPath for { From 42e131536ffae2ab763161b53fdad312733f08ce Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Sat, 13 Jun 2026 11:16:46 +0300 Subject: [PATCH 8/9] PMM-15138 Remove the period from the error message --- build/ansible/roles/nginx/files/conf.d/pmm.conf | 2 +- managed/services/grafana/auth_server.go | 4 ++-- managed/services/grafana/auth_server_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/build/ansible/roles/nginx/files/conf.d/pmm.conf b/build/ansible/roles/nginx/files/conf.d/pmm.conf index 7061eb56e3..63cb588fc4 100644 --- a/build/ansible/roles/nginx/files/conf.d/pmm.conf +++ b/build/ansible/roles/nginx/files/conf.d/pmm.conf @@ -133,7 +133,7 @@ location @access_denied { auth_request off; default_type application/json; - return 403 '{"code":7,"error":"Access denied.","message":"Access denied."}'; + return 403 '{"code":7,"error":"Access denied","message":"Access denied"}'; } # PMM UI diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index dd30bb0d2b..e4b748e32b 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -557,7 +557,7 @@ func (s *AuthServer) authenticate(ctx context.Context, req *http.Request, l *log l = l.WithField("role", user.role.String()) if user.role == grafanaAdmin { - l.Debugf("Grafana admin, allowing access.") + l.Debugf("Grafana admin, granting access.") return user, nil } @@ -567,7 +567,7 @@ func (s *AuthServer) authenticate(ctx context.Context, req *http.Request, l *log } l.Warnf("Minimal required role is %s, denying access.", minRole) - return nil, &authError{code: codes.PermissionDenied, message: "Access denied."} + return nil, &authError{code: codes.PermissionDenied, message: "Access denied"} } func cleanPath(p string) (string, error) { diff --git a/managed/services/grafana/auth_server_test.go b/managed/services/grafana/auth_server_test.go index e9451c4be0..a009ab5d18 100644 --- a/managed/services/grafana/auth_server_test.go +++ b/managed/services/grafana/auth_server_test.go @@ -145,7 +145,7 @@ func TestAuthServerAuthenticate(t *testing.T) { if minRole <= role { assert.Nil(t, res) } else { - assert.Equal(t, &authError{code: codes.PermissionDenied, message: "Access denied."}, res) + assert.Equal(t, &authError{code: codes.PermissionDenied, message: "Access denied"}, res) } }) } From 2c3a045b57f0f686c5645a866304b31838afa8bb Mon Sep 17 00:00:00 2001 From: Alex Demidoff Date: Sat, 13 Jun 2026 11:45:51 +0300 Subject: [PATCH 9/9] PMM-15138 Enhance comments in auth_server.go --- managed/services/grafana/auth_server.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/managed/services/grafana/auth_server.go b/managed/services/grafana/auth_server.go index e4b748e32b..d49583b268 100644 --- a/managed/services/grafana/auth_server.go +++ b/managed/services/grafana/auth_server.go @@ -46,6 +46,10 @@ const ( ) // rules maps original URL prefix to minimal required role. +// In case of multiple matches, the longest prefix wins. The prefix is matched +// against the original URL path, not the cleaned one. The prefix must end with a slash, +// dot, or colon, so that "/v1/inventory" does not match "/v1/inventoryX". +// If several methods share the same path, they must be distinguished by methodRules. var rules = map[string]role{ // TODO https://jira.percona.com/browse/PMM-4420 connectionEndpointV2: admin, // compatibility for v2 agents @@ -63,8 +67,6 @@ var rules = map[string]role{ "/qan.v1.CollectorService.": viewer, "/qan.v1.QANService.": viewer, - // Viewers may list templates; creating rules needs editor. Template writes - // share read paths, so they are method-qualified in methodRules. "/v1/alerting": viewer, "/v1/alerting/rules": editor, "/v1/advisors": editor,