sebadob / rauthy

OpenID Connect Single Sign-On Identity & Access Management
https://sebadob.github.io/rauthy/
Apache License 2.0
297 stars 15 forks source link

UseDpopNonce in openapi spec make oapi-codegen fail generating client-code #544

Closed andoks closed 2 weeks ago

andoks commented 2 weeks ago

I am trying to generate a http client for Go using oapi-codegen from the open-api specification exposed at docs/v1/api-doc/openapi.json but I get an error due to UseDpopNonce being defined as possibly being a string or a nullable string

UseDpopNonce definition

{
 "type": "object",
 "required": [
   "UseDpopNonce"
 ],
 "properties": {
   "UseDpopNonce": {
     "type": "array",
     "items": {
       "allOf": [
         {
           "type": "string",
           "nullable": true
         },
         {
           "type": "string"
         }
       ]
     }
   }
 }
}

The only change I need to do is to remove the {"type": "string"} from the allOf, and it seems to generate fine. Is this something I could see if I find how to change the code for in the project (so that the definition always only can be a nullable string) and would that potentially be a welcomed change? From what I understand the rauthy open-api spec is code-first, then some tool generates the spec from the code, and I have not yet dug that far into the codebase to know how exactly I would have to change this, so wanted to ask before I spend that much effort on it. Hope you don't mind.

If so desired, I could also look into how easy (or hard) it would be to create some sort of CI job that generates client-code to continuously verify that it is possible to generate said http client, although I think that would require some way of generating the openapi-spec via the cli without spinning up a full rauthy system (at least that would make it easier).

oapi-codegen error `error generating code: error generating type definitions: error generating Go types for component schemas: error converting Schema ErrorResponseType to Go type: error generating type for oneOf: error generating Go schema for property 'UseDpopNonce': error resolving primitive type: error generating type for array: error merging schemas: error merging schemas for AllOf: merging two schemas with different Nullable`
oapi-codegen-config.yml ``` package: rauthy_api output: rauthy-api/client.gen.go generate: models: true client: true ```
sebadob commented 2 weeks ago

This is not an or but an and, this is a tuple. It's coming from the ErrorResponseType::UseDpopNonce in src/error/src/error_impls.rs:98.

The first Option<String> is an ACCESS_CONTROL_ALLOW_ORIGIN header value which may not exist.
The second String is the nonce value itself.

I don't know how to translate this into proper go code (last time using go was a long time ago for me), but the type is a tuple (Option<String>, String) inside a rust enum.

I am not sure what exactly you are trying to do, but if you want to have a type safe mapping for the ErrorResponseType, you can probably ignore both of these values. These will never be returned directly inside the JSON. The tuple values will be converted to header values.
The same is true for all the other ErrorResponseType values like DPoP, TooManyRequests and WWWAuthenticate. All values they wrap will be returned as different header values.

From what I understand the rauthy open-api spec is code-first, then some tool generates the spec from the code

That's correct. The OpenAPI file and SwaggerUI docs are generated automatically. This is the reason why these inner values do exist for the ErrorResponseType even though they would only ever be returned as header values, not as the error type itself.
The only solution I could think of right now, to get rid of these in the OpenAPI json is to define the type twice, but then we would not have the possibly additional header values. Not sure what the best approach would be right now.

Something else I see here is the

"required": [
   "UseDpopNonce"
 ],

which I am not sure about if this is correct. Depends on what you want to achieve. Each of these values is only an option because the type is an enum, so only one of them is required while all others cannot exist then.

andoks commented 2 weeks ago

This is not an or but an and, this is a tuple. It's coming from the ErrorResponseType::UseDpopNonce in src/error/src/error_impls.rs:98.

Indeed you are right, I am not that familiar with raw openapi YAML, so I initially thought it was a union-value, where the value could be either string or optional string.

don't know how to translate this into proper go code (last time using go was a long time ago for me), but the type is a tuple (Option, String) inside a rust enum.

AFAIK go does not have "real" support for tuples, but it can be emulated to a degree using a struct with anonymous field.

I am not sure what exactly you are trying to do...

I wanted to create a http client from the openapi spec to use during bootstrapping of rauthy.

The only solution I could think of right now, to get rid of these in the OpenAPI json is to define the type twice, but then we would not have the possibly additional header values. Not sure what the best approach would be right now.

Neither do I honestly, but at least I know I can't just fix the type as I was alluding to. I am probably going to look into how to configure the codegenerator to either handle it, or ignore generating code for the spec it has issues with for now, if it isn't something I need immediately.

Thanks for the help!