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

Add support for role based security in ATC generator. #26

Closed
5 of 9 tasks
MrAndersPedersen opened this issue Dec 10, 2020 · 6 comments
Closed
5 of 9 tasks

Add support for role based security in ATC generator. #26

MrAndersPedersen opened this issue Dec 10, 2020 · 6 comments
Assignees
Labels
design approved Design approved enhancement New feature or request

Comments

@MrAndersPedersen
Copy link

MrAndersPedersen commented Dec 10, 2020

Tasks:

  • Define custom OpenApi x-roles #137
    • Check if $ref can be used
  • Implement generator
    • Read schema
    • Validate security schema
    • Validate path schema
    • Validate role usage
    • Generate Authorize attribute
    • Demo project sample

Design

Setup in yaml file will be done using security schemes and the first type to support is OAuth2 with client id + client secret as authetication. Custom roles will be used to support access based on rights.

This can be done like shown below:

Definition of the security schema:

components:
  securitySchemes:

    oAuth2Sample:
      x-atc-azure-ad-application-roles:
          api.execute.all: 
            description:Access to all operations
            name: everything
          api.execute.read: 
            description: Access to smart charging operations.
            name: read

      type: oauth2
      flows:
        clientCredentials:
          tokenUrl: 'https://login.microsoftonline.com/7b8ae8aa-f7d3-4bfe-a333-eb138ce54b98/oauth2/v2.0/token'
          refreshUrl: ''
          scopes:
            .default 
      description: OAuth2 using client id + client secret

Usage in a operation:

paths:
  '/items/{id}':
    put:
      summary: 'Updates an item'
      description: 'Updates an item'
      operationId: updateItem
      responses:
        '200':
          description: OK
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/UpdateItemRequest'
      parameters:
        - name: id
          in: path
          description: The id of the order
          required: true
          schema:
            type: string
            format: uuid
      security:
        - oAuth2Sample:
            - .default
        - x-atc-azure-ad-application-roles:
            - api.execute.read (can this be a $ref?)
            - api.execute.write

The generated code will affect the controller class(es) and will add an Authorize attribute to each method that is setup with security in its operation definition. The Roles property in the Authorize attribute maps to OAuth2 scopes and these are defined in the securiy scheme.

As the Roles property of the Authorize attribute treats multiple attributes on the same method as AND requirements it will be needed to have only 1 attribute per method to support OR and the roles will have to be comma separated. To make the code a bit nicer a static class with scope definitions are generated for each defined security schema.

Make sure Swagger UI shows the required roles per operation.

Validation:
ATC generator will validate the setup:

  • Unused role(s) (warning)
  • Undefined role(s) (error)
  • Roles not defined in OAuth2 schema (error)

The generated code could look something like this:

namespace Demo.Api.Generated.Security
{
    public static class OAuth2ClientCredentials
    {
        // Single roles
        public const string ApiExecuteAll = "api.execute.all";
        public const string ApiExecuteRead = "api.execute.read";

        // Combined roles
        [System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores", Justification = "Better readability for combined scopes.")]
        [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Better readability for combined scopes.")]
        public const string ApiExecuteAll_ApiExecuteRead = ApiExecuteAll + "," + ApiExecuteRead;
    }
}
namespace Demo.Api.Generated.Endpoints
{
    /// <summary>
    /// Endpoint definitions.
    /// Area: Items.
    /// </summary>
    [ApiController]
    [Authorize]
    [Route("api/v1/items")]
    [GeneratedCode("ApiGenerator", "1.0.216.0")]
    public class ItemsController : ControllerBase
    {
        /// <summary>
        /// Description: Updates an item.
        /// Operation: UpdateItem.
        /// Area: Items.
        /// </summary>
        [HttpPut("{id}")]
        [Authorize(Roles = OAuth2ClientCredentials.ApiExecuteAll_ApiExecuteRead)]
        [ProducesResponseType(typeof(string), StatusCodes.Status200OK)]
        public Task<ActionResult> UpdateItemAsync(UpdateItemParameters parameters, [FromServices] IUpdateItemHandler handler, CancellationToken cancellationToken)
        {
            if (handler is null)
            {
                throw new ArgumentNullException(nameof(handler));
            }

            return InvokeUpdateItemAsync(parameters, handler, cancellationToken);
        }

        private static async Task<ActionResult> InvokeUpdateItemAsync(UpdateItemParameters parameters, IUpdateItemHandler handler, CancellationToken cancellationToken)
        {
            return await handler.ExecuteAsync(parameters, cancellationToken);
        }
    }
}
@LarsSkovslund
Copy link

The problem with using scopes are that any external client generating code from this OpenApi spec will assume it is the required OAuth scope, but what it really is, is just a role-claim in the JWT token.

@davidkallesen davidkallesen transferred this issue from atc-net/atc Jan 12, 2021
@davidkallesen davidkallesen added the enhancement New feature or request label Jan 18, 2021
@davidkallesen
Copy link
Collaborator

davidkallesen commented Jan 20, 2021

We should just to be sure, that we follow https://swagger.io/docs/specification/authentication/ 4 definitions with sub-schemas for securitySchemes (=> 6 definitions):

