From 67c56145ba4997901c0890e7050ca5a7fe7c1f76 Mon Sep 17 00:00:00 2001 From: Lisa Kim Date: Thu, 2 Jan 2025 10:17:13 -0800 Subject: [PATCH] Lax regex and remove prefix versioning on new web clients --- constants.go | 3 --- lib/auth/trustedcluster.go | 2 +- lib/client/https_client.go | 3 +-- lib/client/weblogin_test.go | 2 +- lib/httplib/httplib.go | 16 ++++++------- lib/web/apiserver.go | 24 ++++++++++++------- lib/web/apiserver_test.go | 17 ++++++++++++- web/packages/teleport/src/services/api/api.ts | 1 - .../src/services/integrations/integrations.ts | 1 - .../src/services/integrations/types.ts | 2 +- .../src/services/joinToken/joinToken.ts | 1 - 11 files changed, 44 insertions(+), 28 deletions(-) diff --git a/constants.go b/constants.go index 39754ec6ca149..79f97ae24bfaf 100644 --- a/constants.go +++ b/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. diff --git a/lib/auth/trustedcluster.go b/lib/auth/trustedcluster.go index 5ae0ad522e331..869dad81f12ec 100644 --- a/lib/auth/trustedcluster.go +++ b/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) } diff --git a/lib/client/https_client.go b/lib/client/https_client.go index e707dfea9c0f6..11e263a6d519e 100644 --- a/lib/client/https_client.go +++ b/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) } diff --git a/lib/client/weblogin_test.go b/lib/client/weblogin_test.go index cca05b892fe2b..1008308411d50 100644 --- a/lib/client/weblogin_test.go +++ b/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 } diff --git a/lib/httplib/httplib.go b/lib/httplib/httplib.go index a0439b4e5dcd2..8b241584817da 100644 --- a/lib/httplib/httplib.go +++ b/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) } diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index dae12e3c42b0c..71bb76af586c9 100644 --- a/lib/web/apiserver.go +++ b/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/ +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) diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 375440b8797c3..e1f1e87e4fcfe 100644 --- a/lib/web/apiserver_test.go +++ b/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) diff --git a/web/packages/teleport/src/services/api/api.ts b/web/packages/teleport/src/services/api/api.ts index ddcff5007493d..d651cc028c84d 100644 --- a/web/packages/teleport/src/services/api/api.ts +++ b/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'; diff --git a/web/packages/teleport/src/services/integrations/integrations.ts b/web/packages/teleport/src/services/integrations/integrations.ts index 25b365e47a332..e2eef4a21c58d 100644 --- a/web/packages/teleport/src/services/integrations/integrations.ts +++ b/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, diff --git a/web/packages/teleport/src/services/integrations/types.ts b/web/packages/teleport/src/services/integrations/types.ts index 57c7424b06ad6..e409136ae2e9e 100644 --- a/web/packages/teleport/src/services/integrations/types.ts +++ b/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: diff --git a/web/packages/teleport/src/services/joinToken/joinToken.ts b/web/packages/teleport/src/services/joinToken/joinToken.ts index dab19f36bcd28..f91a0d1f0e883 100644 --- a/web/packages/teleport/src/services/joinToken/joinToken.ts +++ b/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';