Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Permissions] Ambiguous conditional decision behavior with empty anyOf criteria #9280

Closed
joeporpeglia opened this issue Jan 31, 2022 · 0 comments · Fixed by #9348
Closed
Assignees
Labels
bug Something isn't working

Comments

@joeporpeglia
Copy link
Contributor

joeporpeglia commented Jan 31, 2022

Conditional policy decisions with empty anyOf criteria have ambiguous behavior depending on how they are evaluated (either applied to resources in memory or converted to a database query).

Expected Behavior

The result should be the same regardless of how conditions are evaluated.

Current Behavior

Actions are denied when applying these conditions to resources in memory.

Actions are allowed when converting these conditions to a query.

Possible Solution

Add validation and possibly update static types to prevent empty allOf / anyOf criteria.

Steps to Reproduce

  1. Enable permissions for backstage
  2. Create a policy with the following implementation:
class TestPolicy implements PermissionPolicy {
  async handle(
    request: PolicyAuthorizeQuery,
    user?: BackstageIdentityResponse,
  ): Promise<PolicyDecision> {
    if (request.permission.resourceType === 'catalog-entity') {
      return createCatalogPolicyDecision({
        anyOf: []
      });
    }
    return {
      result: AuthorizeResult.ALLOW,
    };
  }
}
  1. Open the catalog page - note that all components are visible
  2. Click into a component and open the context menu in the page header
  3. The "Unregister entity" option is disabled

Context

The permission framework allows rules to define

  • how they are applied to resources in memory (apply)
  • how they are converted to a query for use with some storage system (toQuery)

Rules can be joined together using anyOf and allOf within a PermissionCriteria object. When applying PermissionCriteria to resources in memory, the framework is responsible for evaluating these logical operators. When converting PermissionCriteria to a query, the plugin that owns the related resource type is responsible for evaluating these logical operators.

The framework will use Array.prototype.every to evaluate allOf and Array.prototype.some to evaluate oneOf. However, the behavior of these APIs is not obvious for empty arrays — [].every(() => false) === true while [].some(() => false) === false. Instead of requiring every plugin to align with this behavior, we should prevent policies from returning conditional decisions with empty criteria.

Your Environment

  • NodeJS Version (v14): v14.17.6
  • Operating System and Version (e.g. Ubuntu 14.04): Darwin 20.6.0 - darwin/x64
  • Browser Information: N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant