Limit scope of the machine token signed requests#11215
Conversation
|
ci-test |
|
ci-test build report: |
|
ci-test |
|
Results of automated E2E tests of Eclipse Che Multiuser on OCP: |
skabashnyuk
left a comment
There was a problem hiding this comment.
ok for me. please wait for @sleshchenko feedback.
| "/workspace", asList("getByKey", "addProject", "updateProject", "deleteProject")); | ||
| allowedMethods.putAll("/ssh", asList("getPair", "generatePair")); | ||
| allowedMethods.put("/preferences", "find"); | ||
| allowedMethods.put("/activity", "active"); |
There was a problem hiding this comment.
There one more service which should be accessible with machine token - it's Factory Service. #10710 (comment)
Are there any plans to make it accessible?
There was a problem hiding this comment.
getFactoryJson is already covered by permissions, so it will be restricted with MachineAuthenticatedSubject, nothing to do there. For the rest of methods, we can add getFactory, getFactoryByAttribute and resolveFactory here.
There was a problem hiding this comment.
getFactoryJson is already covered by permissions
It's true. But it won't be allowed since it is not listed in allowed methods here. Am I wrong?
There was a problem hiding this comment.
Oh, yes, you're correct
| @Path("/{path:.*}") | ||
| public class MachineTokenAccessFilter extends CheMethodInvokerFilter { | ||
|
|
||
| private final SetMultimap<String, String> allowedMethods = HashMultimap.create(); |
There was a problem hiding this comment.
It makes sense to change variable name to make clear what is key and value, like parentPathToAllowedMethods. WDYT?
There was a problem hiding this comment.
allowedMethodsByPath ?
What does this PR do?
What issues does this PR fix or reference?
#10710
Release Notes
Docs PR