Use the Upstream header for scope impersonation

Note: the upstream function does not forcibliy remove potential existing impersonation headers, and it also uses the existing ones ignoring any new headers we may be setting. This i ok, because we do not depend nor probably want to allow multiple levels of impersonation. Signed-off-by: Simo Sorce <[email protected]>
openshift · Feb 1, 2018 · 23d319a · 23d319a
1 parent 3dcad95
commit 23d319a
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 39 deletions.
diff --git a/hack/import-restrictions.json b/hack/import-restrictions.json
@@ -367,7 +367,6 @@
     "allowedImportPackages": [
       "vendor/github.com/golang/glog",
       "vendor/k8s.io/kubernetes/pkg/api/legacyscheme",
-      "github.com/openshift/origin/pkg/authorization/apis/authorization",
       "github.com/openshift/origin/pkg/cmd/server/api",
       "github.com/openshift/origin/pkg/util/http/links",
       "github.com/openshift/origin/pkg/authorization/authorizer/scope",

diff --git a/pkg/cmd/server/origin/handlers.go b/pkg/cmd/server/origin/handlers.go
@@ -24,7 +24,6 @@ import (
 
 	authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
 	configapi "github.com/openshift/origin/pkg/cmd/server/api"
-	authenticationapi "github.com/openshift/origin/pkg/oauthserver/api"
 )
 
 // cacheExcludedPaths is small and simple until the handlers include the cache headers they need
@@ -155,10 +154,15 @@ func (c *MasterConfig) versionSkewFilter(handler http.Handler, contextMapper api
 	})
 }
 