  • http – for Basic, Bearer and other HTTP authentications schemes
  • apiKey – for API keys and cookie authentication
  • oauth2 – for OAuth 2
  • openIdConnect – for OpenID Connect Discovery

And as I read it, is only OAuth2 and OpenID that supports Scopes - as @LarsSkovslund mentions.

@cjakobsen cjakobsen added the design approved Design approved label Jan 25, 2021
@cjakobsen
Copy link
Contributor

Started working on this.

@perkops
Copy link
Member

perkops commented Nov 18, 2022

 security:
        - oAuth2Sample:
            - .default
        - x-atc-azure-ad-application-roles:
            - api.execute.read (can this be a $ref?)
            - api.execute.write

The above suggestion is not valid. It will be parsed by the Microsoft.OpenApi.Readers Nuget Package as an invalid reference identifier, since theres no SecurityScheme named x-atc-azure-ad-application-roles and we cant extend with our own custom security scheme. Theres also only the possibility to have 1 Security Scheme pr. type. Also we cant reference the Extensibility point with the roles under the oAuth2Sample SecuritySchema from the OpenApiOperation (PathItem).

@perkops
Copy link
Member

perkops commented Nov 24, 2022

The solution ended up with us defining 3 new extension "tags":

  • x-authorize-roles (role array)
  • x-authentication-schemes (auth-scheme array)
  • x-authentication-required (true/false boolean)

At the root level of the specification we specify the available roles and auth-schemes possible to use in paths (controllers) /path-items (actions/methods). Validation is in place when reading the YAML file to ensure that any path/path-item does not have roles and/or auth-schemes defined, which are not defined globally. Other validations are also in place to ensure that the combination of the 3 new extensions "tags" are set correctly.

The 3 extension "tags" can be specified at path/path-item level as can be seen in the below example.

If all path-items under a given path all have x-authentication-required set to true, then a [Authorize] header will still be added to the generated controller class. Otherwise [Authorize(Roles=x,y,z)] and [AllowAnonymous] will be applied the necessary places on the actions/methods in the controller.

Authentication-Schemes and Authorize-Roles defined at path/controller level is taken into consideration when generating [Authorize] attributes for path-item/action/method level.

Example specification using the new extension "tags":
NOTE: Tags, parameters, responses, request-bodies, schemas etc. are removed for brevity, so the references in spec below are not valid - The specification is only illustrating the various places the 3 new extension "tags" can be applied.

info:
  title: DEMO API
  description: DEMO API
  version: v1
  contact:
    name: ATC
  license:
    name: MIT
servers:
  - url: /api/v1
    description: Api version 1.0
x-authorize-roles:
  - api.execute.read
  - api.execute.write
  - admin
  - operator
x-authentication-schemes:
  - OpenIddict.Validation.AspNetCore
tags:
  - name: DEMO
    description: ''
paths:
  /data-templates:
    x-authentication-required: true
    x-authorize-roles: [operator]
    x-authentication-schemes: [OpenIddict.Validation.AspNetCore]
    get:
      summary: Returns a list of the groups data templates
      operationId: getDataTemplates
      x-authorize-roles: [admin,operator]
      x-authentication-schemes: [OpenIddict.Validation.AspNetCore]
    post:
      summary: Create a new data template
      operationId: createDataTemplate
      x-authentication-required: false
  '/data-templates/{dataTemplateId}':
    x-authentication-required: true
    x-authorize-roles: [operator]
    get:
      summary: Returns a specific data template
      operationId: getDataTemplateById
      x-authentication-required: true
      x-authorize-roles: [api.execute.read]
    delete:
      summary: Removes a specific data template
      operationId: deleteDataTemplateById
    put:
      summary: Updates a specific data template
      operationId: updateDataTemplateById
  '/data-templates/{dataTemplateId}/tags':
    post:
      summary: Creates a new data template tag
      operationId: createDataTemplateTag
      x-authorize-roles: [api.execute.read]
    delete:
      summary: Deletes a data template tag
      operationId: deleteDataTemplateTag
      x-authentication-schemes: [OpenIddict.Validation.AspNetCore]
  '/data-templates/{dataTemplateId}/tags/{dataTemplateTagId}':
    x-authentication-required: false
    put:
      summary: Updates a specific data template tag
      operationId: updateDataTemplateTagById
components:
  schemas: {}
  requestBodies: {}
  responses: {}
  securitySchemes: {}

@perkops
Copy link
Member

perkops commented Nov 28, 2022

Implemented with PR #168

@perkops perkops closed this as completed Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design approved Design approved enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants