implementing endpoint for approval submission#2812
implementing endpoint for approval submission#2812openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
| """Submit user decision for a pending tool approval request.""" | ||
| del auth # Auth dependency enforces request authentication. | ||
|
|
||
| result = set_approval_decision(request.approval_id, request.approved) |
There was a problem hiding this comment.
Only the same user should be able to approve its own pending approvals.
We should also use user_id here and raise if a different user is trying to approve pending approvals from someone else.
There was a problem hiding this comment.
Not really, approval ID is unique and is reported back to the user as a response. A different user just will not know the ID
There was a problem hiding this comment.
So it is "security by obscurity".
We have all the information, it's just about adding user_id as an extra key. Do you have any particular reason why you don't want to do it?
There was a problem hiding this comment.
This requires changes in approval implementation. Currently, it does not have a user ID. We can do this. but it will make the overall implementation heavier. Right now, the approval event does not have a user ID. I would like to at least commit what exists now and then do another PR to add the user ID into this. I am open to having an additional PR if we need this, but at the moment I want to have what I am doing for the last month completed.
There was a problem hiding this comment.
I really prefer to complete the initial implementation first
blublinsky
force-pushed
the
approval-submission
branch
from
4a7b8b3 to
6cb4b7f
Compare
|
LGTM - @blublinsky - Ready to merge once the test pass. |
blublinsky
force-pushed
the
approval-submission
branch
from
6cb4b7f to
22e22d0
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xrajesh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
@blublinsky: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Description
Type of change
Related Tickets & Documents
OLS-2659
OLS-2659
Checklist before requesting a review
Testing