Skip to content
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

ClaimsPrincipal doesn't include identity when authType is anonymous #3857

Closed
johndowns opened this issue Dec 5, 2018 · 16 comments
Closed

ClaimsPrincipal doesn't include identity when authType is anonymous #3857

johndowns opened this issue Dec 5, 2018 · 16 comments
Assignees

Comments

@johndowns
Copy link

johndowns commented Dec 5, 2018

Investigative information

Please provide the following:

  • Timestamp: From December 4 through 6, 2018
  • Function App version (1.0 or 2.0): 2.0
  • Function App name: applies across multiple (every one I've tried)
  • Function name(s) (as appropriate): multiple
  • Invocation ID: N/A
  • Region: multiple; I've mostly tested australiasoutheast

Repro steps

Provide the steps required to reproduce the problem:

  1. Configure Azure AD authentication on a function app and disallow anonymous requests.
  2. Create an HTTP-triggered function (either C# script or precompiled C#) with a ClaimsPrincipal parameter.
  3. Include code within the function to enumerate and log the identities attached to the ClaimsPrincipal.
  4. Change the function authLevel to anonymous, instead of using a function key.
  5. Obtain an Azure AD token and send a request to the function.

Expected behavior

The ClaimsPrincipal.Identities property should be populated with the Azure AD identity, and all claims.

Actual behavior

The ClaimsPrincipal.Identities property does not include the Azure AD identity when authLevel is set to anonymous.

Known workarounds

Change the function's authLevel to a different value.

Related information

Example function code:

#r "Newtonsoft.Json"

using System.Security.Claims;
using System.Linq;
using System.Net;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Primitives;
using Newtonsoft.Json;

public static async Task<IActionResult> Run(HttpRequest req, ClaimsPrincipal principal, ILogger log)
{
    log.LogInformation($"Found {principal.Identities.Count()} identities.");
    foreach (var identity in principal.Identities)
    {
        log.LogInformation($"Identity {identity.Name}:");
        log.LogInformation($"Auth type is {identity.AuthenticationType}");
        foreach (var claim in identity.Claims)
        {
            log.LogInformation($"Claim '{claim.Type}' = '{claim.Value}'");
        }
    }

    return new OkResult();
}

Example function.json:

{
  "bindings": [
    {
      "authLevel": "anonymous",
      "name": "req",
      "type": "httpTrigger",
      "direction": "in",
      "methods": [
        "get",
        "post"
      ]
    },
    {
      "name": "$return",
      "type": "http",
      "direction": "out"
    }
  ]
}

(Also note the documentation says that you should set the HTTP-triggered function authentication level to anonymous when using AAD authentication.)

@ConnorMcMahon ConnorMcMahon self-assigned this Dec 5, 2018
@fabiocav
Copy link
Member

@ConnorMcMahon would it be ok to assign this to the current sprint? I'll have it in Triage for now

@fabiocav fabiocav added this to the Triaged milestone Dec 14, 2018
@ConnorMcMahon
Copy link
Contributor

Sounds good.

@myusrn
Copy link

myusrn commented Dec 18, 2018

@fabiocav and @ConnorMcMahon is this still on the backlog to be addressed in the current sprint?

I ask because i'm working on functions app solution for a customer and have an end of week milestone to deliver. Trying to determine if i'll be able to get away with not having to implement and document inclusion of function ?code=<function code> in addition to oauth Authorization header security.

Should I just watch for an azure functions runtime update from the current 20.12246.0 (~2) to determine that its been addressed an one no longer has to set HttpTrigger(AuthorizationLevel.Function, . . . and pass code to EasyAuth secured function app endpoints?

Have you folks decided on whether or not the fix for this will involving moving to HttpTrigger(AuthorizationLevel.User, . . . to denote an EasyAuth openid connect & oauth secured endpoint or if it'll be HttpTrigger(AuthorizationLevel.Anonymous, . . . with [Authorize] attribute supported?

@ConnorMcMahon
Copy link
Contributor

I have a PR that addresses this issue (#3904).

@myusrn, we are planning on future integrations with EasyAuth, but we are still deciding whether we want to utilize the AuthorizationLevel.User field, or whether we want to do a more robust configuration that would allow users to do similar things to the Authorize attribute within the function.json/HttpTriggerAttribute.

@maryammadzadeh
Copy link

maryammadzadeh commented Jan 8, 2019

Hi there,

I have defined my own roles in AAD and I can see that in the bearer token received by the http trigger (in the req param) is populated with the roles my predefined roles.

public static async Task<IActionResult> RunAsync([HttpTrigger(AuthorizationLevel.Anonymous, "post", Route = null)]HttpRequest req, ILogger log, ClaimsPrincipal principal)
However, when I examine the principal variable (no matter what I set the AuthorizationLevel to), the I only see the following identity:

Found 1 identities.
[1/8/2019 5:12:37 PM] Identity :
[1/8/2019 5:12:37 PM] Auth type is WebJobsAuthLevel
[1/8/2019 5:12:37 PM] Claim 'http://schemas.microsoft.com/2017/07/functions/claims/authlevel' = 'Admin'

Any idea where the missing link is?

@myusrn
Copy link

myusrn commented Jan 8, 2019

@maryammadzadeh I would check . . .

  1. that you have EasyAuth + Express configuration enabled your function app

  2. that you have configured function app api with [HttpTrigger(AuthorizationLevel.Function, . . . setting and are including the api specific or master or _default host key as ?code=<key> query string parameter in calls along with Authorization header bearer token. Essentially the matter that @johndowns raised by opening this issue.

  3. ensure that your bearer token acquisition calls are using scope "<function app registration client id>/user_impersonation" as the bearer token you attach to call need claim iss to be <function app registration client id> for it to be accepted and unwound into injected principal. if you haven't used it before visit jwt.io to claims in bearer tokens you are using.

@myusrn
Copy link

myusrn commented Feb 5, 2019

@ConnorMcMahon with the fix in place do we . . .

  1. Wait for | settings | runtime version to increment from current 2.0.12285.0 value to know when your merged PR fix is deployed?

  2. Once deployed do we change deployment of functions from using AuthorizationLevel.Function to AuthorizationLevel.Anonymous or AuthorizationLevel.User setting?

@ConnorMcMahon
Copy link
Contributor

  1. Correct. You can also track releases here: https://github.com/Azure/azure-functions-host/releases
  2. I would change it to AuthorizationLevel.Anonymous for now. Right now, AuthorizationLevel.User is an unused legacy value, and may remain so, as all of the other AuthorizationLevel values just have to do with Key-based authorization. This could be subject to change in the future, so it is best just to avoid using that until if/when we announce what we will use AuthorizationLevel.User for.

@matt-dib
Copy link

matt-dib commented Feb 28, 2019

@ConnorMcMahon

I see this fix went out with: https://github.com/Azure/azure-functions-host/releases/tag/v2.0.12309

However, I am still experiencing this issue.

My function app's runtime is 2.0.12332.0. Is it expected that this issue should not occur on this runtime running in Azure?

@ConnorMcMahon
Copy link
Contributor

ConnorMcMahon commented Feb 28, 2019

@matt-dib, I just tested against my app running on 2.0.12332, and it appears to be working. Can you share your function name and application name with me? If you want to share your app name without giving it publicly, you can share this information instead.

@matt-dib
Copy link

@ConnorMcMahon
Id=190d3b25-1a95-4fcd-b39a-14aead2d9fe2
2019-02-28T22:48:14.936
East US

@ConnorMcMahon
Copy link
Contributor

@matt-dib From what I can tell everything looks good configwise on your site. Can you check that inside of your function the HTTP request has the header X-MS-CLIENT-PRINCIPAL?

@matt-dib
Copy link

matt-dib commented Mar 1, 2019

@ConnorMcMahon

I checked inside the function and the X-MS-CLIENT-PRINCIPAL is present on the HttpRequest object. After base64 decode on the value the claims do exist in the header value.

However, the System.Security.Claims.ClaimsPrincipal.Current is still null.

I did a little bit more experimentation as well to rule out any other potential influences.

  • Function was being executed through a proxy. I removed the proxy and no change.
  • Function was executed as an instance function. I added static back and no change.
  • Switched to AuthorizationLevel.Function. No change.

By no change, what I mean is System.Security.Claims.ClaimsPrincipal.Current is always null.

@myusrn
Copy link

myusrn commented Mar 1, 2019

@matt-dib an additional thing to confirm is that the token being providing in api calls has the audience "aud" claim set to <application (client) id>/user_impersonation and not the function app's azure ad | <application> | expose an api | scope displayed value of api://<application (client) id>/user_impersonation. In application code this was achieved by setting the microsoft authentication library [msal] scopes parameter to just <application (client) id>/user_impersonation vs api://<application (client) id>/user_impersonation.

@ConnorMcMahon
Copy link
Contributor

ConnorMcMahon commented Mar 1, 2019

@matt-dib, we don't actually populate ClaimsPrincipal.Current. In .NET Core, that is no longer the standard way of passing ClaimsPrincipal data. See here for more info on that.

Instead, we support passing in the ClaimsPrincipal as a parameter to your function, or grabbing it from the HttpRequest object via req.HttpContext.User;

You can see our documentation on the feature here.

@matt-dib
Copy link

matt-dib commented Mar 1, 2019

@ConnorMcMahon That was indeed the issue. Thanks so much.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants