Skip to content

Commit

Permalink
honor redirect to SAML SSO path if user is already authenticated (#50436
Browse files Browse the repository at this point in the history
)

* redirect if path is saml idp sso path

* fix test and add more tests

* add login component tests

* update test url path value

* update test var name

* rename non-base to base
  • Loading branch information
flyinghermit authored Dec 20, 2024
1 parent 9d45d66 commit 8f91da0
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 2 deletions.
24 changes: 24 additions & 0 deletions web/packages/teleport/src/Login/Login.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { render, fireEvent, screen, waitFor } from 'design/utils/testing';

import auth from 'teleport/services/auth/auth';
import history from 'teleport/services/history';
import session from 'teleport/services/websession';
import cfg from 'teleport/config';

import { Login } from './Login';
Expand All @@ -31,6 +32,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();
Expand Down Expand Up @@ -194,3 +196,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(<Login />);

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(<Login />);
expect(history.push).toHaveBeenCalledWith(samlIdPPath, true);
});
100 changes: 100 additions & 0 deletions web/packages/teleport/src/Login/useLogin.test.tsx
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.
*/

import history from 'teleport/services/history';
import session from 'teleport/services/websession';
import cfg from 'teleport/config';

import { renderHook } from '@testing-library/react';

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();
});
27 changes: 25 additions & 2 deletions web/packages/teleport/src/Login/useLogin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

import { useState, useEffect } from 'react';
import { matchPath } from 'react-router';
import { useAttempt } from 'shared/hooks';
import { AuthProvider } from 'shared/services';
import { TrustedDeviceRequirement } from 'gen-proto-ts/teleport/legacy/types/trusted_device_requirement_pb';
Expand Down Expand Up @@ -69,8 +70,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);
}, []);
Expand Down Expand Up @@ -149,6 +167,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) {
Expand Down
3 changes: 3 additions & 0 deletions web/packages/teleport/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ const cfg = {
accessGraph: {
crownJewelAccessPath: '/web/accessgraph/crownjewels/access/:id',
},

/** samlIdpSso is an exact path of the service provider initiated SAML SSO endpoint. */
samlIdpSso: '/enterprise/saml-idp/sso',
},

api: {
Expand Down

0 comments on commit 8f91da0

Please sign in to comment.