Skip to content

Proxy supports authentication / authorization for specific routes #122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Tratcher opened this issue Apr 28, 2020 · 11 comments · Fixed by #265
Closed

Proxy supports authentication / authorization for specific routes #122

Tratcher opened this issue Apr 28, 2020 · 11 comments · Fixed by #265
Assignees
Labels
Priority:0 Used for divisional .NET planning Type: Enhancement New feature or request User Story Used for divisional .NET planning

Comments

@Tratcher
Copy link
Member

When defining proxy routes we should be able to specify Authorization criteria for that route and avoid proxying requests until that criteria is met.

This will need some support to be specified from config (probably by specifying the name of an authz policy that's configured in code), but it should also be configurable when using other data sources.

@Tratcher Tratcher added the Type: Idea This issue is a high-level idea for discussion. label Apr 28, 2020
@Kahbazi
Copy link
Member

Kahbazi commented Apr 29, 2020

avoid proxying requests until that criteria is met

This would be done by AuthorizationMiddleware, YARP doesn't need to do anything, right?

This could be the config.

"Routes": [
  {
    "RouteId": "backend1/route1",
    "BackendId": "backend1",
    "Match": {
      "Methods": [ "GET", "POST" ],
      "Host": "localhost",
      "Path": "/{**catchall}"
    },
    "AllowAnonymous": false,
    "Authorize": [
      {
        "Policy": "Policy1",
        "Roles": "Role1,Role2",
        "AuthenticationSchemes": "jwt"
      },
      {
        "Policy": "Policy2",
        "Roles": "Role3",
        "AuthenticationSchemes": "jwt"
      }
    ]
  }
]

and we could create IAuthorizeData, IAllowAnonymous in
https://github.com/microsoft/reverse-proxy/blob/ea7164c689ea06343b99a30aa7feecef4eda215d/src/ReverseProxy.Core/Service/DynamicEndpoints/RuntimeRouteBuilder.cs#L51-L66

My only question is that do we need two identical class for Authorize? One for ProxyRoute and one for ParsedRoute?

@Tratcher
Copy link
Member Author

avoid proxying requests until that criteria is met

This would be done by AuthorizationMiddleware, YARP doesn't need to do anything, right?

Right. MVC does have an extra safety check in it to make sure the AuthorizationMiddleware ran, and we might copy that, but that should be all we need.

I hesitate to actually craft Authorize policies from scratch as part of the proxy config. If we really want to build those in config then that should be an AspNetCore feature outside of the proxy section that we reference by policy name.

@Kahbazi
Copy link
Member

Kahbazi commented Apr 30, 2020

I hesitate to actually craft Authorize policies from scratch as part of the proxy config. If we really want to build those in config then that should be an AspNetCore feature outside of the proxy section that we reference by policy name.

I'm on the same page with you. Policy1 and Policy2 that I wrote in config would be a reference to ASP.NET Core policies. Just like the authentication scheme.

Do you think there should be also a config to set authorization for all routes?

@Tratcher
Copy link
Member Author

Do you think there should be also a config to set authorization for all routes?

I don't know. I wonder how often you'd be able to use the same policy for every route? And if you did, that doesn't seem like dynamic data anymore so you'd probably do it in code?

@karelz karelz added Type: Enhancement New feature or request and removed Type: Idea This issue is a high-level idea for discussion. labels May 12, 2020
@karelz karelz added this to the 1.0.0 milestone May 12, 2020
@WeihanLi
Copy link
Contributor

WeihanLi commented May 25, 2020

Do you think there should be also a config to set authorization for all routes?

I don't know. I wonder how often you'd be able to use the same policy for every route? And if you did, that doesn't seem like dynamic data anymore so you'd probably do it in code?

maybe compose Backend authorization and Route authorization is better, like controller and action in mvc, use route authorization policy if route policy exists, otherwise use backend authorization policy

@Tratcher
Copy link
Member Author

maybe compose Backend authorization and Route authorization is better, like controller and action in mvc, use route authorization policy if route policy exists, otherwise use backend authorization policy

I don't follow. There wasn't any proposal here to have backend authorization policies, only route auth policies.

@WeihanLi
Copy link
Contributor

maybe compose Backend authorization and Route authorization is better, like controller and action in mvc, use route authorization policy if route policy exists, otherwise use backend authorization policy

I don't follow. There wasn't any proposal here to have backend authorization policies, only route auth policies.

Mostly the same backend(aka the same service) use the same authorize policy in our cases.
For example, routes in the same service need the same scope, if only routes authorize, I had to config every route with the same policy, it's not so convenient for this case.

@jmezach
Copy link
Contributor

jmezach commented Jun 28, 2020

I agree with @WeihanLi that it is useful to be able to configure the authorization policy once for a backend (or cluster as it is now called) as most routes going to that backend will need to have the same policy applied.

I'm guessing we might need to have the ability to override it for specific routes though, if that makes sense. For example, we currently have some API's that are available anonymously, while most of them require authentication.

@Tratcher
Copy link
Member Author

@WeihanLi, @jmezach we have #164 tracking ways to reduce redundant config across routes, which should also be useful in this scenario.

I hesitate to enable authorization at multiple layers because then you need to deal with the scenarios where they conflict. People are already confused about how that works in MVC when you define authorization at both the controller and action level.

@WeihanLi
Copy link
Contributor

@WeihanLi, @jmezach we have #164 tracking ways to reduce redundant config across routes, which should also be useful in this scenario.

I hesitate to enable authorization at multiple layers because then you need to deal with the scenarios where they conflict. People are already confused about how that works in MVC when you define authorization at both the controller and action level.

I think there's no confused, just like mvc controller authz and action authz, if there's action authz then action authz takes effect, that's not conflicts but something like priority, action authz priority is higher than the controller authz.

@Tratcher
Copy link
Member Author

You'd be surprised how many issues get filed for the mvc auth scenarios.

@samsp-msft samsp-msft changed the title Configure authorization for proxy routes Proxy supports authentication / authorization for specific routes Oct 21, 2020
@samsp-msft samsp-msft added the User Story Used for divisional .NET planning label Oct 21, 2020
@samsp-msft samsp-msft added the Priority:0 Used for divisional .NET planning label Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:0 Used for divisional .NET planning Type: Enhancement New feature or request User Story Used for divisional .NET planning
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants