prevent privilege escalation

[deads2k]

Prevent a user who has rights to modify role bindings from binding a user to a role that has more power than the granting user has.

/cc @liggitt

[deads2k]

@smarterclayton comments up to "prevent escalation" addressed.

[deads2k]

@smarterclayton created a rolebinding.Registry to abstract out the ugly from rolebinding/rest.go. It felt more registry-like. If you agree with the approach, I'll fill in unit tests.

[deads2k]

Plumbed kapi.Context through the authorizer. I think it really hurts readability, especially in places where there is logically more than one context in play and in places where only part of the context is used. It ends up reading like a magic container.

@liggitt Your thoughts on the "plumb context through authorizer" commit?

[deads2k]

@smarterclayton I think I made it through all the comments. The only one I don't feel good about is plumbing context through the authorizer. It feels like I've gone from a way of passing arguments that allows the compiler to ensure that I'm passing the correct data where its required and I've replaced with with a way of passing through a data object that is mostly blackblox, that sometimes needs certain fields and sometimes needs others, and destroys the compiler's ability to help me write valid code without having runtime failures.

[deads2k]

[test]

[openshift-bot]

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1122/)

[smarterclayton]

If the authorizer is making nested calls it needs to be using the context of the person who called it. A context is the thread that ties an incoming request from the client across all the backend systems that fulfill it. See https://godoc.org/golang.org/x/net/context and http://research.google.com/pubs/pub36356.html for why we use contexts.

----- Original Message -----

@smarterclayton I think I made it through all the comments. The only one I
don't feel good about is plumbing context through the authorizer. It feels
like I've gone from a way of passing arguments that allows the compiler to
ensure that I'm passing the correct data where its required and I've
replaced with with a way of passing through a data object that is mostly
blackblox, that sometimes needs certain fields and sometimes needs others,
and destroys the compiler's ability to help me write valid code without
having runtime failures.

---

Reply to this email directly or view it on GitHub:
#1051 (comment)

[smarterclayton]

You can hide the context underneath your getters if necessary - it doesn't have to be a field on the object. It just has to be preserved.

----- Original Message -----

Plumbed kapi.Context through the authorizer. I think it really hurts
readability, especially in places where there is logically more than one
context in play and in places where only part of the context is used. It
ends up reading like a magic container.

@liggitt Your thoughts on the "plumb context through authorizer" commit?

---

Reply to this email directly or view it on GitHub:
#1051 (comment)

[deads2k]

jenkins failed in test-integration on TestSimpleImageChangeBuildTrigger. re[test]?

[smarterclayton]

Squash and you'll get a merge.

[deads2k]

squashed.

[smarterclayton]

Hope I don't regret this... LGTM [merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/987/) (Image: devenv-fedora_852)

[openshift-bot]

Evaluated for origin up to e72c121