diff --git a/lib/web/app/handler.go b/lib/web/app/handler.go index 0e38cb86df21f..817df67b25133 100644 --- a/lib/web/app/handler.go +++ b/lib/web/app/handler.go @@ -29,8 +29,8 @@ import ( "net" "net/http" "net/url" - "path" "slices" + "strings" "time" "github.com/gravitational/trace" @@ -667,7 +667,17 @@ func makeAppRedirectURL(r *http.Request, proxyPublicAddr, addr string, req launc } } - u.Path = path.Join(urlPath...) + // Percent-encode every segment so that slashes in ARNs + // are not interpreted as path separators during URL + // serialization. Use strings.Join instead of path.Join + // to preserve the percent-encoded segments. + for i, s := range urlPath { + urlPath[i] = url.PathEscape(s) + } + u.RawPath = "/" + strings.Join(urlPath, "/") + // Error is unreachable: RawPath was built from + // PathEscape output above. + u.Path, _ = url.PathUnescape(u.RawPath) } else { // Hitting this case means the user has hit an endpoint directly diff --git a/lib/web/app/handler_test.go b/lib/web/app/handler_test.go index 4aa46948f741c..b2b7ce1ef4083 100644 --- a/lib/web/app/handler_test.go +++ b/lib/web/app/handler_test.go @@ -736,14 +736,16 @@ func TestMakeAppRedirectURL(t *testing.T) { expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost?path=&required-apps=&state=abc123", }, { + // ARN inputs are decoded (matching the real flow through q.Get("arn") + // which URL-decodes query parameters). name: "OK - with clusterId, publicAddr, and arn", launderURLParams: launcherURLParams{ stateToken: "abc123", clusterName: "im-a-cluster-name", publicAddr: "grafana.localhost", - arn: "arn:aws:iam::123456789012:role%2Frole-name", + arn: "arn:aws:iam::123456789012:role/role-name", }, - expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%252Frole-name?path=&required-apps=&state=abc123", + expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%2Frole-name?path=&required-apps=&state=abc123", }, { name: "OK - with clusterId, publicAddr, arn and path", @@ -751,10 +753,10 @@ func TestMakeAppRedirectURL(t *testing.T) { stateToken: "abc123", clusterName: "im-a-cluster-name", publicAddr: "grafana.localhost", - arn: "arn:aws:iam::123456789012:role%2Frole-name", + arn: "arn:aws:iam::123456789012:role/role-name", path: "/foo/bar?qux=qex", }, - expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%252Frole-name?path=%2Ffoo%2Fbar%3Fqux%3Dqex&required-apps=&state=abc123", + expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%2Frole-name?path=%2Ffoo%2Fbar%3Fqux%3Dqex&required-apps=&state=abc123", }, { name: "OK - with clusterId, publicAddr, arn, path, and required-apps", @@ -762,11 +764,31 @@ func TestMakeAppRedirectURL(t *testing.T) { stateToken: "abc123", clusterName: "im-a-cluster-name", publicAddr: "grafana.localhost", - arn: "arn:aws:iam::123456789012:role%2Frole-name", + arn: "arn:aws:iam::123456789012:role/role-name", path: "/foo/bar?qux=qex", requiredAppFQDNs: "api.example.com,grafana.localhost", }, - expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%252Frole-name?path=%2Ffoo%2Fbar%3Fqux%3Dqex&required-apps=api.example.com%2Cgrafana.localhost&state=abc123", + expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%2Frole-name?path=%2Ffoo%2Fbar%3Fqux%3Dqex&required-apps=api.example.com%2Cgrafana.localhost&state=abc123", + }, + { + name: "OK - with ARN containing multi-level path", + launderURLParams: launcherURLParams{ + stateToken: "abc123", + clusterName: "im-a-cluster-name", + publicAddr: "grafana.localhost", + arn: "arn:aws:iam::123456789012:role/path/to/role-name", + }, + expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%2Fpath%2Fto%2Frole-name?path=&required-apps=&state=abc123", + }, + { + name: "OK - with ARN containing special characters", + launderURLParams: launcherURLParams{ + stateToken: "abc123", + clusterName: "im-a-cluster-name", + publicAddr: "grafana.localhost", + arn: "arn:aws:iam::123456789012:role/path+with=chars", + }, + expectedURL: "https://proxy.com/web/launch/grafana.localhost/im-a-cluster-name/grafana.localhost/arn:aws:iam::123456789012:role%2Fpath+with=chars?path=&required-apps=&state=abc123", }, } { t.Run(test.name, func(t *testing.T) { diff --git a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx index 2290c4a5713d5..e243c470fcf23 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx @@ -16,11 +16,11 @@ * along with this program. If not, see . */ -import { MemoryRouter, Route, Routes } from 'react-router'; +import { MemoryRouter, Route, Routes, useParams } from 'react-router'; import { render, screen, waitFor } from 'design/utils/testing'; -import cfg from 'teleport/config'; +import cfg, { UrlLauncherParams } from 'teleport/config'; import api from 'teleport/services/api'; import service from 'teleport/services/apps'; @@ -44,16 +44,18 @@ const launcherPathTestCases: { expectedPath: 'x-teleport-auth?path=%2Ffoo%2Fbar', }, { - name: 'no state with other path params (clusterId, publicAddr, publicArn', - path: '/some-cluster-id/some-public-addr/arn::123/name', + // The ARN is percent-encoded in the URL path so that the slash in + // "arn::123/name" does not split it into two path segments. + name: 'no state with other path params (clusterId, publicAddr, arn)', + path: '/some-cluster-id/some-public-addr/arn%3A%3A123%2Fname', expectedPath: - 'x-teleport-auth?cluster=some-cluster-id&addr=some-public-addr&arn=arn%3A%3A123', + 'x-teleport-auth?cluster=some-cluster-id&addr=some-public-addr&arn=arn%3A%3A123%2Fname', }, { name: 'no state with path and with other path params', - path: '/some-cluster-id/some-public-addr/arn::123/name?path=%2Ffoo%2Fbar', + path: '/some-cluster-id/some-public-addr/arn%3A%3A123%2Fname?path=%2Ffoo%2Fbar', expectedPath: - 'x-teleport-auth?path=%2Ffoo%2Fbar&cluster=some-cluster-id&addr=some-public-addr&arn=arn%3A%3A123', + 'x-teleport-auth?path=%2Ffoo%2Fbar&cluster=some-cluster-id&addr=some-public-addr&arn=arn%3A%3A123%2Fname', }, { name: 'with state', @@ -129,13 +131,24 @@ const appSessionTestCases: { expectedArn: string; }[] = [ { + // The ARN is percent-encoded in the URL path. React Router's + // useParams() auto-decodes the %2F back to /, so the ARN arrives + // fully decoded for createAppSession. name: 'ARN URL', - path: 'test-app.test.teleport/test.teleport/test-app.test.teleport/arn:aws:iam::joe123:role%2FEC2FullAccess?state=ABC', + path: 'test-app.test.teleport/test.teleport/test-app.test.teleport/arn%3Aaws%3Aiam%3A%3Ajoe123%3Arole%2FEC2FullAccess?state=ABC', returnedFqdn: 'test-app.test.teleport', expectedFqdn: 'test-app.test.teleport', expectedPublicAddr: 'test-app.test.teleport', expectedArn: 'arn:aws:iam::joe123:role/EC2FullAccess', }, + { + name: 'ARN URL with multi-level path', + path: 'test-app.test.teleport/test.teleport/test-app.test.teleport/arn%3Aaws%3Aiam%3A%3Ajoe123%3Arole%2Fpath%2Fto%2FEC2FullAccess?state=ABC', + returnedFqdn: 'test-app.test.teleport', + expectedFqdn: 'test-app.test.teleport', + expectedPublicAddr: 'test-app.test.teleport', + expectedArn: 'arn:aws:iam::joe123:role/path/to/EC2FullAccess', + }, { name: 'uppercase resolved FQDN', path: 'test-app.test.teleport/test.teleport/test-app.test.teleport?state=ABC', @@ -358,3 +371,45 @@ describe('fqdn is matched', () => { expect(windowLocation.replace).not.toHaveBeenCalled(); }); }); + +// Round-trip test: verifies that an ARN with slashes survives the full +// cycle from URL generation through React Router matching and param +// decoding. +describe('ARN round-trips through URL generation and routing', () => { + function ArnCapture({ onArn }: { onArn: (arn: string) => void }) { + const params = useParams(); + const arn = params.arn ? decodeURIComponent(params.arn) : undefined; + if (arn) { + onArn(arn); + } + return
captured
; + } + + test.each([ + 'arn:aws:iam::123456789012:role/my-role', + 'arn:aws:iam::123456789012:role/path/to/my-role', + 'arn:aws:iam::123456789012:role/path+with=chars', + ])('round-trips ARN: %s', async rawArn => { + const url = cfg.getAppLauncherRoute({ + fqdn: 'app.example.com', + clusterId: 'cluster1', + publicAddr: 'app.example.com', + arn: rawArn, + }); + + let capturedArn: string | undefined; + render( + + + (capturedArn = arn)} />} + /> + + + ); + + await screen.findByText('captured'); + expect(capturedArn).toBe(rawArn); + }); +}); diff --git a/web/packages/teleport/src/AppLauncher/AppLauncher.tsx b/web/packages/teleport/src/AppLauncher/AppLauncher.tsx index 45a6f25d8b8a4..1ea5342dfb628 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.tsx @@ -131,6 +131,10 @@ export function AppLauncher({ } // Continue the auth exchange. + // decodeURIComponent is redundant here: React Router's useParams() + // already decodes percent-encoded path segments (e.g. %2F → /), + // so params.arn contains the decoded ARN. Retained as defensive + // code in case a caller constructs the URL without encoding. if (params.arn) { params.arn = decodeURIComponent(params.arn); } diff --git a/web/packages/teleport/src/config.test.ts b/web/packages/teleport/src/config.test.ts index 5c8bbdd682569..d29c23df77f8e 100644 --- a/web/packages/teleport/src/config.test.ts +++ b/web/packages/teleport/src/config.test.ts @@ -135,6 +135,36 @@ test('getClusterEventsUrl does not corrupt startKey when start is set', () => { expect(url).not.toContain('00.000ZKey?'); }); +test('getAppLauncherRoute encodes slash in AWS role ARN', () => { + const url = cfg.getAppLauncherRoute({ + fqdn: 'app.example.com', + clusterId: 'cluster1', + publicAddr: 'app.example.com', + arn: 'arn:aws:iam::123456789012:role/my-role', + }); + expect(url).toContain('role%2Fmy-role'); + expect(url).not.toContain('role/my-role'); +}); + +test('getAppLauncherRoute encodes multi-level ARN path', () => { + const url = cfg.getAppLauncherRoute({ + fqdn: 'app.example.com', + clusterId: 'cluster1', + publicAddr: 'app.example.com', + arn: 'arn:aws:iam::123456789012:role/path/to/my-role', + }); + expect(url).toContain('role%2Fpath%2Fto%2Fmy-role'); +}); + +test('getAppLauncherRoute without ARN leaves route unchanged', () => { + const url = cfg.getAppLauncherRoute({ + fqdn: 'app.example.com', + clusterId: 'cluster1', + publicAddr: 'app.example.com', + }); + expect(url).toBe('/web/launch/app.example.com/cluster1/app.example.com'); +}); + test('getRoleUrl listv2 encodes query params', () => { expect( cfg.getRoleUrl({