From 667fac8db122bfb8946c22284394cdea6a1b13d0 Mon Sep 17 00:00:00 2001 From: Mark Anderson Date: Fri, 25 Mar 2022 12:34:59 -0700 Subject: [PATCH] Fixups for error messages from ACL Errors (#12620) Fixups for error messages from ACL Errors Alter error messages to be more verbose and explanatory, something like: Permission denied: token with AccessorID '8a2d52a0-6b41-7077-8374-09d4fafa2d30 ' lacks permission 'service:read' on "foobar" on "foobar" in partition "foo" in namespace "bar" Signed-off-by: Mark Anderson --- acl/errors.go | 4 ++-- acl/errors_oss.go | 2 +- acl/errors_test.go | 4 ++-- acl/testing.go | 9 +++++++-- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/acl/errors.go b/acl/errors.go index c2363e2a16..7c88704b34 100644 --- a/acl/errors.go +++ b/acl/errors.go @@ -98,9 +98,9 @@ func (e PermissionDeniedError) Error() string { } if e.Accessor == "" { - message.WriteString(": provided accessor") + message.WriteString(": provided token") } else { - fmt.Fprintf(&message, ": accessor '%s'", e.Accessor) + fmt.Fprintf(&message, ": token with AccessorID '%s'", e.Accessor) } fmt.Fprintf(&message, " lacks permission '%s:%s'", e.Resource, e.AccessLevel.String()) diff --git a/acl/errors_oss.go b/acl/errors_oss.go index 9d605b34e9..ef8dc993cd 100644 --- a/acl/errors_oss.go +++ b/acl/errors_oss.go @@ -14,5 +14,5 @@ func NewResourceDescriptor(name string, _ *AuthorizerContext) ResourceDescriptor } func (od *ResourceDescriptor) ToString() string { - return od.Name + return "\"" + od.Name + "\"" } diff --git a/acl/errors_test.go b/acl/errors_test.go index 5b73a156ee..7c651f1ec3 100644 --- a/acl/errors_test.go +++ b/acl/errors_test.go @@ -29,11 +29,11 @@ func TestPermissionDeniedError(t *testing.T) { }, { err: PermissionDeniedByACL(&auth1, nil, ResourceService, AccessRead, "foobar"), - expected: "Permission denied: provided accessor lacks permission 'service:read' on foobar", + expected: "Permission denied: provided token lacks permission 'service:read' on \"foobar\"", }, { err: PermissionDeniedByACLUnnamed(&auth1, nil, ResourceService, AccessRead), - expected: "Permission denied: provided accessor lacks permission 'service:read'", + expected: "Permission denied: provided token lacks permission 'service:read'", }, } diff --git a/acl/testing.go b/acl/testing.go index 01399c6301..303bd1de66 100644 --- a/acl/testing.go +++ b/acl/testing.go @@ -1,6 +1,7 @@ package acl import ( + "fmt" "github.com/stretchr/testify/require" "regexp" "testing" @@ -23,20 +24,24 @@ func RequirePermissionDeniedError(t testing.TB, err error, authz Authorizer, _ * func RequirePermissionDeniedMessage(t testing.TB, msg string, authz interface{}, _ *AuthorizerContext, resource Resource, accessLevel AccessLevel, resourceID string) { require.NotEmpty(t, msg, "expected non-empty error message") + baseRegex := ` lacks permission '(\S*):(\S*)' on \"([^\"]*)\"(?: in partition \"([^\"]*)\" in namespace \"([^\"]*)\")?\s*$` + var resourceIDFound string if authz == nil { - expr := "^Permission denied" + `: provided accessor lacks permission '(\S*):(\S*)' on (.*)\s*$` + expr := "^Permission denied" + `: provided token` + baseRegex re, _ := regexp.Compile(expr) matched := re.FindStringSubmatch(msg) + require.NotNil(t, matched, fmt.Sprintf("RE %q didn't match %q", expr, msg)) require.Equal(t, string(resource), matched[1], "resource") require.Equal(t, accessLevel.String(), matched[2], "access level") resourceIDFound = matched[3] } else { - expr := "^Permission denied" + `: accessor '(\S*)' lacks permission '(\S*):(\S*)' on (.*)\s*$` + expr := "^Permission denied" + `: token with AccessorID '(\S*)'` + baseRegex re, _ := regexp.Compile(expr) matched := re.FindStringSubmatch(msg) + require.NotNil(t, matched, fmt.Sprintf("RE %q didn't match %q", expr, msg)) require.Equal(t, extractAccessorID(authz), matched[1], "auth") require.Equal(t, string(resource), matched[2], "resource") require.Equal(t, accessLevel.String(), matched[3], "access level")