OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.52k stars 6.51k forks source link

[REQ] [CPP] [pistache-server] Add Basic and Bearer Authorization #19695

Open winkler-pixop opened 1 week ago

winkler-pixop commented 1 week ago

Background

As of writing, the cpp-pistache-server generator does not support neither HTTP Basic nor Bearer authorization. This feature request proposes an apporach to remedy that.

If a consensus on how to implement this can be established, I am willing to implement it myself.

Suggestion

It is suggested, that

  1. Any endpoint+method can have one authorization mechanism. Either Basic or Bearer. If an endpoint+method has both specified in yaml, compilation of the genereated C++ sources should be made impossible and appropriate errors should be reported.

  2. The generated c++ sources should only extract and assert the existance of bearer token and credentials from headers.

  3. Credentials and tokens should be forwarded to the relevant handler for validation.

The reason for 3. is that the approaches for Basic and for Bearer should be identical. For Basic, a virtual method for simply asserting the validity could suffice. The verified username could then be forwarded to the handler. For Bearer, however, the approach might be remarkable different. For several cases, JWT for example, the entire token might be needed to extract claims in the handler, while merely determining validity could still be determined in a virtual method.

For that reason, it is suggested that both credentials for Basic and the complete token for Bearer are forwarded to the handlers in their entirey.

The code generated buy openapi-generator should ensure that the relevant headers are present and well formed and, for Basic perform decoding and preprocessing. Validation should be deferred to the handlers.

Examples

Basic

This yaml


components:
  securitySchemes:
    credentials:
      type: http
      scheme: basic

paths:
  /foo:

    get:
      summary: Lists foos
      operationId: listFoos
      tags: [ Foos ]

      security:
        - credentials: []

      responses:
        '200':
          description: A Foo fresh from the Bar.
          content:
            text/plain:
              schema:
                type: string
                example: "Foo"

should result in the following generated c++.

void FoosApiImpl::list_foos(
        const BasicCredentials &credentials, 
        Pistache::Http::ResponseWriter &response) 
{
    response.send(Pistache::Http::Code::Ok, "Do some Magic!");
}

...

typedef struct
{
   std::string username;
   std::string password;
} BasicCredentials;

Bearer

This yaml


components:
  securitySchemes:
    jwt:
      type: http
      scheme: bearer

paths:
  /foo:

    get:
      summary: Lists foos
      operationId: listFoos
      tags: [ Foos ]

      security:
        - jwt: []

      responses:
        '200':
          description: A Foo fresh from the Bar.
          content:
            text/plain:
              schema:
                type: string
                example: "Foo"

should result in c++ code similar to this:

void FoosApiImpl::list_foos(
        const std::string &token, 
        Pistache::Http::ResponseWriter &response) 
{
    response.send(Pistache::Http::Code::Ok, "Do some Magic!");
}

Both Basic and Bearer

This yaml


components:
  securitySchemes:
    jwt:
      type: http
      scheme: bearer
    credentials:
      type: http
      scheme: basic

paths:
  /foo:

    get:
      summary: Lists foos
      operationId: listFoos
      tags: [ Foos ]

      security:
        - jwt: []
        - credentials: []

      responses:
        '200':
          description: A Foo fresh from the Bar.
          content:
            text/plain:
              schema:
                type: string
                example: "Foo"

should result in c++ code similar to this:

void FoosApiImpl::list_foos( ... , Pistache::Http::ResponseWriter &response) 
{
    This code will not compile because c++ pistache server only supports either bearer or basic. Not both.
}

Closing Remarks

winkler-pixop commented 1 week ago

Allow me to ping you again, @ravinikam @stkrwork @etherealjoy @MartinDelille @muttleyxd .

I need this functionality and will implement it and create a PR if I know the approach is acceptable.

MartinDelille commented 1 week ago

If I'm in the cpp technical commitee, I unfortunately only work on the Qt client. Maybe @wing328 would be able to review your proposition and your code.

winkler-pixop commented 1 week ago

If I'm in the cpp technical commitee, I unfortunately only work on the Qt client. Maybe @wing328 would be able to review your proposition and your code.

Oh. I am sorry for the inconvenience. I found the members of the committee somewhere, but now i cannot find that page again. I will omit you from any future annoyances ;-)

muttleyxd commented 1 week ago

The approach looks kinda fine to me, but I wonder how it would work with global authorization?

Would we have to also handle authorization in every endpoint manually?

In the project I worked with, we used to have layered handlers for this (you set custom entry handler by calling setHandler on Pistache::Http::Endpoint) and first layer would be responsible for all authorization related stuff.

winkler-pixop commented 1 week ago

I envisioned that each endpoint performs the authorization individually. The reason for that is, that in most cases the token and the username need to propagate to the handler anyawy. Just because "John" and his "password" match, we would still need to know if "John" is allowed to access the specific resource. And it is the same thing for tokens.

However, I do understand your point, and upon second thought, we probably shouldn't enforce a specific way to handle authorization. So how about the possibility to do both:


FoosApiImpl::FoosApiImpl( ... )
{
  /*  If no handler is set, authentication is left to the individual enpoint handler as suggestged  below.*/
  setBasicCredentialsauthenticator([=](const BasicCredentials &creds)
  {
    return creds.username == "John" && creds.password == "s3cr3t";
  });
}

void FoosApiImpl::list_foos(
        const BasicCredentials &credentials, 
        Pistache::Http::ResponseWriter &response) 
{
   /* If no handler is set in constrtuctor, then this could be done here
   if ( ! ( creds.username == "John" && creds.password == "s3cr3t") ) throw 401;
   */
    response.send(Pistache::Http::Code::Ok, allFoosForUsernameAsJSON(credentials.username));
}

and then a identical solution for the tokens.

What do you think about this?

winkler-pixop commented 14 hours ago

Have you had a chance to consider my proposal, @muttleyxd ?

muttleyxd commented 4 hours ago

Yeah, sorry for that - been busy recently and I missed the notification that you replied to me.

Design looks good, I think you could try to implement it - I also think we shouldn't enforce authorization in case it's not specified at all in yaml (probably there's some property which could be used for detecting that)