fix: Invite link doesn't work when logging in via SAML (#31989)

Co-authored-by: gabriellsh <40830821+gabriellsh@users.noreply.github.com>
pull/32164/head^2
Matheus Barbosa Silva 2 years ago committed by GitHub
parent da45cb6998
commit 4bbc009889
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 5
      .changeset/eleven-news-stare.md
  2. 5
      apps/meteor/.meteorMocks/index.ts
  3. 3
      apps/meteor/app/meteor-accounts-saml/server/definition/IServiceProviderOptions.ts
  4. 12
      apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts
  5. 5
      apps/meteor/app/meteor-accounts-saml/server/lib/Utils.ts
  6. 27
      apps/meteor/client/views/root/SAMLLoginRoute.tsx
  7. 1
      apps/meteor/jest.config.ts
  8. 49
      apps/meteor/tests/e2e/saml.spec.ts
  9. 14
      apps/meteor/tests/mocks/client/RouterContextMock.tsx
  10. 106
      apps/meteor/tests/unit/client/views/root/SAMLLoginRoute.spec.tsx

@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": patch
---
Fixed issue with login via SAML not redirecting to invite link

@ -0,0 +1,5 @@
import sinon from 'sinon';
export const Meteor = {
loginWithSamlToken: sinon.stub(),
};

@ -21,6 +21,7 @@ export interface IServiceProviderOptions {
metadataTemplate: string;
callbackUrl: string;
// The id attribute is filled midway through some operations
// The id and redirectUrl attributes are filled midway through some operations
id?: string;
redirectUrl?: string;
}

