search

aspnet / Security

[Archived] Middleware for security and authorization of web apps. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
1.27k stars 600 forks source link

RequireClaim method signature is a little confusing #1883

Closed benmccallum closed 5 years ago

benmccallum commented 5 years ago 
public AuthorizationPolicyBuilder RequireClaim(string claimType, params string[] requiredValues);
public AuthorizationPolicyBuilder RequireClaim(string claimType, IEnumerable<string> requiredValues);

"requiredValues" is a confusing name as the method in reality only checks for the presence of one of these values, not all. The parameter description is perfect, but I'm not sure if that's enough. I certainly got confused until I confirmed it in the docs/saw comment.

It's a petty thing I know, but maybe something more generic like "values" would suffice? Or go super explicit like "anyOfTheseValues"?

HaoK commented 5 years ago

Sure, do you want to submit a PR for this change?

benmccallum commented 5 years ago

Sure thing. Any preference on a new param name?

dgarciarubio commented 5 years ago

How about "possibleValues", or "permittedValues"? just "values" seems vague, but "anyOfTheseValues" sounds too informal to me.

HaoK commented 5 years ago

acceptedValues maybe?

benmccallum commented 5 years ago

How about valuesAllowed? Or along the lines of auth, valuesAuthorized?

HaoK commented 5 years ago

allowedValues is fine

benmccallum commented 5 years ago

PR up with allowedValues. Cheers!

HaoK commented 5 years ago

Merged into master for 3.0, thanks @benmccallum !