Filter should call through to AuthorizationService #4385
Conversation
| @@ -75,7 +75,6 @@ public AuthorizeFilter(AuthorizationPolicy policy) | |||
|
|
|||
| // Note: Default Anonymous User is new ClaimsPrincipal(new ClaimsIdentity()) | |||
| if (httpContext.User == null || | |||
There was a problem hiding this comment.
I'm pretty sure @Tratcher told me once that this property couldn't be null, but maybe things have changed.
There was a problem hiding this comment.
Still can't, at least not with our DefaultHttpContext, but who knows what implementation is under this HttpContext... :-)
There was a problem hiding this comment.
Hehe yeah, people do unexpected things sometimes 😄
FWIW, I'd personally remove this null check and treat null principals like empty ones, specially since the authorization service is fine with null values.
There was a problem hiding this comment.
I guess we could flow null into the auth service as well.
|
@Eilon who should sign off on this change? |
|
Assigned to @kichalla to review. |
| // Arrange | ||
| var authorizeFilter = new AuthorizeFilter(new AuthorizationPolicyBuilder().RequireAssertion(_ => true).Build()); | ||
| var authorizationContext = GetAuthorizationContext(services => services.AddAuthorization(), anonymous: true); | ||
| authorizationContext.HttpContext.User = new ClaimsPrincipal(); |
There was a problem hiding this comment.
add a variation for null user?
|
|
after responding to the minor comment
Fixes #4287
cc @blowdart @Tratcher @rynowak