+// legacyImpersonateUserScopeHeader is the header name older servers were using
+// just for scopes, so we need to translate it from clients that may still be
+// using it.
+const legacyImpersonateUserScopeHeader = "Impersonate-User-Scope"
+
 // translateLegacyScopeImpersonation is a filter that will translates user scope impersonation for openshift into the equivalent kube headers.
 func translateLegacyScopeImpersonation(handler http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		for _, scope := range req.Header[authenticationapi.ImpersonateUserScopeHeader] {
+		for _, scope := range req.Header[legacyImpersonateUserScopeHeader] {
 			req.Header[authenticationv1.ImpersonateUserExtraHeaderPrefix+authorizationapi.ScopesKey] =
 				append(req.Header[authenticationv1.ImpersonateUserExtraHeaderPrefix+authorizationapi.ScopesKey], scope)
 		}

diff --git a/pkg/oauthserver/client/impersonate.go b/pkg/oauthserver/client/impersonate.go
@@ -5,47 +5,23 @@ import (
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
-	utilnet "k8s.io/apimachinery/pkg/util/net"
 	"k8s.io/apiserver/pkg/authentication/user"
 	restclient "k8s.io/client-go/rest"
+	"k8s.io/client-go/transport"
 	"k8s.io/client-go/util/flowcontrol"
 	kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
-
-	authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
-	authenticationapi "github.com/openshift/origin/pkg/oauthserver/api"
 )
 
-type impersonatingRoundTripper struct {
-	user     user.Info
-	delegate http.RoundTripper
-}
-
-// newImpersonatingRoundTripper will add headers to impersonate a user, including user, groups, and scopes
-func newImpersonatingRoundTripper(user user.Info, delegate http.RoundTripper) http.RoundTripper {
-	return &impersonatingRoundTripper{user: user, delegate: delegate}
-}
-
-func (rt *impersonatingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
-	req = utilnet.CloneRequest(req)
-	req.Header.Del(authenticationapi.ImpersonateUserHeader)
-	req.Header.Del(authenticationapi.ImpersonateGroupHeader)
-	req.Header.Del(authenticationapi.ImpersonateUserScopeHeader)
-
-	req.Header.Set(authenticationapi.ImpersonateUserHeader, rt.user.GetName())
-	for _, group := range rt.user.GetGroups() {
-		req.Header.Add(authenticationapi.ImpersonateGroupHeader, group)
-	}
-	for _, scope := range rt.user.GetExtra()[authorizationapi.ScopesKey] {
-		req.Header.Add(authenticationapi.ImpersonateUserScopeHeader, scope)
-	}
-	return rt.delegate.RoundTrip(req)
-}
-
 // NewImpersonatingConfig wraps the config's transport to impersonate a user, including user, groups, and scopes
 func NewImpersonatingConfig(user user.Info, config restclient.Config) restclient.Config {
 	oldWrapTransport := config.WrapTransport
 	config.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
-		return newImpersonatingRoundTripper(user, oldWrapTransport(rt))
+		newConfig := transport.ImpersonationConfig{
+			UserName: user.GetName(),
+			Groups:   user.GetGroups(),
+			Extra:    user.GetExtra(),
+		}
+		return transport.NewImpersonatingRoundTripper(newConfig, oldWrapTransport(rt))
 	}
 	return config
 }
@@ -68,9 +44,11 @@ func NewImpersonatingRESTClient(user user.Info, client restclient.Interface) res
 
 // Verb does the impersonation per request by setting the proper headers
 func (c impersonatingRESTClient) impersonate(req *restclient.Request) *restclient.Request {
-	req.SetHeader(authenticationapi.ImpersonateUserHeader, c.user.GetName())
-	req.SetHeader(authenticationapi.ImpersonateGroupHeader, c.user.GetGroups()...)
-	req.SetHeader(authenticationapi.ImpersonateUserScopeHeader, c.user.GetExtra()[authorizationapi.ScopesKey]...)
+	req.SetHeader(transport.ImpersonateUserHeader, c.user.GetName())
+	req.SetHeader(transport.ImpersonateGroupHeader, c.user.GetGroups()...)
+	for k, vv := range c.user.GetExtra() {
+		req.SetHeader(transport.ImpersonateUserExtraHeaderPrefix+k, vv...)
+	}
 	return req
 }
 

diff --git a/test/integration/scopes_test.go b/test/integration/scopes_test.go
@@ -3,12 +3,14 @@ package integration
 import (
 	"testing"
 
+	authenticationv1 "k8s.io/api/authentication/v1"
 	kapierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apiserverserviceaccount "k8s.io/apiserver/pkg/authentication/serviceaccount"
 	"k8s.io/client-go/rest"
 	kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 
+	authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
 	"github.com/openshift/origin/pkg/authorization/authorizer/scope"
 	buildapi "github.com/openshift/origin/pkg/build/apis/build"
 	buildclient "github.com/openshift/origin/pkg/build/generated/internalclientset"
@@ -106,7 +108,7 @@ func TestScopedImpersonation(t *testing.T) {
 
 	err = clusterAdminBuildClient.Build().RESTClient().Get().
 		SetHeader(authenticationapi.ImpersonateUserHeader, "harold").
-		SetHeader(authenticationapi.ImpersonateUserScopeHeader, "user:info").
+		SetHeader(authenticationv1.ImpersonateUserExtraHeaderPrefix+authorizationapi.ScopesKey, "user:info").
 		Namespace(projectName).Resource("builds").Name("name").Do().Into(&buildapi.Build{})
 	if !kapierrors.IsForbidden(err) {
 		t.Fatalf("unexpected error: %v", err)
@@ -115,7 +117,7 @@ func TestScopedImpersonation(t *testing.T) {
 	user := &userapi.User{}
 	err = userclient.NewForConfigOrDie(clusterAdminClientConfig).RESTClient().Get().
 		SetHeader(authenticationapi.ImpersonateUserHeader, "harold").
-		SetHeader(authenticationapi.ImpersonateUserScopeHeader, "user:info").
+		SetHeader(authenticationv1.ImpersonateUserExtraHeaderPrefix+authorizationapi.ScopesKey, "user:info").
 		Resource("users").Name("~").Do().Into(user)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)