[v17] Device Trust web: Ensure query params of redirect URL are not lost

lib/web/device_trust.go

@@ -100,6 +100,12 @@ func (h *Handler) deviceWebConfirm(w http.ResponseWriter, r *http.Request, _ htt
 // It returns a full URL if the unsafeRedirectURI points to SAML IdP SSO endpoint.
 // In any other case, as long as the redirect URL is parsable, it returns
 // a path ensuring its prefixed with "/web".
+//
+// Nobody seems to know why we need to prepend the base path to the URL, so we keep doing it. It
+// might be related to the URLs we get from SSO redirects [1], but it's unclear why we'd be getting
+// a URL that's missing the base path and becomes valid only after appending the base path.
+//
+// [1]: https://github.com/gravitational/teleport/pull/47221#discussion_r1792248868
 func (h *Handler) getRedirectURL(host, unsafeRedirectURI string) (string, error) {
 	const (
 		basePath                = "/web"
@@ -145,7 +151,11 @@ func (h *Handler) getRedirectURL(host, unsafeRedirectURI string) (string, error)
 
 	// Prepend "/web" only if it's not already present
 	if !strings.HasPrefix(cleanPath, basePath) {
-		return path.Join(basePath, cleanPath), nil
+		cleanPath = path.Join(basePath, cleanPath)
+	}
+
+	if parsedURL.RawQuery != "" {
+		return cleanPath + "?" + parsedURL.RawQuery, nil
 	}
 	return cleanPath, nil
 }

lib/web/device_trust_test.go

@@ -22,6 +22,7 @@ import (
 	"io"
 	"net/http"
 	"net/url"
+	"strings"
 	"sync"
 	"testing"
 
@@ -51,7 +52,6 @@ func TestHandler_DeviceWebConfirm(t *testing.T) {
 		name               string
 		redirectURI        string
 		expectedRedirectTo string
-		redirectsToFullURL bool
 		statusCode         int
 	}{
 		{
@@ -89,6 +89,12 @@ func TestHandler_DeviceWebConfirm(t *testing.T) {
 			expectedRedirectTo: "/web",
 			statusCode:         http.StatusSeeOther,
 		},
+		{
+			name:               "with empty path with query params",
+			redirectURI:        "https://example.com/?foo=bar",
+			expectedRedirectTo: "/web?foo=bar",
+			statusCode:         http.StatusSeeOther,
+		},
 		{
 			name:               "with relative path",
 			redirectURI:        "/custom/path",
@@ -105,27 +111,53 @@ func TestHandler_DeviceWebConfirm(t *testing.T) {
 			name:               "saml idp service provider initiated sso endpoint",
 			redirectURI:        fmt.Sprintf("https://%s/enterprise/saml-idp/sso?SAMLRequest=example-authn-request", proxy.webURL.Host),
 			expectedRedirectTo: fmt.Sprintf("https://%s/enterprise/saml-idp/sso?SAMLRequest=example-authn-request", proxy.webURL.Host),
-			redirectsToFullURL: true,
 			statusCode:         http.StatusSeeOther,
 		},
 		{
 			name:               "saml idp identity provider initiated sso endpoint",
 			redirectURI:        fmt.Sprintf("https://%s/enterprise/saml-idp/login/example-app", proxy.webURL.Host),
 			expectedRedirectTo: fmt.Sprintf("https://%s/enterprise/saml-idp/login/example-app", proxy.webURL.Host),
-			redirectsToFullURL: true,
 			statusCode:         http.StatusSeeOther,
 		},
 		{
-			name:               "saml idp sso endpoint with redirect_uri pointing to a different host",
-			redirectURI:        "https://example.com/enterprise/saml-idp/sso?SAMLRequest=example-authn-request",
-			redirectsToFullURL: true,
-			statusCode:         http.StatusBadRequest,
+			name:        "saml idp sso endpoint with redirect_uri pointing to a different host",
+			redirectURI: "https://example.com/enterprise/saml-idp/sso?SAMLRequest=example-authn-request",
+			statusCode:  http.StatusBadRequest,
+		},
+		{
+			name:        "saml idp sso endpoint with redirect_uri pointing to a malformed URL",
+			redirectURI: "https://%s.//example.com/enterprise/saml-idp/sso?SAMLRequest=example-authn-request",
+			statusCode:  http.StatusBadRequest,
+		},
+		{
+			name:               "query params with base path",
+			redirectURI:        "/web/foo?bar=quux",
+			expectedRedirectTo: "/web/foo?bar=quux",
+			statusCode:         http.StatusSeeOther,
+		},
+		{
+			name:               "query params without base path",
+			redirectURI:        "/foo?bar=quux",
+			expectedRedirectTo: "/web/foo?bar=quux",
+			statusCode:         http.StatusSeeOther,
+		},
+		{
+			name:               "query params with base path in full URL",
+			redirectURI:        "https://example.com/web/foo?bar=quux",
+			expectedRedirectTo: "/web/foo?bar=quux",
+			statusCode:         http.StatusSeeOther,
 		},
 		{
-			name:               "saml idp sso endpoint with redirect_uri pointing to a malformed URL",
-			redirectURI:        "https://%s.//example.com/enterprise/saml-idp/sso?SAMLRequest=example-authn-request",
-			redirectsToFullURL: true,
-			statusCode:         http.StatusBadRequest,
+			name:               "query params without base path in full URL",
+			redirectURI:        "https://example.com/foo?bar=quux",
+			expectedRedirectTo: "/web/foo?bar=quux",
+			statusCode:         http.StatusSeeOther,
+		},
+		{
+			name:               "query params with nested URL with query params",
+			redirectURI:        "https://teleport.example.com:3030/web/launch/dumper.teleport.example.com?path=%2Fhello&query=custom_url%3Dhttps%253A%252F%252Fcluster.example.com%253A3030%252Fweb%252Fcluster%252Fenterprise-local%252Fresources%253Fsort%253Dname%25253Adesc%2526pinnedOnly%253Dfalse%2526kinds%253Dapp",
+			expectedRedirectTo: "/web/launch/dumper.teleport.example.com?path=%2Fhello&query=custom_url%3Dhttps%253A%252F%252Fcluster.example.com%253A3030%252Fweb%252Fcluster%252Fenterprise-local%252Fresources%253Fsort%253Dname%25253Adesc%2526pinnedOnly%253Dfalse%2526kinds%253Dapp",
+			statusCode:         http.StatusSeeOther,
 		},
 	}
 
@@ -143,10 +175,12 @@ func TestHandler_DeviceWebConfirm(t *testing.T) {
 			httpClient := webClient.HTTPClient()
 			httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
 				redirected = true
-				actualRedirectTo = req.URL.Path
-				if test.redirectsToFullURL {
-					actualRedirectTo = req.URL.String()
+				isExpectRedirectToRelative := strings.HasPrefix(test.expectedRedirectTo, "/")
+				if isExpectRedirectToRelative {
+					req.URL.Scheme = ""
+					req.URL.Host = ""
 				}
+				actualRedirectTo = req.URL.String()
 				return http.ErrUseLastResponse
 			}
 

package.json

@@ -32,20 +32,19 @@
   "private": true,
   "pnpm": {
     "overrides": {
-      "@bundled-es-modules/cookie>cookie": "0.7.1",
-      "jsdom@^20.0.3>nwsapi@^2": "2.2.9"
+      "@bundled-es-modules/cookie>cookie": "0.7.1"
     }
   },
   "devDependencies": {
     "@gravitational/build": "workspace:*",
     "@ianvs/prettier-plugin-sort-imports": "^4.4.0",
     "@storybook/react-vite": "^9.0.17",
     "@storybook/test-runner": "^0.23.0",
-    "@testing-library/jest-dom": "^6.5.0",
+    "@testing-library/jest-dom": "^6.9.1",
     "@testing-library/react": "^16.0.0",
     "@testing-library/user-event": "^14.5.2",
     "@types/history": "^4.7.11",
-    "@types/jest": "^29.5.13",
+    "@types/jest": "^30.0.0",
     "@types/node": "^22.14.0",
     "@types/react": "^18.3.10",
     "@types/react-dom": "^18.3.0",
@@ -54,7 +53,7 @@
     "@types/react-router-dom": "^5.1.1",
     "@types/react-transition-group": "^4.4.11",
     "@types/wicg-file-system-access": "^2023.10.5",
-    "jest": "^29.7.0",
+    "jest": "^30.2.0",
     "jest-canvas-mock": "^2.5.2",
     "jsdom-testing-mocks": "^1.13.1",
     "msw": "^2.4.9",