-
Notifications
You must be signed in to change notification settings - Fork 2
Feature | Extend Swagger Coverage for controller OAuth2TagsApiController
#400
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
base: main
Are you sure you want to change the base?
Conversation
c265146 to
c35ff80
Compare
b81a87d to
a1f1c38
Compare
caseylocker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matiasperrone-exo The security schema needs to be updated/created with the correct scopes.
Also, your missing Security on GET /api/v1/tags/{id}
This endpoint has NO security parameter, but the seeder shows it requires authentication:
operationIds needed
| Endpoint | Suggested operationId |
|---|---|
| GET /api/v1/tags | getAllTags |
| GET /api/v1/tags/{id} | getTag |
| POST /api/v1/tags | createTag |
| PUT /api/v1/tags/{id} | updateTag |
| DELETE /api/v1/tags/{id} | deleteTag |
Missing x: ['required-groups'] for Write Operations
POST, PUT, DELETE endpoints require authorization groups per the seeder:
x: [
'required-groups' => [
IGroup::SuperAdmins,
IGroup::Administrators,
IGroup::SummitAdministrators,
]
],
f713895 to
a1c41ab
Compare
caseylocker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong x: Parameter Key
All 3 write endpoints use 'authz_groups' instead of the project standard 'required-groups'. Please change these to 'required-groups'
Affected locations: createTag, updateTag, deleteTag
47724c0 to
0ab93f0
Compare
|
Thanks @caseylocker for the comments. Now is ready to review again |
0ab93f0 to
391fe7f
Compare
caseylocker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
|
@matiasperrone please rebase against main and review conflicts |
1 similar comment
|
@matiasperrone please rebase against main and review conflicts |
c6ecdd0 to
728ae67
Compare
391fe7f to
b755506
Compare
Signed-off-by: Matias Perrone <github@matiasperrone.com>
|
@smarcet the branch is now rebased and conflicts free. |
Task:
Ref: https://app.clickup.com/t/86b6wkh6w