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

Make RequiredScopeAttribute and AuthorizeAttribute self exclusive #1641

Closed
aremo-ms opened this issue Feb 23, 2022 · 4 comments
Closed

Make RequiredScopeAttribute and AuthorizeAttribute self exclusive #1641

aremo-ms opened this issue Feb 23, 2022 · 4 comments
Assignees
Milestone

Comments

@aremo-ms
Copy link

When I'm using the next attributes, I'm forced to have both Roles and Scopes in my token
Please make it OR relationship, so I can have at least one of them.

[RequiredScope(new string[] { "ToDoList.Read", "ToDoList.Write" })]
[Authorize(Roles = "ToDoList.Read.All, ToDoList.Write.All")]
@kalyankrishna1
Copy link

Since Authorize attribute is already used by asp.net middleware, it's okay to either

  1. extend RequiredScope to suuport app permissions
  2. Create a new RequiredAppPermissions attribute (please don't call it RequiredAppRoles :)). Maybe create a copy of RequiredScope and call it RequiredDelegatedPermissions. It has become an issue to explain app roles and scopes and how they map to app and delegated permissions and I think we should work towards using standard terms and not OAuth terms.

@jmprieur
Copy link
Collaborator

jmprieur commented Apr 4, 2022

Spec proposal:

  • create a new attribute RequiredScopeOrAppPermissionAttribute, deriving RequiredScopeAttribute: from:

    public class RequiredScopeAttribute : Attribute, IAuthRequiredScopeMetadata

  • add a property AppPermissions to that class, as well as a contructor taking both the requiredDelegatedPermissions and requiredAppPermissions.

          /// <summary>
          /// App permissions accepted by this web API.
          /// </summary>
          public IEnumerable<string>? AcceptedAppPermissions { get; set; }
    
          /// <summary>
          /// Fully qualified name of the configuration key containing the required app permissions (separated
          /// by spaces).
          /// </summary>
          /// <example>
          /// If the appsettings.json file contains a section named "AzureAd", in which
          /// a property named "Scopes" contains the required scopes, the attribute on the
          /// controller/page/action to protect should be set to the following:
          /// <code>
          /// [RequiredScopeOrAppPermissionAttribute(RequiredScopesConfigurationKey="AzureAd:Scopes",
          ///                                                                      RequiredAppPermissionsConfigurationKey="AzureAd:AppPermissions")]
          /// </code>
          /// </example>
          public string? RequiredAppPermissionsConfigurationKey { get; set; }
    
          /// <summary>
          /// Verifies that the web API is called with the right scopes (delegated permissions) or app permissions.
          /// If the token obtained for this API is on behalf of the authenticated user does not have
          /// any of these <paramref name="acceptedScopes"/> or in its scope claim, or 
          /// any of the <paramref name="requestedAppPermissions"/> in its role/roles claims, the
          /// method updates the HTTP response providing a status code 403 (Forbidden)
          /// and writes to the response body a message telling which scopes are expected in the token.
          /// </summary>
          /// <param name="acceptedScopes">Scopes accepted by this web API.</param>
          /// <param name="acceptedAppPermissions">App permissions accepted by this web API.</param>
          /// <remarks>When the scopes don't match, the response is a 403 (Forbidden),
          /// because the user is authenticated (hence not 401), but not authorized.</remarks>
          /// <example>
          /// Add the following attribute on the controller/page/action to protect:
          ///
          /// <code>
          /// [RequiredScopeOrAppPermissionAttribute(acceptedScopes = new []{"access_as_user"}, acceptedAppPermissions = new []{"access_as_app"}, )]
          /// </code>
          /// </example>
          /// <seealso cref="M:RequiredScopeAttribute(string,string)"/>
          /// if you want to express the required scopes from the configuration.
          public RequiredScopeOrAppPermissionAttribute(string[] acceptedScopes, string[] acceptedAppPermissions)
  • Same thing for IAuthRequiredScopeMetadata (have a new interface inheriting from this one and named IAuthRequiredScopeAndAppPermissionMetadata)

  • Update the code that uses it:

    image

@jennyf19 jennyf19 removed this from the 1.24.0 milestone Apr 20, 2022
@jmprieur
Copy link
Collaborator

When done, also update the docs.ms page. See MicrosoftDocs/azure-docs#82732

@jennyf19
Copy link
Collaborator

jennyf19 commented Jun 4, 2022

Included in 1.25.0 release

@jennyf19 jennyf19 closed this as completed Jun 4, 2022
@jennyf19 jennyf19 added this to the 1.25.0 milestone Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants