fabric8io/openshift-elasticsearch-plugin

Invalid method for determining the user's permissions

enj opened this issue · 7 comments

enj commented

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 commented

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

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 commented

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.

I just opened BZ 1446217 for this.

@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.

@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.

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