umbraco-community / umbraco-graphql

An implementation of GraphQL for Umbraco 8 using GraphQL for .NET.
MIT License
64 stars 32 forks source link

How to handle permissions when not casting to a Doctype #32

Closed JackPenney closed 5 years ago

JackPenney commented 6 years ago

We need to think about how we should handle permissions for generic PublishedContent Currently requests 1. & 2. will use the permissions we've set on the home Doctype while example 3. they won't apply, even then we're accessing the same pieces of content.

I think we need a step where we lookup the doctype of what's being requested in cases like this and apply the appropriate doctype permissions.

Another option is to have a separate set of permissions for all generic requests but i don't think that's the best way to go about it because we could end up leaking details about doctypes we want totally hidden

# 1. Request for Home content 
{
  home {
     _contentData {
         createDate
  }
 }
}

# 2. Request on home content with cast
{
 content(id:1106) {
  ...on Home {
    _contentData {
     createDate
   }
  }
 }
}
# 3. Generic request for PublishedContent 
{
 content(id:1106) {
   _contentData {
    createDate
   }
  }
}

Thoughts?

PeteDuncanson commented 6 years ago

As discussed I believe the issue here is we loop over and store the doctype name in the meta data and use that to build up the Claims key (ie "docType:Field:Permission" so "Homepage:pageTitle:read"). But when we use the contentBy stuff it doesn't have that meta data available. If we could get it injected in then it should "just work" but that would require that we do a look up to get the data (ie resolve it) before we check the authentication (which is currently done with a filter which fires before we resolve).

Bit of a chicken and egg situation.

rasmusjp commented 6 years ago

I guess the problem is that the validation rules is run against the query (which doesn't know the correct types).

One solution is to move the auth to a field middleware since they are run at execution time and at that point we know which type that's gonna be returned.

But if we #20 are getting implemented I'm not sure if this is gonna be a problem since you'll only get the schema you'll have access to and it'll just fail if you try to query for a field that "doesn't exist"

rasmusjp commented 5 years ago

Closing this for now as I've removed the permissions for now