Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions lib/web/app/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import (
"net"
"net/http"
"net/url"
"path"
"slices"
"strings"
"time"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -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)
Comment thread
claude[bot] marked this conversation as resolved.

} else {
// Hitting this case means the user has hit an endpoint directly
Expand Down
34 changes: 28 additions & 6 deletions lib/web/app/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,37 +736,59 @@ 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",
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",
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",
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",
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) {
Expand Down
71 changes: 63 additions & 8 deletions web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

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';

Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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<UrlLauncherParams>();
const arn = params.arn ? decodeURIComponent(params.arn) : undefined;
if (arn) {
onArn(arn);
}
return <div>captured</div>;
}

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(
<MemoryRouter initialEntries={[url]}>
<Routes>
<Route
path={`${cfg.routes.appLauncher}/*`}
element={<ArnCapture onArn={arn => (capturedArn = arn)} />}
/>
</Routes>
</MemoryRouter>
);

await screen.findByText('captured');
expect(capturedArn).toBe(rawArn);
});
});
4 changes: 4 additions & 0 deletions web/packages/teleport/src/AppLauncher/AppLauncher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
30 changes: 30 additions & 0 deletions web/packages/teleport/src/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
Loading