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

.net core 7 AddAuthorizationRule not work #993

Closed
triumphtang opened this issue Feb 22, 2023 · 29 comments
Closed

.net core 7 AddAuthorizationRule not work #993

triumphtang opened this issue Feb 22, 2023 · 29 comments
Labels
question Further information is requested

Comments

@triumphtang
Copy link

triumphtang commented Feb 22, 2023

Program:

builder.Services.AddAuthorization(options =>
{
    options.AddPolicy("111", policy =>
        policy.Requirements.Add(new MinimumAgeRequirement()));
});

builder.Services.AddTransient<IAuthorizationHandler, MinimumAgeHandler>();

builder.Services.AddGraphQL(b => b
    .AddSchema<Common_Base_Schemas>()
    .AddSchema<IMS_Schemas>()

    .AddAutoClrMappings()
    .AddSystemTextJson()
    .AddAuthorizationRule());

var app = builder.Build();

app.UseDeveloperExceptionPage();

app.UseAuthentication();
app.UseAuthorization();

app.UseWebSockets();

Query:

    public class Query
    {
        [AllowAnonymous]
        public static CompanyEntity? companyEntity()
        {
            return new CompanyEntity() { rowkey = Guid.NewGuid().ToString(), CompanyId = "11", CompanyName = "12" };
        }
        [Authorize(Policy = "111")]
        public static IEnumerable<CompanyEntity>? companyAll()
        {
            return new CompanyEntity[] { new CompanyEntity() { rowkey = Guid.NewGuid().ToString(), CompanyId = "A11"} };
        }
    }

MinimumAgeRequirement :

    public class MinimumAgeRequirement : IAuthorizationRequirement
    {
    }

MinimumAgeHandler :

    public class MinimumAgeHandler : AuthorizationHandler<MinimumAgeRequirement>
    {
        protected override Task HandleRequirementAsync(
            AuthorizationHandlerContext context, MinimumAgeRequirement requirement)
        {
            context.Succeed(requirement);

            return Task.CompletedTask;
        }
    }

Error accessing "companyAll":

{
   "error": {
     "errors": [
       {
         "message": "Access denied for field 'companyAll' on type 'Query'.",
         "locations": [
           {
             "line": 10,
             "column": 3
           }
         ],
         "extensions": {
           "code": "ACCESS_DENIED",
           "codes": [
             "ACCESS_DENIED"
           ]
         }
       }
     ]
   }
}
@Shane32
Copy link
Member

Shane32 commented Feb 22, 2023

Can you post a list of the GraphQL nuget packages you have installed ?

@triumphtang
Copy link
Author

Thanks for you.
GraphQL.Server.All 7.2.0

@triumphtang
Copy link
Author

IMS_Schemas:

public class IMS_Schemas : Schema
{
    public IMS_Schemas(IServiceProvider serviceProvider) : base(serviceProvider)
    {
        Query = new AutoRegisteringObjectGraphType<Query>();
    }
}

@Shane32
Copy link
Member

Shane32 commented Feb 22, 2023

I tested the code you provided and it seems to work for me. See PR #994 -- all tests pass, both successful and access-denied tests for the modified sample project:

  • Samples.Authorization.Tests.EndToEndTests.GraphQLGet_Success
  • Samples.Authorization.Tests.EndToEndTests.GraphQLGet_AccessDenied
  • Samples.Authorization.Tests.EndToEndTests.GraphQLPost_Success
  • Samples.Authorization.Tests.EndToEndTests.GraphQLWebSocket_AccessDenied
  • Samples.Authorization.Tests.EndToEndTests.GraphQLWebSocket_AccessDenied

Can you paste the app.UseGraphQL() call(s) that you have?

Can you verify that your [Authorize] attribute references GraphQL.AuthorizeAttribute and not Microsoft.AspNetCore.Authorization.AuthorizeAttribute ?

Can you verify that you are not referencing any other GraphQL nuget packages?

@triumphtang
Copy link
Author

app.UseGraphQL<IMS_Schemas>("/IMS_Schemas/graphql");
app.UseGraphQLPlayground(
"/IMS_Schemas/ui/playground",
new GraphQL.Server.Ui.Playground.PlaygroundOptions
{
GraphQLEndPoint = "../graphql",
SubscriptionsEndPoint = "../graphql",
});


