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

SupervisorManager: Add authorization checks to bulk endpoints. #8044

Merged
merged 1 commit into from
Jul 9, 2019

Conversation

gianm
Copy link
Contributor

@gianm gianm commented Jul 9, 2019

The endpoints added in #6272 were missing authorization checks. This patch removes the bulk
methods from SupervisorManager, and instead has SupervisorResource run the full list through
filterAuthorizedSupervisorIds before calling resume/suspend/terminate one by one.

Noticed this since they were caught by the watchdog PreResponseAuthorizationCheckFilter.

The endpoints added in apache#6272 were missing authorization checks. This patch removes the bulk
methods from SupervisorManager, and instead has SupervisorResource run the full list through
filterAuthorizedSupervisorIds before calling resume/suspend/terminate one by one.
@gianm gianm added this to the 0.16.0 milestone Jul 9, 2019
Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

@gianm
Copy link
Contributor Author

gianm commented Jul 9, 2019

The only CI step failing is "packaging check", which has been wonky since #7998. I spoke with @jihoonson offline and he said he's working to figure out what is going on. In the meantime, I'll merge this.

@gianm gianm merged commit 338b8b3 into apache:master Jul 9, 2019
@gianm gianm deleted the fix-supervisor-resource-authz-checks branch July 9, 2019 20:16
gianm added a commit to implydata/druid-public that referenced this pull request Jul 9, 2019
…e#8044)

The endpoints added in apache#6272 were missing authorization checks. This patch removes the bulk
methods from SupervisorManager, and instead has SupervisorResource run the full list through
filterAuthorizedSupervisorIds before calling resume/suspend/terminate one by one.
clintropolis pushed a commit that referenced this pull request Jul 24, 2019
The endpoints added in #6272 were missing authorization checks. This patch removes the bulk
methods from SupervisorManager, and instead has SupervisorResource run the full list through
filterAuthorizedSupervisorIds before calling resume/suspend/terminate one by one.
@clintropolis clintropolis modified the milestones: 0.16.0, 0.15.1 Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants