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

Invalid method for determining the user's permissions #72

Closed
enj opened this issue Apr 26, 2017 · 7 comments · Fixed by #73
Closed

Invalid method for determining the user's permissions #72

enj opened this issue Apr 26, 2017 · 7 comments · Fixed by #73
Assignees

Comments

@enj
Copy link

enj commented Apr 26, 2017

The isOperationsUser method incorrectly assumes that a user must be directly bound to cluster-admins or cluster-readers to see the logs. This is incorrect in two scenarios:

  1. The user has rights due to a different binding
  2. The user belongs to a group that grants the rights

Instead, the method should perform a SubjectAccessReview based on the username and group information (you will probably need to build a reverse index of groups). The following SAR is equivalent to cluster admin:

{
	"kind" : "SubjectAccessReview",
	"apiVersion" : "v1",
	"user" : "<username>",
	"groups" : ["<groups>"],
	"verb" : "*",
	"resource" : "*",
}

The following SAR could serve as a proxy for cluster-reader but that is up to you guys:

{
	"kind" : "SubjectAccessReview",
	"apiVersion" : "v1",
	"user" : "<username>",
	"groups" : ["<groups>"],
	"verb" : "list",
	"resource" : "pods/log",
}

cc @ewolinetz @jcantrill @richm

@gabemontero may be able to provide guidance on how to perform a SAR in Java.

@enj
Copy link
Author

enj commented Apr 26, 2017

cc @deads2k @liggitt @smarterclayton in case you guys have any opinions.

@ewolinetz
Copy link
Contributor

This is only specific to a user seeing the logs in the .operations index. Any normal application logs is allowed based on the user having access to a namespace. E.g. if oc get projects would return something (so long as it isn't rolled into the .operations index) they would have access to it.

@enj
Copy link
Author

enj commented Apr 26, 2017

This is only specific to a user seeing the logs in the .operations index

@ewolinetz We have multiple instances of customers who are trying to see those specific logs but are failing to do so because they belong to a group that grants them cluster admin privileges instead of being directly bound to the role.

if oc get projects would return something

That seems like a very broad check. I am not familiar with the contents of the logs so I cannot say who should be able to see them. A SubjectRulesReview may be more appropriate.

@niconosenzo
Copy link

I just opened BZ 1446217 for this.

@jcantrill
Copy link
Collaborator

@ewolinetz Maybe we should do SAR for viewing on all 'operations' namespaces and determine ops user on being able to view them all. I think we have a list in the plugin that is configurable.

@deads2k
Copy link

deads2k commented Apr 27, 2017

@ewolinetz Maybe we should do SAR for viewing on all 'operations' namespaces and determine ops user on being able to view them all. I think we have a list in the plugin that is configurable.

If this is about viewing logs, it may be more appropriate to look for the power to get pods/logs across all namespaces or the one you're interested in.

@liggitt
Copy link

liggitt commented Apr 27, 2017

I think the second SAR is almost right, but should be checking a get verb

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 a pull request may close this issue.

6 participants