[Authorize] attribute references GraphQL.GraphQLAttribute

Thank for you.

@triumphtang
Copy link
Author

<PackageReference Include="GraphQL.Server.All" Version="7.2.0" />
<PackageReference Include="GraphQL.Server.Transports.AspNetCore" Version="7.2.0" />
<PackageReference Include="System.Reactive" Version="5.0.0" />

@triumphtang
Copy link
Author

triumphtang commented Feb 23, 2023

Program.cs:

using GraphQL;
using KDRun.Model.GraphType.Common.Base;
using KDRun.Model.GraphType.IMS;
using KDRun.Service.ServiceAPI;
using Microsoft.AspNetCore.Authorization;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddAuthorization(options =>
{
options.AddPolicy("111", policy =>
policy.Requirements.Add(new MinimumAgeRequirement()));
});

builder.Services.AddTransient<IAuthorizationHandler, MinimumAgeHandler>();

builder.Services.AddRazorPages();

builder.Services.AddGraphQL(b => b
.AddSchema<Common_Base_Schemas>()
.AddSchema<IMS_Schemas>()

.AddAutoClrMappings()
.AddSystemTextJson()
.AddAuthorizationRule());

//builder.Services.AddRouting();

//builder.Services.AddCors(options => options.AddPolicy("all", p => p.AllowAnyOrigin().AllowAnyMethod().AllowAnyHeader()));

var app = builder.Build();

app.UseDeveloperExceptionPage();

app.UseAuthentication();
app.UseAuthorization();

app.UseWebSockets();

//app.UseRouting();

//app.UseCors("all");

// configure the graphql endpoint at "/Common_Base_Schemas/graphql"
app.UseGraphQL<Common_Base_Schemas>("/Common_Base_Schemas/graphql");

// configure Playground at "/Common_Base_Schemas/ui/playground" with relative link to api
app.UseGraphQLPlayground(
"/Common_Base_Schemas/ui/playground",
new GraphQL.Server.Ui.Playground.PlaygroundOptions
{
GraphQLEndPoint = "../graphql",
SubscriptionsEndPoint = "../graphql",
});

// configure the graphql endpoint at "/IMS_Schemas/graphql"
app.UseGraphQL<IMS_Schemas>("/IMS_Schemas/graphql");

// configure Playground at "/IMS_Schemas/ui/playground" with relative link to api
app.UseGraphQLPlayground(
"/IMS_Schemas/ui/playground",
new GraphQL.Server.Ui.Playground.PlaygroundOptions
{
GraphQLEndPoint = "../graphql",
SubscriptionsEndPoint = "../graphql",
});

app.MapRazorPages();
await app.RunAsync();

@triumphtang
Copy link
Author

Query.cs:

using GraphQL;

namespace KDRun.Model.GraphType.IMS
{
public class Query
{
[AllowAnonymous]
public static CompanyEntity? companyEntity()
{
return new CompanyEntity() { rowkey = Guid.NewGuid().ToString(), CompanyId = "11", CompanyName = "12" };
}

    [Authorize(Policy = "111")]
    public static IEnumerable<CompanyEntity>? companyAll()
    {
        return new CompanyEntity[] { new CompanyEntity() { rowkey = Guid.NewGuid().ToString(), CompanyId = "A11" } };
    }

}

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

I just realized what the issue is. Your authorization policy does not require the user to be authenticated, as it always returns Success. However, in our authorization attribute, we implicitly require the user to be authenticated before policies or roles are checked. See here:

https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL/Attributes/Authorization/AuthorizeAttribute.cs#L41..L110

And here:

https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL/Extensions/AuthorizationExtensions.cs#L124

And here:

https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL/Extensions/AuthorizationExtensions.cs#L158

And here:

https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL/Extensions/AuthorizationExtensions.cs#L190

And here:

https://github.com/graphql-dotnet/graphql-dotnet/blob/master/src/GraphQL/Extensions/AuthorizationExtensions.cs#L83

This was implemented as an optimization so that policies need not be checked if the user is not authenticated in the first place. This may differ from ASP.NET Core's logic.

We can consider changing this in a future version of GraphQL.NET if you can provide reasoning why this should not be the case.

To change behavior in the current version of GraphQL.NET, you would need to:

