Route Level security

[jchannon]

Was thinking of something like this.

Thoughts?

this.Get("/", async ctx => await this.RequiresAuthentication(async innerctx => await innerctx.WriteAsync("authed")));

[jchannon]

Pinging @RagingKore

[JoeStead]

It's almost as if someone else has wrote this code :) I still don't like it for the record

[jchannon]

I just made it better

On 24 May 2018 at 09:17, Joe Stead notifications@github.com wrote:

It's almost as if someone else has wrote this code :) I still don't like it for the record

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/112#issuecomment-391629695, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGappPIJdSCiQM9T9qwxPelr2eu_YUfks5t1mybgaJpZM4ULwYN .

[JoeStead]

But broken ;)

Options I can see are:

• Make the route return something, so you can return the result of a requires auth method if necessary, and exit early
• Throw some kind of exception that gets converted into the correct response 🤢
• An optional pre-hook you can add to the routes individually, and return a boolean similar ot the global before hook
• Just work it out in the routes:

if(!IsAuthenticated)
{
  ctx.Response.StatusCode = 401;
   return;
}

//route code

That said, how often do you have a module where some routes will have different auth settings than others? There's nothing stopping you having multiple modules with the same base path if you wanted. I'm interested in usecase, mostly

[jchannon]

Route pre-hook example...

            this.Get("/", async ctx =>
            {
                var authenticated = ctx?.User?.Identity != null && ctx.User.Identity.IsAuthenticated;

                if (!authenticated)
                {
                    ctx.Response.StatusCode = 401;
                    return false;
                }

                return true;
            }, async (request, response, arg3) => await response.WriteAsync("Authed"));

[JoeStead]

Yeah, but you'd probably move that into a method so it would look more like

