Skip to content
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

Improve security decorator #78

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

jackghicks
Copy link

Currently, the @Security decorator as defined in typescript-rest does not match the decorate as defined in typescript-rest-swagger.

This pull request adds support for the @Security decorator as defined in typescript-rest, without breaking support for the decorator as it stands, to maximize backwards compatibility.

Where no name is specified for the @Security decorator (in typescript-rest, this equates to roles(scopes) only) the contents of the swagger.config.json will be used to determine all appropriate matching securityDefinition entries, based on the scopes required.

Where the name is specified, the behavior is as it was before, albeit with some error checking now in place to ensure that the defined security rule matches something existing in the securityDefinitions. This could be construed as a breaking change, however for any users affected by this, their resulting swagger.yaml/json would have been invalid regardless.

@jackghicks
Copy link
Author

This pull request has come as a result of the issue outlined originally by this issue: https://github.com/thiagobustamante/typescript-rest-swagger/issues/76

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.5%) to 76.287% when pulling 1ebe71d on jackghicks:improve-security-decorator into 478aeb3 on thiagobustamante:master.

@tnrich
Copy link
Collaborator

tnrich commented Feb 21, 2020

@jackghicks if you want to fix the merge conflicts I'll take a look at this PR

@joaosousafranco
Copy link

Anyone taking ownership on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants