Authorize Directive

[michaelstaib]

Current behavior:

The authorize directive currently can be added to object types and object type field definitions.

• If added to a field definition it will protect that annotated field with the specified policy.
• If added to the object type it will protect all of the fields of that type.

Proposed new behavior:

The authorize directive currently can be added to object types and object type field definitions.

• If added to a field definition it will protect that annotated field with the specified policy.
• If added to the object type it will protect type no matter from which field it has been resolved.

[michaelstaib]

This essentially would also introduce new issues.... consider the following schema:

type Query {
   fooOrBar: FooOrBar
}

type Foo {
}

type Bar @authroize(policy: "xyz") {

}

union FooOrBar = Foo | Bar

In the above example the auth middleware would have execute resolver and pull in the data in order check if the type Foo or Bar is being resolved just to return null because authorize declined to allow that type.

[drowhunter]

I admit I wasn't thinking of how it would affect unions

[michaelstaib]

The same issue would arise when we are using interfaces.

[drowhunter]

Was thinking about this over the weekend. I think it might make more sense if the Authorize directive affected execution similar to how skip and include work. If the policy should fail it would not return the node at all.

As it currently works, if the policy predicate returns false, the object returns null, even for types that you wouldn't expect a null from such as Non-Nullable value types

[michaelstaib]

If an error is thrown while resolving a field, it should be treated as though the field returned null, and an error must be added to the "errors" list in the response.

If the result of resolving a field is null (either because the function to resolve the field returned null or because an error occurred), and that field is of a Non-Null type, then a field error is thrown. The error must be added to the "errors" list in the response.

Errors and Non Nullability

So, a sever has to return null for a field that raised an error. The current implementation meets the spec.

If we want to have something that skips fields (excludes them from the result) the directive must convey that.@authorize does not imply that it will just hide fields that do not comply.

Furthermore, does that not fix the issue that if @authorize would be implemented in the way described (type-wise) the query engine would have to first load the data.

@skip and @include are special directives since they are specified in the GraphQL Spec.

In order to allow a directive to skip fields if they are not authorized we would have to either add a new directive or extend the current directive like the following:

@authorize(policy: "foo" action: SKIP)

If the action is not set the default behavior will be used which would be to throw an error.

We would have to implement a QuerySyntaxRewriter for that which would effectively rewrite the incoming query to remove fields that are not authorized.

This could be done with the new query middleware.

Still, this would work field-wise.

What do you think?

[drowhunter]

I really like Plan B!

I know skip and include are special. I assume they are evaluated in a special place where the can skip resolving the field. Normal directives seem to actually get closer to resolving as the fields are still placed in the output.

Would plan A. put the authorization at the same place as normal directives.

Will this solve the issue with unions though?

[michaelstaib]

Will this solve the issue with unions though?

Since with the authorize directive is essentially a field directive yes. since we won't put it on a type. Maybe, we should limit the scope and only allow authorize to be added to field definitions. This way this becomes more clear.

I really like Plan B!

You mean @authorize(policy: "foo" action: SKIP)

So basically @authorize would have three arguments:

• Roles
• Profile
• Action -> Action is an enum [ERROR, SKIP] and ERROR is the default.

[michaelstaib]

directive @authorize(profile: String roles: [String] action: AuthorizeAction = ERROR) on FIELD_DEFINITION

enum AuthorizeAction {
  ERROR
  SKIP
}

[drowhunter]

so when you say only allowed on fields, that means you are ruling out the ability to protect types?

[michaelstaib]

I can't see how I can lookup the type before executing the resolver. If you think about the following schema:

type Query {
   allFoos: [Foo] 
}

interface Foo {

}

type Bar implements Foo @authorize(policy: "foo") {
}

type Baz implements Foo {

}

Only bar has the authorize directive. Since, the authorize-directive is not annotated to the field on the query type it is not possible to validate the directive without executing the resolver, which should pull in the data.

I think this can be done in a custom directive better than in the standardized one. The problem that I have with it is that the query runtime would become unpredictable.

