Lax regex and remove prefix versioning on new web clients

constants.go

@@ -25,9 +25,6 @@ import (
 	"github.com/gravitational/trace"
 )
 
-// WebAPIVersionOne is a current webapi version
-const WebAPIVersionOne = "v1"
-
 const (
 	// SSHAuthSock is the environment variable pointing to the
 	// Unix socket the SSH agent is running on.

lib/auth/trustedcluster.go

@@ -673,7 +673,7 @@ func (a *Server) sendValidateRequestToProxy(host string, validateRequest *authcl
 		opts = append(opts, roundtrip.HTTPClient(insecureWebClient))
 	}
 
-	clt, err := roundtrip.NewClient(proxyAddr.String(), teleport.WebAPIVersionOne, opts...)
+	clt, err := roundtrip.NewClient(proxyAddr.String(), "", opts...)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

lib/client/https_client.go

@@ -28,7 +28,6 @@ import (
 	"github.com/gravitational/trace"
 	"golang.org/x/net/http/httpproxy"
 
-	"github.com/gravitational/teleport"
 	tracehttp "github.com/gravitational/teleport/api/observability/tracing/http"
 	apiutils "github.com/gravitational/teleport/api/utils"
 	"github.com/gravitational/teleport/lib/httplib"
@@ -62,7 +61,7 @@ func httpTransport(insecure bool, pool *x509.CertPool) *http.Transport {
 
 func NewWebClient(url string, opts ...roundtrip.ClientParam) (*WebClient, error) {
 	opts = append(opts, roundtrip.SanitizerEnabled(true))
-	clt, err := roundtrip.NewClient(url, teleport.WebAPIVersionOne, opts...)
+	clt, err := roundtrip.NewClient(url, "", opts...)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

lib/client/weblogin_test.go

@@ -74,7 +74,7 @@ func TestHostCredentialsHttpFallback(t *testing.T) {
 		// Start an http server (not https) so that the request only succeeds
 		// if the fallback occurs.
 		var handler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
-			if r.RequestURI != "/v1/webapi/host/credentials" {
+			if r.RequestURI != "/webapi/host/credentials" {
 				w.WriteHeader(http.StatusNotFound)
 				return
 			}

lib/httplib/httplib.go

@@ -223,24 +223,24 @@ type Version struct {
 func ReplyRouteNotFoundJSONWithVersionField(w http.ResponseWriter, versionStr string) {
 	SetDefaultSecurityHeaders(w.Header())
 
+	errObj := &trace.TraceErr{
+		Err: trace.NotFound("path not found"),
+	}
+
 	ver, err := semver.NewVersion(versionStr)
-	verObj := Version{}
 	if err == nil {
-		verObj = Version{
+		verObj := Version{
 			Major:      ver.Major,
 			Minor:      ver.Minor,
 			Patch:      ver.Patch,
 			String:     versionStr,
 			PreRelease: string(ver.PreRelease),
 		}
+		fields := make(map[string]interface{})
+		fields["proxyVersion"] = verObj
+		errObj.Fields = fields
 	}
 
-	fields := make(map[string]interface{})
-	fields["proxyVersion"] = verObj
-	errObj := &trace.TraceErr{
-		Err:    trace.NotFound("path not found"),
-		Fields: fields,
-	}
 	roundtrip.ReplyJSON(w, http.StatusNotFound, errObj)
 }
 

lib/web/apiserver.go

@@ -109,8 +109,8 @@ import (
 	"github.com/gravitational/teleport/lib/web/ui"
 )
 
-// apiPrefixRegex matches pathname starting with /vN/webapi or /vN/enterprise
-var apiPrefixRegex = regexp.MustCompile(`^/v(\d+)/(webapi|enterprise)`)
+// apiPrefixRegex matches pathnames starting with /v<version num>/<any characters>
+var apiPrefixRegex = regexp.MustCompile(`^/v(\d+)/(.+)`)
 
 const (
 	// SSOLoginFailureMessage is a generic error message to avoid disclosing sensitive SSO failure messages.
@@ -616,13 +616,21 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {
 
 	notFoundRoutingHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Request is going to the API?
-		// If no routes were matched, it could be because certain paths don't expect any version prefixes.
-		// Only prefix "v1" will be stripped to preserve the legacy behavior before introducing v1 + N.
-		if matches := apiPrefixRegex.FindStringSubmatch(r.URL.Path); matches != nil && len(matches) > 1 {
+		// If no routes were matched, it could be because it's a path with `v1` prefix
+		// (eg: the Teleport web app will call "most" endpoints with v1 prefixed).
+		//
+		// `v1` paths are not defined with `v1` prefix. If the path turns out to be prefixed
+		// with `v1`, it will be stripped and served again. Historically, that's how it started
+		// and should be kept that way to prevent breakage.
+		//
+		// v2+ prefixes will be expected by both caller and definition and will not be stripped.
+		if matches := apiPrefixRegex.FindStringSubmatch(r.URL.Path); matches != nil && len(matches) == 3 {
+			postVersionPrefixPath := fmt.Sprintf("/%s", matches[2])
 			versionNum := matches[1]
-			if versionNum == "1" {
-				// Try path matching again with the stripped prefix.
-				http.StripPrefix("/"+teleport.WebAPIVersionOne, h).ServeHTTP(w, r)
+
+			// Regex check the rest of path to ensure we aren't allowing paths like /v1/v2/webapi
+			if matches := apiPrefixRegex.FindStringSubmatch(postVersionPrefixPath); matches == nil && versionNum == "1" {
+				http.StripPrefix("/v1", h).ServeHTTP(w, r)
 				return
 			}
 			httplib.ReplyRouteNotFoundJSONWithVersionField(w, teleport.Version)

lib/web/apiserver_test.go

@@ -3484,6 +3484,21 @@ func TestEndpointNotFoundHandling(t *testing.T) {
 			endpoint:  "v1/something/else",
 			shouldErr: true,
 		},
+		{
+			name:      "invalid triple version prefixes",
+			endpoint:  "v1/v1/v1/webapi/token",
+			shouldErr: true,
+		},
+		{
+			name:      "invalid just prefix",
+			endpoint:  "v1/",
+			shouldErr: true,
+		},
+		{
+			name:      "invalid prefix",
+			endpoint:  "v1s/webapi/token",
+			shouldErr: true,
+		},
 	}
 
 	for _, tc := range tt {
@@ -5110,7 +5125,7 @@ func TestDeleteMFA(t *testing.T) {
 	jar, err := cookiejar.New(nil)
 	require.NoError(t, err)
 	opts := []roundtrip.ClientParam{roundtrip.BearerAuth(pack.session.Token), roundtrip.CookieJar(jar), roundtrip.HTTPClient(client.NewInsecureWebClient())}
-	rclt, err := roundtrip.NewClient(proxy.webURL.String(), teleport.WebAPIVersionOne, opts...)
+	rclt, err := roundtrip.NewClient(proxy.webURL.String(), "", opts...)
 	require.NoError(t, err)
 	clt := client.WebClient{Client: rclt}
 	jar.SetCookies(&proxy.webURL, pack.cookies)

web/packages/teleport/src/services/api/api.ts

@@ -23,7 +23,6 @@ import websession from 'teleport/services/websession';
 
 import { MfaChallengeResponse } from '../mfa';
 import { storageService } from '../storageService';
-
 import parseError, { ApiError, parseProxyVersion } from './parseError';
 
 export const MFA_HEADER = 'Teleport-Mfa-Response';

web/packages/teleport/src/services/integrations/integrations.ts

@@ -24,7 +24,6 @@ import makeApp from '../apps/makeApps';
 import auth, { MfaChallengeScope } from '../auth/auth';
 import makeNode from '../nodes/makeNode';
 import { withUnsupportedLabelFeatureErrorConversion } from '../version/unsupported';
-
 import {
   AwsDatabaseVpcsResponse,
   AwsOidcDeployDatabaseServicesRequest,

web/packages/teleport/src/services/integrations/types.ts

@@ -18,8 +18,8 @@
 
 import { Label } from 'teleport/types';
 
-import { Node } from '../nodes';
 import { ResourceLabel } from '../agents';
+import { Node } from '../nodes';
 
 /**
  * type Integration v. type Plugin:

web/packages/teleport/src/services/joinToken/joinToken.ts

@@ -21,7 +21,6 @@ import api from 'teleport/services/api';
 
 import { makeLabelMapOfStrArrs } from '../agents/make';
 import { withUnsupportedLabelFeatureErrorConversion } from '../version/unsupported';
-
 import makeJoinToken from './makeJoinToken';
 import { JoinRule, JoinToken, JoinTokenRequest } from './types';