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

IAuthorizationRequirement is not sent the same variable names as those in IResolveField context #139

Open
JRDrummond opened this issue Apr 12, 2021 · 2 comments
Labels
needs confirmation The problem is most likely resolved and requires verification by the author

Comments

@JRDrummond
Copy link

JRDrummond commented Apr 12, 2021

Take the following AuthorizationRequirement:

namespace GraphQLNet.AuthRequirements
{
	public class IsSelf : IAuthorizationRequirement
	{
		public Task Authorize(AuthorizationContext context)
		{
			var user = context.User;
			object idArg;
			if (context.InputVariables.TryGetValue("userID", out idArg) 
				&& user.HasClaim(c => c.Type == "ID" && c.Value == idArg.ToString()) 
				|| user.IsInRole(Roles.Admin))
			{
				return Task.CompletedTask;
			};

			context.ReportError("You cannot access private fields of another user.");

			return Task.CompletedTask;
		}
	}
}

It can be defeated with the following query:

query users($user: String) {
  users(id: $user) {
	id
	privateStuff // Secured with IsSelf
  }
}
//Variables:
{
	"userID": "myUserID",
	"user": "someoneElse'sUserID"
}

This is because AuthorizationContext.InputVariables accesses the variables object whereas IResolveFieldContext.GetArgument<>() accesses variables from the query arguments, whose names are defined by the schema.

As it stands now, there is no way to secure fields based on what arguments are allowed to be passed in. In fact, as shown above, there is no point in securing the variables object's key names since any other variable name can be passed in as a query argument.

Resolution:

Pass IResolveFieldContext.Arguments as AuthorizationContext.Arguments into IAuthorizationRequirement.Authorize.

This only makes sense as AuthorizationRequirements are supposed to define authorization for fields, therefore, they should look at the same context that they are securing.

@sungam3r sungam3r added the needs investigation The described problem requires additional time to study label Apr 14, 2021
@sungam3r
Copy link
Member

Some thoughts:

  1. Probably this was fixed in Added authorization for VariableReference fields #179
  2. What version do you use? I see InputVariables. In master I see Inputs.

This is because AuthorizationContext.InputVariables accesses the variables object whereas IResolveFieldContext.GetArgument<>() accesses variables from the query arguments, whose names are defined by the schema.

Sorry, I don't understand.

This only makes sense as AuthorizationRequirements are supposed to define authorization for fields, therefore, they should look at the same context that they are securing.

Policies (policy is a named set of 1 or more AuthorizationRequirements) may be applied on any IProvideMetadata object namely: input type, object type, object field, input objet field, etc. What do you mean same context?

@sungam3r sungam3r added needs confirmation The problem is most likely resolved and requires verification by the author and removed needs investigation The described problem requires additional time to study labels Dec 11, 2021
@alexistamand
Copy link

Hi @JRDrummond ,
I just came across this issue. I'm working on implementing something similar as stated in your "Resolution".
I was wondering how you were accessing the IResolveFieldContext.Arguments in your AuthorizationEvaluator?
This would greatly help me,
Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs confirmation The problem is most likely resolved and requires verification by the author
Projects
None yet
Development

No branches or pull requests

3 participants