DuendeSoftware / Support

Support for Duende Software products
20 stars 0 forks source link

Auth Policy M2M vs Interactive - Require Client Claims? #1290

Closed MH61Aus closed 4 weeks ago

MH61Aus commented 1 month ago

Which version of Duende IdentityServer are you using?

7

Which version of .NET are you using?

8

Describe the bug

Not a bug, just a question. This is more to do with dotnet's Authorization handlers, but since I'm using identity server config to handle to issue, I still feel its relevant to ask.

I'm trying to figure out the authorization strategy for my API. I've implemented auth policies, and have been following along with one of Roland's courses on Pluralsight.

I've got an endpoint that I want to protect with a scope. but also if a user is logged in, make sure that user has permission to do that action.

So I decorate the controller action with

[Authorize(Policy = DemoApiConstants.AuthorizationPolicies.ClientApplicationCanWrite)]
[Authorize(Policy = DemoApiConstants.AuthorizationPolicies.UserIsAdministrator)]

That works fine for my web client, but I also have a console client using m2m. that second policy fails - so when its an m2m client, In only care about that first attribute.

From what I can see, there's no claims that tell me whether a client is an m2m client. I suppose i could explicitly check for client_Ids, but that seems tacky.

So, to fix this, I've changed my auth policy from .RequireRole("admin") to

return new AuthorizationPolicyBuilder()
    .RequireAuthenticatedUser()
    //.RequireRole("admin")
    .RequireAssertion(context =>
    {
        //Check if the user has the admin role claim
        return context.User.HasClaim(c => c.Type == ClaimTypes.Role && c.Value == "admin")
        || context.User.HasClaim(c => c.Type == "client_ClientType" && c.Value == "m2m");
    })
    .Build();

And then I've added a client claim to the console app as such:

    Claims = new List<ClientClaim>
    {
        new ClientClaim("ClientType", "m2m")
    }

This all seems to work - from what I can tell, for the interactive client, you need to be logged in with a role to hit the endpoint, and for the m2m client it just requires the scope and the client type.

I just wanted to confirm that this is the correct strategy, or if I'm totally misunderstanding something here.

RolandGuijt commented 1 month ago

When you say m2m I'm assuming you're using client credentials flow. Is that right? If so, there is no user involved using that flow. It's machine to machine. Identity scopes, like one that would contain "role", are irrelevant, it's just about API scopes. You can configure multiple scopes per API though. They are just not tied to users and you can check for those in your authorization policies. I would choose using API scopes if possible instead of letting the policy depend on the flow used to obtain the token.

MH61Aus commented 1 month ago

I'm a little confused by this response. yes - I meant client credentials.

So, in my clients list I have a console app and a web app. console app uses client credentials. Web app uses auth code and allows users to login.

for my API endpoints I want the console app to be able to hit them. I also want website users with certain claims to be able to hit them.

If I only checked API scopes (as you are suggesting from my understanding), it would mean all website users would be able to update/delete/etc.

To get around this, I'm finding I need two policies on my endpoint (in this example an Update endpoint):

    [Authorize(Policy = DemoApiConstants.AuthorizationPolicies.ClientApplicationCanWrite)]
    [Authorize(Policy = DemoAuthorizationConstants.AuthorizationPolicies.UserCanWrite)]

This means that for the website, we first check that the website has the API scope, but we also check that the user is allowed to write.

But, since there's no way of conditinally bypassing the UserCanWrite policy when I'm using the console app, I've had to add a client claim.

    public static AuthorizationPolicy UserCanWrite()
    {
        //Build an Authorization Policy
        return new AuthorizationPolicyBuilder()
            .RequireAuthenticatedUser()
            .RequireAssertion(context =>
            {
                //Check if the user has the admin full or read claim
                return context.User.HasClaim(c => c.Type == "demo_access" && (c.Value == "full" || c.Value == "write"))
                // For client credentials, we'll check the client type, because there will not ber a logged in user
                || context.User.HasClaim(c => c.Type == "client_ClientType" && c.Value == "m2m");
            })
            .Build();
    }
RolandGuijt commented 1 month ago

Thanks for the clarification. My point was that your solution works but my suggestion was to not tie your policies to the manner the token was obtained and to instead check for a scope that only clients that use client credentials use e.g. For further discussion around policies please use Microsoft's issue tracker as this is not really an IdentityServer issue.

brockallen commented 1 month ago

I can chime here a bit... what I'd say is that your authorization logic depends on 1) the client, or 2) the user. So I'd suggest building a policy that knows this. For (1) you know the client id -- that's one way to identify the client. Would that not work?

Also, perhaps part of what is historically problematic here is that often folks want to use claims in the token as permissions and that's not ideal. Perhaps the permissions are in the app's DB (not from IdentityServer). You'd still use claims in the token to know who's calling, and then there's a second step to load perms for the caller.

So in short, authorization is complicated and there's no one-size-fits-all; very often it depends on your architecture and OIDC/OAuth is not designed as a general purpose permission system.

MH61Aus commented 1 month ago

Thanks for your responses.

Just to clarify I'm following along correctly - Roland is suggesting that I could consider using different scopes for client credentials. So I could potentially change the scopes to: { "DemoApi.FullAccess.ClientCredentials", "DemoApi.Read.ClientCredentials", "DemoApi.Write.ClientCredentials","DemoApi.FullAccess.Interactive", "DemoApi.Read.Interactive", "DemoApi.Write.Interactive" },

and then I wouldn't need the clientclaim because I could differentiate based off requested scope.

Brock is suggesting I could simply check the clientID instead of the clientclaim. I can of course do this - I'm just trying to plan for an API that'd going to have multiple consumers, so I thought checking for a claim or scope would make more sense than explicitly specifying the clientId.

Brock - your second paragraph is actually something I've been thinking about, though I'm still a little confused about exactly what you are saying. Moving forward I'm looking at client apps managing their own permissions to some degree - I'm already getting to the point where the identity server config is starting to contain claims that I feel the identity server doesn't need to know about, but I wasn't sure how else to do it. I've seen examples of a client manipulating the claims principal after signin to add claims that contain client specific data - but these aren't going to be passed with the access token to the API. I suppose in many of these cases, the API would have access to the DB containing the client related permissions, so the auth policy would just be hitting that (passing in the sub) and would otherwise have nothing to do with the access token/identity server?

brockallen commented 1 month ago

Just to clarify I'm following along correctly - Roland is suggesting that I could consider using different scopes for client credentials.

So that would be one way, but you need to ensure the clients that are using user-based authorization can't request those higher/special scopes.

so I thought checking for a claim or scope would make more sense than explicitly specifying the clientId.

Ok, so then you could also configure a custom Claims for those higher privileged clients: https://docs.duendesoftware.com/identityserver/v7/reference/models/client/#token

I'm already getting to the point where the identity server config is starting to contain claims that I feel the identity server doesn't need to know about,

Yes, this is somewhat my point. I don't know your architecture or requirements, so this is up to you to decide. But we always suggest putting those things where it makes the most sense. As a starting point then, at IdentityServer you might set things that re "global" to your architecture, and then things that are local/app-specific at the app. I know this might sound obvious, but the point is that there's nothing magic here :)

MH61Aus commented 4 weeks ago

Thanks for all the responses - I've got enough to work with, so I'm going to close out this issue.