From f1d4e197363f08dd64db05a70f8b160e2a1ee001 Mon Sep 17 00:00:00 2001 From: Sakshyam Shah Date: Tue, 7 Jan 2025 09:14:05 -0500 Subject: [PATCH] [v16] honor redirect to SAML SSO path if user is already authenticated (#50794) * honor redirection if redirect path is saml idp sso path * honor redirection if redirect path is saml idp sso path --- .../teleport/src/Login/Login.test.tsx | 24 +++++ .../teleport/src/Login/useLogin.test.tsx | 100 ++++++++++++++++++ web/packages/teleport/src/Login/useLogin.ts | 27 ++++- web/packages/teleport/src/config.ts | 3 + 4 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 web/packages/teleport/src/Login/useLogin.test.tsx diff --git a/web/packages/teleport/src/Login/Login.test.tsx b/web/packages/teleport/src/Login/Login.test.tsx index ac5af29894efe..122845265c9d0 100644 --- a/web/packages/teleport/src/Login/Login.test.tsx +++ b/web/packages/teleport/src/Login/Login.test.tsx @@ -25,6 +25,7 @@ import { fireEvent, render, screen, waitFor } from 'design/utils/testing'; import cfg from 'teleport/config'; import auth from 'teleport/services/auth/auth'; import history from 'teleport/services/history'; +import session from 'teleport/services/websession'; import { Login } from './Login'; @@ -33,6 +34,7 @@ let user: UserEvent; beforeEach(() => { jest.restoreAllMocks(); jest.spyOn(history, 'push').mockImplementation(); + jest.spyOn(history, 'replace').mockImplementation(); jest.spyOn(history, 'getRedirectParam').mockImplementation(() => '/'); jest.spyOn(history, 'hasAccessChangedParam').mockImplementation(() => false); user = userEvent.setup(); @@ -196,3 +198,25 @@ describe('test MOTD', () => { expect(screen.getByText(/Your access has changed/i)).toBeInTheDocument(); }); }); + +test('redirect to root if session is valid and path is not "/enterprise/saml-idp/sso"', () => { + jest.spyOn(session, 'isValid').mockImplementation(() => true); + jest + .spyOn(history, 'getRedirectParam') + .mockReturnValue( + 'http://localhost/web/login?redirect_url=http://localhost/web/cluster/localhost/resources' + ); + render(); + + expect(history.replace).toHaveBeenCalledWith('/web'); +}); + +test('redirect if session is valid and path matches "/enterprise/saml-idp/sso"', () => { + const samlIdPPath = new URL('http://localhost' + cfg.routes.samlIdpSso); + jest.spyOn(session, 'isValid').mockImplementation(() => true); + jest + .spyOn(history, 'getRedirectParam') + .mockReturnValue(samlIdPPath.toString()); + render(); + expect(history.push).toHaveBeenCalledWith(samlIdPPath, true); +}); diff --git a/web/packages/teleport/src/Login/useLogin.test.tsx b/web/packages/teleport/src/Login/useLogin.test.tsx new file mode 100644 index 0000000000000..58c847a98ee6d --- /dev/null +++ b/web/packages/teleport/src/Login/useLogin.test.tsx @@ -0,0 +1,100 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { renderHook } from '@testing-library/react'; + +import cfg from 'teleport/config'; +import history from 'teleport/services/history'; +import session from 'teleport/services/websession'; + +import useLogin from './useLogin'; + +beforeEach(() => { + jest.restoreAllMocks(); + jest.spyOn(session, 'isValid').mockImplementation(() => true); + jest.spyOn(history, 'push').mockImplementation(); + jest.spyOn(history, 'replace').mockImplementation(); + jest.mock('shared/hooks', () => ({ + useAttempt: () => { + return [ + { status: 'success', statusText: 'Success Text' }, + { + clear: jest.fn(), + }, + ]; + }, + })); +}); + +afterEach(() => { + jest.resetAllMocks(); +}); + +it('redirect to root on path not matching "/enterprise/saml-idp/sso"', () => { + jest.spyOn(history, 'getRedirectParam').mockReturnValue('http://localhost'); + renderHook(() => useLogin()); + expect(history.replace).toHaveBeenCalledWith('/web'); + + jest + .spyOn(history, 'getRedirectParam') + .mockReturnValue('http://localhost/web/cluster/name/resources'); + renderHook(() => useLogin()); + expect(history.replace).toHaveBeenCalledWith('/web'); +}); + +it('redirect to SAML SSO path on matching "/enterprise/saml-idp/sso"', () => { + const samlIdpPath = new URL('http://localhost' + cfg.routes.samlIdpSso); + cfg.baseUrl = 'http://localhost'; + jest + .spyOn(history, 'getRedirectParam') + .mockReturnValue(samlIdpPath.toString()); + renderHook(() => useLogin()); + expect(history.push).toHaveBeenCalledWith(samlIdpPath, true); +}); + +it('non-base domain redirects with base domain for a matching "/enterprise/saml-idp/sso"', async () => { + const samlIdpPath = new URL('http://different-base' + cfg.routes.samlIdpSso); + jest + .spyOn(history, 'getRedirectParam') + .mockReturnValue(samlIdpPath.toString()); + renderHook(() => useLogin()); + const expectedPath = new URL('http://localhost' + cfg.routes.samlIdpSso); + expect(history.push).toHaveBeenCalledWith(expectedPath, true); +}); + +it('base domain with different path is redirected to root', async () => { + const nonSamlIdpPath = new URL('http://localhost/web/cluster/name/resources'); + jest + .spyOn(history, 'getRedirectParam') + .mockReturnValue(nonSamlIdpPath.toString()); + renderHook(() => useLogin()); + expect(history.replace).toHaveBeenCalledWith('/web'); +}); + +it('invalid session does nothing', async () => { + const samlIdpPathWithDifferentBase = new URL( + 'http://different-base' + cfg.routes.samlIdpSso + ); + jest + .spyOn(history, 'getRedirectParam') + .mockReturnValue(samlIdpPathWithDifferentBase.toString()); + jest.spyOn(session, 'isValid').mockImplementation(() => false); + renderHook(() => useLogin()); + expect(history.replace).not.toHaveBeenCalled(); + expect(history.push).not.toHaveBeenCalled(); +}); diff --git a/web/packages/teleport/src/Login/useLogin.ts b/web/packages/teleport/src/Login/useLogin.ts index e9de6ea143da7..2a3a6f4126e4f 100644 --- a/web/packages/teleport/src/Login/useLogin.ts +++ b/web/packages/teleport/src/Login/useLogin.ts @@ -17,6 +17,7 @@ */ import { useEffect, useState } from 'react'; +import { matchPath } from 'react-router'; import { TrustedDeviceRequirement } from 'gen-proto-ts/teleport/legacy/types/trusted_device_requirement_pb'; import { useAttempt } from 'shared/hooks'; @@ -70,8 +71,25 @@ export default function useLogin() { useEffect(() => { if (session.isValid()) { - history.replace(cfg.routes.root); - return; + try { + const redirectUrlWithBase = new URL(getEntryRoute()); + const matched = matchPath(redirectUrlWithBase.pathname, { + path: cfg.routes.samlIdpSso, + strict: true, + exact: true, + }); + if (matched) { + history.push(redirectUrlWithBase, true); + return; + } else { + history.replace(cfg.routes.root); + return; + } + } catch (e) { + console.error(e); + history.replace(cfg.routes.root); + return; + } } setCheckingValidSession(false); }, []); @@ -150,6 +168,11 @@ function loginSuccess() { history.push(redirect, withPageRefresh); } +/** + * getEntryRoute returns a base ensured redirect URL value that is safe + * for redirect. + * @returns base ensured URL string. + */ function getEntryRoute() { let entryUrl = history.getRedirectParam(); if (entryUrl) { diff --git a/web/packages/teleport/src/config.ts b/web/packages/teleport/src/config.ts index 4078ab39aed95..30cd11cd0b4f8 100644 --- a/web/packages/teleport/src/config.ts +++ b/web/packages/teleport/src/config.ts @@ -200,6 +200,9 @@ const cfg = { oidcHandler: '/v1/webapi/oidc/*', samlHandler: '/v1/webapi/saml/*', githubHandler: '/v1/webapi/github/*', + + /** samlIdpSso is an exact path of the service provider initiated SAML SSO endpoint. */ + samlIdpSso: '/enterprise/saml-idp/sso', }, api: {