From bb384a6dba8664653faeb10139d8f11fb62a8882 Mon Sep 17 00:00:00 2001 From: Roberto Jimenez Sanchez Date: Wed, 17 Dec 2025 20:21:40 +0100 Subject: [PATCH] provisioning: remove legacy access checker tests and refactor access checker implementation Deleted the access_checker_test.go file to streamline the codebase and focus on the updated access checker implementation. Refactored the access checker to enhance clarity and maintainability, ensuring it supports role-based fallback behavior. Updated the access checker integration in the API builder to utilize the new fallback role configuration, improving authorization logic across resources. --- apps/provisioning/pkg/auth/access_checker.go | 141 +-------------- .../pkg/auth/access_checker_mock.go | 135 +++++++++++++++ .../pkg/auth/session_access_checker.go | 93 ++++++++++ ...test.go => session_access_checker_test.go} | 153 ++++------------- .../pkg/auth/token_access_checker.go | 63 +++++++ .../pkg/auth/token_access_checker_test.go | 162 ++++++++++++++++++ pkg/registry/apis/provisioning/register.go | 13 +- 7 files changed, 499 insertions(+), 261 deletions(-) create mode 100644 apps/provisioning/pkg/auth/access_checker_mock.go create mode 100644 apps/provisioning/pkg/auth/session_access_checker.go rename apps/provisioning/pkg/auth/{access_checker_test.go => session_access_checker_test.go} (56%) create mode 100644 apps/provisioning/pkg/auth/token_access_checker.go create mode 100644 apps/provisioning/pkg/auth/token_access_checker_test.go diff --git a/apps/provisioning/pkg/auth/access_checker.go b/apps/provisioning/pkg/auth/access_checker.go index 72f34d7c92c..841c5421f2a 100644 --- a/apps/provisioning/pkg/auth/access_checker.go +++ b/apps/provisioning/pkg/auth/access_checker.go @@ -2,146 +2,21 @@ package auth import ( "context" - "fmt" - - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" authlib "github.com/grafana/authlib/types" "github.com/grafana/grafana/pkg/apimachinery/identity" ) -// AccessChecker provides access control checks with mode-aware behavior. -// It encapsulates the differences between multi-tenant (MT) and single-tenant (ST) modes: -// - MT mode: uses AuthInfo from access tokens, no role-based fallback -// - ST mode: uses Requester from Grafana sessions, optional role-based fallback +//go:generate mockery --name AccessChecker --structname MockAccessChecker --inpackage --filename access_checker_mock.go --with-expecter + +// AccessChecker provides access control checks with optional role-based fallback. type AccessChecker interface { // Check performs an access check and returns nil if allowed, or an appropriate - // API error if denied. Behavior depends on the mode: - // - MT mode: gets identity from AuthInfoFrom(ctx), no fallback - // - ST mode: gets identity from GetRequester(ctx), applies fallback if configured - // If req.Namespace is empty, it will be filled from the identity's namespace. + // API error if denied. If req.Namespace is empty, it will be filled from the + // identity's namespace. Check(ctx context.Context, req authlib.CheckRequest, folder string) error - // WithFallback returns a new AccessChecker configured with the specified fallback role. - // The fallback is only applied in ST mode. - WithFallback(role identity.RoleType) AccessChecker -} - -// accessChecker implements AccessChecker by wrapping authlib.AccessChecker. -type accessChecker struct { - inner authlib.AccessChecker - multiTenant bool - fallbackRole identity.RoleType -} - -// NewAccessChecker creates an AccessChecker with mode-aware behavior. -// -// Parameters: -// - inner: the underlying authlib.AccessChecker to delegate to -// - multiTenant: when true (MT), uses AuthInfoFrom and no fallback; -// when false (ST), uses GetRequester and applies fallback if configured -func NewAccessChecker(inner authlib.AccessChecker, multiTenant bool) AccessChecker { - return &accessChecker{ - inner: inner, - multiTenant: multiTenant, - fallbackRole: "", // no fallback by default - } -} - -// WithFallback returns a new AccessChecker with the specified fallback role. -// The fallback role is only applied in ST mode. -func (c *accessChecker) WithFallback(role identity.RoleType) AccessChecker { - return &accessChecker{ - inner: c.inner, - multiTenant: c.multiTenant, - fallbackRole: role, - } -} - -// Check performs an access check with mode-aware identity resolution and fallback. -// Returns nil if access is allowed, or an appropriate API error if denied. -func (c *accessChecker) Check(ctx context.Context, req authlib.CheckRequest, folder string) error { - // Get identity based on mode - id, err := c.getIdentity(ctx) - if err != nil { - return apierrors.NewUnauthorized(err.Error()) - } - - // AccessPolicy identities are trusted internal callers (ST->MT flow) - if authlib.IsIdentityType(id.GetIdentityType(), authlib.TypeAccessPolicy) { - return nil - } - - // Fill in namespace from identity if not provided - if req.Namespace == "" { - req.Namespace = id.GetNamespace() - } - - // Perform the access check - rsp, err := c.inner.Check(ctx, id, req, folder) - - // Build the GroupResource for error messages - gr := schema.GroupResource{Group: req.Group, Resource: req.Resource} - - // In MT mode or no fallback configured, return result directly - if c.multiTenant || c.fallbackRole == "" { - if err != nil { - return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("access check failed: %w", err)) - } - if !rsp.Allowed { - return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("permission denied")) - } - return nil - } - - // ST mode with fallback: apply fallback logic - requester, ok := id.(identity.Requester) - if !ok { - // Can't apply fallback without Requester interface - if err != nil { - return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("access check failed: %w", err)) - } - if !rsp.Allowed { - return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("permission denied")) - } - return nil - } - - if err != nil { - if requester.GetOrgRole().Includes(c.fallbackRole) { - return nil // Fallback succeeded - } - return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("access check failed: %w", err)) - } - - if rsp.Allowed { - return nil - } - - // Fall back to role for backwards compatibility - if requester.GetOrgRole().Includes(c.fallbackRole) { - return nil // Fallback succeeded - } - - return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("permission denied")) -} - -// getIdentity returns the appropriate identity based on the mode. -func (c *accessChecker) getIdentity(ctx context.Context) (authlib.AuthInfo, error) { - if c.multiTenant { - // MT mode: get identity from access token in context - info, ok := authlib.AuthInfoFrom(ctx) - if !ok { - return nil, fmt.Errorf("no auth info in context for multi-tenant mode") - } - return info, nil - } - - // ST mode: get identity from Grafana requester - id, err := identity.GetRequester(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get requester: %w", err) - } - return id, nil + // WithFallbackRole returns an AccessChecker configured with the specified fallback role. + // Whether the fallback is actually applied depends on the implementation. + WithFallbackRole(role identity.RoleType) AccessChecker } diff --git a/apps/provisioning/pkg/auth/access_checker_mock.go b/apps/provisioning/pkg/auth/access_checker_mock.go new file mode 100644 index 00000000000..d0f1cddd7fc --- /dev/null +++ b/apps/provisioning/pkg/auth/access_checker_mock.go @@ -0,0 +1,135 @@ +// Code generated by mockery v2.53.4. DO NOT EDIT. + +package auth + +import ( + context "context" + + identity "github.com/grafana/grafana/pkg/apimachinery/identity" + mock "github.com/stretchr/testify/mock" + + types "github.com/grafana/authlib/types" +) + +// MockAccessChecker is an autogenerated mock type for the AccessChecker type +type MockAccessChecker struct { + mock.Mock +} + +type MockAccessChecker_Expecter struct { + mock *mock.Mock +} + +func (_m *MockAccessChecker) EXPECT() *MockAccessChecker_Expecter { + return &MockAccessChecker_Expecter{mock: &_m.Mock} +} + +// Check provides a mock function with given fields: ctx, req, folder +func (_m *MockAccessChecker) Check(ctx context.Context, req types.CheckRequest, folder string) error { + ret := _m.Called(ctx, req, folder) + + if len(ret) == 0 { + panic("no return value specified for Check") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, types.CheckRequest, string) error); ok { + r0 = rf(ctx, req, folder) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockAccessChecker_Check_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Check' +type MockAccessChecker_Check_Call struct { + *mock.Call +} + +// Check is a helper method to define mock.On call +// - ctx context.Context +// - req types.CheckRequest +// - folder string +func (_e *MockAccessChecker_Expecter) Check(ctx interface{}, req interface{}, folder interface{}) *MockAccessChecker_Check_Call { + return &MockAccessChecker_Check_Call{Call: _e.mock.On("Check", ctx, req, folder)} +} + +func (_c *MockAccessChecker_Check_Call) Run(run func(ctx context.Context, req types.CheckRequest, folder string)) *MockAccessChecker_Check_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(types.CheckRequest), args[2].(string)) + }) + return _c +} + +func (_c *MockAccessChecker_Check_Call) Return(_a0 error) *MockAccessChecker_Check_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockAccessChecker_Check_Call) RunAndReturn(run func(context.Context, types.CheckRequest, string) error) *MockAccessChecker_Check_Call { + _c.Call.Return(run) + return _c +} + +// WithFallbackRole provides a mock function with given fields: role +func (_m *MockAccessChecker) WithFallbackRole(role identity.RoleType) AccessChecker { + ret := _m.Called(role) + + if len(ret) == 0 { + panic("no return value specified for WithFallbackRole") + } + + var r0 AccessChecker + if rf, ok := ret.Get(0).(func(identity.RoleType) AccessChecker); ok { + r0 = rf(role) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(AccessChecker) + } + } + + return r0 +} + +// MockAccessChecker_WithFallbackRole_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WithFallbackRole' +type MockAccessChecker_WithFallbackRole_Call struct { + *mock.Call +} + +// WithFallbackRole is a helper method to define mock.On call +// - role identity.RoleType +func (_e *MockAccessChecker_Expecter) WithFallbackRole(role interface{}) *MockAccessChecker_WithFallbackRole_Call { + return &MockAccessChecker_WithFallbackRole_Call{Call: _e.mock.On("WithFallbackRole", role)} +} + +func (_c *MockAccessChecker_WithFallbackRole_Call) Run(run func(role identity.RoleType)) *MockAccessChecker_WithFallbackRole_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(identity.RoleType)) + }) + return _c +} + +func (_c *MockAccessChecker_WithFallbackRole_Call) Return(_a0 AccessChecker) *MockAccessChecker_WithFallbackRole_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockAccessChecker_WithFallbackRole_Call) RunAndReturn(run func(identity.RoleType) AccessChecker) *MockAccessChecker_WithFallbackRole_Call { + _c.Call.Return(run) + return _c +} + +// NewMockAccessChecker creates a new instance of MockAccessChecker. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMockAccessChecker(t interface { + mock.TestingT + Cleanup(func()) +}) *MockAccessChecker { + mock := &MockAccessChecker{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/apps/provisioning/pkg/auth/session_access_checker.go b/apps/provisioning/pkg/auth/session_access_checker.go new file mode 100644 index 00000000000..8a6ca55dad6 --- /dev/null +++ b/apps/provisioning/pkg/auth/session_access_checker.go @@ -0,0 +1,93 @@ +package auth + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + + authlib "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/apimachinery/identity" +) + +// sessionAccessChecker implements AccessChecker using Grafana session identity. +type sessionAccessChecker struct { + inner authlib.AccessChecker + fallbackRole identity.RoleType +} + +// NewSessionAccessChecker creates an AccessChecker that gets identity from Grafana +// sessions via GetRequester(ctx). Supports optional role-based fallback via +// WithFallbackRole for backwards compatibility. +func NewSessionAccessChecker(inner authlib.AccessChecker) AccessChecker { + return &sessionAccessChecker{ + inner: inner, + fallbackRole: "", + } +} + +// WithFallbackRole returns a new AccessChecker with the specified fallback role. +func (c *sessionAccessChecker) WithFallbackRole(role identity.RoleType) AccessChecker { + return &sessionAccessChecker{ + inner: c.inner, + fallbackRole: role, + } +} + +// Check performs an access check with optional role-based fallback. +// Returns nil if access is allowed, or an appropriate API error if denied. +func (c *sessionAccessChecker) Check(ctx context.Context, req authlib.CheckRequest, folder string) error { + // Get identity from Grafana session + requester, err := identity.GetRequester(ctx) + if err != nil { + return apierrors.NewUnauthorized(fmt.Sprintf("failed to get requester: %v", err)) + } + + // AccessPolicy identities are trusted internal callers + if authlib.IsIdentityType(requester.GetIdentityType(), authlib.TypeAccessPolicy) { + return nil + } + + // Fill in namespace from identity if not provided + if req.Namespace == "" { + req.Namespace = requester.GetNamespace() + } + + // Perform the access check + rsp, err := c.inner.Check(ctx, requester, req, folder) + + // Build the GroupResource for error messages + gr := schema.GroupResource{Group: req.Group, Resource: req.Resource} + + // No fallback configured, return result directly + if c.fallbackRole == "" { + if err != nil { + return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("access check failed: %w", err)) + } + if !rsp.Allowed { + return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("permission denied")) + } + return nil + } + + // Fallback is configured - apply fallback logic + if err != nil { + if requester.GetOrgRole().Includes(c.fallbackRole) { + return nil // Fallback succeeded + } + return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("access check failed: %w", err)) + } + + if rsp.Allowed { + return nil + } + + // Fall back to role for backwards compatibility + if requester.GetOrgRole().Includes(c.fallbackRole) { + return nil // Fallback succeeded + } + + return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("permission denied")) +} + diff --git a/apps/provisioning/pkg/auth/access_checker_test.go b/apps/provisioning/pkg/auth/session_access_checker_test.go similarity index 56% rename from apps/provisioning/pkg/auth/access_checker_test.go rename to apps/provisioning/pkg/auth/session_access_checker_test.go index 093b77ccb71..ac9411c993f 100644 --- a/apps/provisioning/pkg/auth/access_checker_test.go +++ b/apps/provisioning/pkg/auth/session_access_checker_test.go @@ -14,20 +14,6 @@ import ( "github.com/stretchr/testify/require" ) -// mockAccessChecker implements authlib.AccessChecker for testing. -type mockAccessChecker struct { - response authlib.CheckResponse - err error -} - -func (m *mockAccessChecker) Check(_ context.Context, _ authlib.AuthInfo, _ authlib.CheckRequest, _ string) (authlib.CheckResponse, error) { - return m.response, m.err -} - -func (m *mockAccessChecker) Compile(_ context.Context, _ authlib.AuthInfo, _ authlib.ListRequest) (authlib.ItemChecker, authlib.Zookie, error) { - return nil, nil, nil -} - // mockRequester implements identity.Requester for testing. type mockRequester struct { identity.Requester @@ -48,7 +34,7 @@ func (m *mockRequester) GetNamespace() string { return m.namespace } -func TestAccessChecker_Check_SingleTenant(t *testing.T) { +func TestSessionAccessChecker_Check(t *testing.T) { ctx := context.Background() req := authlib.CheckRequest{ Verb: "get", @@ -132,17 +118,17 @@ func TestAccessChecker_Check_SingleTenant(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - mock := &mockAccessChecker{ + mock := &mockInnerAccessChecker{ response: tt.innerResponse, err: tt.innerErr, } - checker := NewAccessChecker(mock, false) // ST mode + checker := NewSessionAccessChecker(mock) if tt.fallbackRole != "" { - checker = checker.WithFallback(tt.fallbackRole) + checker = checker.WithFallbackRole(tt.fallbackRole) } - // Add requester to context (ST mode uses GetRequester) + // Add requester to context testCtx := identity.WithRequester(ctx, tt.requester) err := checker.Check(testCtx, req, "") @@ -157,107 +143,26 @@ func TestAccessChecker_Check_SingleTenant(t *testing.T) { } } -func TestAccessChecker_Check_MultiTenant(t *testing.T) { - req := authlib.CheckRequest{ - Verb: "get", - Group: "provisioning.grafana.app", - Resource: "repositories", - Name: "test-repo", - Namespace: "default", - } - - tests := []struct { - name string - fallbackRole identity.RoleType - innerResponse authlib.CheckResponse - innerErr error - authInfo authlib.AuthInfo - expectAllow bool - }{ - { - name: "allowed by checker", - fallbackRole: identity.RoleAdmin, - innerResponse: authlib.CheckResponse{Allowed: true}, - authInfo: &mockRequester{orgRole: identity.RoleViewer, identityType: authlib.TypeUser}, - expectAllow: true, - }, - { - name: "denied by checker, no fallback even with admin role", - fallbackRole: identity.RoleAdmin, - innerResponse: authlib.CheckResponse{Allowed: false}, - authInfo: &mockRequester{orgRole: identity.RoleAdmin, identityType: authlib.TypeUser}, - expectAllow: false, // MT mode: no fallback - }, - { - name: "error from checker, no fallback even with admin role", - fallbackRole: identity.RoleAdmin, - innerErr: errors.New("access check failed"), - authInfo: &mockRequester{orgRole: identity.RoleAdmin, identityType: authlib.TypeUser}, - expectAllow: false, // MT mode: no fallback - }, - { - name: "AccessPolicy identity is always allowed", - innerResponse: authlib.CheckResponse{Allowed: false}, - authInfo: &mockRequester{orgRole: identity.RoleViewer, identityType: authlib.TypeAccessPolicy}, - expectAllow: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - mock := &mockAccessChecker{ - response: tt.innerResponse, - err: tt.innerErr, - } - - checker := NewAccessChecker(mock, true) // MT mode - if tt.fallbackRole != "" { - checker = checker.WithFallback(tt.fallbackRole) - } - - // Add auth info to context (MT mode uses AuthInfoFrom) - testCtx := authlib.WithAuthInfo(context.Background(), tt.authInfo) - - err := checker.Check(testCtx, req, "") - - if tt.expectAllow { - require.NoError(t, err) - } else { - require.Error(t, err) - assert.True(t, apierrors.IsForbidden(err), "expected Forbidden error, got: %v", err) - } - }) - } -} - -func TestAccessChecker_Check_NoIdentity(t *testing.T) { - mock := &mockAccessChecker{ +func TestSessionAccessChecker_NoRequester(t *testing.T) { + mock := &mockInnerAccessChecker{ response: authlib.CheckResponse{Allowed: true}, } - t.Run("ST mode without requester", func(t *testing.T) { - checker := NewAccessChecker(mock, false) // ST mode - err := checker.Check(context.Background(), authlib.CheckRequest{}, "") - require.Error(t, err) - assert.True(t, apierrors.IsUnauthorized(err), "expected Unauthorized error") - }) + checker := NewSessionAccessChecker(mock) + err := checker.Check(context.Background(), authlib.CheckRequest{}, "") - t.Run("MT mode without auth info", func(t *testing.T) { - checker := NewAccessChecker(mock, true) // MT mode - err := checker.Check(context.Background(), authlib.CheckRequest{}, "") - require.Error(t, err) - assert.True(t, apierrors.IsUnauthorized(err), "expected Unauthorized error") - }) + require.Error(t, err) + assert.True(t, apierrors.IsUnauthorized(err), "expected Unauthorized error") } -func TestAccessChecker_WithFallback_ImmutableOriginal(t *testing.T) { - mock := &mockAccessChecker{ +func TestSessionAccessChecker_WithFallbackRole_ImmutableOriginal(t *testing.T) { + mock := &mockInnerAccessChecker{ response: authlib.CheckResponse{Allowed: false}, } - original := NewAccessChecker(mock, false) // ST mode - withAdmin := original.WithFallback(identity.RoleAdmin) - withEditor := original.WithFallback(identity.RoleEditor) + original := NewSessionAccessChecker(mock) + withAdmin := original.WithFallbackRole(identity.RoleAdmin) + withEditor := original.WithFallbackRole(identity.RoleEditor) ctx := identity.WithRequester(context.Background(), &mockRequester{ orgRole: identity.RoleEditor, @@ -279,15 +184,15 @@ func TestAccessChecker_WithFallback_ImmutableOriginal(t *testing.T) { require.NoError(t, err, "editor fallback should allow for editor") } -func TestAccessChecker_WithFallback_ChainedCalls(t *testing.T) { - mock := &mockAccessChecker{ +func TestSessionAccessChecker_WithFallbackRole_ChainedCalls(t *testing.T) { + mock := &mockInnerAccessChecker{ response: authlib.CheckResponse{Allowed: false}, } - // Ensure chained WithFallback calls work correctly - checker := NewAccessChecker(mock, false). // ST mode - WithFallback(identity.RoleAdmin). - WithFallback(identity.RoleEditor) // This should override admin + // Ensure chained WithFallbackRole calls work correctly + checker := NewSessionAccessChecker(mock). + WithFallbackRole(identity.RoleAdmin). + WithFallbackRole(identity.RoleEditor) // This should override admin ctx := identity.WithRequester(context.Background(), &mockRequester{ orgRole: identity.RoleEditor, @@ -298,12 +203,12 @@ func TestAccessChecker_WithFallback_ChainedCalls(t *testing.T) { require.NoError(t, err, "last fallback (editor) should be used") } -func TestAccessChecker_RealSignedInUser(t *testing.T) { - mock := &mockAccessChecker{ +func TestSessionAccessChecker_RealSignedInUser(t *testing.T) { + mock := &mockInnerAccessChecker{ response: authlib.CheckResponse{Allowed: false}, } - checker := NewAccessChecker(mock, false).WithFallback(identity.RoleAdmin) // ST mode + checker := NewSessionAccessChecker(mock).WithFallbackRole(identity.RoleAdmin) // Use a real SignedInUser signedInUser := &user.SignedInUser{ @@ -318,12 +223,12 @@ func TestAccessChecker_RealSignedInUser(t *testing.T) { require.NoError(t, err, "admin user should be allowed via fallback") } -func TestAccessChecker_Check_FillsNamespace(t *testing.T) { - mock := &mockAccessChecker{ +func TestSessionAccessChecker_FillsNamespace(t *testing.T) { + mock := &mockInnerAccessChecker{ response: authlib.CheckResponse{Allowed: true}, } - checker := NewAccessChecker(mock, false) // ST mode + checker := NewSessionAccessChecker(mock) ctx := identity.WithRequester(context.Background(), &mockRequester{ orgRole: identity.RoleAdmin, @@ -342,5 +247,5 @@ func TestAccessChecker_Check_FillsNamespace(t *testing.T) { err := checker.Check(ctx, req, "") require.NoError(t, err) - // The namespace should have been filled from the identity } + diff --git a/apps/provisioning/pkg/auth/token_access_checker.go b/apps/provisioning/pkg/auth/token_access_checker.go new file mode 100644 index 00000000000..659a9905124 --- /dev/null +++ b/apps/provisioning/pkg/auth/token_access_checker.go @@ -0,0 +1,63 @@ +package auth + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + + authlib "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/apimachinery/identity" +) + +// tokenAccessChecker implements AccessChecker using access tokens from context. +type tokenAccessChecker struct { + inner authlib.AccessChecker +} + +// NewTokenAccessChecker creates an AccessChecker that gets identity from access tokens +// via AuthInfoFrom(ctx). Role-based fallback is not supported. +func NewTokenAccessChecker(inner authlib.AccessChecker) AccessChecker { + return &tokenAccessChecker{inner: inner} +} + +// WithFallbackRole returns the same checker since fallback is not supported. +func (c *tokenAccessChecker) WithFallbackRole(_ identity.RoleType) AccessChecker { + return c +} + +// Check performs an access check using AuthInfo from context. +// Returns nil if access is allowed, or an appropriate API error if denied. +func (c *tokenAccessChecker) Check(ctx context.Context, req authlib.CheckRequest, folder string) error { + // Get identity from access token in context + id, ok := authlib.AuthInfoFrom(ctx) + if !ok { + return apierrors.NewUnauthorized("no auth info in context") + } + + // AccessPolicy identities are trusted internal callers + if authlib.IsIdentityType(id.GetIdentityType(), authlib.TypeAccessPolicy) { + return nil + } + + // Fill in namespace from identity if not provided + if req.Namespace == "" { + req.Namespace = id.GetNamespace() + } + + // Perform the access check + rsp, err := c.inner.Check(ctx, id, req, folder) + + // Build the GroupResource for error messages + gr := schema.GroupResource{Group: req.Group, Resource: req.Resource} + + if err != nil { + return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("access check failed: %w", err)) + } + if !rsp.Allowed { + return apierrors.NewForbidden(gr, req.Name, fmt.Errorf("permission denied")) + } + return nil +} + diff --git a/apps/provisioning/pkg/auth/token_access_checker_test.go b/apps/provisioning/pkg/auth/token_access_checker_test.go new file mode 100644 index 00000000000..8028c29b7bf --- /dev/null +++ b/apps/provisioning/pkg/auth/token_access_checker_test.go @@ -0,0 +1,162 @@ +package auth + +import ( + "context" + "errors" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + + authlib "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTokenAccessChecker_Check(t *testing.T) { + req := authlib.CheckRequest{ + Verb: "get", + Group: "provisioning.grafana.app", + Resource: "repositories", + Name: "test-repo", + Namespace: "default", + } + + tests := []struct { + name string + innerResponse authlib.CheckResponse + innerErr error + authInfo *mockAuthInfo + expectAllow bool + }{ + { + name: "allowed by checker", + innerResponse: authlib.CheckResponse{Allowed: true}, + authInfo: &mockAuthInfo{identityType: authlib.TypeUser}, + expectAllow: true, + }, + { + name: "denied by checker", + innerResponse: authlib.CheckResponse{Allowed: false}, + authInfo: &mockAuthInfo{identityType: authlib.TypeUser}, + expectAllow: false, + }, + { + name: "error from checker", + innerErr: errors.New("access check failed"), + authInfo: &mockAuthInfo{identityType: authlib.TypeUser}, + expectAllow: false, + }, + { + name: "AccessPolicy identity is always allowed", + innerResponse: authlib.CheckResponse{Allowed: false}, + authInfo: &mockAuthInfo{identityType: authlib.TypeAccessPolicy}, + expectAllow: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock := &mockInnerAccessChecker{ + response: tt.innerResponse, + err: tt.innerErr, + } + + checker := NewTokenAccessChecker(mock) + + // Add auth info to context + testCtx := authlib.WithAuthInfo(context.Background(), tt.authInfo) + + err := checker.Check(testCtx, req, "") + + if tt.expectAllow { + require.NoError(t, err) + } else { + require.Error(t, err) + assert.True(t, apierrors.IsForbidden(err), "expected Forbidden error, got: %v", err) + } + }) + } +} + +func TestTokenAccessChecker_NoAuthInfo(t *testing.T) { + mock := &mockInnerAccessChecker{ + response: authlib.CheckResponse{Allowed: true}, + } + + checker := NewTokenAccessChecker(mock) + err := checker.Check(context.Background(), authlib.CheckRequest{}, "") + + require.Error(t, err) + assert.True(t, apierrors.IsUnauthorized(err), "expected Unauthorized error") +} + +func TestTokenAccessChecker_WithFallbackRole_IsNoOp(t *testing.T) { + mock := &mockInnerAccessChecker{ + response: authlib.CheckResponse{Allowed: false}, + } + + checker := NewTokenAccessChecker(mock) + checkerWithFallback := checker.WithFallbackRole(identity.RoleAdmin) + + // They should be the same instance + assert.Same(t, checker, checkerWithFallback, "WithFallbackRole should return same instance") +} + +func TestTokenAccessChecker_FillsNamespace(t *testing.T) { + mock := &mockInnerAccessChecker{ + response: authlib.CheckResponse{Allowed: true}, + } + + checker := NewTokenAccessChecker(mock) + + ctx := authlib.WithAuthInfo(context.Background(), &mockAuthInfo{ + identityType: authlib.TypeUser, + namespace: "org-123", + }) + + // Request without namespace + req := authlib.CheckRequest{ + Verb: "get", + Group: "provisioning.grafana.app", + Resource: "repositories", + Name: "test-repo", + // Namespace intentionally empty + } + + err := checker.Check(ctx, req, "") + require.NoError(t, err) +} + +// mockAuthInfo implements authlib.AuthInfo for testing. +type mockAuthInfo struct { + identityType authlib.IdentityType + namespace string +} + +func (m *mockAuthInfo) GetIdentityType() authlib.IdentityType { + return m.identityType +} + +func (m *mockAuthInfo) GetNamespace() string { + return m.namespace +} + +func (m *mockAuthInfo) GetSubject() string { + return "user:1" +} + +// mockInnerAccessChecker implements authlib.AccessChecker for testing. +type mockInnerAccessChecker struct { + response authlib.CheckResponse + err error +} + +func (m *mockInnerAccessChecker) Check(_ context.Context, _ authlib.AuthInfo, _ authlib.CheckRequest, _ string) (authlib.CheckResponse, error) { + return m.response, m.err +} + +func (m *mockInnerAccessChecker) Compile(_ context.Context, _ authlib.AuthInfo, _ authlib.ListRequest) (authlib.ItemChecker, authlib.Zookie, error) { + return nil, nil, nil +} + diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index 956abf4b175..3ce8185a713 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -161,8 +161,13 @@ func NewAPIBuilder( parsers := resources.NewParserFactory(clients) resourceLister := resources.NewResourceListerForMigrations(unified) - // Create access checker with fallback behavior based on mode - accessChecker := auth.NewAccessChecker(access, useExclusivelyAccessCheckerForAuthz) + // Create access checker based on mode + var accessChecker auth.AccessChecker + if useExclusivelyAccessCheckerForAuthz { + accessChecker = auth.NewTokenAccessChecker(access) + } else { + accessChecker = auth.NewSessionAccessChecker(access) + } b := &APIBuilder{ onlyApiServer: onlyApiServer, @@ -177,8 +182,8 @@ func NewAPIBuilder( dashboardAccess: dashboardAccess, unified: unified, access: accessChecker, - accessWithAdmin: accessChecker.WithFallback(identity.RoleAdmin), - accessWithEditor: accessChecker.WithFallback(identity.RoleEditor), + accessWithAdmin: accessChecker.WithFallbackRole(identity.RoleAdmin), + accessWithEditor: accessChecker.WithFallbackRole(identity.RoleEditor), jobHistoryConfig: jobHistoryConfig, extraWorkers: extraWorkers, restConfigGetter: restConfigGetter,