  1. Inherit from AuthorizationVisitor and override ValidateAsync so it calls a custom implementation of IsAuthorizationRequired which only returns provider.GetMetadata(AUTHORIZE_KEY, false). See https://github.com/graphql-dotnet/server/blob/master/src/Transports.AspNetCore/AuthorizationVisitorBase.Validation.cs#L74

  2. Inherit from AuthorizationValidationRule so it uses your custom AuthorizationVisitor implementation

  3. Call .AddValidationRule<MyAuthorizationValidationRule>() instead of .AddAuthorizationRule()

  4. Write a custom AuthorizationAttribute which sets the metadata directly instead of calling AuthorizeWithPolicy and AuthorizeWithRole.

  5. Use the custom attribute on your methods.

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

Another option is to use the authorization project. There are some limitations, such as AllowAnonymous and roles not being supported, but was designed to be able to be implemented without ASP.NET Core's authorization framework. I'm pretty sure it ignores the AUTHORIZE_KEY metadata (currently), and so would just run any policies through your implementation. In the future we may update the code to have the same logic as the one in this project, at which time it would not work for you anymore.

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

Another option would be to derive from AuthorizationValidationVisitor, and override IsAuthenticated to always return true. This is very simple to implement, so long as you have a policy or role defined for each method requiring authorization (and not just [Authorize]). Of course methods not requiring authentication would continue to work as intended.

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

Here is a sample of the latter:

public class MyAuthorizationVisitor : AuthorizationVisitor
{
    public MyAuthorizationVisitor(ValidationContext context, ClaimsPrincipal claimsPrincipal, IAuthorizationService authorizationService)
        : base(context, claimsPrincipal, authorizationService)
    {
    }

    protected override bool IsAuthenticated => true;
}

public class MyAuthorizationRule : IValidationRule
{
    public virtual async ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
    {
        var user = context.User
            ?? throw new InvalidOperationException("User could not be retrieved from ValidationContext. Please be sure it is set in ExecutionOptions.User.");
        var provider = context.RequestServices
            ?? throw new MissingRequestServicesException();
        var authService = provider.GetService<IAuthorizationService>()
            ?? throw new InvalidOperationException("An instance of IAuthorizationService could not be pulled from the dependency injection framework.");

        var visitor = new MyAuthorizationVisitor(context, user, authService);
        // if the schema fails authentication, report the error and do not perform any additional authorization checks.
        return await visitor.ValidateSchemaAsync(context) ? visitor : null;
    }
}

builder.AddValidationRule<MyAuthorizationRule>();
// remove builder.AddAuthorizationRule()

@triumphtang
Copy link
Author

Thank you for your reply. In our plan, the authentication framework of .net core is not used. We customize the authentication process and combine it with radis cluster. When the user accesses a specific method, we use the custom authentication to check, that it is legal Let it go, and we will terminate the access if it is illegal.

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

Then perhaps you would just take the sample above, and instead of deriving from AuthorizationValidationRule, derive from AuthorizationValidationRuleBase and override all three methods: IsAuthenticated, IsInRole and AuthorizeAsync to fit your needs, skipping the ASP.NET Core authorization framework altogether.

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

Sample of the latter:

public class MyAuthorizationVisitor : AuthorizationVisitorBase
{
    public MyAuthorizationVisitor(ValidationContext context)
        : base(context)
    {
        // optional: code to load the identity of the user here
    }

    protected override bool IsAuthenticated => false; // implement your logic here

    protected override bool IsInRole(string role) => false; // implement your logic here

    protected override ValueTask<AuthorizationResult> AuthorizeAsync(string policy)
        => new(AuthorizationResult.Failed()); // implement your logic here
}

public class MyAuthorizationRule : IValidationRule
{
    // this is a singleton so there should be no user-specific code in the constructor

