JamesRandall / FunctionMonkey

Write more elegant Azure Functions with less boilerplate, more consistency, and support for REST APIs. Docs can be found at https://functionmonkey.azurefromthetrenches.com
MIT License
294 stars 50 forks source link

Http Trigger: AuthorizationLevel.Anonymous runs ClaimsPrincipalAuthorization but it should be ignored #162

Open alexeymarkov opened 4 years ago

alexeymarkov commented 4 years ago

ClaimsPrincipalAuthorization requires ClaimsPrincipal which is not resolved for AuthorizationLevel.Anonymous. As a consequence ClaimsPrincipal null is passed to ClaimsPrincipalAuthorization:

Change `

    {{#if ValidatesToken}}
        if (req.Headers["{{TokenHeader}}"].Count == 0)
        {
            return new UnauthorizedResult();
        }
        string authorizationHeader = req.Headers["{{TokenHeader}}"][0];
        if (string.IsNullOrWhiteSpace(authorizationHeader))
        {
            return new UnauthorizedResult();
        }

        principal = await pluginFunctions.ValidateToken(authorizationHeader);
        if (principal == null)
        {
            return new UnauthorizedResult();
        }
        contextSetter.SetHttpContext(principal, requestUrl, headerDictionary);    
    {{/if}}

    {{#if AuthorizesClaims}}
        var claimsPrincipalAuthorizationResult = await pluginFunctions.IsAuthorized(principal, req.Method, requestUrl);
        if (!claimsPrincipalAuthorizationResult)
        {
            return new UnauthorizedResult();
        }
    {{/if}}

`

to `

    {{#if ValidatesToken}}
        if (req.Headers["{{TokenHeader}}"].Count == 0)
        {
            return new UnauthorizedResult();
        }
        string authorizationHeader = req.Headers["{{TokenHeader}}"][0];
        if (string.IsNullOrWhiteSpace(authorizationHeader))
        {
            return new UnauthorizedResult();
        }

        principal = await pluginFunctions.ValidateToken(authorizationHeader);
        if (principal == null)
        {
            return new UnauthorizedResult();
        }
        contextSetter.SetHttpContext(principal, requestUrl, headerDictionary);

        {{#if AuthorizesClaims}}
            var claimsPrincipalAuthorizationResult = await pluginFunctions.IsAuthorized(principal, req.Method, requestUrl);
            if (!claimsPrincipalAuthorizationResult)
            {
                return new UnauthorizedResult();
            }
        {{/if}}
    {{/if}}

`

jlocans commented 4 years ago

Does this mean that after this change anonymous endpoints will skip ClaimsPrincipalAuthorizationDefault? If so looking forward to this change.

Current behavior doesn't seem right. Why would anonymous endpoint be checked in ClaimsPrincipalAuthorizationDefault? This is causing issues if you set up some custom data (like user context) in TokenValidator and afterwards check it in ClaimsPrincipalAuthorizationDefault.

alexeymarkov commented 4 years ago

Yes, this will be ignore. Alternatively you can also use a custom IClaimsPrincipalAuthorization implementation and apply it to anonymous endpoints public class AllowAnonymousClaimsAuthorization : IClaimsPrincipalAuthorization { public Task<bool> IsAuthorized(ClaimsPrincipal claimsPrincipal, string httpVerb, string url) { return Task.FromResult(true); } }

jlocans commented 4 years ago

@alexeymarkov Thanks for the tip. Works great!