From f79b39603639126c8eaaf59da6931d2bdd3505de Mon Sep 17 00:00:00 2001 From: Julia Ogris Date: Mon, 30 Mar 2026 17:28:42 +1100 Subject: [PATCH 1/2] app: Fix ARN encoding in web launcher redirect URL When an AWS console app role ARN contains a `/` in the path component (e.g. `arn:aws:iam::123:role/my-role`), the web launcher redirect truncates the ARN at the slash. This is a regression from the react-router v7 upgrade (PR #63496). Both v5 and v7 encode `/` as `%2F` in the URL path via `generatePath`. The difference is in `useParams()`: v5 returned the raw percent-encoded value, v7 auto-decodes it. In v5, `useParams().arn` was still percent-encoded (e.g. `role%2Fmy-role`). `getNewAuthExchangeUrl` put that encoded value into the query string, so Go's `q.Get("arn")` decoded one layer and got `role%2Fmy-role` (literal `%2F`, not a slash). `path.Join` treated `%2F` as ordinary characters and the full ARN survived. In v7, `useParams().arn` is auto-decoded to `role/my-role` (literal `/`). `getNewAuthExchangeUrl` puts that decoded value into the query string, Go's `q.Get("arn")` gets a literal `/`, and `path.Join` in `makeAppRedirectURL` splits the ARN at that slash. The redirect URL carries a truncated ARN, the session cert is issued with the truncated value, and the access check rejects the mismatch. Percent-encode the ARN with `url.PathEscape` before appending it to the URL path, and replace `path.Join` with `strings.Join` plus explicit `RawPath`/`Path` setting so that `%2F` survives URL serialization. --- lib/web/app/handler.go | 15 +++- lib/web/app/handler_test.go | 34 +++++++-- .../src/AppLauncher/AppLauncher.test.tsx | 70 +++++++++++++++++-- .../teleport/src/AppLauncher/AppLauncher.tsx | 4 ++ web/packages/teleport/src/config.test.ts | 30 ++++++++ 5 files changed, 137 insertions(+), 16 deletions(-) diff --git a/lib/web/app/handler.go b/lib/web/app/handler.go index 0e38cb86df21f..2b7ef95c4a801 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" @@ -663,11 +663,20 @@ func makeAppRedirectURL(r *http.Request, proxyPublicAddr, addr string, req launc urlPath = append(urlPath, req.clusterName, req.publicAddr) if req.arn != "" { - urlPath = append(urlPath, req.arn) + urlPath = append(urlPath, url.PathEscape(req.arn)) } } - u.Path = path.Join(urlPath...) + // Use strings.Join instead of path.Join to preserve + // percent-encoded segments like %2F in ARNs. + u.RawPath = "/" + strings.Join(urlPath, "/") + // Fall back to RawPath if PathUnescape fails, which can + // only happen if a non-ARN segment contains a bare percent. + var err error + u.Path, err = url.PathUnescape(u.RawPath) + if err != nil { + u.Path = 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..4a8416e409955 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx @@ -16,11 +16,12 @@ * 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 { UrlLauncherParams } from 'teleport/config'; import api from 'teleport/services/api'; import service from 'teleport/services/apps'; @@ -44,16 +45,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 +132,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 +372,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({ From 12cbf72a2e20b02f110d82aaa91517e95903d8a5 Mon Sep 17 00:00:00 2001 From: Julia Ogris Date: Tue, 31 Mar 2026 16:22:11 +1100 Subject: [PATCH 2/2] app: Percent-encode all path segments in launcher redirect Escape every segment in the redirect URL path, not just the ARN. This prevents any unexpected percent sequences in cluster names or public addresses from being emitted raw in the Location header. --- lib/web/app/handler.go | 21 ++++++++++--------- .../src/AppLauncher/AppLauncher.test.tsx | 3 +-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/web/app/handler.go b/lib/web/app/handler.go index 2b7ef95c4a801..817df67b25133 100644 --- a/lib/web/app/handler.go +++ b/lib/web/app/handler.go @@ -663,20 +663,21 @@ func makeAppRedirectURL(r *http.Request, proxyPublicAddr, addr string, req launc urlPath = append(urlPath, req.clusterName, req.publicAddr) if req.arn != "" { - urlPath = append(urlPath, url.PathEscape(req.arn)) + urlPath = append(urlPath, req.arn) } } - // Use strings.Join instead of path.Join to preserve - // percent-encoded segments like %2F in ARNs. - u.RawPath = "/" + strings.Join(urlPath, "/") - // Fall back to RawPath if PathUnescape fails, which can - // only happen if a non-ARN segment contains a bare percent. - var err error - u.Path, err = url.PathUnescape(u.RawPath) - if err != nil { - u.Path = u.RawPath + // 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/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx index 4a8416e409955..e243c470fcf23 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx @@ -20,8 +20,7 @@ import { MemoryRouter, Route, Routes, useParams } from 'react-router'; import { render, screen, waitFor } from 'design/utils/testing'; -import cfg from 'teleport/config'; -import { UrlLauncherParams } from 'teleport/config'; +import cfg, { UrlLauncherParams } from 'teleport/config'; import api from 'teleport/services/api'; import service from 'teleport/services/apps';