davidparks8 / owin-authorization Goto Github PK

Done Criteria: 100% class code coverage

Right now there is no obvious support for multiple authorization handlers. The purpose of this ticket is to make it easier for external developers to integrate their own handlers.

Requires:

• A sample
• Documentation
• Unit tests

aspnet/Security@e8f55bd

Synchronize with the official repo.

I would like to have a hook point into the end of the authorization process.

One option that isn't possible at the moment is to inherit from DefaultAuthorizationService and have something like

public class LoggingAuthorizationService : DefaultAuthorizationService
{
    // Inject any other dependencies for contextual logging here
    public LoggingAuthorizationService(
        IMyLoggingService loggingService,
        IMyNotificationService notificationService,
        IAuthorizationPolicyProvider policyProvider, 
        IEnumerable<IAuthorizationHandler> handlers) : base(policyProvider, handlers)
    {
    }

    public override async Task<bool> AuthorizeAsync(ClaimsPrincipal user, object resource, IEnumerable<IAuthorizationRequirement> requirements)
    {
        bool isAuthorized = await base.AuthorizeAsync(user, resource, requirements);

        // log here

        return isAuthorized;
    }
}

I could then register my LoggingAuthorizationService implementation with my DI container (Simple Injector) and have all the information I need to log when a request is not authorized.

But since AuthorizeAsync is not virtual I cannot use the above approach.

Do you have a recommended approach for this?

The reason I would like to have a hook point is that currently I'm using my Global Exception Handler as a central logging point for collecting 400, 401, 403, 404 and 500 responses and ensuring these failed requests along with their request context is logged out to Application Insights and to my logging infrastructure via Serilog.

This project's approach is a really nice solution for authorization but it bypasses all of that centralization.

Also, I'm not sure how to customize the response message, which is "Authorization has been denied for this request" and the response status code is 401, which is correct for being un-authenticated, but not un-authorized (which should be a 403). I know that ASP.NET has these mixed up and returns 401 for failed authZ checks but there should be a way to correct this.

Details mentioned by Dominick at https://leastprivilege.com/2014/10/02/401-vs-403/ and more details here aspnet/Security#720 (comment) concerning the wrong status codes in ASP.NET 4 and the need to overwrite them.

All public classes should be commented for use with intellisense.

AuthorizeDependencies class no longer needs Func objects. The policy provider and handler properties can also be removed. Service should just become an IAuthorizationService object.

@brockallen can I please get a code review? As far as I can tell, the only things the library is missing are unit tests and nuspec files. I was planning on turning it into three different nuget packages. One for the common classes, one for mvc, and one for web api.

using scopes for access (similar to github - see https://developer.github.com/v3/oauth/#scopes) I correlate e.g. ResourceAuthorize[Policy="READ-X"] with the existence of a claim with key="READ-X" (value= true resp. 1)

this works if I register all policies (READ-X, WRITE-X, READ-Y,...) upfront but try to avoid that with a custom policy provider using RequireClaim(name of policy) for policy creation during runtime

what do you think?

To match the documentation of Asp.net core, the following commits should be reviewed and merged in.

• Doc Comments
• Fix misc feedback
• Doc comment update

This includes the owin and the httpContext version of the accessor

I can see this class AuthorizationDependencyHelper is internal which is used to extract the authorization options and therefore the IAuthorizationService but i can't see a way to get access to the IAuthorizationService inside of a WebApi controller without duplicating this code. I know i can create an instance of AuthorizationHelper but that doesn't let me execute AuthorizeAsync with a resource specified (such as AuthorizeAsync(this IAuthorizationService service, ClaimsPrincipal user, object resource, AuthorizationPolicy policy))

Any tips? I can make a PR to update AuthorizationHelper to have an overload or publicize AuthorizationDependencyHelper if needed

MIT license is not what Microsoft used for their code. It should be switched over to the apache license.

Hello

does this solution also handle View Based Authorization
as detailed here : https://docs.microsoft.com/en-us/aspnet/core/security/authorization/views

or in our case calling the AuthorizationService from within the controller

Make the changes included in this commit from the Asp.net core repo.

Create guidelines similar to https://github.com/aspnet/Home/wiki/Engineering-guidelines#basics

The mechanism for dependency injection is way over complicated. It could be simplified to just a dependency object that is overridable. That way, one could use a simple DI framework to return whatever objects one wants.

The interfaces for dependency injection have changed. The wiki needs to be updated to match them.

Add new features introduced in #934 of the Asp Net Core Security repository.

Need to publish everything to nuget in pre-release format

Port the relevant sections of asp.net core's documentation over for use in this project.

Currently, the default authorization policy is completely ignored. It should be used as a fallback for default authorization behavior when no other behaviors are specified.

Right now the logging is commented out. Implement this feature.

Version 1.0.1 of the packages have breaking changes related to dependency injection. The samples need to be updated to utilize these new changes.

Only in MVC, the ResourceAuthorizeAttribute does not check for or user any present AllowAnonymousAttribute.

Technically speaking, authentication should be separate from authorization, but Asp.Net old (4.6 and below) by default enforce this.

I'm running into an issue where my application deadlocks when using async/await in methods called by my AuthorizationHandler. If I call my async methods with ConfigureAwait(false) then the deadlock goes away.

It looks like in ResourceAuthorizeAttribute there is a line

return helper.IsAuthorizedAsync(controller, user, this).Result;

This is really dangerous since accessing the HttpContext anywhere downstream of this call will cause a deadlock since this line performs sync over async.

Is there an alternative to this design? Was this intentional?

The result of this should return the root of what a user has access too