Create project context from route and authorize based on user context#14
Create project context from route and authorize based on user context#14
Conversation
| string.IsNullOrEmpty(roleClaim) || string.IsNullOrEmpty(projectRolesClaim)) | ||
| { | ||
| throw new ArgumentException($"User is missing required claims. Claims: {user.Claims}."); | ||
| return null; |
There was a problem hiding this comment.
@hahn-kev I decided this makes more sense in the end, because when we get requests from unauthenticated users there can still be a WebContext, it just doesn't include a user.
Ultimately I made this change, because (to my surprise) my ProjectAuthorizationHandler gets instantiated for requests that don't need it (i.e. requests where AllowAnonymous is present). So, I either had to call ExtractLfUser from the authorization handler or make this change. I don't have a super strong opinion either way.
There was a problem hiding this comment.
My first thought was to just lazily get the user - and leave this code as is.
But that means we could miss bugs where we access the user from some code outside of an authorized context, in this case that code has to decide what to do if the user is null now.
| //default policy is when there's no parameters specified on the auth attribute | ||
| //this will make sure that all endpoints require auth unless they have the AllowAnonymous attribute | ||
| options.FallbackPolicy = options.DefaultPolicy; | ||
| options.FallbackPolicy = AuthorizationPolicy.Combine(options.DefaultPolicy, options.GetPolicy(nameof(ProjectAuthorizationRequirement))); |
There was a problem hiding this comment.
So this modifies the fallback policy. But not the default, so if we were to put the authorize attribute somewhere then we would no longer be applying the project auth requirement without any trigger that this was happening.
There was a problem hiding this comment.
True. Then, I'll change the Default-Policy.
|
Implemented in #18 |
2 participants
Fixes #15