@ -54,7 +54,7 @@ export class SAML {
case 'sloRedirect':
return this.processSLORedirectAction(req, res);
case 'authorize':
return this.processAuthorizeAction(res, service, samlObject);
return this.processAuthorizeAction(req, res, service, samlObject);
case 'validate':
return this.processValidateAction(req, res, service, samlObject);
default:
@ -378,12 +378,20 @@ export class SAML {
}
private static async processAuthorizeAction(
req: IIncomingMessage,
res: ServerResponse,
service: IServiceProviderOptions,
samlObject: ISAMLAction,
): Promise<void> {
service.id = samlObject.credentialToken;
// Allow redirecting to internal domains when login process is complete
const { referer } = req.headers;
const siteUrl = settings.get<string>('Site_Url');
if (typeof referer === 'string' && referer.startsWith(siteUrl)) {
service.redirectUrl = referer;
}
const serviceProvider = new SAMLServiceProvider(service);
let url: string | undefined;
@ -430,7 +438,7 @@ export class SAML {
};
await this.storeCredential(credentialToken, loginResult);
const url = Meteor.absoluteUrl(SAMLUtils.getValidationActionRedirectPath(credentialToken));
const url = Meteor.absoluteUrl(SAMLUtils.getValidationActionRedirectPath(credentialToken, service.redirectUrl));
res.writeHead(302, {
Location: url,
});

@ -131,9 +131,10 @@ export class SAMLUtils {
return newTemplate;
}
public static getValidationActionRedirectPath(credentialToken: string): string {
public static getValidationActionRedirectPath(credentialToken: string, redirectUrl?: string): string {
const redirectUrlParam = redirectUrl ? `&redirectUrl=${encodeURIComponent(redirectUrl)}` : '';
// the saml_idp_credentialToken param is needed by the mobile app
return `saml/${credentialToken}?saml_idp_credentialToken=${credentialToken}`;
return `saml/${credentialToken}?saml_idp_credentialToken=${credentialToken}${redirectUrlParam}`;
}
public static log(obj: any, ...args: Array<any>): void {

@ -1,19 +1,42 @@
import { useRouter, useToastMessageDispatch, useUserId } from '@rocket.chat/ui-contexts';
import type { LocationPathname } from '@rocket.chat/ui-contexts';
import { useRouter, useToastMessageDispatch, useUserId, useAbsoluteUrl } from '@rocket.chat/ui-contexts';
import { Meteor } from 'meteor/meteor';
import { useEffect } from 'react';
const SAMLLoginRoute = () => {
const rootUrl = useAbsoluteUrl()('');
const router = useRouter();
const dispatchToastMessage = useToastMessageDispatch();
useEffect(() => {
const { token } = router.getRouteParameters();
const { redirectUrl } = router.getSearchParameters();
Meteor.loginWithSamlToken(token, (error?: unknown) => {
if (error) {
dispatchToastMessage({ type: 'error', message: error });
}
const decodedRedirectUrl = decodeURIComponent(redirectUrl || '');
if (decodedRedirectUrl?.startsWith(rootUrl)) {
const redirect = new URL(decodedRedirectUrl);
router.navigate(
{
pathname: redirect.pathname as LocationPathname,
search: Object.fromEntries(redirect.searchParams.entries()),
},
{ replace: true },
);
} else {
router.navigate(
{
pathname: '/home',
},
{ replace: true },
);
}
});
}, [dispatchToastMessage, router]);
}, [dispatchToastMessage, rootUrl, router]);
const userId = useUserId();
useEffect(() => {

@ -22,6 +22,7 @@ const config: Config = {
'\\.css$': 'identity-obj-proxy',
'^react($|/.+)': '<rootDir>/node_modules/react$1',
'^@tanstack/(.+)': '<rootDir>/node_modules/@tanstack/$1',
'^meteor/(.*)': '<rootDir>/.meteorMocks/index.ts',
},
},
{

@ -1,6 +1,7 @@
import child_process from 'child_process';
import path from 'path';
import { faker } from '@faker-js/faker';
import { Page } from '@playwright/test';
import { v2 as compose } from 'docker-compose'
import { MongoClient } from 'mongodb';
@ -48,6 +49,10 @@ const resetTestData = async (cleanupOnly = false) => {
await Promise.all(
[
{
_id: 'Accounts_AllowAnonymousRead',
value: false,
},
{
_id: 'SAML_Custom_Default_logout_behaviour',
value: 'SAML',
@ -93,6 +98,9 @@ test.describe('SAML', () => {
let poRegistration: Registration;
let samlRoleId: string;
let targetInviteGroupId: string;
let targetInviteGroupName: string;
let inviteId: string;
const containerPath = path.join(__dirname, 'containers', 'saml');
@ -116,6 +124,19 @@ test.describe('SAML', () => {
});
});
test.beforeAll(async ({ api }) => {
const groupResponse = await api.post('/groups.create', { name: faker.string.uuid() });
expect(groupResponse.status()).toBe(200);
const { group } = await groupResponse.json();
targetInviteGroupId = group._id;
targetInviteGroupName = group.name;
const inviteResponse = await api.post('/findOrCreateInvite', { rid: targetInviteGroupId, days: 1, maxUses: 0 });
expect(inviteResponse.status()).toBe(200);
const { _id } = await inviteResponse.json();
inviteId = _id;
});
test.afterAll(async ({ api }) => {
await compose.down({
cwd: containerPath,
@ -139,6 +160,10 @@ test.describe('SAML', () => {
}
});
test.afterAll(async ({ api }) => {
expect((await api.post('/groups.delete', { roomId: targetInviteGroupId })).status()).toBe(200);
});
test.beforeEach(async ({ page }) => {
poRegistration = new Registration(page);
@ -175,7 +200,7 @@ test.describe('SAML', () => {
});
});
const doLoginStep = async (page: Page, username: string) => {
const doLoginStep = async (page: Page, username: string, redirectUrl = '/home') => {
await test.step('expect successful login', async () => {
await poRegistration.btnLoginWithSaml.click();
// Redirect to Idp
@ -187,7 +212,7 @@ test.describe('SAML', () => {
await page.locator('role=button[name="Login"]').click();
// Redirect back to rocket.chat
await expect(page).toHaveURL('/home');
await expect(page).toHaveURL(redirectUrl);
await expect(page.getByRole('button', { name: 'User menu' })).toBeVisible();
});
@ -314,6 +339,26 @@ test.describe('SAML', () => {
});
});
test('Redirect to a specific group after login when using a valid invite link', async ({ page }) => {
await page.goto(`/invite/${inviteId}`);
await page.getByRole('link', { name: 'Back to Login' }).click();
await doLoginStep(page, 'samluser1', `${constants.BASE_URL}/invite/${inviteId}`);
await test.step('expect to be redirected to the invited room after succesful login', async () => {
await expect(page).toHaveURL(`/group/${targetInviteGroupName}`);
});
});
test('Redirect to home after login when no redirectUrl is provided', async ({ page }) => {
await doLoginStep(page, 'samluser2');
await test.step('expect to be redirected to the homepage after succesful login', async () => {
await expect(page).toHaveURL('/home');
});
});
test.fixme('User Merge - By Custom Identifier', async () => {
// Test user merge with a custom identifier configured in the fieldmap
});

@ -59,9 +59,17 @@ type RouterContextMockProps = {
children?: ReactNode;
navigate?: (toOrDelta: number | To) => void;
currentPath?: MutableRefObject<string | undefined>;
searchParameters?: Record<string, any>;
routeParameters?: Record<string, any>;
};
const RouterContextMock = ({ children, navigate, currentPath }: RouterContextMockProps): ReactElement => {
const RouterContextMock = ({
children,
navigate,
currentPath,
searchParameters = {},
routeParameters = {},
}: RouterContextMockProps): ReactElement => {
const history = useRef<{ stack: To[]; index: number }>({ stack: ['/'], index: 0 });
if (currentPath) {
@ -75,8 +83,8 @@ const RouterContextMock = ({ children, navigate, currentPath }: RouterContextMoc
subscribeToRouteChange: () => () => undefined,
getLocationPathname: () => '/',
getLocationSearch: () => '',
getRouteParameters: () => ({}),
getSearchParameters: () => ({}),
getRouteParameters: () => routeParameters,
getSearchParameters: () => searchParameters,
getRouteName: () => 'home',
buildRoutePath,
navigate:

@ -0,0 +1,106 @@
import { MockedServerContext, MockedUserContext } from '@rocket.chat/mock-providers';
import { render } from '@testing-library/react';
import '@testing-library/jest-dom';
import { Meteor } from 'meteor/meteor';
import React from 'react';
import sinon from 'sinon';
import SAMLLoginRoute from '../../../../../client/views/root/SAMLLoginRoute';
import RouterContextMock from '../../../../mocks/client/RouterContextMock';
const loginWithSamlTokenStub = Meteor.loginWithSamlToken as sinon.SinonStub;
const navigateStub = sinon.stub();
describe('views/root/SAMLLoginRoute', () => {
beforeEach(() => {
jest.clearAllMocks();
navigateStub.resetHistory();
loginWithSamlTokenStub.reset();
loginWithSamlTokenStub.callsFake((_token, callback) => callback());
});
it('should redirect to /home when userId is not null', async () => {
render(
<MockedServerContext>
<MockedUserContext>
<RouterContextMock searchParameters={{ redirectUrl: 'http://rocket.chat' }} navigate={navigateStub}>
<SAMLLoginRoute />
</RouterContextMock>
</MockedUserContext>
</MockedServerContext>,
);
expect(navigateStub.calledTwice).toBe(true);
expect(
navigateStub.calledWith(
sinon.match({
pathname: '/home',
}),
),
).toBe(true);
});
it('should redirect to /home when userId is null and redirectUrl is not within the workspace domain', async () => {
render(
<MockedServerContext>
<RouterContextMock searchParameters={{ redirectUrl: 'http://rocket.chat' }} navigate={navigateStub}>
<SAMLLoginRoute />
</RouterContextMock>
</MockedServerContext>,
);
expect(
navigateStub.calledOnceWith(
sinon.match({
pathname: '/home',
}),
),
).toBe(true);
});
it('should redirect to the provided redirectUrl when userId is null and redirectUrl is within the workspace domain', async () => {
render(
<MockedServerContext>
<RouterContextMock searchParameters={{ redirectUrl: 'http://localhost:3000/invite/test' }} navigate={navigateStub}>
<SAMLLoginRoute />
</RouterContextMock>
</MockedServerContext>,
);
expect(
navigateStub.calledOnceWith(
sinon.match({
pathname: '/invite/test',
}),
),
).toBe(true);
});
it('should call loginWithSamlToken when component is mounted', async () => {
render(
<MockedServerContext>
<RouterContextMock searchParameters={{ redirectUrl: 'http://rocket.chat' }} navigate={navigateStub}>
<SAMLLoginRoute />
</RouterContextMock>
</MockedServerContext>,
);
expect(loginWithSamlTokenStub.calledOnceWith(undefined)).toBe(true);
});
it('should call loginWithSamlToken with the token when it is present', async () => {
render(
<MockedUserContext>
<RouterContextMock
searchParameters={{ redirectUrl: 'http://rocket.chat' }}
routeParameters={{ token: 'testToken' }}
navigate={navigateStub}
>
<SAMLLoginRoute />
</RouterContextMock>
</MockedUserContext>,
);
expect(loginWithSamlTokenStub.calledOnceWith('testToken')).toBe(true);
});
});
Loading…
Cancel
Save