goauthentik / authentik

The authentication glue you need.
https://goauthentik.io
Other
13.47k stars 899 forks source link

API: mixing additionalProperties with properties #3901

Open commonism opened 2 years ago

commonism commented 2 years ago

Describe your question/

The OpenAPI description document specifies schemas which mix named properties and additionalProperties:{}. While this is valid, it severely impacts the choice/quality of clients being able to interface the API.

Relevant infos I author/use aiopenapi3, a python3 OpenAPI client/validator. aiopenapi3 is making use of pydantic to take care of the Schema things, even additionalProperties: True -the json schema default- is a problem (to pydantic or any other Validator).

Additionally the named property of a schema (PromptChallengeResponseRequest) with additionalProperties is used as a propertyName in a discriminated union (FlowChallengeResponseRequest).

Removing the additionalProperties from ValidationError & PromptChallengeResponseRequest allows me to parse the description document.

import aiopenapi3.plugin
from aiopenapi3 import OpenAPI

class AuthentikDocument(aiopenapi3.plugin.Document):
    def parsed(self, ctx):
        for c in ["ValidationError","PromptChallengeResponseRequest"]:
            if "additionalProperties" in ctx.document["components"]["schemas"][c]:
                del ctx.document["components"]["schemas"][c]["additionalProperties"]
        return ctx

def test_authentik_HEAD():
    api = OpenAPI.load_sync("https://raw.githubusercontent.com/goauthentik/authentik/main/schema.yml",
                            plugins=[AuthentikDocument()])

I do not know any better than pydantic, I have to use it. Therefore I'd be glad if you could have a look in getting the description document to use the subset of OpenAPI/json schema which allows using it with pydantic/aiopenapi3.

And I propose to test the OpenAPI description document with a bunch of clients/validators/generators.

BeryJu commented 6 months ago

not really a solution for this, but we have our own python API client which is also generated from the same API schema: https://pypi.org/project/authentik-client/ (this client also uses pydantic for schema validation)

commonism commented 6 months ago

pydanticv2 made this work.

I tried loading the current version of the schema using aiopenapi3 and now we disagree on valid semantics for OpenAPI 3.0 description documents.

aiopenapi3 validate https://raw.githubusercontent.com/goauthentik/authentik/main/schema.yml

pydantic_core._pydantic_core.ValidationError: 2 validation errors for Root
components.securitySchemes.authentik.SecurityScheme.apiKey.scheme
Extra inputs are not permitted [type=extra_forbidden, input_value='bearer', input_type=str]
For further information visit https://errors.pydantic.dev/2.6/v/extra_forbidden
components.securitySchemes.authentik.Reference.$ref
Field required [type=missing, input_value={'type': 'apiKey', 'in': ...on', 'scheme': 'bearer'}, input_type=dict]
For further information visit https://errors.pydantic.dev/2.6/v/missing

section is

  securitySchemes:
    authentik:
      type: apiKey
      in: header
      name: Authorization
      scheme: bearer

Specification I think I'm correct on this one, scheme is not a field for type apiKey.

Fixing this it'll warn for

DiscriminatorWarning: Discriminated Union member #/components/schemas/UserLoginChallengeResponseRequest without const/enum key property component

and finally quit with

NotImplementedError: primitive type string allOf is not implemented

This would be my duty, supporting allOf for primitive types.

BeryJu commented 6 months ago

@commonism with the PR above merged this shouldn't be an issue anymore

commonism commented 6 months ago

I've added the required bits to aiopenapi3 to deal with allOf applicators for primitive types to HEAD.

Due to our combined efforts you could use aiopenapi3 for unit testing the authentik api, interested?

Current setup time is ~15s, but may be easier to work with than codegen api clients.

time aiopenapi3 --verbose validate https://raw.githubusercontent.com/goauthentik/authentik/main/schema.yml
…  0:00:13.185635 (processing time)
… 742 #operations
… 742 #operations (with operationId)
… 0 https://raw.githubusercontent.com/goauthentik/authentik/main/schema.yml: 558
… 558 schemas total
OK

real    0m14,776s
user    0m14,329s
sys 0m0,272s

Currently these warnings remain:

DiscriminatorWarning: Discriminated Union member #/components/schemas/UserLoginChallengeResponseRequest without const/enum key property component

Setting const (OpenAPI 3.1) or enum (3.0) the discriminator key value on the discriminated union members improves usability of models, as it prohibits creating models with mismatching key value.