Skip to content

Schema queries are allowed even when ACL rules are set #4105

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

Closed
danielmai opened this issue Sep 30, 2019 · 10 comments
Closed

Schema queries are allowed even when ACL rules are set #4105

danielmai opened this issue Sep 30, 2019 · 10 comments
Assignees
Labels
area/acl Related to Access Control Lists area/querylang Issues related to the query language specification and implementation. kind/bug Something is broken. status/accepted We accept to investigate/work on it.

Comments

@danielmai
Copy link
Contributor

What version of Dgraph are you using?

v1.1.0

Have you tried reproducing the issue with the latest release?

Yes

What is the hardware spec (RAM, OS)?

N/A

Steps to reproduce the issue (command/config used to run Dgraph).

  1. Run Dgraph cluster with enterprise ACLs enabled (set the Alpha flag --hmac_secret_file).
  2. Create a user and group that does not have any read/write/modify permissions.
  3. Without logging in, run a schema {} query.

Expected behaviour and actual result.

Actual result:

The schema query succeeds and returns the schema of the db.

Expected result:

I expect the schema query to be disallowed unless I am 1) logged in and 2) logged in as a user with the appropriate permissions.

@danielmai danielmai added kind/bug Something is broken. area/querylang Issues related to the query language specification and implementation. status/accepted We accept to investigate/work on it. area/acl Related to Access Control Lists labels Sep 30, 2019
@martinmr
Copy link
Contributor

I can take a look at this.

@martinmr martinmr self-assigned this Sep 30, 2019
@martinmr
Copy link
Contributor

I think number 2 doesn't make a lot of sense when it comes to schema queries since they are not associated with a particular predicate. I think it makes sense to just do #1 for now.

I just opened a #4107 to fix that.

@mangalaman93
Copy link
Member

I am wondering if access to predicate is (currently) acceptable when no rules are defined, why access to schema is not acceptable. It would be a good idea to create a matrix with scenarios before adding allow/block rules.

@martinmr
Copy link
Contributor

martinmr commented Oct 1, 2019

Yeah, that is weird and I think there's another issue to make predicates not accessible by default. Usually it's a good idea to allow the least access by default but this was overlooked.

@mangalaman93
Copy link
Member

Related to #4082.
Yeah, agreed. We should consider raising a holistic PR that does it all. Or at least, identify the access/block matrix before adding more default rules.

@mangalaman93
Copy link
Member

mangalaman93 commented Oct 1, 2019

Now, on further thought, I feel like we currently have no rules around reading schema. We have no way of blocking to read the schema. We can specify rules for modifying schema, though. This seems like this will require additional ACL rules if we want to do something about it. I think this needs more thought.

cc: @campoy

@martinmr
Copy link
Contributor

martinmr commented Oct 1, 2019

Not a fan of doing it all in a single PR because a lot of tests will break.
We can add schema rules later but the PR I opened is just to make sure only logged in users can query the schema.

@mangalaman93
Copy link
Member

But I think before that, we need to fix on the behaviour of access to predicates by default.

  • If we have access to predicates by default (which is current behaviour), it seems okay to me to be able to read schema.
  • If we do not have access to predicates by default, then we could consider blocking access to reading schema. I am still not sure why reading the schema is such a bad thing that it needs to be blocked by default.

@martinmr
Copy link
Contributor

martinmr commented Oct 1, 2019

Here's the other issue for blocking predicates: #4082

It's not a terrible thing to be able to read the schema but the default behavior should be as strict as possible.

@martinmr
Copy link
Contributor

martinmr commented Nov 5, 2019

Fixed by #4082

@martinmr martinmr closed this as completed Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/acl Related to Access Control Lists area/querylang Issues related to the query language specification and implementation. kind/bug Something is broken. status/accepted We accept to investigate/work on it.
Development

No branches or pull requests

3 participants