-
Notifications
You must be signed in to change notification settings - Fork 57
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
Wrong Security swagger generation #76
Comments
@Leogiciel Hi, did you come up with a workaround for this issue by any chance? I'm curious to know how you deal with it, as I have come up against it myself. |
Yes @jackghicks, you can use our fork of typescript-rest package : https://github.com/nexussmart/typescript-rest This should do the trick : |
@Leogiciel thanks for sharing this. have you published yours to npm? for now, I am able to use @Security() without params for most of my application but this will be a problem later, and I'd like to keep my deployment as simple and unsurprising as possible :) |
@jackghicks, I've made the package public : https://www.npmjs.com/package/@dev-volpy/typescript-rest |
I've just realized the dichotomy behind this bug. I think the issue is really with the Swagger generator. It seems to me that in the Swagger spec, you need to specify the name of the securityDefinition, together with the scopes (only if oauth2, also it is misnamed variable 'roles' in typescript-rest's Security decorator). However, in the typescript-rest @Security implementation, the type of authentication is not taken into account during authorization at all, which is why it is missing. Perhaps the developer intended to allow flexibility here, by omitting the name and making the roles optional too, you can easily specify with @Security() that there must be some form (any valid) of authentication for the decorated routes, or that the scopes, where supplied, must be satisfied, but by any available authentication type. This would be a bit odd, since only oauth2 would allow scopes anyway. If this is the case, then the typescript-rest-swagger generator should respect the securityDefinition defined in swagger.config.json, and generate all the possibilities where the scopes or name has not been supplied. Alternatively, the name parameter should become required, as in your patch, and perhaps passed through so that it can be used in authorization if required. All this said, it does seem reasonable to simply leave the name parameter as an unused variable like you have done here. At least temporarily. |
In the end I opened a pull request for typescript-rest-swagger, to retroactively add support for the typescript-rest style of @Security decorator to typescript-rest-swagger. In this way, you can continue to use typescript-rest on the main repo, but switch your typescript-rest-swagger dependency to use the patched version. I published it here: Pull request is here: #78 |
Can you give an example on how to use this patched version with I'm not sure how to use your version @jackghicks ? Do you have an example? Is this compatible with |
Security decorator needs two params to be generated correctfully.
eg :
@Security("bearer", ["admin","operator"])
In typescript-rest, you only implemented the second parameters (roles), so Swagger can't recognize it and generate swagger.yaml file properly :
security: - '[object Object]': []
instead of
security: - 'bearer': ["admin", "operator"]
The text was updated successfully, but these errors were encountered: