Closed sigmanil closed 4 years ago
You need to register the module in the route as it is created. You are starting to use the less elegant parts of the project, sorry about that.
just replace the route.provider.child()
with route.provider.child().also { it.registerModule(this) }
Essentially route modules are there to handle the metadata and the behavior, but due to typing authentication is an exception to that (modules cannot enforce types at compile time) and requires both the module and the special route class to preserve the authentication type.
Thanks for the quick answer! I'm assuming you mean com.papsign.ktor.openapigen.modules.registerModule
for the method to be called in that "also{...}"? (Because the registerModule available on the CachingModuleProvider object available there also requires a KType parameter.)
It compiles and runs, but doesn't seem to make any difference to the output. 🤔
I tried adding this.provider.child().also { it.registerModule(authProvider) }
to the NormalOpenAPIRoute.auth()-function as well, but that didn't have any effect either.
Odd, try and see if the authProvider is in the module provider, and what type it is associated to. What version do you use ? One possiblity is that a latest version does not query erased types properly, due to the change to KType.
We're currently on version 0.2-beta.9, we started using this project after the KType-rewrite. I'll try downgrading to before that change to see if that helps, and try to (understand and) find out what you asked about with regards to the module provider.
I can confirm that securitySchemes (that look correct) and security tags (these are empty though 🤔 ) are generated with 0.2-beta.7 (last one before the KType-change as far as I can tell) - whereas they are NOT generated with 0.2-beta.9.
Which in my head means it's likely that this is a bug introduced with the KType rewrite - would you agree? How do we proceed in that case? Would it be helpful if I submitted a (failing) test for this?
Hello, I actually wrote that change to KType and must have missed something. I will take a look at it shortly (as I noticed this problem myself with our my project). A failing test would be helpful.
I'll see what I can do about the test as soon as I'm able then! Thanks for looking into it!
Actually I'm mistaken, my latest KType changes have not been released yet - but I will take a look at it regardless as I have the same problem.
@davidhiendl I opened a PR with a test that demonstrates this issue, as you can see above.
@davidhiendl Your changes are not to be blamed.
I know exactly where the bug is:
Currently the Module provider stores a hash map with Type -> associated modules. When it was KClass based erased types could be used as all subtypes were registered as erasures. Now With KType the subtypes are no longer registering as AuthProvider<*>
but as AuthProvider<Principal>
, and thus there is no entry when the module provider is queried for AuthProvider<*>
.
The registration should stop registering the subtypes, and get all matching keys and Aggregate the result. Problem is the module provider guarantees registration order, which means aggregation is out of the question, we can only use an ArrayList and filter for subtypes upon query, increasing time complexity.
The alternative is to store all possible erasure combinations for subtypes, but that would create an exponential memory use for types having a generic-heavy hierarchy, which is far worse than the overhead of detecting a module on O(n) complexity once during initialization. There is one exception to that, the respond function queries a module during every response to get the appropriate response code. It could be cached, but there is currently no mechanism to do so.
Does registration order even matter for distinct principles? Perhaps a Map<Principal, List<AuthProvider<Principal>>
can be used to mitigate the impact.
This is a bug in the fundamental module system, not specific to the AuthProvider, it just happened to be the one that was affected. There being less than 20 modules per route generally speaking, the ArrayList solution seems to be the appropriate one.
The bug is fixed.
@Wicpar Thank you so much for the fix :-) Would it be possible for you to release a version with it?
Oh, right !
Done
Thank you! :-)
We've been able to implement JWT-authenticated endpoints through the Ktor-OpenAPI-Generator API, and authentication works - but the generated OpenAPI.json doesn't contain any information about the security scheme. It should, right?
We have written the following code to tell Ktor-OpenAPI-Generator about the authentication:
In our Application.apirouting, we wrap the required paths with auth{}, and make sure we use the routing functions from com.papsign.ktor.openapigen.route.path.auth.* in those cases. We are able to access the authenticated user through the principal()-function. An example:
Everything seems to work fine for actual use - but as mentioned, we're not finding any information about the security scheme in the generated openapi.json. :-(
We are calling
addModules(authProvider)
as part of the installation of OpenAPIGen, but we're under the impression this isn't working as intended. (We are supposed to add it as a module, right?) Here, we're ending up in the object com.papsign.ktor.openapigen.modules.handlers.AuthHandler:Here, in the configure method, provider seems to contain our authProvider object, but it isn't returned from the "provider.ofType<AuthProvider<*>>()" call. But at this point we're a bit uncertain if we're barking up the wrong tree, and if we're not, why our AuthProvider isn't recognized as one by ofType...
We're not sure if this is a bug in OpenAPIGen or just something we haven't understood about how to use the framework. Any help would be greatly appreciated.