rh-che traefik#532: Adding extra workarounds for identity_id processi…

…ng (preserve order of query params / supporting & in path) (fabric8-services#25) Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
nurali-techie · Jun 26, 2018 · bd536a1 · bd536a1
1 parent df8e2d1
commit bd536a1
Show file tree

Hide file tree

Showing 2 changed files with 135 additions and 14 deletions.
diff --git a/middlewares/osio/auth.go b/middlewares/osio/auth.go
@@ -266,27 +266,44 @@ func removeUserID(req *http.Request) {
 	if req.Header.Get(UserIDHeader) != "" {
 		req.Header.Del(UserIDHeader)
 	}
-	if req.URL.Query().Get(UserIDParam) != "" {
-		userID := req.URL.Query().Get(UserIDParam)
+	userID := req.URL.Query().Get(UserIDParam)
+	if userID != "" {
+		// case when userID query parameter contains path e.g.
 		if strings.Contains(userID, "/") {
-			q := req.URL.Query()
-			q.Del(UserIDParam)
 
+			// Processing query params
+			rawQuery := req.URL.RawQuery;
+			extraPath := ""
+			if strings.Contains(rawQuery, "?") {
+				queryIndex := strings.LastIndex(rawQuery, "?")
+				rawQuery = rawQuery[queryIndex+1:]
+			} else if strings.Contains(rawQuery, "&") {
+				ampersandIndex := strings.Index(rawQuery, "&")
+				extraPath = rawQuery[ampersandIndex:]
+				rawQuery = ""
+			} else {
+				rawQuery = ""
+			}
+			req.URL.RawQuery = rawQuery
+
+
+			// Removing query params from userID
 			if strings.Contains(userID, "?") {
 				indexOfQuery := strings.Index(userID, "?")
+				userID = userID[:indexOfQuery]
+			}
 
-				// adding missing query parameters to request URL query
-				missingQueryParam := userID[indexOfQuery+1:]
-				keyValue := strings.Split(missingQueryParam, "=")
-				q.Add(keyValue[0], keyValue[1])
+			// obtaining path
+			indexOfFirstSlash := strings.Index(userID, "/")
+			path := userID[indexOfFirstSlash:]
 
-				// removing query params from userID
-				userID = userID[:indexOfQuery]
+			if extraPath != "" {
+				path += extraPath
 			}
 
-			req.URL.RawQuery = q.Encode()
-			startInd := strings.Index(userID, "/")
-			req.URL.Path = userID[startInd:]
+			req.URL.Path = path
+
+			// setting requestURI
 			req.RequestURI = req.URL.RequestURI()
 		} else {
 			q := req.URL.Query()

diff --git a/middlewares/osio/auth_test.go b/middlewares/osio/auth_test.go
@@ -59,12 +59,19 @@ func TestExtractUserID(t *testing.T) {
 		assert.Equal(t, expectedUserID, userID)
 	})
 
-	t.Run("UserID as part of ugly url produced by kubernetes client for 'exec' calls", func(t *testing.T) {
+	t.Run("UserID as part of ugly url parameter produced by kubernetes client for 'exec' calls", func(t *testing.T) {
 		adhocURL:= fmt.Sprintf("http://f8osoproxy.com?%s=%s/some/path/to/pod/exec?command=date&tty=true&stdin=true&stdout=true&stderr=true", UserIDParam, expectedUserID)
 		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
 		userID := extractUserID(req)
 		assert.Equal(t, expectedUserID, userID)
 	})
+
+	t.Run("UserID as part of ugly url segment produced by kubernetes client for 'exec' calls", func(t *testing.T) {
+		adhocURL:= fmt.Sprintf("http://f8osoproxy.com/?%s=%s/some/path/to/pod/exec?command=date&tty=true&stdin=true&stdout=true&stderr=true", UserIDParam, expectedUserID)
+		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
+		userID := extractUserID(req)
+		assert.Equal(t, expectedUserID, userID)
+	})
 }
 
 func TestRemoveUserID(t *testing.T) {
@@ -76,6 +83,7 @@ func TestRemoveUserID(t *testing.T) {
 		removeUserID(req)
 		actualUserID := req.Header.Get(UserIDHeader)
 		assert.Empty(t, actualUserID)
+		assert.Equal(t, "http://f8osoproxy.com", req.URL.String())
 	})
 
 	t.Run("UserID as query param", func(t *testing.T) {
@@ -84,6 +92,7 @@ func TestRemoveUserID(t *testing.T) {
 		removeUserID(req)
 		actualUserID := req.URL.Query().Get(UserIDParam)
 		assert.Empty(t, actualUserID)
+		assert.Equal(t, "http://f8osoproxy.com/some/path", req.URL.String())
 	})
 
 	t.Run("UserID as part of path", func(t *testing.T) {
@@ -95,6 +104,7 @@ func TestRemoveUserID(t *testing.T) {
 		assert.Empty(t, actualUserID)
 		assert.Equal(t, path, req.URL.Path)
 		assert.Equal(t, path, req.RequestURI)
+		assert.Equal(t, "http://f8osoproxy.com/some/path", req.URL.String())
 	})
 
 	t.Run("UserID as part of path with query parameters", func(t *testing.T) {
@@ -111,6 +121,7 @@ func TestRemoveUserID(t *testing.T) {
 
 		assert.Equal(t, "key=value", req.URL.RawQuery)
 		assert.Equal(t, pathWithQueryParam, req.RequestURI)
+		assert.Equal(t, "http://f8osoproxy.com/some/path?key=value", req.URL.String())
 	})
 
 	t.Run("UserID as part of ugly url produced by kubernetes client for 'exec' calls with single query parameter", func(t *testing.T) {
@@ -125,6 +136,7 @@ func TestRemoveUserID(t *testing.T) {
 		commandQueryParam := req.URL.Query().Get("command")
 		assert.Equal(t, "date", commandQueryParam)
 		assert.Equal(t, pathWithParam, req.RequestURI)
+		assert.Equal(t, "http://f8osoproxy.com/some/path/to/pod/exec?command=date", req.URL.String())
 	})
 
 	t.Run("UserID as part of ugly url produced by kubernetes client for 'exec' calls with multiple query parameters", func(t *testing.T) {
@@ -150,5 +162,97 @@ func TestRemoveUserID(t *testing.T) {
 
 		stderr := req.URL.Query().Get("stderr")
 		assert.Equal(t, "false", stderr)
+
+		assert.Equal(t, pathWithParams, req.RequestURI)
+		assert.Equal(t, "http://f8osoproxy.com/some/path/to/pod/exec?command=date&tty=true&stdin=true&stdout=true&stderr=false", req.URL.String())
+	})
+
+	t.Run("UserID as part of url produced by rh-che via kubernetes client for 'event' calls", func(t *testing.T) {
+		pathWithParams := "/api/v1/anamespaces/namespace-che/events\u0026watch=true"
+		adhocURL := fmt.Sprintf("http://f8osoproxy.com?%s=%s%s", UserIDParam, userID, pathWithParams)
+		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
+		removeUserID(req)
+
+		actualUserID := req.URL.Query().Get(UserIDParam)
+		assert.Empty(t, actualUserID)
+
+		assert.Equal(t, pathWithParams, req.RequestURI)
+		assert.Equal(t, "http://f8osoproxy.com/api/v1/anamespaces/namespace-che/events\u0026watch=true", req.URL.String())
+	})
+
+	t.Run("UserID as part of url produced by rh-che via kubernetes client for 'exec' calls", func(t *testing.T) {
+		pathWithParams := "/api/v1/namespaces/osio-ci-ee1-preview-che/pods/workspacetest.dockerimage/exec?command=mkdir\u0026command=-p\u0026command=%2Ftmp%2Fbootstrapper%2F\u0026command=%2Fworkspace_logs%2Fbootstrapper\u0026container=container\u0026stderr=true"
+		adhocURL := fmt.Sprintf("http://f8osoproxy.com?%s=%s%s", UserIDParam, userID, pathWithParams)
+		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
+		removeUserID(req)
+
+		actualUserID := req.URL.Query().Get(UserIDParam)
+		assert.Empty(t, actualUserID)
+
+		assert.Equal(t, pathWithParams, req.RequestURI)
+		assert.Equal(t, req.URL.RawQuery, "command=mkdir\u0026command=-p\u0026command=%2Ftmp%2Fbootstrapper%2F\u0026command=%2Fworkspace_logs%2Fbootstrapper\u0026container=container\u0026stderr=true")
+		assert.Equal(t, "http://f8osoproxy.com/api/v1/namespaces/osio-ci-ee1-preview-che/pods/workspacetest.dockerimage/exec?command=mkdir\u0026command=-p\u0026command=%2Ftmp%2Fbootstrapper%2F\u0026command=%2Fworkspace_logs%2Fbootstrapper\u0026container=container\u0026stderr=true", req.URL.String())
+	})
+
+	t.Run("UserID as part of url produced by rh-che via kubernetes client for pod 'watch' calls", func(t *testing.T) {
+		pathWithParams := "/api/v1/namespaces/osio-ci-ee1-preview-che/pods\u0026fieldSelector=metadata.name%3Drm-workspace41v9261pdzqs84c4\u0026watch=true"
+		adhocURL := fmt.Sprintf("http://f8osoproxy.com/?%s=%s%s", UserIDParam, userID, pathWithParams)
+		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
+		removeUserID(req)
+
+		actualUserID := req.URL.Query().Get(UserIDParam)
+		assert.Empty(t, actualUserID)
+
+		assert.Equal(t, pathWithParams, req.URL.Path)
+	})
+
+	t.Run("UserID as part of url produced by rh-che via kubernetes client for pod 'watch' calls", func(t *testing.T) {
+		pathWithParams := "/api/v1/namespaces/osio-ci-ee1-preview-che/pods\u0026fieldSelector=metadata.name%3Dworkspacertz5iv86ez29v6bp.dockerimage\u0026watch=true"
+		adhocURL := fmt.Sprintf("http://f8osoproxy.com?%s=%s%s", UserIDParam, userID, pathWithParams)
+		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
+		removeUserID(req)
+
+		actualUserID := req.URL.Query().Get(UserIDParam)
+		assert.Empty(t, actualUserID)
+	})
+
+	t.Run("UserID as part of url produced by rh-che via kubernetes client for pod removal calls", func(t *testing.T) {
+		pathWithParams := "/api/v1/namespaces/osio-ci-ee1-preview-che/pods\u0026fieldSelector=metadata.name%3Drm-workspace41v9261pdzqs84c4\u0026watch=true"
+		adhocURL := fmt.Sprintf("http://f8osoproxy.com?%s=%s%s", UserIDParam, userID, pathWithParams)
+		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
+		removeUserID(req)
+
+		actualUserID := req.URL.Query().Get(UserIDParam)
+		assert.Empty(t, actualUserID)
+	})
+
+	t.Run("UserID as part of url produced by rh-che via kubernetes client for routes calls", func(t *testing.T) {
+		pathWithParams := "/apis/route.openshift.io/v1/namespaces/osio-ci-ee1-preview-che/routes\u0026labelSelector=che.workspace_id%3Dworkspacertz5iv86ez29v6b"
+		adhocURL := fmt.Sprintf("http://f8osoproxy.com?%s=%s%s", UserIDParam, userID, pathWithParams)
+		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
+		removeUserID(req)
+
+		actualUserID := req.URL.Query().Get(UserIDParam)
+		assert.Empty(t, actualUserID)
+	})
+
+	t.Run("UserID as part of url produced by rh-che via kubernetes client for services calls", func(t *testing.T) {
+		pathWithParams := "/api/v1/namespaces/osio-ci-ee1-preview-che/services\u0026labelSelector=che.workspace_id%3Dworkspacertz5iv86ez29v6bp"
+		adhocURL := fmt.Sprintf("http://f8osoproxy.com/?%s=%s%s", UserIDParam, userID, pathWithParams)
+		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
+		removeUserID(req)
+
+		actualUserID := req.URL.Query().Get(UserIDParam)
+		assert.Empty(t, actualUserID)
+	})
+
+	t.Run("UserID as part of url produced by rh-che via kubernetes client for pod 'watch' calls", func(t *testing.T) {
+		pathWithParams := "/api/v1/namespaces/osio-ci-ee1-preview-che/pods\u0026fieldSelector=metadata.name%3Drm-workspace41v9261pdzqs84c4\u0026watch=true"
+		adhocURL := fmt.Sprintf("http://f8osoproxy.com/?%s=%s%s", UserIDParam, userID, pathWithParams)
+		req, _ := http.NewRequest(http.MethodGet, adhocURL, nil)
+		removeUserID(req)
+
+		actualUserID := req.URL.Query().Get(UserIDParam)
+		assert.Empty(t, actualUserID)
 	})
 }