public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

unistore: check namespace (#102020)

Browse Source 
* check namespace in unistore

* fix tests

* fix trace status

* Use capital letter

---------

Co-authored-by: Karl Persson <23356117+kalleep@users.noreply.github.com>
This commit is contained in:
Georges Chaudy
2025-03-12 14:37:17 +01:00
committed by GitHub
co-authored by Karl Persson
parent 00d9916113
commit 1f637d07eb
3 changed files with 215 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/authn/grpcutils/grpc_authenticator.go
+5 -1
View File
@@ -105,6 +105,10 @@ func FallbackUsed(ctx context.Context) bool {
return ctx.Value(contextFallbackKey{}) != nil
}
func WithFallback(ctx context.Context) context.Context {
return context.WithValue(ctx, contextFallbackKey{}, true)
}
func (f *authenticatorWithFallback) Authenticate(ctx context.Context) (context.Context, error) {
ctx, span := f.tracer.Start(ctx, "grpcutils.AuthenticatorWithFallback.Authenticate")
defer span.End()
@@ -122,7 +126,7 @@ func (f *authenticatorWithFallback) Authenticate(ctx context.Context) (context.C
span.SetAttributes(attribute.Bool("fallback_used", true))
newCtx, err = f.fallback.Authenticate(ctx)
if newCtx != nil {
newCtx = context.WithValue(newCtx, contextFallbackKey{}, true)
newCtx = WithFallback(newCtx)
}
f.metrics.requestsTotal.WithLabelValues("true", fmt.Sprintf("%t", err == nil)).Inc()
return newCtx, err
pkg/storage/unified/resource/access.go
+44 -3
View File
@@ -9,6 +9,7 @@ import (
"github.com/prometheus/client_golang/prometheus"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/noop"
@@ -120,10 +121,28 @@ func (c authzLimitedClient) Check(ctx context.Context, id claims.AuthInfo, req c
attribute.Bool("fallback_used", grpcutils.FallbackUsed(ctx)),
))
defer span.End()
if grpcutils.FallbackUsed(ctx) {
if req.Namespace == "" {
// cross namespace queries are not allowed when fallback is used
span.SetAttributes(attribute.Bool("allowed", false))
span.SetStatus(codes.Error, "Namespace empty")
err := fmt.Errorf("namespace empty")
span.RecordError(err)
return claims.CheckResponse{Allowed: false}, err
}
span.SetAttributes(attribute.Bool("allowed", true))
return claims.CheckResponse{Allowed: true}, nil
}
if !claims.NamespaceMatches(id.GetNamespace(), req.Namespace) {
span.SetAttributes(attribute.Bool("allowed", false))
span.SetStatus(codes.Error, "Namespace missmatch")
span.RecordError(claims.ErrNamespaceMissmatch)
return claims.CheckResponse{Allowed: false}, claims.ErrNamespaceMissmatch
}
if !c.IsCompatibleWithRBAC(req.Group, req.Resource) {
span.SetAttributes(attribute.Bool("allowed", true))
return claims.CheckResponse{Allowed: true}, nil
@@ -132,7 +151,8 @@ func (c authzLimitedClient) Check(ctx context.Context, id claims.AuthInfo, req c
if err != nil {
c.logger.Error("Check", "group", req.Group, "resource", req.Resource, "error", err, "duration", time.Since(t), "traceid", tracing.TraceIDFromContext(ctx, false))
c.metrics.errorsTotal.WithLabelValues(req.Group, req.Resource, req.Verb).Inc()
span.SetAttributes(attribute.String("error", err.Error()))
span.SetStatus(codes.Error, fmt.Sprintf("check failed: %v", err))
span.RecordError(err)
return resp, err
}
span.SetAttributes(attribute.Bool("allowed", resp.Allowed))
@@ -152,7 +172,27 @@ func (c authzLimitedClient) Compile(ctx context.Context, id claims.AuthInfo, req
attribute.Bool("fallback_used", fallbackUsed),
))
defer span.End()
if fallbackUsed || !c.IsCompatibleWithRBAC(req.Group, req.Resource) {
if fallbackUsed {
if req.Namespace == "" {
// cross namespace queries are not allowed when fallback is used
span.SetAttributes(attribute.Bool("allowed", false))
span.SetStatus(codes.Error, "Namespace empty")
err := fmt.Errorf("namespace empty")
span.RecordError(err)
return nil, err
}
return func(name, folder string) bool {
return true
}, nil
}
if !claims.NamespaceMatches(id.GetNamespace(), req.Namespace) {
span.SetAttributes(attribute.Bool("allowed", false))
span.SetStatus(codes.Error, "Namespace missmatch")
span.RecordError(claims.ErrNamespaceMissmatch)
return nil, claims.ErrNamespaceMissmatch
}
if !c.IsCompatibleWithRBAC(req.Group, req.Resource) {
return func(name, folder string) bool {
return true
}, nil
@@ -161,7 +201,8 @@ func (c authzLimitedClient) Compile(ctx context.Context, id claims.AuthInfo, req
if err != nil {
c.logger.Error("Compile", "group", req.Group, "resource", req.Resource, "error", err, "traceid", tracing.TraceIDFromContext(ctx, false))
c.metrics.errorsTotal.WithLabelValues(req.Group, req.Resource, req.Verb).Inc()
span.SetAttributes(attribute.String("error", err.Error()))
span.SetStatus(codes.Error, fmt.Sprintf("compile failed: %v", err))
span.RecordError(err)
return nil, err
}
c.metrics.compileDuration.WithLabelValues(req.Group, req.Resource, req.Verb).Observe(time.Since(t).Seconds())
pkg/storage/unified/resource/access_test.go
+166 -8
View File
@@ -5,9 +5,12 @@ import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
authlib "github.com/grafana/authlib/types"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/apimachinery/utils"
"github.com/grafana/grafana/pkg/services/authn/grpcutils"
)
func TestAuthzLimitedClient_Check(t *testing.T) {
@@ -26,11 +29,12 @@ func TestAuthzLimitedClient_Check(t *testing.T) {
for _, test := range tests {
req := authlib.CheckRequest{
Group: test.group,
Resource: test.resource,
Verb: utils.VerbGet,
Group: test.group,
Resource: test.resource,
Verb: utils.VerbGet,
Namespace: "stacks-1",
}
resp, err := client.Check(context.Background(), nil, req)
resp, err := client.Check(context.Background(), &identity.StaticRequester{Namespace: "stacks-1"}, req)
assert.NoError(t, err)
assert.Equal(t, test.expected, resp.Allowed)
}
@@ -52,11 +56,12 @@ func TestAuthzLimitedClient_Compile(t *testing.T) {
for _, test := range tests {
req := authlib.ListRequest{
Group: test.group,
Resource: test.resource,
Verb: utils.VerbGet,
Group: test.group,
Resource: test.resource,
Verb: utils.VerbGet,
Namespace: "stacks-1",
}
checker, err := client.Compile(context.Background(), nil, req)
checker, err := client.Compile(context.Background(), &identity.StaticRequester{Namespace: "stacks-1"}, req)
assert.NoError(t, err)
assert.NotNil(t, checker)
@@ -64,3 +69,156 @@ func TestAuthzLimitedClient_Compile(t *testing.T) {
assert.Equal(t, test.expected, result)
}
}
// TestNamespaceMatching tests namespace matching in Check and Compile methods
func TestNamespaceMatching(t *testing.T) {
// Create a mock client that always returns allowed=true
mockClient := authlib.FixedAccessClient(true)
client := NewAuthzLimitedClient(mockClient, AuthzOptions{})
// Create a context with fallback disabled
ctx := context.Background()
tests := []struct {
name string
authNamespace string
reqNamespace string
expectError bool
}{
{
name: "matching namespaces",
authNamespace: "ns1",
reqNamespace: "ns1",
expectError: false,
},
{
name: "mismatched namespaces",
authNamespace: "ns1",
reqNamespace: "ns2",
expectError: true,
},
{
name: "empty request namespace",
authNamespace: "ns1",
reqNamespace: "",
expectError: true,
},
{
name: "empty auth namespace",
authNamespace: "",
reqNamespace: "ns1",
expectError: true,
},
{
name: "wildcard auth namespace",
authNamespace: "*",
reqNamespace: "ns1",
expectError: false,
},
{
name: "both empty namespaces",
authNamespace: "",
reqNamespace: "",
expectError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Test Check method with namespace matching
checkReq := authlib.CheckRequest{
Group: "unknown.group", // Use unknown group to bypass RBAC check
Resource: "unknown.resource",
Verb: utils.VerbGet,
Namespace: tt.reqNamespace,
}
// Create a mock auth info with the specified namespace
// Test Check method
user := &identity.StaticRequester{Namespace: tt.authNamespace}
_, checkErr := client.Check(ctx, user, checkReq)
// Test Compile method
compileReq := authlib.ListRequest{
Group: "unknown.group", // Use unknown group to bypass RBAC check
Resource: "unknown.resource",
Verb: utils.VerbGet,
Namespace: tt.reqNamespace,
}
_, compileErr := client.Compile(ctx, user, compileReq)
if tt.expectError {
require.Error(t, checkErr, "Check should return error")
require.Error(t, compileErr, "Compile should return error")
assert.ErrorIs(t, checkErr, authlib.ErrNamespaceMissmatch, "Check should return namespace mismatch error")
assert.ErrorIs(t, compileErr, authlib.ErrNamespaceMissmatch, "Compile should return namespace mismatch error")
} else {
assert.NoError(t, checkErr, "Check should not return error when namespaces match")
assert.NoError(t, compileErr, "Compile should not return error when namespaces match")
}
})
}
}
// TestNamespaceMatchingFallback tests namespace matching in Check and Compile methods when fallback is used
func TestNamespaceMatchingFallback(t *testing.T) {
// Create a mock client that always returns allowed=true
mockClient := authlib.FixedAccessClient(true)
client := NewAuthzLimitedClient(mockClient, AuthzOptions{})
// Create a context with fallback disabled
ctx := context.Background()
tests := []struct {
name string
authNamespace string
reqNamespace string
expectError bool
}{
{
name: "with namespace fallback",
reqNamespace: "ns1",
expectError: false,
},
{
name: "empty request namespace with fallback",
reqNamespace: "",
expectError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Test Check method with namespace matching
checkReq := authlib.CheckRequest{
Group: "unknown.group", // Use unknown group to bypass RBAC check
Resource: "unknown.resource",
Verb: utils.VerbGet,
Namespace: tt.reqNamespace,
}
ctx = grpcutils.WithFallback(ctx)
// Create a mock auth info with the specified namespace
// Test Check method
user := &identity.StaticRequester{Namespace: tt.authNamespace}
_, checkErr := client.Check(ctx, user, checkReq)
// Test Compile method
compileReq := authlib.ListRequest{
Group: "unknown.group", // Use unknown group to bypass RBAC check
Resource: "unknown.resource",
Verb: utils.VerbGet,
Namespace: tt.reqNamespace,
}
_, compileErr := client.Compile(ctx, user, compileReq)
if tt.expectError {
require.Error(t, checkErr, "Check should return error")
require.Error(t, compileErr, "Compile should return error")
assert.ErrorContains(t, checkErr, "namespace empty", "Check should return namespace mismatch error")
assert.ErrorContains(t, compileErr, "namespace empty", "Compile should return namespace mismatch error")
} else {
assert.NoError(t, checkErr, "Check should not return error when namespaces match")
assert.NoError(t, compileErr, "Compile should not return error when namespaces match")
}
})
}
}