Add Authentication/Authorization support

[kreuzhofer]

Hardening the Code and add unit Tests.

[Jark]

Hi @kreuzhofer,

Apologies for the late code review! I (and I'm sure Tom as well) have been quite busy with work.

Thanks for all the work, like Tom said, a pretty solid implementation, even more so with the extra hardening/ error cases you added in the last few commits.

I've got a couple of comments / questions, let me know what you think:

1. How users would provide the IAuthorizationProvider

The IAuthorizationProvider could be passed in on the http server level. That way the instantiation for RestRouteHandler and StaticFileRouteHandler wouldn't have to change and I think in most cases the authorization provider would be the same for the entire server.. The authorization provider could then be provided on the IHttpServerRequest to the RestRouteHandler and the StaticFileRouteHandler.

The authorisation provider could then be added to the HttpServerConfiguration so the user can use that to specify it.

2. RestRouteHandler, authorize attribute

More a nice to have then a must have, but when RestControllerRequestHandler.RegisterController is called, we could parse it for Authorize attributes on the controller level and pass that into the GetRestMethods call.

In the GetRestMethods method we would then either cache the authorize attribute from the controller, or the (override) authorize attribute on the method in RestControllerMethodInfo.

3. StaticFileRouteHandler

I think for now what you did to authorize everything if an IAuthorizationProvider is passed in is great.

If we go with my suggestion of point 1 then we do lose a bit of flexibility there so I think we then need some way in the static file route handler to tell if it should execute the authorization procedure.

I would then suggest passing an interface into the StaticFileRouteHandler like: AuthorizedFilePaths = new []{ AuthorizedFilePath("\directory"), AuthorizedFilePath("\directory\file")};

The reason for the AuthorizedFilePath being a class is so in the future we can add roles / exceptions, etc.

4. minor renaming / refactorings, not 100% required for pull request merging imo

In RestRouteHandler _authenticationProvider should actually be named _authorizationProvider.

Could potentially do the parsing of the Authorisation header in a custom header (like ContentLengthHeader, etc.)

In DemoCredentialValidator.AuthenticateHelper you could do Task.FromResult((username == "user" && password == "pass");) and remove the async keyword instead of adding the pragma

Why does IAuthorizationProvider Task as a return type and ICredentialValidator use a IAsyncOperation as a return type?

PassThroughCredentialValidator seems to not be used anywhere, are you going to do anything with this?

When using AuthenticatedPerCallControllerSample TotalCallsHandled is never increased since the InstanceCreationType is PerCall.

[Jark]

@kreuzhofer do you mind if I apply some of the review notes above so we can get this pull request merged back in? I think it's a nice piece of functionality to have.

[kreuzhofer]

Hi @Jark, sorry for the late answer. I had some other urgent stuff, which blocked me completely from continuing on this. I will review this again and make some fixes to make it work with the development branch this week. Lets then review again.

about your comments:

1. Yes, this makes sense. I will have a look into it and move the code to the upper level.
2. Maybe we could chat about this. Is this about performance?
3. this makes totally sense, but I would suggest taking it as it is for now and then make another step forward to improve it.
4. Renamings I will do. Async fix, will do. The rest is mostly for demo purposes. The PassThroughCredentialValidator can be used to validate the credentials agains the local admin user credentials for the management web-portal. I will test this a bit further if it works and if yes, will include a sample for it. Otherwise, I will remove it.