conventions icon indicating copy to clipboard operation
conventions copied to clipboard

Adding authorization with Conventions

Open Dispersia opened this issue 4 years ago • 9 comments

Hello, is there any example of using the graphql-dotnet/authorization with this package? I would really like to use these together, but can't figure them out.

The two things I'm stuck on:

  1. My query is marked with [ImplementViewer(OperationType.Query)]. With this, I try and mark my endpoint with [GraphQLAuthorize()], and it seems completely ignored. It does not apply the policy requirement to the endpoint.

  2. How do you add an IProvideClaimsPrincipal? I'm confused on the difference between the IUserContext, and IProvideClaimsPrincipal. Do they go on the same object? I assumed yes, however following the authorization tutorial, they do https://github.com/graphql-dotnet/authorization/blob/master/src/Harness/Startup.cs#L66 which you not do with this package, so I don't know how to add the User from HttpContext.

Any help would be appreciated, thank you

Dispersia avatar Oct 03 '19 20:10 Dispersia

Have you implemented authorization? I've got the same problem and don't know how to implement it.

blissi avatar Dec 07 '19 05:12 blissi

Unfortunately not. This issue caused me to use HotChocolate now, I hope one day this is shown, as I still prefer the conventions API the most.

Dispersia avatar Dec 08 '19 02:12 Dispersia

Hi guys. Sorry to hear that you're struggling to get this to work. Let me look into it and get back to you. Will need to make a couple of tweaks to make the auth functionality injectable.

tlil avatar Dec 10 '19 06:12 tlil

Experiencing the same problem. GraphQl.Authorizations assumes that UserContext implements IProvideClaimsPrincipal, but GraphQLEngine treats UserContext as dictionary (with IUserContext and IDependencyInjector inside). So today there is no option to combine GraphQLEngine and authorization.

I hope you will implement this, because this is really usefull feature.

syaylev-minbox avatar Apr 09 '21 10:04 syaylev-minbox

May be related to https://github.com/graphql-dotnet/authorization/pull/128

sungam3r avatar Apr 09 '21 21:04 sungam3r

@Shane32 You have made quite a lot of changes recently in this project. Did any of them change the situation here?

At least with the current release, the [Authorize] and [AllowAnonymous] still seem to do nothing. I saw in the readme of the server project the AddAuthorizationRule() on the GraphQLBuilder (but I'm not sure if it is translatable to this project).

I also tried to enable the AuthorizationRequired option, but that just gives me this error on every request when I'm not logged in, even when selecting Methods on the Query type that have [AllowAnonymous]: {"errors":[{"message":"Access denied for schema.","extensions":{"code":"ACCESS_DENIED","codes":["ACCESS_DENIED"]}}]}

floge07 avatar Jan 03 '23 09:01 floge07

Yes... and no.

GraphQL.NET 7 adds the User property to IResolveFieldContext in order to provide a dedicated field in which to hold the identity and claims in the form of a ClaimsPrincipal instance. GraphQL.NET Server v7 includes an authorization validation rule which operates on this value, so there is no need to store user identity/claims within the user context.

Within this repo, I've made changes to remove the server-type components and ensure that the Server project is compatible with the Conventions project. This means that (once released) you'll be able to easily use the Server project with the Conventions project, including the Server project's authentication rule.

However..... the [Authorize] and [AllowAnonymous] attributes within the GraphQL.NET base repository are designed for use with the AutoRegisteringObjectGraphType class and will not add the necessary metadata to the generated graph types when used with the Conventions project. @tlil will need to provide guidance on how to add this metadata.

The transport-level authentication configuration options within GraphQL.NET Server 7 will still function as intended as they do not require metadata set on the graph types for field definitions.

Shane32 avatar Jan 03 '23 11:01 Shane32

@tlil It is likely possible without too much difficulty to support GraphQLAttribute-based attributes for metadata. During construction of the graph type or field definition, these methods would need to be called on the attribute instance as appropriate:

public virtual void Modify(IGraphType graphType);
public virtual void Modify(FieldType fieldType, bool isInputType);
public virtual void Modify(QueryArgument queryArgument);

This would future-proof compatibility with metadata-based attributes such as [Authorize], [AllowAnonymous], [Metadata] and any future ones added for the complexity analyzer. For better or worse, it would also enable compatibility with [Name] and [Scoped] among others, and would not be compatible with attributes such as [Id] which utilize other methods on GraphQLAttribute.

I leave this up to you.

Shane32 avatar Jan 03 '23 11:01 Shane32

@tlil It is likely possible without too much difficulty to support GraphQLAttribute-based attributes for metadata. During construction of the graph type or field definition, these methods would need to be called on the attribute instance as appropriate:

Yes; let me take a look at this next week. Apologies for being slow at responding to this. Been caught up at work 😅

tlil avatar Jan 26 '23 06:01 tlil