There is another issue, what If you use our paging capabilities .... since some of the data could be Bars and some Bazs we would have to filter through them in the list and return a reduced set. So, the pagesize would not anymore be reliable.... There are many issues that we would have for our standard component.

But when we put everything in place it would be fairly easy for a custom schema to implement an @authorizeType that does exactly what you ask. It would just have to pull in data in order to validate the data and paging would be a challenge which could be acceptable for some use-cases.

I mean, as far as I understood you are not using Interfaces in the place that you want to secure a type. So, for your use-case a custom @authorizeType would work like a charm.

What are your thoughts?

[drowhunter]

Maybe you would need to add the ability to reflect the inherited types in the code? This way you wouldn't have to dig into the instance to find out?

perhaps when putting it all together you could have a hidden property that stuck the inherited types in.

[michaelstaib]

Maybe you would need to add the ability to reflect the inherited types in the code? This way you wouldn't have to dig into the instance to find out?

But you need the instance. If you have a list of unions or interfaces than some of the instances would have to be authorized and some not. The type cannot help here.

Moreover, I do not necessary have a fixed type in dotnet. The ClrType of Bar and Baz could be object.

[michaelstaib]

The type cannot help here.

What I mean with that is .... would we implement a solution where the type has some authorization logic then we could only execute it once the type is clear. The type is only clear once you have an instance. Otherwise you can only lookup the possible types which would not help since they are known to the schema anyway.

[OneCyrus]

we have another use case for authorization where we are searching for a streamlined way. let's say our graphql reponse is something like the following structure. basically have an array with multiple objects which have different permissions.

{
   customers: [
      customerA: {
         data: "sensitive content scoped to customer A",
         serviceA {
            data: "serviceA data"
         },
         serviceB {
            data: "serviceB data"
         }
      },
      customerB: {
         data: "sensitive content scoped to customer B",
         serviceA {
            data: "serviceA data"
         },
         serviceB {
            data: "serviceB data"
         }
      }
   ]
}

filtering the list (customers) to the user's permission is easy. but now we have fields (serviceA, serviceB) in those customer (customerA, customerB) objects. now we are looking into a solution where we could use the field path/graph to scope the permissions. so that we can check if the authorization is not only valid for serviceA or serviceB but also check if it's behind customerA or customerB.

basically @authorize("serviceA", "customerA") but automatically scoped to the customer in the path. may be we can have custom authorize implementations where we can implement our own logic if it's out of scope for the OOB implementation.

[michaelstaib]

That should already be possible. Since the resolver context has access to all the results that were produced in its path you can use those in an authorization policy. Let me verify this and get back to you on that.

[michaelstaib]

Hi @OneCyrus,

with the current version it is not possible. But we will integrate it.

I think the object could be send as a resource to the policies and the authorization policy can then decide if a user has permission on a certain field of a certain instance of an object.

Would that satisfy your need?

[michaelstaib]

@OneCyrus @drowhunter

So the reworked authorization directives will be included in 0.8.0 ... so I think end of January.

[michaelstaib]

may be we can have custom authorize implementations

So, so the policies are basically the part where you can write your own authorization logic.

With the current implementation this is not really helpful since you can only write the authorization logic on the ClaimsPrincipal. With the new authorize directive we will pass in the IResolverContext. So, you basically then could write authorization logic ontop of that. This will give you information about the query, the current query selection, all resolved objects in the path of the field and more. This should then enable you to write complex authorization policies if needed.

[OneCyrus]

With the current implementation this is not really helpful since you can only write the authorization logic on the ClaimsPrincipal. With the new authorize directive we will pass in the IResolverContext. So, you basically then could write authorization logic ontop of that. This will give you information about the query, the current query selection, all resolved objects in the path of the field and more. This should then enable you to write complex authorization policies if needed.

that sounds perfect! if we can write our own policies and still have access to the IResolverContext that should be enough. thanks!

[michaelstaib]

@OneCyrus with 9.0.0-preview.42 we are passing now the IResolverContext as resource to the policy. So, part of this is now implemented.

[OneCyrus]

perfect. thanks!