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

Inner read (selections) authorization behavior #74

Closed
anasbarg opened this issue Nov 24, 2020 · 4 comments
Closed

Inner read (selections) authorization behavior #74

anasbarg opened this issue Nov 24, 2020 · 4 comments
Labels
authorizer bug Something isn't working P0 Do it NOW!!!

Comments

@anasbarg
Copy link
Collaborator

Inner read (selections) authorization behavior

Pragmafile: https://gist.github.com/anasbarg/f5617d17e45e35488689408322e14a16

Problem

I'm trying to run this query against the above Pragmafile:

query ReadBusiness {
  Business {
    read(username: "test") {
      username
      name {
        ar
        en
      }
      name1: name {
        ar
      }
    }
  }
}

and I'm getting the following result:

{
  "errors": [
    {
      "message": "No `allow` rule exists that allows `READ` operations on `Localized`"
    },
    {
      "message": "No `allow` rule exists that allows `READ` operations on `Localized`"
    },
    {
      "message": "No `allow` rule exists that allows `READ` operations on `Localized`"
    },
    {
      "message": "No `allow` rule exists that allows `READ` operations on `Localized`"
    },
    {
      "message": "No `allow` rule exists that allows `READ` operations on `Localized`"
    }
  ]
}

Expected

  • Not to return any errors
  • Even if an error must be returned, it shouldn't be duplicated as above. The problem here might be that the authorizer is checking the children selections (inner read ops) even if the parent selection (inner read op) is rejected; short circuiting on children will solve the problem.
@anasbarg anasbarg added bug Something isn't working P0 Do it NOW!!! authorizer labels Nov 24, 2020
@m-tabaza
Copy link
Contributor

The errors are returned because the PermissionTree doesn't return rules that reference fields for inner-read operations (selections.) Adding these rules to the return of PermissionTree#innerReadRules should solve the issue.

@anasbarg
Copy link
Collaborator Author

This will be solved #76

@anasbarg
Copy link
Collaborator Author

This will be solved #76

Still, we need to fix the redundant error messages by short-circuiting on children of a failed parent in the authorizer.

@anasbarg
Copy link
Collaborator Author

This will be solved #76

Still, we need to fix the redundant error messages by short-circuiting on children of a failed parent in the authorizer.

I created a issue #80 for this one since it's of a different priority.

m-tabaza added a commit that referenced this issue Nov 26, 2020
* .

* Metals updtes

* Allowed `Int`s as default values for `Float` model fields #45

* Made `ALL` permission include `PUSH_TO` and `REMOVE_FROM` for array fields #65

* * Moved JWT logic to `core`
* Added `root-jwt` command to CLI (#67)

* Metals updates

* Fixed #72

* Made it possible to define enum variants only through identifiers. #78

* Removed recursive selection checking in authorization. #74

* Dummy commit to test GitHub Actions

* Fixed broken tests. Should finalize #76 and #74

* Updated installation page

* Changed license to Apache 2.0

Co-authored-by: Anas Albarghouthy <[email protected]>
@anasbarg anasbarg closed this as completed Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authorizer bug Something isn't working P0 Do it NOW!!!
Projects
None yet
Development

No branches or pull requests

2 participants