requiredScope Any or All check for scopes

[jsquire]

Issue Transfer

This issue has been transferred from the Azure SDK for .NET repository, #32647.

Please be aware that @1Jesper1 is the author of the original issue and include them for any questions or replies.

Details

Does the requiredScope attribute do an Any or All check for the scopes?

---

Document Details

⚠ Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

• ID: eea099b1-d4fe-5ec7-71e3-e4f79e972be0
• Version Independent ID: 935de5b5-6042-4514-3688-cb794d3a9eed
• Content: RequiredScopeAttribute Class (Microsoft.Identity.Web.Resource) - Azure for .NET Developers
• Content Source: xml/Microsoft.Identity.Web.Resource/RequiredScopeAttribute.xml
• Service: active-directory
• GitHub Login: @rloutlaw
• Microsoft Alias: routlaw

[1Jesper1]

I did some research:

1. If only 1 scope exists => allow [RequireScope("test", "example")]

2. Only 1 scope exist => allow

services.AddAuthorization(options =>
options.AddPolicy("testPolicy2",
policy => policy.RequireScope("test", "example"))) 

3. Both scopes must exist => allow

services.AddAuthorization(options =>
options.AddPolicy("testPolicy",
policy => policy.RequireScope("test").RequireScope("example"))); 

Is this true @jsquire ?

[jsquire]

@1Jesper1: I do not have insight. My role was only to triage the original issue over to the folks who would be best able to assist.

[1Jesper1]

@jsquire Ah, I see.

[1Jesper1]

@jsquire Has anyone checked this yet? :)

[jsquire]

@jsquire Has anyone checked this yet? :)

Unfortunately, this is not something that I have insight into. @jmprieur: Are you able to assist?

[tonycoelho]

I did some research:

1. If only 1 scope exists => allow [RequireScope("test", "example")]
2. Only 1 scope exist => allow

services.AddAuthorization(options =>
options.AddPolicy("testPolicy2",
policy => policy.RequireScope("test", "example"))) 

3. Both scopes must exist => allow

services.AddAuthorization(options =>
options.AddPolicy("testPolicy",
policy => policy.RequireScope("test").RequireScope("example"))); 

Is this true @jsquire ?

@1Jesper1 I had the same question and after looking at the source code, I came to the same conclusion that you did above. Using RequireScopeAttribute appears to apply an Any() condition to the scopes in the list whereas invoking multiple policy.RequireScope().RequireScope() methods applies a combination of Any and All() conditions. Meaning if multiple scopes are defined in each innovation then Any scope is allowed from that list, but then invoking RequireScope again applies an AND condition to the second list. Example below:

The following requires ((scopea or scopeb) and scopec).

services.AddAuthorization(options => options.AddPolicy("example", policy => policy.RequireScope("scopea", "scopeb").RequireScope("scopec")));

It doesn't appear that you can use the RequireScopeAttribute in a way that it applies All() logic, especially since the attribute has AllowMultiple set to false.

[1Jesper1]

@tonycoelho Interesting, it's a bit weird. I think RequireScopeAttribute should have been "All".

[tonycoelho]

@1Jesper1 I think the better solution would be for RequiredScopeAttribute to allow multiple instances, i.e.:

[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true)]
public class RequiredScopeAttribute : Attribute, IAuthRequiredScopeMetadata
{}

and then update the handler as follows. This would ensure that the attribute condition works the same as a policy where you can chain RequireScope() innovations on the policy builder. Note : the code below hasn't been tested and probably could be optimized a bit.

@jmprieur any thoughts?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Options;

namespace Microsoft.Identity.Web
{
    internal class ScopeAuthorizationHandler : AuthorizationHandler<ScopeAuthorizationRequirement>
    {
        private readonly IConfiguration _configuration;

        public ScopeAuthorizationHandler(IConfiguration configuration)
        {
            _configuration = configuration;
        }

        protected override Task HandleRequirementAsync(
            AuthorizationHandlerContext context,
            ScopeAuthorizationRequirement requirement)
        {
            _ = Throws.IfNull(context);
            _ = Throws.IfNull(requirement);

            // The resource is either the HttpContext or the Endpoint directly when used with the
            // authorization middleware
            var endpoint = context.Resource switch
            {
                HttpContext httpContext => httpContext.GetEndpoint(),
                Endpoint ep => ep,
                _ => null,
            };

            var metadatas = endpoint?.Metadata.GetOrderedMetadata<IRequiredScopeMetadata>();
            var scopeGroups = new List<IEnumerable<string>>();

            string[] GetScopesFromConfig(string key) => _configuration.GetValue<string>(key)?.Split(' ');

            if (requirement.RequiredScopesConfigurationKey is not null || requirement.AcceptedScopes is not null)
            {
                var requirementScopes = requirement.RequiredScopesConfigurationKey is not null ? GetScopesFromConfig(requirement.RequiredScopesConfigurationKey) : requirement.AcceptedScopes;
                if (requirementScopes is not null)
                {
                    scopeGroups.Add(requirementScopes);
                }
            }
            else if (metadatas is not null)
            {
                scopeGroups.AddRange(metadatas
                    .Select(m => m.RequiredScopesConfigurationKey is not null ? GetScopesFromConfig(m.RequiredScopesConfigurationKey) : m.AcceptedScopes)
                    .Where(s => s is not null));
            }

            if (!scopeGroups.Any())
            {
                context.Succeed(requirement);
                return Task.CompletedTask;
            }

            var claimScopes = context.User.FindAll(ClaimConstants.Scp).Union(context.User.FindAll(ClaimConstants.Scope));
            if (!claimScopes.Any())
            {
                return Task.CompletedTask;
            }

            if (scopeGroups.All(scopes => claimScopes.SelectMany(s => s.Value.Split(' ')).Intersect(scopes).Any()))
            {
                context.Succeed(requirement);
                return Task.CompletedTask;
            }

            return Task.CompletedTask;
        }
    }
}

[1Jesper1]

@tonycoelho Agree, or add an optional param "RequireAll" to the attribute.