this.Get("/", this.RequiresAuth, async (req, resp, route) => await response.WriteAsync("Authed"));` 

Otherwise there's no point using a pre-hook for it, and it should just be part of the route body, which I think should be the case anyway

[jchannon]

Which has now led me to wonder if routes should be

this.Get("/", async (req, resp, routeData, next) => {await response.WriteAsync("First"); await next()), async (req, resp, route) => await response.WriteAsync("Second"); await next())

Basically middleware....

So first can be a security check and if not authed dont call next()

Get becomes protected void Get(string path, params Func<HttpContext, Func<Task>, Task>[] middleware)

[JoeStead]

I wonder if it would be better to have a slightly different signature for that, so you could have each func in the array return true/false before determining whether or not it should fall through, rather than manually calling next(). Obviously signature would need to be different so it didn't make a single func call look weird

[RagingKore]

Dude I wrote the issue and didn't submit XD I was going to submit some Nancy like thing like:

Maybe something similar to Nancy like this:

public class PlayersModule : CarterModule {
  public PlayersModule() {
    Get("/players", async ctx => {
      ctx.RequiresAuthentication();
      // do something authorized
    });
  }
}

[JoeStead]

How would you stop the rest of the handler being executed if that method results in a 401, that's the issue :/

[RagingKore]

true true... what about something like:

public class PlayersModule : CarterModule {
  public PlayersModule() {
    Get("/players", async ctx => {
      // do something authorized
    }).RequiresAuthentication();
  }
}

[RagingKore]

It would be like composing a pipe (inverted).

[JoeStead]

That could work, provided the response stream hasn't been written to before the check. Maybe something like

public class PlayersModule : CarterModule {
  public PlayersModule() {
    RequiresAuthentication().Get("/players", async ctx => {
      // do something authorized
    });
  }
}

and rely on something being set to prevent the execution of the handler if RequiresAuth fails

[RagingKore]

that looks more atractive and doable. I would go with full pipeline composition as an alternative.

Here are some examples:

public class PlayersModule : CarterModule {
  public PlayersModule() {
    Route("/players/{:id}")
      .Authorize()
      .Get(ctx => // do something authorized );

    Route("/players/{:id}")
      .AuthorizeWithClaims(x => x.Type == "Admin")
      .Post(ctx => // do something authorized );

    Route("/players/{:id}")
      .Anonymous()
      .Put(ctx => // do something);

    Route("/players/{:id}")
      .Pipe(ctx => // do whatever my heart desires)
      .Delete(ctx => // do something authorized );
  }
}

[JoeStead]

This reminds me of Ktor, I approve of this

[RagingKore]

The posibilities are endless when it comes to create extensions for specific pipe actions.

One of the things that becomes really atractive is the idea that I could build everything right there, without auto magical assembly scanning of validators, binders or negotiators if I wished.

I still remember that the first thing I would do in Nancy would be to disable all magic and register my own stuff (and yes it was a pita).

PS - Had to google Ktor.

[JoeStead]

It will require some sort of Carter pipeline intertwined with the ASP.NET one (I don't think the details of which need to be exposed), so will require a bit more upfront thought, but it avoids specific CarterRequest and CarterResponse objects, whilst taking the extensibility up to 11.

[RagingKore]

Would be great if we could also pass data down the pipe.

public class PlayersModule : CarterModule {
  public PlayersModule() {
    Route("/players/{:id}")
      .Authorize()
      .Bind<Messages.V1.GetPlayer>(ctx => {
        var message = new Messages.V1.GetPlayer();
        // ...
        return message;
      })
      .Validate((msg, ctx) => {
        // ...
      })
      .Get((msg, ctx) => {
        // do something authorized with the message already deserialized and validated
      });
  }
}

[JoeStead]

You could provide overloads for that message passing system which could work with relative ease, I just think the C# typing system would let us down there and it would become quite ugly quite quickly

[davidfowl]

@jchannon Just FYI, endpoint routing makes it possible to hook into the ASP.NET pipeline for authorization. Here's what a vanilla app looks like with auth:

using System.Security.Claims;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;

namespace WebApplication66
{
    public class Startup
    {
        public void ConfigureServices(IServiceCollection services)
        {
            services.AddAuthentication("Cookies")
                    .AddCookie();

            services.AddAuthorization();

            services.AddAuthorizationPolicyEvaluator();
        }

        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
            }

            app.UseAuthentication();

            app.UseRouting(routes =>
            {
                routes.MapGet("/", async context =>
                {
                    await context.Response.WriteAsync($"Hello {context.User.Identity.Name}");
                })
                .RequireAuthorization(new AuthorizeAttribute());

                routes.MapGet("/account/login", async context =>
                {
                    var claims = new[] {
                        new Claim(ClaimTypes.NameIdentifier, "id"),
                        new Claim(ClaimTypes.Name, "David")
                    };

                    var identity = new ClaimsIdentity(claims, "Cookies");
                    var user = new ClaimsPrincipal(identity);
                    await context.SignInAsync(user);
                });

                routes.MapGet("/account/logout", async context =>
                {
                    // Handle logout
                    await context.SignOutAsync();
                });
            });

            app.UseAuthorization();
        }
    }
}

Not the prettiest (RequireAuthorization) but there isn't much flexibility here because of C# syntax.

[jchannon]

Interesting. Carter has Before hooks which are wrapped in this.RequiresAuthenticatio() but we also talked about a design that is pretty much the same as what you showed here https://github.com/CarterCommunity/Carter/issues/112 I wonder if instead of it taking an attribute in your method it could be made more generic?

On Mon, 21 Jan 2019 at 08:10, David Fowler notifications@github.com wrote:

@jchannon https://github.com/jchannon Just FYI, endpoint routing makes it possible to hook into the ASP.NET pipeline for authorization. Here's what a vanilla app looks like with auth:

using System.Security.Claims;using Microsoft.AspNetCore.Authentication;using Microsoft.AspNetCore.Authorization;using Microsoft.AspNetCore.Builder;using Microsoft.AspNetCore.Hosting;using Microsoft.AspNetCore.Http;using Microsoft.Extensions.DependencyInjection; namespace WebApplication66 { public class Startup { public void ConfigureServices(IServiceCollection services) { services.AddAuthentication("Cookies") .AddCookie();

        services.AddAuthorization();

        services.AddAuthorizationPolicyEvaluator();
    }

    public void Configure(IApplicationBuilder app, IHostingEnvironment env)
    {
        if (env.IsDevelopment())
        {
            app.UseDeveloperExceptionPage();
        }

        app.UseAuthentication();

        app.UseRouting(routes =>
        {
            routes.MapGet("/", async context =>
            {
                await context.Response.WriteAsync($"Hello {context.User.Identity.Name}");
            })
            .RequireAuthorization(new AuthorizeAttribute());

            routes.MapGet("/account/login", async context =>
            {
                var claims = new[] {
                    new Claim(ClaimTypes.NameIdentifier, "id"),
                    new Claim(ClaimTypes.Name, "David")
                };

                var identity = new ClaimsIdentity(claims, "Cookies");
                var user = new ClaimsPrincipal(identity);
                await context.SignInAsync(user);
            });

            routes.MapGet("/account/logout", async context =>
            {
                // Handle logout
                await context.SignOutAsync();
            });
        });

        app.UseAuthorization();
    }
}

}

Not the prettiest (RequireAuthorization) but there isn't much flexibility here because of C# syntax.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/112#issuecomment-455982354, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGapocwk3WSM84qbeVZu0GvforkzE2Dks5vFXYOgaJpZM4ULwYN .

[davidfowl]

It takes an IAuthorizeData which AuthorizeAttribute implements. You can also pass a policy. Under the covers though, it's just adding metadata to the endpoints.

[jchannon]

I'll take a look at that as it could basically match the issue linked above.

Are there any other extension methods planned/implemented?

On Mon, 21 Jan 2019 at 09:09, David Fowler notifications@github.com wrote:

It takes an IAuthorizeData which AuthorizeAttribute implements. You can also pass a policy. Under the covers though, it's just adding metadata to the endpoints.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/112#issuecomment-455998857, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGapi_g0-aozGzBFOqYxAAYu0kkKoOyks5vFYPagaJpZM4ULwYN .

[davidfowl]

There's WithCorsPolicy and RequireHost

cc @JamesNK

[JamesNK]

Note that extension method names may change post preview 2.

[tim-arheit]

Nancy handled ctx.RequiresAuthentication(); by throwing an exception, effectively exiting the route and was then handled by the framework returning a proper 401, etc. This let us add additional functionality doing the same thing, but allowing for other permissions. ie. ctx.RequiresPermission(whatever);.

This was particularly useful because different permissions might be needed base on the contents of the request, data being accessed, etc.

[abazanov]

Hi Guys, So I am looking at Carter and the whole issue of Authorisation, rather than Authentication. Hence, I came across this very intersting discussion.

I need route-level claimes-based authorisation. What is the best way I can achieve that in Carter. Any examples would be great.

Thanks.

[jchannon]

At the moment you can't however I have plans to make this possible when ASP.NET Core 3 is released.

Only way you can do it is for each handler to have an if statement in each handler however you might be able to use the this.RequiresPolicy feature but obviously only for those routes that need that policy check

On Fri, 28 Jun 2019 at 09:54, Andrei Bazanov notifications@github.com wrote:

Hi Guys, So I am looking at Carter and the whole issue of Authorisation, rather than Authentication. Hence, I came across this very intersting discussion.

I need route-level claimes-based authorisation. What is the best way I can achieve that in Carter. Any examples would be great.

Thanks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/112?email_source=notifications&email_token=AAAZVJTXEABA4AW3VKZKIP3P4XGWHA5CNFSM4FBPAYG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYZQBBA#issuecomment-506658948, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZVJQHDENTA4T5ZQCV4P3P4XGWHANCNFSM4FBPAYGQ .

[abazanov]

Thanks @jchannon

Not too sure how I can do it through this.RequiresPolicy(). Will have to look into it. I was thinking to do it in the Before hook and have a swicth statement that goes through all the routes and checks if the claim is there.

Otherwise ofcourse, it mens doing it in the actual handler, which I so do not want to clutter it repetitive code.

[jchannon]

You could use the Before hook and check it there for the routes you are interested in because potentially you may have routes in the module that won't need the auth check

On Fri, 28 Jun 2019 at 10:06, Andrei Bazanov notifications@github.com wrote:

Thanks @jchannon https://github.com/jchannon

Not too sure how I can do it through this.RequiresPolicy(). Will have to look into it. I was thinking to do it in the Before hook and have a swicth statement that goes through all the routes and checks if the claim is there.

Otherwise ofcourse, it mens doing it in the actual handler, which I so do not want to clutter it repetitive code.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/112?email_source=notifications&email_token=AAAZVJU4XICDYST4LDDODLLP4XIBHA5CNFSM4FBPAYG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYZQ5AY#issuecomment-506662531, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZVJUCMA7Z7MWULS32CPDP4XIBHANCNFSM4FBPAYGQ .

[abazanov]

That is what I had in mind. I was not sure if that was the most elegant solution for now. Although, now I am thinking of adding multiple Before hooks, one for each route and do specific route stuff in there. Not sure if the overhead of multiple Before hooks is something to consider or not.

[jchannon]

Looks good and highlights we need to improve this :)

On Fri, 28 Jun 2019 at 10:40, Andrei Bazanov notifications@github.com wrote:

That is what I had in mind. I was not sure if that was the most elegant solution for now. In other words, something like this.

Before += ctx => { var identity = ctx?.User?.Identity;

            if (identity == null)
            {
                ctx.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
                return Task.FromResult(false);
            }

            var claimsIdentity = identity as ClaimsIdentity;
            var route = ctx.Request.Path.Value;
            string requiredClaim = string.Empty;

            switch(route.ToLowerInvariant())
            {
                case "/organisations":
                    requiredClaim = "view-organisations";
                    break;
            }

            if (claimsIdentity.FindFirst(requiredClaim) == null)
            {
                ctx.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
                ctx.Response.Negotiate($"Missing { requiredClaim } claim.");
                return Task.FromResult(false);
            }

            return Task.FromResult(true);
        };

What do you think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/112?email_source=notifications&email_token=AAAZVJUYWEE75ONKSRVBH3LP4XL7HA5CNFSM4FBPAYG2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYZTM2Q#issuecomment-506672746, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZVJXJSAZP6NAH6VFCEWLP4XL7HANCNFSM4FBPAYGQ .