    public virtual async ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
    {
        var visitor = new MyAuthorizationVisitor(context);
        // if the schema fails authentication, report the error and do not perform any additional authorization checks.
        return await visitor.ValidateSchemaAsync(context) ? visitor : null;
    }
}

builder.AddValidationRule<MyAuthorizationRule>();

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

Please note that if you add transport-level authentication requirements within the options of UseGraphQL, then you need to override the middleware authentication method(s) to tie it to your own implementation. However, you can instead just set an authorization policy on a schema or on the Query or Mutation or Subscription type as appropriate and just use your modified validation rule.

@triumphtang
Copy link
Author

Thanks for your reply, I'll try it!

@triumphtang
Copy link
Author

Thank you for your reply,

How to access http headers in Query or Mutation or Subscription method?

and

How to access http headers in MyAuthorizationRule?

thanks.

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

Inject IHttpContextAccessor into the constructor of MyAuthorizationRule

@triumphtang
Copy link
Author

Yes,

public class MyAuthorizationRule : IValidationRule
{
IHttpContextAccessor _httpContext;
public MyAuthorizationRule(IHttpContextAccessor httpContext)
{
_httpContext = httpContext;
}

// this is a singleton so there should be no user-specific code in the constructor

public virtual async ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
{
    var visitor = new MyAuthorizationVisitor(context);
    Console.Write(_httpContext.HttpContext.Request.Headers);
    // if the schema fails authentication, report the error and do not perform any additional authorization checks.
    //return await visitor.ValidateSchemaAsync(context) ? visitor : null;
    return null;
}

}

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

I’d probably just pass the relevant header(s) into the visitor, but there’s a few ways to implement it. Is it working?

@triumphtang
Copy link
Author

Yes, nject IHttpContextAccessor into the constructor of MyAuthorizationRule is good.

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

Another option is to use the user context builder to initialize the UserContext based on the HttpContext. Then both the validation rule and individual methods have access to it.

@triumphtang
Copy link
Author

That's a good idea, how to do this:
"Another option is to use the user context builder to initialize the UserContext based on the HttpContext. Then both the validation rule and individual methods have access to it.",

@Shane32
Copy link
Member

Shane32 commented Feb 23, 2023

@triumphtang
Copy link
Author

Thank for you.

@sungam3r sungam3r added the question Further information is requested label Mar 1, 2023
@Shane32 Shane32 closed this as completed Mar 2, 2023
@yuchen-w
Copy link

Here is a sample of the latter:

public class MyAuthorizationVisitor : AuthorizationVisitor
{
    public MyAuthorizationVisitor(ValidationContext context, ClaimsPrincipal claimsPrincipal, IAuthorizationService authorizationService)
        : base(context, claimsPrincipal, authorizationService)
    {
    }

    protected override bool IsAuthenticated => true;
}

public class MyAuthorizationRule : IValidationRule
{
    public virtual async ValueTask<INodeVisitor?> ValidateAsync(ValidationContext context)
    {
        var user = context.User
            ?? throw new InvalidOperationException("User could not be retrieved from ValidationContext. Please be sure it is set in ExecutionOptions.User.");
        var provider = context.RequestServices
            ?? throw new MissingRequestServicesException();
        var authService = provider.GetService<IAuthorizationService>()
            ?? throw new InvalidOperationException("An instance of IAuthorizationService could not be pulled from the dependency injection framework.");

        var visitor = new MyAuthorizationVisitor(context, user, authService);
        // if the schema fails authentication, report the error and do not perform any additional authorization checks.
        return await visitor.ValidateSchemaAsync(context) ? visitor : null;
    }
}

builder.AddValidationRule<MyAuthorizationRule>();
// remove builder.AddAuthorizationRule()

Thank you. I just wanted to comment that I had the same problem building an internal enterprise application. This particular piece of code solved my problem.

@Shane32
Copy link
Member

Shane32 commented May 23, 2023

@sungam3r Perhaps we should not check that IsAuthenticated == true before checking policy or role membership.

@fl99kl
Copy link

fl99kl commented Aug 8, 2023

I just ran into the same issue. Setting IsAuthenticated == true solved it, but finding the reason for this behaviour caused some headaches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants