DavidParks8 / Owin-Authorization

Backport of Asp.Net core's policy based authorization to Asp.Net 4
Other
60 stars 16 forks source link

make dependency construction more explicit and simpler to adapt #40

Closed 8snit closed 8 years ago

8snit commented 8 years ago

With OWIN middleware there is no "default" way to use/consume/provide IOC container functionality, the best guess is to make it simple for the app developer to plug-in the own container implementation.

The current implementation for these dependendies doesn't make me totally happy:

The proposed changes put all the construction work in the ResourceAuthorizationMiddleware -> Invoke call, the construction can be customized via the AuthorizationDependenciesProvider.

The WebApi Custom Handler sample has been adapted for demonstration:

           app.UseAuthorization(options =>
            {
                options.AddPolicy(ExampleConstants.EmployeeNumber2Policy, policyBuilder =>
                {
                    policyBuilder.AddRequirements(new EmployeeNumber2Requirement());
                });
                options.Handlers = new IAuthorizationHandler[] {new EmployeeNumber2Handler()};
                options.PolicyProvider = new CustomAuthorizationPolicyProvider(options);
           });

Existing tests targeting old implementation were removed/adaped accordingly.

Please let me know if you need further samples/tests :)

DavidParks8 commented 8 years ago

If you make a separate pull request for the WebApi Custom Handler sample, I'd be happy to merge it as is. You might be on the right track with your idea to influence lifetime, but I think we need to refine it a bit more before merging.

8snit commented 8 years ago

Thanks for your input, I tried to make things more explicit now and added a sample using Autofac to demonstrate how it could work (using per request lifetime scope).

With the AuthorizationDependenciesProvider OnCreate/OnDispose approach, I tried to stay close what is e.g.done in Microsoft.AspNet.Identity.Owin -> CreatePerOwinContext but I think there is plenty of space for further refinement :) Afterwards I'm happy to split things up for pulling in...

8snit commented 8 years ago

hi, thx for your input, I tried to incorporate your suggestions with my latest commit, I also removed the disposable stuff as it is not needed together with an IOC container...hope we can move forward :-)

DavidParks8 commented 8 years ago

Looks good enough to me!