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

Adding authorization with Conventions #180

Open
Dispersia opened this issue Oct 3, 2019 · 9 comments
Open

Adding authorization with Conventions #180

Dispersia opened this issue Oct 3, 2019 · 9 comments
Assignees

Comments

@Dispersia
Copy link

Dispersia commented Oct 3, 2019

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

@blissi
Copy link

blissi commented Dec 7, 2019

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

@Dispersia
Copy link
Author

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.

@tlil
Copy link
Collaborator

tlil commented Dec 10, 2019

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 tlil added the question label Oct 29, 2020
@tlil tlil self-assigned this Jan 25, 2021
@syaylev-minbox
Copy link

syaylev-minbox commented Apr 9, 2021

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.

@sungam3r
Copy link
Member

sungam3r commented Apr 9, 2021

May be related to graphql-dotnet/authorization#128

@floge07
Copy link

floge07 commented Jan 3, 2023

@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"]}}]}

@Shane32
Copy link
Member

Shane32 commented Jan 3, 2023

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
Copy link
Member

Shane32 commented Jan 3, 2023

@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.

@tlil
Copy link
Collaborator

tlil commented Jan 26, 2023

@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 tlil added enhancement and removed question labels Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants