diff --git a/apps/provisioning/pkg/auth/session_access_checker.go b/apps/provisioning/pkg/auth/session_access_checker.go deleted file mode 100644 index 00348ef8e59..00000000000 --- a/apps/provisioning/pkg/auth/session_access_checker.go +++ /dev/null @@ -1,88 +0,0 @@ -package auth - -import ( - "context" - "fmt" - "strings" - - 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)) - } - - // 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("%s role is required", strings.ToLower(string(c.fallbackRole)))) -} diff --git a/apps/provisioning/pkg/auth/session_access_checker_test.go b/apps/provisioning/pkg/auth/session_access_checker_test.go deleted file mode 100644 index 1e99a6e46db..00000000000 --- a/apps/provisioning/pkg/auth/session_access_checker_test.go +++ /dev/null @@ -1,244 +0,0 @@ -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/grafana/grafana/pkg/services/user" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// mockRequester implements identity.Requester for testing. -type mockRequester struct { - identity.Requester - orgRole identity.RoleType - identityType authlib.IdentityType - namespace string -} - -func (m *mockRequester) GetOrgRole() identity.RoleType { - return m.orgRole -} - -func (m *mockRequester) GetIdentityType() authlib.IdentityType { - return m.identityType -} - -func (m *mockRequester) GetNamespace() string { - return m.namespace -} - -func TestSessionAccessChecker_Check(t *testing.T) { - ctx := context.Background() - 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 - requester *mockRequester - expectAllow bool - }{ - { - name: "allowed by checker", - fallbackRole: identity.RoleAdmin, - innerResponse: authlib.CheckResponse{Allowed: true}, - requester: &mockRequester{orgRole: identity.RoleViewer, identityType: authlib.TypeUser}, - expectAllow: true, - }, - { - name: "denied by checker, fallback to admin role succeeds", - fallbackRole: identity.RoleAdmin, - innerResponse: authlib.CheckResponse{Allowed: false}, - requester: &mockRequester{orgRole: identity.RoleAdmin, identityType: authlib.TypeUser}, - expectAllow: true, - }, - { - name: "denied by checker, fallback to admin role fails for viewer", - fallbackRole: identity.RoleAdmin, - innerResponse: authlib.CheckResponse{Allowed: false}, - requester: &mockRequester{orgRole: identity.RoleViewer, identityType: authlib.TypeUser}, - expectAllow: false, - }, - { - name: "error from checker, fallback to admin role succeeds", - fallbackRole: identity.RoleAdmin, - innerErr: errors.New("access check failed"), - requester: &mockRequester{orgRole: identity.RoleAdmin, identityType: authlib.TypeUser}, - expectAllow: true, - }, - { - name: "error from checker, fallback fails for viewer", - fallbackRole: identity.RoleAdmin, - innerErr: errors.New("access check failed"), - requester: &mockRequester{orgRole: identity.RoleViewer, identityType: authlib.TypeUser}, - expectAllow: false, - }, - { - name: "denied, editor fallback succeeds for editor", - fallbackRole: identity.RoleEditor, - innerResponse: authlib.CheckResponse{Allowed: false}, - requester: &mockRequester{orgRole: identity.RoleEditor, identityType: authlib.TypeUser}, - expectAllow: true, - }, - { - name: "denied, editor fallback fails for viewer", - fallbackRole: identity.RoleEditor, - innerResponse: authlib.CheckResponse{Allowed: false}, - requester: &mockRequester{orgRole: identity.RoleViewer, identityType: authlib.TypeUser}, - expectAllow: false, - }, - { - name: "no fallback configured, denied stays denied", - fallbackRole: "", // no fallback - innerResponse: authlib.CheckResponse{Allowed: false}, - requester: &mockRequester{orgRole: identity.RoleAdmin, identityType: authlib.TypeUser}, - expectAllow: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - mock := &mockInnerAccessChecker{ - response: tt.innerResponse, - err: tt.innerErr, - } - - checker := NewSessionAccessChecker(mock) - if tt.fallbackRole != "" { - checker = checker.WithFallbackRole(tt.fallbackRole) - } - - // Add requester to context - testCtx := identity.WithRequester(ctx, tt.requester) - - 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 TestSessionAccessChecker_NoRequester(t *testing.T) { - mock := &mockInnerAccessChecker{ - response: authlib.CheckResponse{Allowed: true}, - } - - checker := NewSessionAccessChecker(mock) - err := checker.Check(context.Background(), authlib.CheckRequest{}, "") - - require.Error(t, err) - assert.True(t, apierrors.IsUnauthorized(err), "expected Unauthorized error") -} - -func TestSessionAccessChecker_WithFallbackRole_ImmutableOriginal(t *testing.T) { - mock := &mockInnerAccessChecker{ - response: authlib.CheckResponse{Allowed: false}, - } - - original := NewSessionAccessChecker(mock) - withAdmin := original.WithFallbackRole(identity.RoleAdmin) - withEditor := original.WithFallbackRole(identity.RoleEditor) - - ctx := identity.WithRequester(context.Background(), &mockRequester{ - orgRole: identity.RoleEditor, - identityType: authlib.TypeUser, - }) - - req := authlib.CheckRequest{} - - // Original should deny (no fallback) - err := original.Check(ctx, req, "") - require.Error(t, err, "original should deny without fallback") - - // WithAdmin should deny for editor - err = withAdmin.Check(ctx, req, "") - require.Error(t, err, "admin fallback should deny for editor") - - // WithEditor should allow for editor - err = withEditor.Check(ctx, req, "") - require.NoError(t, err, "editor fallback should allow for editor") -} - -func TestSessionAccessChecker_WithFallbackRole_ChainedCalls(t *testing.T) { - mock := &mockInnerAccessChecker{ - response: authlib.CheckResponse{Allowed: false}, - } - - // 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, - identityType: authlib.TypeUser, - }) - - err := checker.Check(ctx, authlib.CheckRequest{}, "") - require.NoError(t, err, "last fallback (editor) should be used") -} - -func TestSessionAccessChecker_RealSignedInUser(t *testing.T) { - mock := &mockInnerAccessChecker{ - response: authlib.CheckResponse{Allowed: false}, - } - - checker := NewSessionAccessChecker(mock).WithFallbackRole(identity.RoleAdmin) - - // Use a real SignedInUser - signedInUser := &user.SignedInUser{ - UserID: 1, - OrgID: 1, - OrgRole: identity.RoleAdmin, - } - - ctx := identity.WithRequester(context.Background(), signedInUser) - - err := checker.Check(ctx, authlib.CheckRequest{}, "") - require.NoError(t, err, "admin user should be allowed via fallback") -} - -func TestSessionAccessChecker_FillsNamespace(t *testing.T) { - mock := &mockInnerAccessChecker{ - response: authlib.CheckResponse{Allowed: true}, - } - - checker := NewSessionAccessChecker(mock) - - ctx := identity.WithRequester(context.Background(), &mockRequester{ - orgRole: identity.RoleAdmin, - 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) -} diff --git a/apps/provisioning/pkg/auth/token_access_checker.go b/apps/provisioning/pkg/auth/token_access_checker.go deleted file mode 100644 index d2c6f9e1c5b..00000000000 --- a/apps/provisioning/pkg/auth/token_access_checker.go +++ /dev/null @@ -1,57 +0,0 @@ -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") - } - - // 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 deleted file mode 100644 index bce0d3a77a0..00000000000 --- a/apps/provisioning/pkg/auth/token_access_checker_test.go +++ /dev/null @@ -1,137 +0,0 @@ -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 *identity.StaticRequester - expectAllow bool - }{ - { - name: "allowed by checker", - innerResponse: authlib.CheckResponse{Allowed: true}, - authInfo: &identity.StaticRequester{Type: authlib.TypeUser}, - expectAllow: true, - }, - { - name: "denied by checker", - innerResponse: authlib.CheckResponse{Allowed: false}, - authInfo: &identity.StaticRequester{Type: authlib.TypeUser}, - expectAllow: false, - }, - { - name: "error from checker", - innerErr: errors.New("access check failed"), - authInfo: &identity.StaticRequester{Type: authlib.TypeUser}, - expectAllow: false, - }, - } - - 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(), &identity.StaticRequester{ - Type: 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) -} - -// 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/apps/provisioning/pkg/auth/unified_access_checker.go b/apps/provisioning/pkg/auth/unified_access_checker.go new file mode 100644 index 00000000000..d62f8ecdb45 --- /dev/null +++ b/apps/provisioning/pkg/auth/unified_access_checker.go @@ -0,0 +1,137 @@ +package auth + +import ( + "context" + "fmt" + "strings" + + 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" +) + +// unifiedAccessChecker implements AccessChecker with the original fallthrough behavior: +// 1. First try to get identity from access token (authlib.AuthInfoFrom) +// 2. If token exists and conditions are met, use the access checker with token identity +// 3. If no token, fall back to session identity (identity.GetRequester) +// 4. Apply role-based fallback if configured +type unifiedAccessChecker struct { + inner authlib.AccessChecker + fallbackRole identity.RoleType + useExclusivelyAccessCheckerForAuthz bool +} + +// NewAccessChecker creates an AccessChecker that implements the original fallthrough behavior. +// +// When useExclusivelyAccessCheckerForAuthz is true (MT mode), it will: +// - Try to get identity from access token first +// - If token exists, use the access checker +// - If no token, fall back to session identity +// +// When useExclusivelyAccessCheckerForAuthz is false (ST mode), it will: +// - Try to get identity from access token first +// - If token exists AND is TypeAccessPolicy, use the access checker +// - Otherwise, fall back to session identity with role-based fallback +func NewAccessChecker(inner authlib.AccessChecker, useExclusivelyAccessCheckerForAuthz bool) AccessChecker { + return &unifiedAccessChecker{ + inner: inner, + fallbackRole: "", + useExclusivelyAccessCheckerForAuthz: useExclusivelyAccessCheckerForAuthz, + } +} + +// WithFallbackRole returns a new AccessChecker with the specified fallback role. +// The fallback role is applied when the access checker denies access but the user +// has the required org role. This is primarily used in ST mode for backwards compatibility. +func (c *unifiedAccessChecker) WithFallbackRole(role identity.RoleType) AccessChecker { + return &unifiedAccessChecker{ + inner: c.inner, + fallbackRole: role, + useExclusivelyAccessCheckerForAuthz: c.useExclusivelyAccessCheckerForAuthz, + } +} + +// Check performs an access check with the original fallthrough behavior. +// Returns nil if access is allowed, or an appropriate API error if denied. +func (c *unifiedAccessChecker) Check(ctx context.Context, req authlib.CheckRequest, folder string) error { + gr := schema.GroupResource{Group: req.Group, Resource: req.Resource} + + // First try: get identity from access token + if info, ok := authlib.AuthInfoFrom(ctx); ok { + // When running as standalone API server, the identity type may not always match TypeAccessPolicy + // so we allow it to use the access checker if there is any auth info available + if authlib.IsIdentityType(info.GetIdentityType(), authlib.TypeAccessPolicy) || c.useExclusivelyAccessCheckerForAuthz { + return c.checkWithAuthInfo(ctx, info, req, folder, gr) + } + } + + // Fallback: get identity from Grafana session + requester, err := identity.GetRequester(ctx) + if err != nil { + return apierrors.NewUnauthorized(fmt.Sprintf("no identity in context: %v", err)) + } + + return c.checkWithRequester(ctx, requester, req, folder, gr) +} + +// checkWithAuthInfo performs access check using AuthInfo from access token. +func (c *unifiedAccessChecker) checkWithAuthInfo(ctx context.Context, info authlib.AuthInfo, req authlib.CheckRequest, folder string, gr schema.GroupResource) error { + // Fill in namespace from identity if not provided + if req.Namespace == "" { + req.Namespace = info.GetNamespace() + } + + // Perform the access check + rsp, err := c.inner.Check(ctx, info, req, folder) + + 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 +} + +// checkWithRequester performs access check using Requester from session with optional role fallback. +func (c *unifiedAccessChecker) checkWithRequester(ctx context.Context, requester identity.Requester, req authlib.CheckRequest, folder string, gr schema.GroupResource) error { + // 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) + + // 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("%s role is required", strings.ToLower(string(c.fallbackRole)))) +} diff --git a/apps/provisioning/pkg/auth/unified_access_checker_test.go b/apps/provisioning/pkg/auth/unified_access_checker_test.go new file mode 100644 index 00000000000..1388c500b42 --- /dev/null +++ b/apps/provisioning/pkg/auth/unified_access_checker_test.go @@ -0,0 +1,282 @@ +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" +) + +// mockInnerAccessChecker is a mock implementation of 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, authlib.NoopZookie{}, nil +} + +func TestUnifiedAccessChecker_WithTokenIdentity(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 + useExclusivelyAccessCheckerForAuthz bool + authInfo *identity.StaticRequester + expectAllow bool + }{ + { + name: "allowed by checker with token in MT mode", + innerResponse: authlib.CheckResponse{Allowed: true}, + useExclusivelyAccessCheckerForAuthz: true, + authInfo: &identity.StaticRequester{Type: authlib.TypeUser}, + expectAllow: true, + }, + { + name: "denied by checker with token in MT mode", + innerResponse: authlib.CheckResponse{Allowed: false}, + useExclusivelyAccessCheckerForAuthz: true, + authInfo: &identity.StaticRequester{Type: authlib.TypeUser}, + expectAllow: false, + }, + { + name: "allowed by checker with AccessPolicy in ST mode", + innerResponse: authlib.CheckResponse{Allowed: true}, + useExclusivelyAccessCheckerForAuthz: false, + authInfo: &identity.StaticRequester{Type: authlib.TypeAccessPolicy}, + expectAllow: true, + }, + { + name: "error from checker", + innerErr: errors.New("access check failed"), + useExclusivelyAccessCheckerForAuthz: true, + authInfo: &identity.StaticRequester{Type: authlib.TypeUser}, + expectAllow: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock := &mockInnerAccessChecker{ + response: tt.innerResponse, + err: tt.innerErr, + } + + checker := NewAccessChecker(mock, tt.useExclusivelyAccessCheckerForAuthz) + + // 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 TestUnifiedAccessChecker_WithSessionIdentity(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 + fallbackRole identity.RoleType + requesterRole identity.RoleType + expectAllow bool + }{ + { + name: "allowed by checker without fallback", + innerResponse: authlib.CheckResponse{Allowed: true}, + fallbackRole: "", + requesterRole: identity.RoleViewer, + expectAllow: true, + }, + { + name: "denied by checker without fallback", + innerResponse: authlib.CheckResponse{Allowed: false}, + fallbackRole: "", + requesterRole: identity.RoleViewer, + expectAllow: false, + }, + { + name: "denied by checker but allowed by admin fallback", + innerResponse: authlib.CheckResponse{Allowed: false}, + fallbackRole: identity.RoleAdmin, + requesterRole: identity.RoleAdmin, + expectAllow: true, + }, + { + name: "denied by checker and fallback role not met", + innerResponse: authlib.CheckResponse{Allowed: false}, + fallbackRole: identity.RoleAdmin, + requesterRole: identity.RoleViewer, + expectAllow: false, + }, + { + name: "error from checker but allowed by fallback", + innerErr: errors.New("access check failed"), + fallbackRole: identity.RoleAdmin, + requesterRole: identity.RoleAdmin, + expectAllow: true, + }, + { + name: "error from checker and fallback role not met", + innerErr: errors.New("access check failed"), + fallbackRole: identity.RoleAdmin, + requesterRole: identity.RoleViewer, + expectAllow: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock := &mockInnerAccessChecker{ + response: tt.innerResponse, + err: tt.innerErr, + } + + // ST mode (useExclusivelyAccessCheckerForAuthz=false) with non-AccessPolicy identity + // will fall through to session-based check + checker := NewAccessChecker(mock, false) + if tt.fallbackRole != "" { + checker = checker.WithFallbackRole(tt.fallbackRole) + } + + // Create context with session identity (no token auth info) + requester := &identity.StaticRequester{ + Type: authlib.TypeUser, + OrgRole: tt.requesterRole, + } + testCtx := identity.WithRequester(context.Background(), requester) + + 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 TestUnifiedAccessChecker_FallsBackToSessionWhenNoToken(t *testing.T) { + req := authlib.CheckRequest{ + Verb: "get", + Group: "provisioning.grafana.app", + Resource: "repositories", + Name: "test-repo", + Namespace: "default", + } + + mock := &mockInnerAccessChecker{ + response: authlib.CheckResponse{Allowed: true}, + } + + // MT mode but no token in context - should fall back to session + checker := NewAccessChecker(mock, true) + + // Create context with only session identity (no token auth info) + requester := &identity.StaticRequester{ + Type: authlib.TypeUser, + OrgRole: identity.RoleAdmin, + } + testCtx := identity.WithRequester(context.Background(), requester) + + err := checker.Check(testCtx, req, "") + require.NoError(t, err, "should succeed using session identity when no token is present") +} + +func TestUnifiedAccessChecker_NoIdentityReturnsUnauthorized(t *testing.T) { + req := authlib.CheckRequest{ + Verb: "get", + Group: "provisioning.grafana.app", + Resource: "repositories", + Name: "test-repo", + Namespace: "default", + } + + mock := &mockInnerAccessChecker{ + response: authlib.CheckResponse{Allowed: true}, + } + + checker := NewAccessChecker(mock, true) + + // Empty context with no identity + err := checker.Check(context.Background(), req, "") + require.Error(t, err) + assert.True(t, apierrors.IsUnauthorized(err), "expected Unauthorized error, got: %v", err) +} + +func TestUnifiedAccessChecker_WithFallbackRole(t *testing.T) { + mock := &mockInnerAccessChecker{} + checker := NewAccessChecker(mock, false) + + // WithFallbackRole should return a new checker with the role configured + checkerWithAdmin := checker.WithFallbackRole(identity.RoleAdmin) + checkerWithEditor := checker.WithFallbackRole(identity.RoleEditor) + + // They should be different instances + assert.NotEqual(t, checker, checkerWithAdmin) + assert.NotEqual(t, checkerWithAdmin, checkerWithEditor) +} + +func TestUnifiedAccessChecker_FillsNamespaceFromIdentity(t *testing.T) { + mock := &mockInnerAccessChecker{ + response: authlib.CheckResponse{Allowed: true}, + } + + checker := NewAccessChecker(mock, true) + + // Request without namespace + req := authlib.CheckRequest{ + Verb: "get", + Group: "provisioning.grafana.app", + Resource: "repositories", + Name: "test-repo", + // Namespace not set + } + + // Create context with identity that has a namespace + requester := &identity.StaticRequester{ + Type: authlib.TypeUser, + OrgRole: identity.RoleAdmin, + Namespace: "stacks-123", + } + testCtx := authlib.WithAuthInfo(context.Background(), requester) + + err := checker.Check(testCtx, req, "") + require.NoError(t, err) +} diff --git a/pkg/registry/apis/provisioning/register.go b/pkg/registry/apis/provisioning/register.go index d211382255e..01888b740cd 100644 --- a/pkg/registry/apis/provisioning/register.go +++ b/pkg/registry/apis/provisioning/register.go @@ -162,13 +162,9 @@ func NewAPIBuilder( parsers := resources.NewParserFactory(clients) resourceLister := resources.NewResourceListerForMigrations(unified) - // Create access checker based on mode - var accessChecker auth.AccessChecker - if useExclusivelyAccessCheckerForAuthz { - accessChecker = auth.NewTokenAccessChecker(access) - } else { - accessChecker = auth.NewSessionAccessChecker(access) - } + // Create unified access checker that handles both token and session identity + // with the original fallthrough behavior + accessChecker := auth.NewAccessChecker(access, useExclusivelyAccessCheckerForAuthz) b := &APIBuilder{ onlyApiServer: onlyApiServer,