Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions internal/server/authz/engine/testdata/rbac_v2.rego
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ has_permission(scope_type, env, ns, action) if {
perm in {"*", action}
}

# Get list of viewable environments
# Get list of viewable environments. Define this rule so ListEnvironments can
# scope results to the caller: an undefined rule falls back to the allow
# decision on the list request itself instead of an unrestricted list.
viewable_environments := ["*"] if {
# If global access, return wildcard
has_global_access
Expand All @@ -131,7 +133,10 @@ viewable_environments := ["*"] if {
}
}

# Get list of viewable namespaces for a specific environment
# Get list of viewable namespaces for a specific environment. Define this rule
# alongside viewable_environments so ListNamespaces stays scoped to both: the
# requested environment must be viewable, and an undefined rule falls back to
# the allow decision on the list request itself.
viewable_namespaces(env) := ["*"] if {
# If wildcard access for this environment, return wildcard
has_wildcard_access(env)
Expand Down
37 changes: 33 additions & 4 deletions internal/server/authz/middleware/grpc/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,38 @@ func authorize(ctx context.Context, logger *zap.Logger, policyVerifier authz.Ver
// request itself is not allowed (there is no environment on a list
// request to evaluate). An empty scope is still passed to the handler
// so the endpoint can return an empty list rather than fail open.
// Policies that do not define the optional scope retain the historical
// unrestricted list behavior.
// Policies that do not define the optional scope must explicitly
// allow the list request itself; otherwise any authenticated caller
// could list every environment regardless of allow.
if viewableEnvironments == nil {
if !allowed {
logger.Error("unauthorized", zap.String("reason", "permission denied"))
return ctx, errUnauthorized
}
viewableEnvironments = []string{"*"}
}
ctx = context.WithValue(ctx, authz.EnvironmentsKey, viewableEnvironments)
continue
case environments.EnvironmentsService_ListNamespaces_FullMethodName:
if request.Environment == nil {
logger.Error("unauthorized", zap.String("reason", "permission denied"))
return ctx, errUnauthorized
}

viewableEnvironments, err := policyVerifier.ViewableEnvironments(ctx, input)
if err != nil {
logger.Error("unauthorized", zap.Error(err))
return ctx, errUnauthorized
}

// A namespace list must not expose an environment hidden by the
// environment scope, even when the namespace scope is undefined
// or wildcard.
if viewableEnvironments != nil && !slices.Contains(viewableEnvironments, "*") && !slices.Contains(viewableEnvironments, *request.Environment) {
logger.Error("unauthorized", zap.String("reason", "permission denied"))
return ctx, errUnauthorized
}

viewableNamespaces, err := policyVerifier.ViewableNamespaces(ctx, *request.Environment, input)
if err != nil {
logger.Error("unauthorized", zap.Error(err))
Expand All @@ -250,9 +274,14 @@ func authorize(ctx context.Context, logger *zap.Logger, policyVerifier authz.Ver
logger.Debug("policy namespaces evaluation", zap.Any("namespaces", viewableNamespaces))
// As with environments, preserve partial access but always attach a
// scope, including an empty one, so the endpoint cannot fail open.
// Policies that do not define the optional scope retain the historical
// unrestricted list behavior.
// Policies that do not define the optional scope must explicitly
// allow the list request itself instead of falling back to an
// unrestricted list.
if viewableNamespaces == nil {
if !allowed {
logger.Error("unauthorized", zap.String("reason", "permission denied"))
return ctx, errUnauthorized
}
viewableNamespaces = []string{"*"}
}
ctx = context.WithValue(ctx, authz.NamespacesKey, viewableNamespaces)
Expand Down
156 changes: 144 additions & 12 deletions internal/server/authz/middleware/grpc/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,19 @@ type mockServer struct {
skipsAuthz bool
}

// nilEnvironmentRequester returns a request with a nil environment to exercise
// the defensive deny guard in the ListNamespaces authorization path.
type nilEnvironmentRequester struct{}

func (nilEnvironmentRequester) Request() []flipt.Request {
return []flipt.Request{
{
Scope: flipt.ScopeEnvironment,
Action: flipt.ActionRead,
},
}
}

func (s *mockServer) SkipsAuthorization(ctx context.Context) bool {
return s.skipsAuthz
}
Expand Down Expand Up @@ -177,16 +190,18 @@ func TestReadOnlyPrincipalsCannotMutateBranches(t *testing.T) {

func TestAuthorizationRequiredInterceptorListScopesFailClosed(t *testing.T) {
tests := []struct {
name string
fullMethod string
req flipt.Requester
viewable []string
viewableErr error
wantScopeKey any
wantScopeValue []string
validatorAllowed bool
wantHandlerCalled bool
wantError bool
name string
fullMethod string
req flipt.Requester
viewable []string
viewableErr error
viewableEnvironments []string
viewableEnvironmentsErr error
wantScopeKey any
wantScopeValue []string
validatorAllowed bool
wantHandlerCalled bool
wantError bool
}{
{
name: "partial environments remain visible",
Expand All @@ -198,9 +213,17 @@ func TestAuthorizationRequiredInterceptorListScopesFailClosed(t *testing.T) {
wantHandlerCalled: true,
},
{
name: "undefined environments scope remains unrestricted",
name: "undefined environments scope denied without allow",
fullMethod: environments.EnvironmentsService_ListEnvironments_FullMethodName,
req: &environments.ListEnvironmentsRequest{},
wantError: true,
wantHandlerCalled: false,
},
{
name: "undefined environments scope allowed with allow",
fullMethod: environments.EnvironmentsService_ListEnvironments_FullMethodName,
req: &environments.ListEnvironmentsRequest{},
validatorAllowed: true,
wantScopeKey: authz.EnvironmentsKey,
wantScopeValue: []string{"*"},
wantHandlerCalled: true,
Expand Down Expand Up @@ -241,13 +264,86 @@ func TestAuthorizationRequiredInterceptorListScopesFailClosed(t *testing.T) {
wantHandlerCalled: true,
},
{
name: "undefined namespaces scope remains unrestricted",
name: "undefined namespaces scope denied without allow",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
wantError: true,
wantHandlerCalled: false,
},
{
name: "undefined namespaces scope allowed with allow",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
validatorAllowed: true,
wantScopeKey: authz.NamespacesKey,
wantScopeValue: []string{"*"},
wantHandlerCalled: true,
},
{
name: "undefined namespaces scope denied despite visible environment",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewableEnvironments: []string{"production"},
wantError: true,
wantHandlerCalled: false,
},
{
name: "undefined namespaces scope allowed in visible environment with allow",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewableEnvironments: []string{"production"},
validatorAllowed: true,
wantScopeKey: authz.NamespacesKey,
wantScopeValue: []string{"*"},
wantHandlerCalled: true,
},
{
name: "namespaces in hidden environment denied",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewable: []string{"*"},
viewableEnvironments: []string{"staging"},
wantError: true,
wantHandlerCalled: false,
},
{
name: "namespaces in hidden environment denied despite namespace scope",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewable: []string{"analytics"},
viewableEnvironments: []string{"staging"},
wantError: true,
wantHandlerCalled: false,
},
{
name: "namespaces in visible environment allowed",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewable: []string{"analytics"},
viewableEnvironments: []string{"production"},
wantScopeKey: authz.NamespacesKey,
wantScopeValue: []string{"analytics"},
wantHandlerCalled: true,
},
{
name: "namespaces with wildcard environments allowed",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewable: []string{"analytics"},
viewableEnvironments: []string{"*"},
wantScopeKey: authz.NamespacesKey,
wantScopeValue: []string{"analytics"},
wantHandlerCalled: true,
},
{
name: "namespaces with empty environments scope denied",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewable: []string{"analytics"},
viewableEnvironments: []string{},
wantError: true,
wantHandlerCalled: false,
},
{
name: "empty namespaces are not unrestricted",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
Expand All @@ -266,6 +362,40 @@ func TestAuthorizationRequiredInterceptorListScopesFailClosed(t *testing.T) {
wantScopeValue: []string{"*"},
wantHandlerCalled: true,
},
{
name: "namespace query failure denies",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewableErr: errors.New("query failed"),
wantError: true,
wantHandlerCalled: false,
},
{
name: "environment scope query failure denies namespaces",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewable: []string{"analytics"},
viewableEnvironmentsErr: errors.New("query failed"),
wantError: true,
wantHandlerCalled: false,
},
{
name: "environment scope query failure denies namespaces despite allow",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: &environments.ListNamespacesRequest{EnvironmentKey: "production"},
viewable: []string{"analytics"},
viewableEnvironmentsErr: errors.New("query failed"),
validatorAllowed: true,
wantError: true,
wantHandlerCalled: false,
},
{
name: "nil environment denied without panic",
fullMethod: environments.EnvironmentsService_ListNamespaces_FullMethodName,
req: nilEnvironmentRequester{},
wantError: true,
wantHandlerCalled: false,
},
}

for _, tt := range tests {
Expand All @@ -278,6 +408,8 @@ func TestAuthorizationRequiredInterceptorListScopesFailClosed(t *testing.T) {
verifier.viewableEnvironments = tt.viewable
verifier.viewableEnvironmentsErr = tt.viewableErr
} else {
verifier.viewableEnvironments = tt.viewableEnvironments
verifier.viewableEnvironmentsErr = tt.viewableEnvironmentsErr
verifier.viewableNamespacesForEnvironment = map[string][]string{"production": tt.viewable}
verifier.viewableNamespacesErr = tt.viewableErr
}
Expand Down
Loading