diff --git a/.changeset/hot-spoons-heal.md b/.changeset/hot-spoons-heal.md new file mode 100644 index 0000000000000..6bcdabec83e39 --- /dev/null +++ b/.changeset/hot-spoons-heal.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Disables SAML login when it is set to validate signatures without the proper configuration for it diff --git a/.changeset/rich-doodles-grow.md b/.changeset/rich-doodles-grow.md new file mode 100644 index 0000000000000..3699a28550a66 --- /dev/null +++ b/.changeset/rich-doodles-grow.md @@ -0,0 +1,9 @@ +--- +'@rocket.chat/model-typings': patch +'@rocket.chat/core-typings': patch +'@rocket.chat/models': patch +'@rocket.chat/i18n': patch +'@rocket.chat/meteor': patch +--- + +Security Hotfix (https://docs.rocket.chat/docs/security-fixes-and-updates) diff --git a/.github/workflows/ci-test-unit.yml b/.github/workflows/ci-test-unit.yml index 7d249f60ff48e..b278eb9cd7a8a 100644 --- a/.github/workflows/ci-test-unit.yml +++ b/.github/workflows/ci-test-unit.yml @@ -53,7 +53,7 @@ jobs: tar -xzf /tmp/RocketChat-packages-build.tar.gz -C . - name: Unit Test - run: yarn testunit + run: yarn testunit --concurrency=1 - uses: codecov/codecov-action@v5 with: diff --git a/apps/meteor/.mocharc.js b/apps/meteor/.mocharc.js index 2ec06b5257b65..5c46d3320e31d 100644 --- a/apps/meteor/.mocharc.js +++ b/apps/meteor/.mocharc.js @@ -11,6 +11,7 @@ module.exports = { exit: true, spec: [ 'server/lib/callbacks.spec.ts', + 'server/lib/cas/*.spec.ts', 'server/lib/ldap/*.spec.ts', 'server/lib/ldap/**/*.spec.ts', 'server/lib/dataExport/**/*.spec.ts', diff --git a/apps/meteor/app/file-upload/server/lib/FileUpload.ts b/apps/meteor/app/file-upload/server/lib/FileUpload.ts index 89878e783a413..a368d28586c0c 100644 --- a/apps/meteor/app/file-upload/server/lib/FileUpload.ts +++ b/apps/meteor/app/file-upload/server/lib/FileUpload.ts @@ -94,7 +94,7 @@ const defaults: Record Partial> = { return { collection: UserDataFiles, getPath(file: IUpload) { - return `${settings.get('uniqueID')}/uploads/userData/${file.userId}`; + return `${settings.get('uniqueID')}/uploads/userData/${file.userId}/${file._id}`; }, onValidate: FileUpload.uploadsOnValidate, async onRead(_fileId: string, file: IUpload, req: http.IncomingMessage, res: http.ServerResponse) { diff --git a/apps/meteor/app/file-upload/server/methods/sendFileMessage.spec.ts b/apps/meteor/app/file-upload/server/methods/sendFileMessage.spec.ts new file mode 100644 index 0000000000000..3a73c375d5d9a --- /dev/null +++ b/apps/meteor/app/file-upload/server/methods/sendFileMessage.spec.ts @@ -0,0 +1,111 @@ +import { expect } from 'chai'; +import { describe, it, beforeEach } from 'mocha'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +const findOneByIdAndUserIdAndRoomId = sinon.stub(); +const updateFileMetadata = sinon.stub().resolves(); +const getPath = sinon.stub().returns('/path/to/file.txt'); +const isImagePreviewSupported = sinon.stub().returns(false); +const getFileExtension = sinon.stub().returns('txt'); + +const { parseFileIntoMessageAttachments } = proxyquire.noCallThru().load('./sendFileMessage', { + '@rocket.chat/models': { + Uploads: { findOneByIdAndUserIdAndRoomId, updateFileMetadata }, + Rooms: { findOneById: sinon.stub() }, + Users: { findOneById: sinon.stub() }, + }, + 'meteor/check': { + check: sinon.stub(), + Match: { + Maybe: sinon.stub(), + Optional: sinon.stub(), + ObjectIncluding: sinon.stub(), + }, + }, + 'meteor/meteor': { + Meteor: { + Error: class Error extends global.Error {}, + methods: sinon.stub(), + }, + }, + '../lib/FileUpload': { + FileUpload: { getPath }, + }, + './isImagePreviewSupported': { isImagePreviewSupported }, + '../../../../lib/utils/getFileExtension': { getFileExtension }, + '../../../../server/lib/callbacks': { callbacks: { runAsync: sinon.stub() } }, + '../../../../server/lib/logger/system': { SystemLogger: { error: sinon.stub() } }, + '../../../authorization/server/functions/canAccessRoom': { canAccessRoomAsync: sinon.stub().resolves(true) }, + '../../../lib/server/methods/sendMessage': { executeSendMessage: sinon.stub().resolves({}) }, +}); + +describe('sendFileMessage - Mass Assignment & Type Pollution Prevention', () => { + const mockUser = { _id: 'user123' }; + const roomId = 'room123'; + + beforeEach(() => { + findOneByIdAndUserIdAndRoomId.reset(); + updateFileMetadata.reset(); + + findOneByIdAndUserIdAndRoomId.resolves({ _id: 'file123' }); + }); + + it('should filter out invalid types, nulls, and malicious fields before updating the database', async () => { + const maliciousFilePayload = { + _id: 'file123', + name: null, // invalid type, must be ignored + type: 'text/plain', + size: 1024, + description: 12345, // invalid type, must be ignored + typeGroup: 'image', // only valid field + content: null, // invalid type, must be ignored + maliciousRoleAssignment: 'admin', // mass assignment, must be ignored + $set: { bypassSecurity: true }, // mongo injection, must be ignored + }; + + await parseFileIntoMessageAttachments(maliciousFilePayload as any, roomId, mockUser as any); + + expect(updateFileMetadata.calledOnce).to.equal(true); + + const [fileId, userId, safeMetadata] = updateFileMetadata.getCall(0).args; + + expect(fileId).to.equal('file123'); + expect(userId).to.equal('user123'); + + expect(safeMetadata).to.deep.equal({ + typeGroup: 'image', + }); + }); + + it('should pass valid fields correctly to the database', async () => { + const validFilePayload = { + _id: 'file123', + name: 'picture.jpg', + type: 'image/jpeg', + size: 2048, + description: 'Description', + typeGroup: 'image', + content: { + algorithm: 'rc.v1.aes-sha2', + ciphertext: 'test', + }, + }; + + await parseFileIntoMessageAttachments(validFilePayload as any, roomId, mockUser as any); + + expect(updateFileMetadata.calledOnce).to.equal(true); + + const [, , safeMetadata] = updateFileMetadata.getCall(0).args; + + expect(safeMetadata).to.deep.equal({ + name: 'picture.jpg', + description: 'Description', + typeGroup: 'image', + content: { + algorithm: 'rc.v1.aes-sha2', + ciphertext: 'test', + }, + }); + }); +}); diff --git a/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts b/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts index 15fcba1875388..961b086ac4ec8 100644 --- a/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts +++ b/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts @@ -15,7 +15,6 @@ import { Meteor } from 'meteor/meteor'; import { isImagePreviewSupported } from './isImagePreviewSupported'; import { getFileExtension } from '../../../../lib/utils/getFileExtension'; -import { omit } from '../../../../lib/utils/omit'; import { callbacks } from '../../../../server/lib/callbacks'; import { SystemLogger } from '../../../../server/lib/logger/system'; import { canAccessRoomAsync } from '../../../authorization/server/functions/canAccessRoom'; @@ -38,7 +37,21 @@ export const parseFileIntoMessageAttachments = async ( ): Promise => { validateFileRequiredFields(file); - await Uploads.updateFileComplete(file._id, user._id, omit(file, '_id')); + const upload = await Uploads.findOneByIdAndUserIdAndRoomId(file._id, user._id, roomId, { projection: { _id: 1 } }); + if (!upload) { + throw new Meteor.Error('error-invalid-file', 'Invalid file', { + method: 'sendFileMessage', + }); + } + + const safeMetadata = { + ...(typeof file.name === 'string' && { name: file.name }), + ...(typeof file.description === 'string' && { description: file.description }), + ...(typeof file.typeGroup === 'string' && { typeGroup: file.typeGroup }), + ...(file.content && typeof file.content === 'object' && { content: file.content }), + }; + + await Uploads.updateFileMetadata(file._id, user._id, safeMetadata); const fileUrl = FileUpload.getPath(`${file._id}/${encodeURI(file.name || '')}`); diff --git a/apps/meteor/app/meteor-accounts-saml/server/definition/IServiceProviderOptions.ts b/apps/meteor/app/meteor-accounts-saml/server/definition/IServiceProviderOptions.ts index e00c967288024..673486d0a3dfe 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/definition/IServiceProviderOptions.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/definition/IServiceProviderOptions.ts @@ -14,6 +14,8 @@ export interface IServiceProviderOptions { defaultUserRole: string; allowedClockDrift: number; signatureValidationType: string; + validateLogoutRequestSignature: boolean; + validateLogoutResponseSignature: boolean; identifierFormat: string; nameIDPolicyTemplate: string; authnContextTemplate: string; diff --git a/apps/meteor/app/meteor-accounts-saml/server/definition/SAMLEnvelope.ts b/apps/meteor/app/meteor-accounts-saml/server/definition/SAMLEnvelope.ts new file mode 100644 index 0000000000000..35378aa450c15 --- /dev/null +++ b/apps/meteor/app/meteor-accounts-saml/server/definition/SAMLEnvelope.ts @@ -0,0 +1,43 @@ +// Other binding values are not supported: HTTP-Artifact, URI, SOAP, PAOS +export type SAMLBinding = 'HTTP-Redirect' | 'HTTP-POST'; + +export type SAMLDocumentType = 'SAMLRequest' | 'SAMLResponse'; + +export type SAMLBaseEnvelope = { + binding: SAMLBinding; + /** + * encodedDocument is the document in the exact format we received it + * */ + encodedDocument: string; + /** + * decodedDocument is the raw XML file; processed according to the binding specification + * */ + decodedDocument: string; + type: T; + relayState?: string; +}; + +/** + * HTTP-Redirect envelope + * For this binding, the document must be a Deflated XML file that is then base64 encoded + **/ +export type SAMLRedirectEnvelope = SAMLBaseEnvelope & { + binding: 'HTTP-Redirect'; + sigAlg?: string; + signature?: string; + + /** + * signedContent is the complete string value that must be matched by the signature + * */ + signedContent?: string; +}; + +/** + * HTTP-POST envelope + * For this binding, the document must be a regular XML file that is base64 encoded + **/ +export type SAMLPOSTEnvelope = SAMLBaseEnvelope & { + binding: 'HTTP-POST'; +}; + +export type SAMLEnvelope = SAMLRedirectEnvelope | SAMLPOSTEnvelope; diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts index fdac8b0b77fa8..adb83a3f3a8b2 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts @@ -9,6 +9,7 @@ import { Meteor } from 'meteor/meteor'; import { SAMLServiceProvider } from './ServiceProvider'; import { SAMLUtils } from './Utils'; +import { getSAMLEnvelope } from './getSAMLEnvelope'; import { ensureArray } from '../../../../lib/utils/arrayUtils'; import { SystemLogger } from '../../../../server/lib/logger/system'; import { addUserToRoom } from '../../../lib/server/functions/addUserToRoom'; @@ -283,11 +284,21 @@ export class SAML { } private static async processLogoutRequest(req: IIncomingMessage, res: ServerResponse, service: IServiceProviderOptions): Promise { + const errorHandler = (err: unknown) => { + SystemLogger.error({ err }); + throw new Meteor.Error('Unable to Validate Logout Request'); + }; + + const envelope = await getSAMLEnvelope(req, 'SAMLRequest', 'HTTP-Redirect').catch(errorHandler); + if (!envelope) { + errorHandler('No envelope found for SAML Logout Request'); + return; + } + const serviceProvider = new SAMLServiceProvider(service); - await serviceProvider.validateLogoutRequest(req.query.SAMLRequest, async (err, result) => { + await serviceProvider.validateLogoutRequest(envelope, async (err, result) => { if (err) { - SystemLogger.error({ err }); - throw new Meteor.Error('Unable to Validate Logout Request'); + errorHandler(err); } if (!result?.nameID || !result?.idpSession) { @@ -350,15 +361,16 @@ export class SAML { } private static async processLogoutResponse(req: IIncomingMessage, res: ServerResponse, service: IServiceProviderOptions): Promise { - if (!req.query.SAMLResponse) { + const envelope = await getSAMLEnvelope(req, 'SAMLResponse', 'HTTP-Redirect'); + if (!envelope) { SAMLUtils.error({ msg: 'Invalid LogoutResponse received: missing SAMLResponse parameter.', query: req.query }); throw new Error('Invalid LogoutResponse received.'); } const serviceProvider = new SAMLServiceProvider(service); - await serviceProvider.validateLogoutResponse(req.query.SAMLResponse, async (err, inResponseTo) => { + await serviceProvider.validateLogoutResponse(envelope, async (err, inResponseTo) => { if (err) { - return; + throw new Error('Logout Response Validation failed'); } if (!inResponseTo) { @@ -420,15 +432,29 @@ export class SAML { res.end(); } - private static processValidateAction( + private static async processValidateAction( req: IIncomingMessage, res: ServerResponse, service: IServiceProviderOptions, _samlObject: ISAMLAction, - ): void { + ): Promise { + const redirect = (url?: string) => { + res.writeHead(302, { + Location: url ?? Meteor.absoluteUrl(), + }); + res.end(); + }; + + const envelope = await getSAMLEnvelope(req, 'SAMLResponse', 'HTTP-POST').catch((err) => SAMLUtils.error(err)); + + if (!envelope) { + SAMLUtils.error({ msg: 'No envelope found for SAML Response' }); + return redirect(); + } + const serviceProvider = new SAMLServiceProvider(service); - SAMLUtils.relayState = req.body.RelayState; - serviceProvider.validateResponse(req.body.SAMLResponse, async (err, profile /* , loggedOut*/) => { + SAMLUtils.relayState = envelope.relayState ?? null; + serviceProvider.validateResponse(envelope, async (err, profile /* , loggedOut*/) => { try { if (err) { SAMLUtils.error(err); @@ -450,16 +476,10 @@ export class SAML { await this.storeCredential(credentialToken, loginResult); const url = Meteor.absoluteUrl(SAMLUtils.getValidationActionRedirectPath(credentialToken)); - res.writeHead(302, { - Location: url, - }); - res.end(); + redirect(url); } catch (err) { SAMLUtils.error({ err }); - res.writeHead(302, { - Location: Meteor.absoluteUrl(), - }); - res.end(); + redirect(); } }); } diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/ServiceProvider.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/ServiceProvider.ts index f60b65952d67e..824b7ec684dcc 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/lib/ServiceProvider.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/ServiceProvider.ts @@ -17,13 +17,8 @@ import { ServiceProviderMetadata } from './generators/ServiceProviderMetadata'; import { LogoutRequestParser } from './parsers/LogoutRequest'; import { LogoutResponseParser } from './parsers/LogoutResponse'; import { ResponseParser } from './parsers/Response'; - -const signatureAlgorithms = { - 'RSA-SHA1': 'http://www.w3.org/2000/09/xmldsig#rsa-sha1', - 'RSA-SHA256': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256', - 'RSA-SHA384': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha384', - 'RSA-SHA512': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512', -} as const; +import type { SAMLPOSTEnvelope, SAMLRedirectEnvelope } from '../definition/SAMLEnvelope'; +import { getSigAlgKeyIfSupported, type SigAlgKey, signatureAlgorithms } from './signature/signatureAlgorithms'; export class SAMLServiceProvider { serviceProviderOptions: IServiceProviderOptions; @@ -36,14 +31,9 @@ export class SAMLServiceProvider { this.serviceProviderOptions = serviceProviderOptions; } - private getSignatureAlgorithm(): keyof typeof signatureAlgorithms { + private getSignatureAlgorithm(): SigAlgKey { const algorithm = `RSA-${this.serviceProviderOptions.signatureAlgorithm}`; - - if (algorithm in signatureAlgorithms) { - return algorithm as keyof typeof signatureAlgorithms; - } - - return 'RSA-SHA256'; + return getSigAlgKeyIfSupported(algorithm) || 'RSA-SHA256'; } private maybeSignRequest(samlObject: Record): Record { @@ -180,37 +170,25 @@ export class SAMLServiceProvider { return this.requestToUrl(request, 'authorize'); } - public async validateLogoutRequest(samlRequest: string, callback: ILogoutRequestValidateCallback): Promise { - await SAMLUtils.inflateXml( - samlRequest, - async (xml: string) => { - const parser = new LogoutRequestParser(this.serviceProviderOptions); - return parser.validate(xml, callback); - }, - async (err: string | object | null) => { - await callback(err, null); - }, - ); + public async validateLogoutRequest( + envelope: SAMLRedirectEnvelope<'SAMLRequest'>, + callback: ILogoutRequestValidateCallback, + ): Promise { + const parser = new LogoutRequestParser(this.serviceProviderOptions); + return parser.validate(envelope, callback); } - public async validateLogoutResponse(samlResponse: string, callback: ILogoutResponseValidateCallback): Promise { - await SAMLUtils.inflateXml( - samlResponse, - async (xml: string) => { - const parser = new LogoutResponseParser(this.serviceProviderOptions); - return parser.validate(xml, callback); - }, - async (err: string | object | null) => { - await callback(err, null); - }, - ); + public async validateLogoutResponse( + envelope: SAMLRedirectEnvelope<'SAMLResponse'>, + callback: ILogoutResponseValidateCallback, + ): Promise { + const parser = new LogoutResponseParser(this.serviceProviderOptions); + return parser.validate(envelope, callback); } - public validateResponse(samlResponse: string, callback: IResponseValidateCallback): void { - const xml = Buffer.from(samlResponse, 'base64').toString('utf8'); - + public validateResponse(envelope: SAMLPOSTEnvelope<'SAMLResponse'>, callback: IResponseValidateCallback): void { const parser = new ResponseParser(this.serviceProviderOptions); - return parser.validate(xml, callback); + return parser.validate(envelope, callback); } public generateServiceProviderMetadata(): string { diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/Utils.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/Utils.ts index ec8924c69a7f4..8d99fa18b70ae 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/lib/Utils.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/Utils.ts @@ -50,6 +50,10 @@ export class SAMLUtils { relayState = value; } + public static get logger(): Logger | undefined { + return logger; + } + public static getServiceProviderOptions(providerName: string): IServiceProviderOptions | undefined { this.log({ providerName, providerList }); @@ -145,25 +149,19 @@ export class SAMLUtils { } } - public static async inflateXml( - base64Data: string, - successCallback: (xml: string) => Promise, - errorCallback: (err: string | object | null) => Promise, - ): Promise { + public static async inflateXml(deflatedXml: Buffer): Promise> { return new Promise((resolve, reject) => { - const buffer = Buffer.from(base64Data, 'base64'); - zlib.inflateRaw(buffer, (err, decoded) => { + zlib.inflateRaw(deflatedXml, (err, inflatedXml) => { if (err) { this.log({ msg: 'Error while inflating.', err }); - return reject(errorCallback(err)); + return reject(err); } - if (!decoded) { - return reject(errorCallback('Failed to extract request data')); + if (!inflatedXml) { + return reject(new Error('Failed to extract request data')); } - const xmlString = this.convertArrayBufferToString(decoded); - return resolve(successCallback(xmlString)); + resolve(Buffer.from(inflatedXml)); }); }); } @@ -367,7 +365,7 @@ export class SAMLUtils { } } - if (mapping.regex && mainValue && mainValue.match) { + if (mapping.regex && mainValue?.match) { let regexValue; const match = mainValue.match(new RegExp(mapping.regex)); if (match?.length) { @@ -393,10 +391,6 @@ export class SAMLUtils { return mainValue; } - public static convertArrayBufferToString(buffer: Buffer, encoding: BufferEncoding = 'utf8'): string { - return Buffer.from(buffer).toString(encoding); - } - public static normalizeUsername(name: string): string { const { globalSettings } = this; diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/getSAMLEnvelope.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/getSAMLEnvelope.ts new file mode 100644 index 0000000000000..6e54417e7de00 --- /dev/null +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/getSAMLEnvelope.ts @@ -0,0 +1,120 @@ +import type { IIncomingMessage } from '@rocket.chat/core-typings'; + +import { SAMLUtils } from './Utils'; +import type { + SAMLEnvelope, + SAMLBinding, + SAMLDocumentType, + SAMLRedirectEnvelope, + SAMLPOSTEnvelope, + SAMLBaseEnvelope, +} from '../definition/SAMLEnvelope'; + +function getStringAttribute(params: Record, attributeName: string): string | null { + if (!(attributeName in params)) { + return null; + } + + if (typeof params[attributeName] !== 'string') { + return null; + } + + return params[attributeName]; +} + +async function performBindingSpecificDecoding(binding: SAMLBinding, buffer: Buffer): Promise> { + switch (binding) { + case 'HTTP-Redirect': + return SAMLUtils.inflateXml(buffer); + default: + return buffer; + } +} + +async function decodeDocument(base64Data: string, binding: SAMLBinding): Promise { + const buffer = await performBindingSpecificDecoding(binding, Buffer.from(base64Data, 'base64')); + + return buffer.toString('utf8'); +} + +function getSignedContent(req: IIncomingMessage, documentType: SAMLDocumentType): string | null { + // We load the signed content directly from the query string, because: + // 1. We need the raw value, without running decodeURIComponent or any other processing/normalization + // 2. We need to respect the order the params came in (although the order should be fixed) + const rawUrl = req.url; + if (!rawUrl) { + return null; + } + + const queryString = rawUrl.split('?')[1]; + if (!queryString) { + return null; + } + const params = queryString.split('&'); + + const signedContent = params + .filter((p) => p.startsWith(`${documentType}=`) || p.startsWith(`RelayState=`) || p.startsWith('SigAlg=')) + .join('&'); + + return signedContent || null; +} + +export async function getSAMLEnvelope( + req: IIncomingMessage, + type: T, + binding: 'HTTP-Redirect', +): Promise | null>; +export async function getSAMLEnvelope( + req: IIncomingMessage, + type: T, + binding: 'HTTP-POST', +): Promise | null>; +export async function getSAMLEnvelope( + req: IIncomingMessage, + type: T, + binding: B, +): Promise | null> { + const params = binding === 'HTTP-Redirect' ? req.query : req.body; + + if (!params) { + return null; + } + + const encodedDocument = getStringAttribute(params, type); + + if (!encodedDocument) { + return null; + } + + const decodedDocument = await decodeDocument(encodedDocument, binding); + + const relayState = getStringAttribute(params, 'RelayState'); + + const envelope: SAMLBaseEnvelope = { + binding, + type, + encodedDocument, + decodedDocument, + ...(typeof relayState === 'string' && { relayState }), + }; + + if (envelope.binding !== 'HTTP-Redirect') { + return envelope; + } + + const signedContent = getSignedContent(req, type); + + const sigAlg = getStringAttribute(params, 'SigAlg'); + const signature = getStringAttribute(params, 'Signature'); + + if (sigAlg && signature && signedContent) { + return { + ...envelope, + sigAlg, + signature, + signedContent, + }; + } + + return envelope; +} diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/LogoutRequest.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/LogoutRequest.ts index c609328ea21bf..917d29acced87 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/LogoutRequest.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/LogoutRequest.ts @@ -1,8 +1,10 @@ import xmldom from '@xmldom/xmldom'; import type { IServiceProviderOptions } from '../../definition/IServiceProviderOptions'; +import type { SAMLRedirectEnvelope } from '../../definition/SAMLEnvelope'; import type { ILogoutRequestValidateCallback } from '../../definition/callbacks'; import { SAMLUtils } from '../Utils'; +import { validateRedirectSignature } from '../signature/validateRedirectSignature'; export class LogoutRequestParser { serviceProviderOptions: IServiceProviderOptions; @@ -11,9 +13,16 @@ export class LogoutRequestParser { this.serviceProviderOptions = serviceProviderOptions; } - public async validate(xmlString: string, callback: ILogoutRequestValidateCallback): Promise { + public async validate(envelope: SAMLRedirectEnvelope<'SAMLRequest'>, callback: ILogoutRequestValidateCallback): Promise { + const { decodedDocument: xmlString } = envelope; + SAMLUtils.log({ msg: 'Validating SAML Logout Request', xmlString }); + if (!this.verifySignature(envelope)) { + SAMLUtils.log({ msg: 'Failed to verify signature from Logout Request' }); + return callback('Signature validation failed'); + } + const doc = new xmldom.DOMParser().parseFromString(xmlString, 'text/xml'); if (!doc) { return callback('No Doc Found'); @@ -46,4 +55,12 @@ export class LogoutRequestParser { return callback(err instanceof Error ? err : String(err), null); } } + + private verifySignature(envelope: SAMLRedirectEnvelope<'SAMLRequest'>): boolean { + if (!this.serviceProviderOptions.validateLogoutRequestSignature) { + return true; + } + + return validateRedirectSignature(envelope, this.serviceProviderOptions.cert); + } } diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/LogoutResponse.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/LogoutResponse.ts index af9c176233cdf..fa62aa4e3f640 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/LogoutResponse.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/LogoutResponse.ts @@ -1,8 +1,10 @@ import xmldom from '@xmldom/xmldom'; import type { IServiceProviderOptions } from '../../definition/IServiceProviderOptions'; +import type { SAMLRedirectEnvelope } from '../../definition/SAMLEnvelope'; import type { ILogoutResponseValidateCallback } from '../../definition/callbacks'; import { SAMLUtils } from '../Utils'; +import { validateRedirectSignature } from '../signature/validateRedirectSignature'; export class LogoutResponseParser { serviceProviderOptions: IServiceProviderOptions; @@ -11,9 +13,16 @@ export class LogoutResponseParser { this.serviceProviderOptions = serviceProviderOptions; } - public async validate(xmlString: string, callback: ILogoutResponseValidateCallback): Promise { + public async validate(envelope: SAMLRedirectEnvelope<'SAMLResponse'>, callback: ILogoutResponseValidateCallback): Promise { + const { decodedDocument: xmlString } = envelope; + SAMLUtils.log({ msg: 'Validating SAML Logout Response', xmlString }); + if (!this.verifySignature(envelope)) { + SAMLUtils.log({ msg: 'Failed to verify signature from Logout Response' }); + return callback('Signature validation failed'); + } + const doc = new xmldom.DOMParser().parseFromString(xmlString, 'text/xml'); if (!doc) { return callback('No Doc Found'); @@ -46,4 +55,12 @@ export class LogoutResponseParser { return callback(null, inResponseTo); } + + private verifySignature(envelope: SAMLRedirectEnvelope<'SAMLResponse'>): boolean { + if (!this.serviceProviderOptions.validateLogoutResponseSignature) { + return true; + } + + return validateRedirectSignature(envelope, this.serviceProviderOptions.cert); + } } diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/Response.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/Response.ts index 87aeb4ad9f6c8..dd6be1371d5f9 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/Response.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/parsers/Response.ts @@ -4,6 +4,7 @@ import xmlenc from 'xml-encryption'; import type { ISAMLAssertion } from '../../definition/ISAMLAssertion'; import type { IServiceProviderOptions } from '../../definition/IServiceProviderOptions'; +import type { SAMLPOSTEnvelope } from '../../definition/SAMLEnvelope'; import type { IResponseValidateCallback } from '../../definition/callbacks'; import { SAMLUtils } from '../Utils'; import { StatusCode } from '../constants'; @@ -17,7 +18,8 @@ export class ResponseParser { this.serviceProviderOptions = serviceProviderOptions; } - public validate(xml: string, callback: IResponseValidateCallback): void { + public validate(envelope: SAMLPOSTEnvelope<'SAMLResponse'>, callback: IResponseValidateCallback): void { + const { decodedDocument: xml } = envelope; // We currently use RelayState to save SAML provider SAMLUtils.log({ msg: 'Validating SAML Response', xml }); @@ -240,10 +242,6 @@ export class ResponseParser { } private verifySignatures(response: Element, assertionData: ISAMLAssertion, xml: string): void { - if (!this.serviceProviderOptions.cert) { - return; - } - const signatureType = this.serviceProviderOptions.signatureValidationType; const checkEither = signatureType === 'Either'; @@ -251,6 +249,14 @@ export class ResponseParser { const checkAssertion = signatureType === 'Assertion' || signatureType === 'All' || checkEither; let anyValidSignature = false; + if (!this.serviceProviderOptions.cert) { + if (checkResponse || checkAssertion) { + SAMLUtils.log('Missing Signature validation params'); + throw new Error('Unable to validate signature'); + } + return; + } + if (checkResponse) { SAMLUtils.log('Verify Document Signature'); if (!this.validateResponseSignature(xml, this.serviceProviderOptions.cert, response)) { diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/settings.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/settings.ts index dacdd014806e4..0316045acb5c8 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/lib/settings.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/settings.ts @@ -49,6 +49,8 @@ const getSamlConfigs = function (service: string): SAMLConfiguration { algorithm: settings.get(`${service}_signature_algorithm`) || 'SHA1', }, signatureValidationType: settings.get(`${service}_signature_validation_type`), + validateLogoutRequestSignature: settings.get(`${service}_validate_logout_request_signature`), + validateLogoutResponseSignature: settings.get(`${service}_validate_logout_response_signature`), userDataFieldMap: settings.get(`${service}_user_data_fieldmap`), allowedClockDrift: settings.get(`${service}_allowed_clock_drift`), customAuthnContext: defaultAuthnContext, @@ -69,6 +71,17 @@ const getSamlConfigs = function (service: string): SAMLConfiguration { return configs; }; +const isValidConfiguration = function (key: string, samlConfigs: SAMLConfiguration): boolean { + const needsCert = samlConfigs.signatureValidationType !== 'None'; + + if (!samlConfigs.secret.cert && needsCert) { + SAMLUtils.log({ msg: 'SAML Configuration missing Custom Certificate setting', key }); + return false; + } + + return true; +}; + const configureSamlService = function (samlConfigs: Record): IServiceProviderOptions { let privateCert = null; let privateKey = null; @@ -99,6 +112,8 @@ const configureSamlService = function (samlConfigs: Record): IServi defaultUserRole: samlConfigs.defaultUserRole, allowedClockDrift: parseInt(samlConfigs.allowedClockDrift) || 0, signatureValidationType: samlConfigs.signatureValidationType, + validateLogoutRequestSignature: samlConfigs.validateLogoutRequestSignature, + validateLogoutResponseSignature: samlConfigs.validateLogoutResponseSignature, identifierFormat: samlConfigs.identifierFormat, nameIDPolicyTemplate: samlConfigs.nameIDPolicyTemplate, authnContextTemplate: samlConfigs.authnContextTemplate, @@ -124,10 +139,15 @@ export const loadSamlServiceProviders = async function (): Promise { services.map(async ([key, value]) => { if (value === true) { const samlConfigs = getSamlConfigs(key); - SAMLUtils.log({ key }); - await LoginServiceConfiguration.createOrUpdateService(serviceName, samlConfigs); - void notifyOnLoginServiceConfigurationChangedByService(serviceName); - return configureSamlService(samlConfigs); + + if (isValidConfiguration(key, samlConfigs)) { + SAMLUtils.log({ msg: 'Loading SAML Provider', key }); + await LoginServiceConfiguration.createOrUpdateService(serviceName, samlConfigs); + void notifyOnLoginServiceConfigurationChangedByService(serviceName); + return configureSamlService(samlConfigs); + } + + SAMLUtils.logger?.warn({ msg: 'SAML Provider not loaded due to invalid configuration', key }); } const service = await LoginServiceConfiguration.findOneByService(serviceName, { projection: { _id: 1 } }); @@ -207,6 +227,7 @@ export const addSettings = async function (name: string): Promise { await this.add(`SAML_Custom_${name}_signature_validation_type`, 'All', { type: 'select', values: [ + { key: 'None', i18nLabel: 'SAML_Custom_signature_validation_none' }, { key: 'Response', i18nLabel: 'SAML_Custom_signature_validation_response' }, { key: 'Assertion', i18nLabel: 'SAML_Custom_signature_validation_assertion' }, { key: 'Either', i18nLabel: 'SAML_Custom_signature_validation_either' }, @@ -215,6 +236,14 @@ export const addSettings = async function (name: string): Promise { i18nLabel: 'SAML_Custom_signature_validation_type', i18nDescription: 'SAML_Custom_signature_validation_type_description', }); + await this.add(`SAML_Custom_${name}_validate_logout_request_signature`, true, { + type: 'boolean', + i18nLabel: 'SAML_Custom_validate_logout_request_signature', + }); + await this.add(`SAML_Custom_${name}_validate_logout_response_signature`, true, { + type: 'boolean', + i18nLabel: 'SAML_Custom_validate_logout_response_signature', + }); await this.add(`SAML_Custom_${name}_private_key`, '', { type: 'string', multiline: true, diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/signature/signatureAlgorithms.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/signature/signatureAlgorithms.ts new file mode 100644 index 0000000000000..90e0a60bf2629 --- /dev/null +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/signature/signatureAlgorithms.ts @@ -0,0 +1,38 @@ +export const signatureAlgorithms = { + 'RSA-SHA1': 'http://www.w3.org/2000/09/xmldsig#rsa-sha1', + 'RSA-SHA256': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256', + 'RSA-SHA384': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha384', + 'RSA-SHA512': 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512', +} as const; + +export type SigAlgKey = keyof typeof signatureAlgorithms; + +export type SigAlgURI = (typeof signatureAlgorithms)[SigAlgKey]; + +export function getSigAlgKeyIfSupported(key: SigAlgKey): SigAlgKey; +export function getSigAlgKeyIfSupported(key: string): SigAlgKey | null; +export function getSigAlgKeyIfSupported(key: string): SigAlgKey | null { + if (key in signatureAlgorithms) { + return key as keyof typeof signatureAlgorithms; + } + + return null; +} + +export function getSigAlgKeyByURI(algURI: SigAlgURI): SigAlgKey; +export function getSigAlgKeyByURI(algURI: string): SigAlgKey | null; +export function getSigAlgKeyByURI(algURI: string): SigAlgKey | null { + for (const [key, uri] of Object.entries(signatureAlgorithms)) { + if (uri === algURI) { + return key as SigAlgKey; + } + } + + return null; +} + +export function getSigAlgURIByKey(key: SigAlgKey): SigAlgURI; +export function getSigAlgURIByKey(key: string): SigAlgURI | null; +export function getSigAlgURIByKey(key: string): SigAlgURI | null { + return signatureAlgorithms[key as SigAlgKey] ?? null; +} diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/signature/validateRedirectSignature.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/signature/validateRedirectSignature.ts new file mode 100644 index 0000000000000..9dd73052b8214 --- /dev/null +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/signature/validateRedirectSignature.ts @@ -0,0 +1,44 @@ +import crypto from 'crypto'; + +import type { SAMLRedirectEnvelope } from '../../definition/SAMLEnvelope'; +import { SAMLUtils } from '../Utils'; +import { getSigAlgKeyByURI } from './signatureAlgorithms'; + +export function validateRedirectSignature(envelope: SAMLRedirectEnvelope, certificate: string): boolean { + try { + if (!envelope.sigAlg || !envelope.signature) { + SAMLUtils.log({ msg: 'Could not validate SAML Document Signature: Missing Signature params', type: envelope.type }); + return false; + } + + if (!envelope.signedContent) { + SAMLUtils.log({ msg: 'Could not validate SAML Document signature: signedContent is empty', type: envelope.type }); + return false; + } + + const algorithm = getSigAlgKeyByURI(envelope.sigAlg); + if (!algorithm) { + SAMLUtils.log({ msg: 'Could not validate SAML Document signature: invalid algorithm', type: envelope.type }); + return false; + } + + const cert = SAMLUtils.certToPEM(certificate); + + SAMLUtils.log({ + msg: 'Verifying Signatures for SAML Document', + algorithm, + cert, + type: envelope.type, + }); + const verifier = crypto.createVerify(algorithm); + + verifier.update(envelope.signedContent, 'utf8'); + verifier.end(); + + return verifier.verify(cert, envelope.signature, 'base64'); + } catch (err) { + SAMLUtils.error({ msg: 'Failed to validate SAML Document Signature', err }); + } + + return false; +} diff --git a/apps/meteor/app/meteor-accounts-saml/server/listener.ts b/apps/meteor/app/meteor-accounts-saml/server/listener.ts index 9af8c4d84c2d1..8747bd59a8714 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/listener.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/listener.ts @@ -66,11 +66,13 @@ const middleware = async function (req: express.Request, res: ServerResponse, ne // @ToDo: Ideally we should send some error message to the client, but there's no way to do it on a redirect right now. SystemLogger.error({ err }); - const url = Meteor.absoluteUrl('home'); - res.writeHead(302, { - Location: url, - }); - res.end(); + if (!res.headersSent) { + const url = Meteor.absoluteUrl('home'); + res.writeHead(302, { + Location: url, + }); + res.end(); + } } }; diff --git a/apps/meteor/app/meteor-accounts-saml/server/loginHandler.ts b/apps/meteor/app/meteor-accounts-saml/server/loginHandler.ts index e9b861b4a511a..8b2059489931c 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/loginHandler.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/loginHandler.ts @@ -1,3 +1,4 @@ +import { CredentialTokens } from '@rocket.chat/models'; import { Accounts } from 'meteor/accounts-base'; import { Meteor } from 'meteor/meteor'; @@ -12,11 +13,18 @@ const makeError = (message: string): Record => ({ }); Accounts.registerLoginHandler('saml', async (loginRequest) => { - if (!loginRequest.saml || !loginRequest.credentialToken || typeof loginRequest.credentialToken !== 'string') { + if ( + !loginRequest.saml || + !loginRequest.credentialToken || + typeof loginRequest.credentialToken !== 'string' || + SAMLUtils.serviceProviders.length === 0 + ) { return undefined; } const loginResult = await SAML.retrieveCredential(loginRequest.credentialToken); + + await CredentialTokens.removeById(loginRequest.credentialToken); SAMLUtils.log({ msg: 'RESULT', loginResult }); if (!loginResult) { diff --git a/apps/meteor/server/lib/cas/loginHandler.spec.ts b/apps/meteor/server/lib/cas/loginHandler.spec.ts new file mode 100644 index 0000000000000..8ba620d2c7f54 --- /dev/null +++ b/apps/meteor/server/lib/cas/loginHandler.spec.ts @@ -0,0 +1,56 @@ +import { expect } from 'chai'; +import { describe, it, beforeEach } from 'mocha'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +const findOneNotExpiredById = sinon.stub().resolves(null); +const removeById = sinon.stub().resolves(); +const findExistingCASUser = sinon.stub().resolves(null); +const settingsGet = sinon.stub().returns(true); + +const { loginHandlerCAS: handler } = proxyquire.noCallThru().load('./loginHandler', { + '@rocket.chat/models': { + CredentialTokens: { findOneNotExpiredById, removeById }, + Users: { updateOne: sinon.stub().resolves() }, + }, + 'meteor/accounts-base': { + Accounts: { + LoginCancelledError: { numericError: 403 }, + }, + }, + 'meteor/meteor': { + Meteor: { Error }, + }, + './createNewUser': { createNewUser: sinon.stub().resolves({ _id: 'newUserId' }) }, + './findExistingCASUser': { findExistingCASUser }, + './logger': { logger: { debug: sinon.stub(), error: sinon.stub() } }, + '../../../app/lib/server/functions/setRealName': { setRealName: sinon.stub().resolves() }, + '../../../app/settings/server': { settings: { get: settingsGet } }, +}); + +describe('loginHandlerCAS', () => { + beforeEach(() => { + findOneNotExpiredById.reset(); + removeById.reset(); + findExistingCASUser.reset(); + settingsGet.reset(); + settingsGet.returns(true); + }); + + it('should reject non-string credentialToken and never query the database (NoSQL injection prevention)', async () => { + expect(await handler({ cas: { credentialToken: { $gt: '' } } })).to.be.undefined; + expect(await handler({ cas: { credentialToken: { $ne: null } } })).to.be.undefined; + expect(await handler({ cas: { credentialToken: 123 } })).to.be.undefined; + expect(await handler({ cas: { credentialToken: ['a'] } })).to.be.undefined; + expect(await handler({ cas: { credentialToken: null } })).to.be.undefined; + + expect(findOneNotExpiredById.called).to.be.false; + }); + + it('should return undefined when CAS is disabled', async () => { + settingsGet.returns(false); + + expect(await handler({ cas: { credentialToken: 'valid-token' } })).to.be.undefined; + expect(findOneNotExpiredById.called).to.be.false; + }); +}); diff --git a/apps/meteor/server/lib/cas/loginHandler.ts b/apps/meteor/server/lib/cas/loginHandler.ts index 9bc2c31a9e245..4cf4b78683a88 100644 --- a/apps/meteor/server/lib/cas/loginHandler.ts +++ b/apps/meteor/server/lib/cas/loginHandler.ts @@ -9,7 +9,7 @@ import { setRealName } from '../../../app/lib/server/functions/setRealName'; import { settings } from '../../../app/settings/server'; export const loginHandlerCAS = async (options: any): Promise => { - if (!options.cas) { + if (!settings.get('CAS_enabled') || !options.cas || typeof options.cas.credentialToken !== 'string') { return undefined; } @@ -19,6 +19,8 @@ export const loginHandlerCAS = async (options: any): Promise('CAS_Sync_User_Data_FieldMap').trim(); const casVersion = parseFloat(settings.get('CAS_version') ?? '1.0'); diff --git a/apps/meteor/server/oauth2-server/oauth.ts b/apps/meteor/server/oauth2-server/oauth.ts index e52e5fabfac4f..45886a2d80807 100644 --- a/apps/meteor/server/oauth2-server/oauth.ts +++ b/apps/meteor/server/oauth2-server/oauth.ts @@ -79,18 +79,40 @@ export class OAuth2Server { return next(); }; - this.app.all('/oauth/token', debugMiddleware, transformRequestsNotUsingFormUrlencodedType, async (req, res, next) => { - const request = new OAuthServer.Request(req); - const response = new OAuthServer.Response(res); - - try { - await oauth.token(request, response); - - handleResponse(res, response, next); - } catch (e: any) { - next(e); + const validateTokenPayload = function (req: Request, res: Response, next: NextFunction) { + const grantParams = ['client_id', 'client_secret', 'refresh_token', 'code', 'grant_type', 'scope', 'redirect_uri']; + + for (const param of grantParams) { + for (const source of [req.body, req.query]) { + if (source?.[param] !== undefined && typeof source[param] !== 'string') { + return res.status(400).send({ + error: 'invalid_request', + error_description: `Invalid parameter: ${param} must be a string`, + }); + } + } } - }); + return next(); + }; + + this.app.all( + '/oauth/token', + debugMiddleware, + transformRequestsNotUsingFormUrlencodedType, + validateTokenPayload, + async (req, res, next) => { + const request = new OAuthServer.Request(req); + const response = new OAuthServer.Response(res); + + try { + await oauth.token(request, response); + + handleResponse(res, response, next); + } catch (e: any) { + next(e); + } + }, + ); this.app.get('/oauth/authorize', debugMiddleware, async (req, res, next) => { if (typeof req.query.client_id !== 'string') { diff --git a/apps/meteor/tests/e2e/containers/saml/Dockerfile b/apps/meteor/tests/e2e/containers/saml/Dockerfile index bf997ae6aacf7..0401d7e09181a 100644 --- a/apps/meteor/tests/e2e/containers/saml/Dockerfile +++ b/apps/meteor/tests/e2e/containers/saml/Dockerfile @@ -1,4 +1,4 @@ -FROM php:7.3-apache +FROM php:7.4-apache # Utilities RUN apt-get update && \ @@ -6,7 +6,7 @@ RUN apt-get update && \ rm -r /var/lib/apt/lists/* # SimpleSAMLphp -ARG SIMPLESAMLPHP_VERSION=1.15.4 +ARG SIMPLESAMLPHP_VERSION=1.19.6 RUN curl -s -L -o /tmp/simplesamlphp.tar.gz https://github.com/simplesamlphp/simplesamlphp/releases/download/v$SIMPLESAMLPHP_VERSION/simplesamlphp-$SIMPLESAMLPHP_VERSION.tar.gz && \ tar xzf /tmp/simplesamlphp.tar.gz -C /tmp && \ rm -f /tmp/simplesamlphp.tar.gz && \ @@ -14,6 +14,7 @@ RUN curl -s -L -o /tmp/simplesamlphp.tar.gz https://github.com/simplesamlphp/sim touch /var/www/simplesamlphp/modules/exampleauth/enable COPY config/simplesamlphp/config.php /var/www/simplesamlphp/config COPY config/simplesamlphp/authsources.php /var/www/simplesamlphp/config +COPY config/simplesamlphp/saml20-idp-hosted.php /var/www/simplesamlphp/metadata COPY config/simplesamlphp/saml20-sp-remote.php /var/www/simplesamlphp/metadata COPY config/simplesamlphp/server_crt /var/www/simplesamlphp/cert/server.crt COPY config/simplesamlphp/server_pem /var/www/simplesamlphp/cert/server.pem diff --git a/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/config.php b/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/config.php index ed9c74b2609f4..71b67214d2982 100644 --- a/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/config.php +++ b/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/config.php @@ -44,6 +44,7 @@ 'session.cookie.path' => '/', 'session.cookie.domain' => null, 'session.cookie.secure' => false, + 'session.cookie.samesite' => 'Lax', 'enable.http_post' => false, 'session.phpsession.cookiename' => 'PHPSESSIDIDP', 'session.phpsession.savepath' => null, diff --git a/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/saml20-idp-hosted.php b/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/saml20-idp-hosted.php new file mode 100644 index 0000000000000..c72fa97803cc9 --- /dev/null +++ b/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/saml20-idp-hosted.php @@ -0,0 +1,23 @@ + 'http://localhost:3000/_saml/metadata/test-sp', + 'host' => '__DEFAULT__', + 'privatekey' => 'server.pem', + 'certificate' => 'server.crt', + 'auth' => 'example-userpass', + 'redirect.sign' => true, + + /* + * Force the NameID to be the raw email attribute value in emailAddress + * format, bypassing SimpleSAMLphp's default persistent (hashed) nameID + * generation. + */ + 'authproc' => [ + 100 => [ + 'class' => 'saml:AttributeNameID', + 'attribute' => 'email', + 'Format' => 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress', + ], + ], +]; diff --git a/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/saml20-sp-remote.php b/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/saml20-sp-remote.php index 7b0dd4b49eaba..4e306b29b97aa 100644 --- a/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/saml20-sp-remote.php +++ b/apps/meteor/tests/e2e/containers/saml/config/simplesamlphp/saml20-sp-remote.php @@ -22,9 +22,9 @@ ), 'NameIDFormat' => 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress', // Other settings that may be useful for testing, but are not compatible with the existing e2e tests - // 'privatekey' => 'server.pem', - // 'certificate' => 'server.crt', - // 'redirect.sign' => true, + 'privatekey' => 'server.pem', + 'certificate' => 'server.crt', + 'redirect.sign' => true, // 'redirect.validate' => true, // 'validate.logout' => true, // 'assertion.encryption' => true, diff --git a/apps/meteor/tests/e2e/saml.spec.ts b/apps/meteor/tests/e2e/saml.spec.ts index 50d59d1b8b402..d3239219e4a50 100644 --- a/apps/meteor/tests/e2e/saml.spec.ts +++ b/apps/meteor/tests/e2e/saml.spec.ts @@ -14,6 +14,7 @@ import { convertHexToRGB } from './utils/convertHexToRGB'; import { createCustomRole, deleteCustomRole } from './utils/custom-role'; import { getUserInfo } from './utils/getUserInfo'; import { parseMeteorResponse } from './utils/parseMeteorResponse'; +import { saveSettings } from './utils/saveSettings'; import { setSettingValueById } from './utils/setSettingValueById'; import type { BaseTest } from './utils/test'; import { test, expect } from './utils/test'; @@ -70,8 +71,12 @@ const resetTestData = async ({ api, cleanupOnly = false }: { api?: any; cleanupO { _id: 'SAML_Custom_Default_mail_overwrite', value: false }, { _id: 'SAML_Custom_Default_name_overwrite', value: false }, { _id: 'SAML_Custom_Default', value: false }, - { _id: 'SAML_Custom_Default_role_attribute_sync', value: true }, - { _id: 'SAML_Custom_Default_role_attribute_name', value: 'role' }, + ...(constants.IS_EE + ? [ + { _id: 'SAML_Custom_Default_role_attribute_sync', value: true }, + { _id: 'SAML_Custom_Default_role_attribute_name', value: 'role' }, + ] + : []), { _id: 'SAML_Custom_Default_user_data_fieldmap', value: '{"username":"username", "email":"email", "name": "cn"}' }, { _id: 'SAML_Custom_Default_provider', value: 'test-sp' }, { _id: 'SAML_Custom_Default_issuer', value: 'http://localhost:3000/_saml/metadata/test-sp' }, @@ -79,9 +84,34 @@ const resetTestData = async ({ api, cleanupOnly = false }: { api?: any; cleanupO { _id: 'SAML_Custom_Default_idp_slo_redirect_url', value: 'http://localhost:8080/simplesaml/saml2/idp/SingleLogoutService.php' }, { _id: 'SAML_Custom_Default_button_label_text', value: 'SAML test login button' }, { _id: 'SAML_Custom_Default_button_color', value: '#185925' }, + { _id: 'SAML_Custom_Default_signature_validation_type', value: 'All' }, + { _id: 'SAML_Custom_Default_validate_logout_request_signature', value: true }, + { _id: 'SAML_Custom_Default_validate_logout_response_signature', value: true }, + { + _id: 'SAML_Custom_Default_cert', + value: `MIIDXTCCAkWgAwIBAgIJALmVVuDWu4NYMA0GCSqGSIb3DQEBCwUAMEUxCzAJBgNV +BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX +aWRnaXRzIFB0eSBMdGQwHhcNMTYxMjMxMTQzNDQ3WhcNNDgwNjI1MTQzNDQ3WjBF +MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50 +ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB +CgKCAQEAzUCFozgNb1h1M0jzNRSCjhOBnR+uVbVpaWfXYIR+AhWDdEe5ryY+Cgav +Og8bfLybyzFdehlYdDRgkedEB/GjG8aJw06l0qF4jDOAw0kEygWCu2mcH7XOxRt+ +YAH3TVHa/Hu1W3WjzkobqqqLQ8gkKWWM27fOgAZ6GieaJBN6VBSMMcPey3HWLBmc ++TYJmv1dbaO2jHhKh8pfKw0W12VM8P1PIO8gv4Phu/uuJYieBWKixBEyy0lHjyix +YFCR12xdh4CA47q958ZRGnnDUGFVE1QhgRacJCOZ9bd5t9mr8KLaVBYTCJo5ERE8 +jymab5dPqe5qKfJsCZiqWglbjUo9twIDAQABo1AwTjAdBgNVHQ4EFgQUxpuwcs/C +YQOyui+r1G+3KxBNhxkwHwYDVR0jBBgwFoAUxpuwcs/CYQOyui+r1G+3KxBNhxkw +DAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAAiWUKs/2x/viNCKi3Y6b +lEuCtAGhzOOZ9EjrvJ8+COH3Rag3tVBWrcBZ3/uhhPq5gy9lqw4OkvEws99/5jFs +X1FJ6MKBgqfuy7yh5s1YfM0ANHYczMmYpZeAcQf2CGAaVfwTTfSlzNLsF2lW/ly7 +yapFzlYSJLGoVE+OHEu8g5SlNACUEfkXw+5Eghh+KzlIN7R6Q7r2ixWNFBC/jWf7 +NKUfJyX8qIG5md1YUeT6GBW9Bm2/1/RiO24JTaYlfLdKK9TYb8sG5B+OLab2DImG +99CJ25RkAcSobWNF5zD0O6lgOo3cEdB/ksCq3hmtlC/DlLZ/D8CJ+7VuZnS1rR2n +aQ==`, + }, ]; - await Promise.all(settings.map(({ _id, value }) => setSettingValueById(api, _id, value))); + await saveSettings(api, settings); }; const setupCustomRole = async (api: BaseTest['api']) => { @@ -238,6 +268,10 @@ test.describe('SAML', () => { }); }); + const doChangeSetting = async (api: any, settingId: string, settingValue: any) => { + await expect((await setSettingValueById(api, settingId, settingValue)).status()).toBe(200); + }; + const doLoginStep = async (page: Page, username: string, redirectUrl: string | null = '/home') => { await test.step('expect successful login', async () => { await poRegistration.btnLoginWithSaml.click(); @@ -321,6 +355,20 @@ test.describe('SAML', () => { }); }); + test('Logout - From IdP', async ({ page }) => { + await page.goto('/home'); + await doLoginStep(page, 'samluser1'); + + // This should trigger a logout request from the IdP, with a redirect to our home on success + await page.goto('http://localhost:8080/simplesaml/saml2/idp/SingleLogoutService.php?ReturnTo=http://localhost:3000'); + + await test.step('expect user to be logged out from Rocket.Chat', async () => { + await expect(page).toHaveURL('/home'); + await expect(page.getByRole('button', { name: 'User menu' })).not.toBeVisible(); + await expect(poRegistration.btnLoginWithSaml).toBeVisible(); + }); + }); + test('User Merge - By Email', async ({ page, api }) => { await test.step('Configure SAML to identify users by email', async () => { await expect((await setSettingValueById(api, 'SAML_Custom_Default_immutable_property', 'EMail')).status()).toBe(200); @@ -576,8 +624,87 @@ test.describe('SAML', () => { // Test user merge with a custom identifier configured in the fieldmap }); - test.fixme('Signature Validation', async () => { - // Test login with signed responses + test.describe('Logout Request Signature Validation', () => { + // This is a valid logout request for user samluser1 + const logoutRequest = + 'http://localhost:3000/_saml/logout/test-sp/?SAMLRequest=lZJBi9swEIX%2FitE9sWRbTiIS00AoBLa7bVN66CWMpVHXYEuuNIb8%2FMpOF7YtXehNPM379GZG%2BwhDP6oH%2F91P9Bl%2FTBgpuw29i2q5ObApOOUhdlE5GDAq0upy%2FPCgijVXY%2FDkte%2FZK8vbDogRA3Xesex8OrBrKTd8V1UaWiNbu%2BPSbrmQtUCorN5IgwWgMHYDBcu%2BYojJeWAJlOwxTnh2kcBRknhRr7hc8eqLqBUvlRTfWHZK3XQOaHE9E40qz3uvoX%2F2kVTJOc%2Bvc%2BYkzv3nlOpXccxZ9ujpyT2FoyUMf9G3C73Zz1a15AjNW%2FQBCQwQvPD3%2BWvnHfOYRnU%2BZZeP8%2BHTBH1nu%2Fnp%2F%2BGy7L0PA9C%2FVyDWYlE6s7JLqcIBuv5oTMAYWTNjp7Qh8Q5vMIw9rrUffsW9J7zHHdUl1aexnp3BW3MtSmnLtkBbbrGuQYPQG2NbY2SlUeykgLLd6dYU8g77w%2F8i%2FvYNm58%3D&RelayState=_658ecfea419894f995dd3d5bc1dedc511ab252a0ad&SigAlg=http%3A%2F%2Fwww.w3.org%2F2001%2F04%2Fxmldsig-more%23rsa-sha256'; + const logoutRequestSignature = + 'MM9fh6cesrjADYxkq9dr0aoyRxjG%2Fsi7lFyqjNex0F80sosmjZedwx%2FZC4iIr1DfK9ac7EuloXpp2sn%2BgLbLrsoo2kqY3AS5%2FQ6zQVpIdg%2BhAk5CcpGXmY%2B8CqvL2QsUnMv9nHuemXzgoSjwnvGeTlpBiuGCHFmcfFFBUVgq1sZO4JM43d5JUojhVI4nEAGPhbOiHFSvKB09pE1pOu6sDhSepb6sAbBIrRfZPW7DLcSGe1DoY1x6vRw%2F3hV88Z9WwZJmogJ69lhQOuv%2BbZwC%2BEWVIgre38GnbLkXOiifMUJnkxkHcCvku7ALhUD%2BPwPEaslFwjIb2eIN0PJUZcKJCg%3D%3D'; + + test.describe('Require Signature', () => { + test.beforeAll(async ({ api }) => { + await doChangeSetting(api, 'SAML_Custom_Default_validate_logout_request_signature', true); + }); + + test('Reject Invalid Signature on Logout Request', async ({ page }) => { + await page.goto('/home'); + await doLoginStep(page, 'samluser1'); + + await page.goto(`${logoutRequest}&Signature=invalid`); + + await test.step('expect to be redirected back to rocket.chat without being logged out', async () => { + await expect(page).toHaveURL('/home'); + await expect(page.getByRole('button', { name: 'User menu' })).toBeVisible(); + }); + }); + + test('Reject Missing Signature on Logout Request', async ({ page }) => { + await page.goto('/home'); + await doLoginStep(page, 'samluser1'); + + await page.goto(logoutRequest); + + await test.step('expect to be redirected back to rocket.chat without being logged out', async () => { + await expect(page).toHaveURL('/home'); + await expect(page.getByRole('button', { name: 'User menu' })).toBeVisible(); + }); + }); + + test('Accept Valid Signature on Logout Request', async ({ page }) => { + await page.goto('/home'); + await doLoginStep(page, 'samluser1'); + + await page.goto(`${logoutRequest}&Signature=${logoutRequestSignature}`); + + await test.step('expect user to be logged out from Rocket.Chat', async () => { + await expect(page).toHaveURL('/home'); + await expect(page.getByRole('button', { name: 'User menu' })).not.toBeVisible(); + await expect(poRegistration.btnLoginWithSaml).toBeVisible(); + }); + }); + }); + + test.describe('Do not require Signature', () => { + test.beforeAll(async ({ api }) => { + await doChangeSetting(api, 'SAML_Custom_Default_validate_logout_request_signature', false); + }); + + test('Ignore Invalid Signature on Logout Request', async ({ page }) => { + await page.goto('/home'); + await doLoginStep(page, 'samluser1'); + + await page.goto(`${logoutRequest}&Signature=invalid`); + + await test.step('expect user to be logged out from Rocket.Chat', async () => { + await expect(page).toHaveURL('/home'); + await expect(page.getByRole('button', { name: 'User menu' })).not.toBeVisible(); + await expect(poRegistration.btnLoginWithSaml).toBeVisible(); + }); + }); + + test('Ignore Missing Signature on Logout Request', async ({ page }) => { + await page.goto('/home'); + await doLoginStep(page, 'samluser1'); + + await page.goto(logoutRequest); + + await test.step('expect user to be logged out from Rocket.Chat', async () => { + await expect(page).toHaveURL('/home'); + await expect(page.getByRole('button', { name: 'User menu' })).not.toBeVisible(); + await expect(poRegistration.btnLoginWithSaml).toBeVisible(); + }); + }); + }); }); test.fixme('Login - User without username', async () => { diff --git a/apps/meteor/tests/e2e/utils/saveSettings.ts b/apps/meteor/tests/e2e/utils/saveSettings.ts new file mode 100644 index 0000000000000..db44ea6ff8915 --- /dev/null +++ b/apps/meteor/tests/e2e/utils/saveSettings.ts @@ -0,0 +1,20 @@ +import type { APIResponse } from '@playwright/test'; +import type { ISetting } from '@rocket.chat/core-typings'; + +import type { BaseTest } from './test'; + +export const saveSettings = ( + api: BaseTest['api'], + changes: { + _id: ISetting['_id']; + value: ISetting['value']; + }[], +): Promise => + api.post('/method.call/saveSettings', { + message: JSON.stringify({ + msg: 'method', + id: '1', + method: 'saveSettings', + params: [changes], + }), + }); diff --git a/apps/meteor/tests/end-to-end/api/oauth-server.ts b/apps/meteor/tests/end-to-end/api/oauth-server.ts index 4a0121734c247..44a9af2be7323 100644 --- a/apps/meteor/tests/end-to-end/api/oauth-server.ts +++ b/apps/meteor/tests/end-to-end/api/oauth-server.ts @@ -106,6 +106,37 @@ describe('[OAuth Server]', () => { }); }); + it('should return bad request if payload has non string parameters in refresh_token grant', async () => { + await request + .post(`/oauth/token`) + .send({ + grant_type: 'refresh_token', + client_id: { $ne: null }, + client_secret: { $ne: null }, + refresh_token: { $ne: null }, + }) + .expect((res: Response) => { + expect(res.status).to.be.equal(400); + expect(res.body).to.have.property('error').that.is.a('string').and.equal('invalid_request'); + }); + }); + + it('should return bad request if payload has non string parameters in authorization_code grant', async () => { + await request + .post(`/oauth/token`) + .send({ + grant_type: 'authorization_code', + client_id: { $ne: null }, + client_secret: { $ne: null }, + code: { $ne: null }, + redirect_uri: { $ne: null }, + }) + .expect((res: Response) => { + expect(res.status).to.be.equal(400); + expect(res.body).to.have.property('error').that.is.a('string').and.equal('invalid_request'); + }); + }); + it('should be able to refresh the access_token', async () => { await request .post(`/oauth/token`) diff --git a/apps/meteor/tests/unit/app/meteor-accounts-saml/data.ts b/apps/meteor/tests/unit/app/meteor-accounts-saml/data.ts index ab145dbbc0f15..7a0c41c7667cc 100644 --- a/apps/meteor/tests/unit/app/meteor-accounts-saml/data.ts +++ b/apps/meteor/tests/unit/app/meteor-accounts-saml/data.ts @@ -13,7 +13,9 @@ export const serviceProviderOptions: IServiceProviderOptions = { authnContextComparison: 'Whatever', defaultUserRole: 'user', allowedClockDrift: 0, - signatureValidationType: 'All', + signatureValidationType: 'None', + validateLogoutRequestSignature: true, + validateLogoutResponseSignature: true, identifierFormat: 'email', nameIDPolicyTemplate: '', authnContextTemplate: '__authnContext__', diff --git a/apps/meteor/tests/unit/app/meteor-accounts-saml/helpers.ts b/apps/meteor/tests/unit/app/meteor-accounts-saml/helpers.ts new file mode 100644 index 0000000000000..be2c1ba020270 --- /dev/null +++ b/apps/meteor/tests/unit/app/meteor-accounts-saml/helpers.ts @@ -0,0 +1,47 @@ +import type { + SAMLDocumentType, + SAMLPOSTEnvelope, + SAMLRedirectEnvelope, +} from '../../../../app/meteor-accounts-saml/server/definition/SAMLEnvelope'; + +export function makeLogoutEnvelope( + type: T, + xml: string, + signedContent?: string, + signature?: string, +): SAMLRedirectEnvelope { + return { + binding: 'HTTP-Redirect', + encodedDocument: xml, + decodedDocument: xml, + type, + relayState: 'relayState', + ...(signedContent && { signedContent }), + ...(signature && { + signature, + sigAlg: 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256', + }), + }; +} + +export function makeLogoutRequestEnvelope(xml: string, signedContent?: string, signature?: string) { + return makeLogoutEnvelope('SAMLRequest', xml, signedContent, signature); +} + +export function makeLogoutResponseEnvelope(xml: string, signedContent?: string, signature?: string) { + return makeLogoutEnvelope('SAMLResponse', xml, signedContent, signature); +} + +export function makeLoginEnvelope(type: T, xml: string): SAMLPOSTEnvelope { + return { + binding: 'HTTP-POST', + encodedDocument: xml, + decodedDocument: xml, + type, + relayState: 'relayState', + }; +} + +export function makeLoginResponseEnvelope(xml: string) { + return makeLoginEnvelope('SAMLResponse', xml); +} diff --git a/apps/meteor/tests/unit/app/meteor-accounts-saml/loginHandler.spec.ts b/apps/meteor/tests/unit/app/meteor-accounts-saml/loginHandler.spec.ts new file mode 100644 index 0000000000000..42e3ba8cde170 --- /dev/null +++ b/apps/meteor/tests/unit/app/meteor-accounts-saml/loginHandler.spec.ts @@ -0,0 +1,74 @@ +import { expect } from 'chai'; +import { describe, it, beforeEach } from 'mocha'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +const retrieveCredential = sinon.stub().resolves(null); +const removeById = sinon.stub().resolves(); +const samlUtilsMock = { + serviceProviders: [{ provider: 'test-saml' }] as any[], + log: sinon.stub(), + mapProfileToUserObject: sinon.stub(), + events: { emit: sinon.stub() }, +}; + +const handler = sinon.stub(); +proxyquire.noCallThru().load('../../../../app/meteor-accounts-saml/server/loginHandler', { + '@rocket.chat/models': { + CredentialTokens: { removeById }, + }, + 'meteor/accounts-base': { + Accounts: { + LoginCancelledError: { numericError: 403 }, + registerLoginHandler: (_name: string, fn: any) => { + handler.callsFake(fn); + }, + }, + }, + 'meteor/meteor': { + Meteor: { Error }, + }, + './lib/SAML': { + SAML: { retrieveCredential }, + }, + './lib/Utils': { + SAMLUtils: samlUtilsMock, + }, + '../../../server/lib/i18n': { i18n: { t: sinon.stub().returns('') } }, + '../../../server/lib/logger/system': { SystemLogger: { error: sinon.stub() } }, +}); + +describe('SAML loginHandler', () => { + beforeEach(() => { + retrieveCredential.reset(); + retrieveCredential.resolves(null); + removeById.reset(); + removeById.resolves(); + samlUtilsMock.serviceProviders = [{ provider: 'test-saml' }]; + }); + + it('should reject non-string credentialToken and never query the database (NoSQL injection prevention)', async () => { + expect(await handler({ saml: true, credentialToken: { $gt: '' } })).to.be.undefined; + expect(await handler({ saml: true, credentialToken: { $ne: null } })).to.be.undefined; + expect(await handler({ saml: true, credentialToken: { $not: { $eq: '__nonexistent__' } } })).to.be.undefined; + expect(await handler({ saml: true, credentialToken: 123 })).to.be.undefined; + expect(await handler({ saml: true, credentialToken: ['a'] })).to.be.undefined; + expect(await handler({ saml: true, credentialToken: null })).to.be.undefined; + + expect(retrieveCredential.called).to.be.false; + }); + + it('should return undefined when no SAML providers are configured', async () => { + samlUtilsMock.serviceProviders = []; + + expect(await handler({ saml: true, credentialToken: 'valid-token' })).to.be.undefined; + expect(retrieveCredential.called).to.be.false; + }); + + it('should delete the credential token after retrieval', async () => { + await handler({ saml: true, credentialToken: 'token-to-delete' }); + + expect(removeById.calledOnce).to.be.true; + expect(removeById.calledWith('token-to-delete')).to.be.true; + }); +}); diff --git a/apps/meteor/tests/unit/app/meteor-accounts-saml/server.tests.ts b/apps/meteor/tests/unit/app/meteor-accounts-saml/server.tests.ts index effb3bee3a00f..fb972157db7ce 100644 --- a/apps/meteor/tests/unit/app/meteor-accounts-saml/server.tests.ts +++ b/apps/meteor/tests/unit/app/meteor-accounts-saml/server.tests.ts @@ -28,6 +28,7 @@ import { privateKeyCert, privateKey, } from './data'; +import { makeLoginResponseEnvelope, makeLogoutRequestEnvelope, makeLogoutResponseEnvelope } from './helpers'; import { SAMLUtils } from '../../../../app/meteor-accounts-saml/server/lib/Utils'; import { AuthorizeRequest } from '../../../../app/meteor-accounts-saml/server/lib/generators/AuthorizeRequest'; import { LogoutRequest } from '../../../../app/meteor-accounts-saml/server/lib/generators/LogoutRequest'; @@ -140,7 +141,7 @@ describe('SAML', () => { it('should extract the idpSession and nameID from the request', () => { const parser = new LogoutRequestParser(serviceProviderOptions); - void parser.validate(simpleLogoutRequest, async (err, data) => { + void parser.validate(makeLogoutRequestEnvelope(simpleLogoutRequest), async (err, data) => { expect(err).to.be.null; expect(data).to.be.an('object'); expect(data).to.have.property('idpSession'); @@ -154,7 +155,7 @@ describe('SAML', () => { it('should fail to parse an invalid xml', () => { const parser = new LogoutRequestParser(serviceProviderOptions); - void parser.validate(invalidXml, async (err, data) => { + void parser.validate(makeLogoutRequestEnvelope(invalidXml), async (err, data) => { expect(err).to.exist; expect(data).to.not.exist; }); @@ -162,7 +163,7 @@ describe('SAML', () => { it('should fail to parse a xml without any LogoutRequest tag', () => { const parser = new LogoutRequestParser(serviceProviderOptions); - void parser.validate(randomXml, async (err, data) => { + void parser.validate(makeLogoutRequestEnvelope(randomXml), async (err, data) => { expect(err).to.be.equal('No Request Found'); expect(data).to.not.exist; }); @@ -171,7 +172,7 @@ describe('SAML', () => { it('should fail to parse a request with no NameId', () => { const parser = new LogoutRequestParser(serviceProviderOptions); - void parser.validate(invalidLogoutRequest, async (err, data) => { + void parser.validate(makeLogoutRequestEnvelope(invalidLogoutRequest), async (err, data) => { expect(err).to.be.an('error').that.has.property('message').equal('SAML Logout Request: No NameID node found'); expect(data).to.not.exist; }); @@ -212,7 +213,7 @@ describe('SAML', () => { const logoutResponse = simpleLogoutResponse.replace('[STATUSCODE]', 'urn:oasis:names:tc:SAML:2.0:status:Success'); const parser = new LogoutResponseParser(serviceProviderOptions); - void parser.validate(logoutResponse, async (err, inResponseTo) => { + void parser.validate(makeLogoutResponseEnvelope(logoutResponse), async (err, inResponseTo) => { expect(err).to.be.null; expect(inResponseTo).to.be.equal('_id-6530db3fcd23dc42a31c'); }); @@ -222,7 +223,7 @@ describe('SAML', () => { const logoutResponse = simpleLogoutResponse.replace('[STATUSCODE]', 'Anything'); const parser = new LogoutResponseParser(serviceProviderOptions); - void parser.validate(logoutResponse, async (err, inResponseTo) => { + void parser.validate(makeLogoutResponseEnvelope(logoutResponse), async (err, inResponseTo) => { expect(err).to.be.equal('Error. Logout not confirmed by IDP'); expect(inResponseTo).to.be.null; }); @@ -230,7 +231,7 @@ describe('SAML', () => { it('should fail to parse an invalid xml', () => { const parser = new LogoutResponseParser(serviceProviderOptions); - void parser.validate(invalidXml, async (err, inResponseTo) => { + void parser.validate(makeLogoutResponseEnvelope(invalidXml), async (err, inResponseTo) => { expect(err).to.exist; expect(inResponseTo).to.not.exist; }); @@ -238,7 +239,7 @@ describe('SAML', () => { it('should fail to parse a xml without any LogoutResponse tag', () => { const parser = new LogoutResponseParser(serviceProviderOptions); - void parser.validate(randomXml, async (err, inResponseTo) => { + void parser.validate(makeLogoutResponseEnvelope(randomXml), async (err, inResponseTo) => { expect(err).to.be.equal('No Response Found'); expect(inResponseTo).to.not.exist; }); @@ -252,7 +253,7 @@ describe('SAML', () => { .replace('InResponseTo=', 'SomethingElse='); const parser = new LogoutResponseParser(serviceProviderOptions); - void parser.validate(logoutResponse, async (err, inResponseTo) => { + void parser.validate(makeLogoutResponseEnvelope(logoutResponse), async (err, inResponseTo) => { expect(err).to.be.equal('Unexpected Response from IDP'); expect(inResponseTo).to.not.exist; }); @@ -261,7 +262,7 @@ describe('SAML', () => { it('should reject a response with no status tag', () => { const parser = new LogoutResponseParser(serviceProviderOptions); - void parser.validate(invalidLogoutResponse, async (err, inResponseTo) => { + void parser.validate(makeLogoutResponseEnvelope(invalidLogoutResponse), async (err, inResponseTo) => { expect(err).to.be.equal('Error. Logout not confirmed by IDP'); expect(inResponseTo).to.be.null; }); @@ -303,7 +304,7 @@ describe('SAML', () => { .replace('[NOTONORAFTER]', notOnOrAfter.toISOString()); const parser = new ResponseParser(serviceProviderOptions); - parser.validate(response, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(response), (err, profile, loggedOut) => { expect(err).to.be.null; expect(profile).to.be.an('object'); expect(profile).to.have.property('inResponseToId').equal('[INRESPONSETO]'); @@ -325,7 +326,7 @@ describe('SAML', () => { const response = samlResponse.replace('[NOTBEFORE]', notBefore.toISOString()).replace('[NOTONORAFTER]', new Date().toISOString()); const parser = new ResponseParser(serviceProviderOptions); - parser.validate(response, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(response), (err, profile, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('NotBefore / NotOnOrAfter assertion failed'); expect(profile).to.be.null; expect(loggedOut).to.be.false; @@ -342,7 +343,7 @@ describe('SAML', () => { const response = samlResponse.replace('[NOTBEFORE]', notBefore.toISOString()).replace('[NOTONORAFTER]', notOnOrAfter.toISOString()); const parser = new ResponseParser(serviceProviderOptions); - parser.validate(response, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(response), (err, profile, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('NotBefore / NotOnOrAfter assertion failed'); expect(profile).to.be.null; expect(loggedOut).to.be.false; @@ -351,7 +352,7 @@ describe('SAML', () => { it('should fail to parse an invalid xml', () => { const parser = new ResponseParser(serviceProviderOptions); - parser.validate(invalidXml, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(invalidXml), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Unknown SAML response message'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -360,7 +361,7 @@ describe('SAML', () => { it('should fail to parse a xml without any Response tag', () => { const parser = new ResponseParser(serviceProviderOptions); - parser.validate(randomXml, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(randomXml), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Unknown SAML response message'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -369,7 +370,7 @@ describe('SAML', () => { it('should reject a xml with multiple responses', () => { const parser = new ResponseParser(serviceProviderOptions); - parser.validate(duplicatedSamlResponse, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(duplicatedSamlResponse), (err, data, loggedOut) => { expect(err).to.be.an('error'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -378,7 +379,7 @@ describe('SAML', () => { it('should fail to parse a reponse with no Status tag', () => { const parser = new ResponseParser(serviceProviderOptions); - parser.validate(samlResponseMissingStatus, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseMissingStatus), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Missing StatusCode'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -387,7 +388,7 @@ describe('SAML', () => { it('should fail to parse a reponse with a failed status', () => { const parser = new ResponseParser(serviceProviderOptions); - parser.validate(samlResponseFailedStatus, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseFailedStatus), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Status is: Failed'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -396,7 +397,7 @@ describe('SAML', () => { it('should reject a response with multiple assertions', () => { const parser = new ResponseParser(serviceProviderOptions); - parser.validate(samlResponseMultipleAssertions, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseMultipleAssertions), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Too many SAML assertions'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -405,7 +406,7 @@ describe('SAML', () => { it('should reject a response with no assertions', () => { const parser = new ResponseParser(serviceProviderOptions); - parser.validate(samlResponseMissingAssertion, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseMissingAssertion), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Missing SAML assertion'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -430,7 +431,7 @@ describe('SAML', () => { .replace('[NOTONORAFTER]', notOnOrAfter.toISOString()); const parser = new ResponseParser(providerOptions); - parser.validate(response, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(response), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('No valid SAML Signature found'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -439,7 +440,7 @@ describe('SAML', () => { it('should reject a document with multiple issuers', () => { const parser = new ResponseParser(serviceProviderOptions); - parser.validate(samlResponseMultipleIssuers, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseMultipleIssuers), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Too many Issuers'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -454,7 +455,7 @@ describe('SAML', () => { }; const parser = new ResponseParser(options); - parser.validate(encryptedResponse, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(encryptedResponse), (err, profile, loggedOut) => { // No way to change the assertion conditions on an encrypted response ¯\_(ツ)_/¯ expect(err).to.be.an('error').that.has.property('message').that.is.equal('NotBefore / NotOnOrAfter assertion failed'); expect(loggedOut).to.be.false; @@ -468,10 +469,11 @@ describe('SAML', () => { privateCert: privateKeyCert, signatureValidationType: 'All', privateKey, + cert: certificate, }; const parser = new ResponseParser(options); - parser.validate(encryptedResponse, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(encryptedResponse), (err, profile, loggedOut) => { // No way to change the assertion conditions on an encrypted response ¯\_(ツ)_/¯ expect(err).to.be.an('error').that.has.property('message').that.is.equal('NotBefore / NotOnOrAfter assertion failed'); expect(loggedOut).to.be.false; @@ -499,7 +501,7 @@ describe('SAML', () => { .replace('[NOTONORAFTER]', notOnOrAfter.toISOString()); const parser = new ResponseParser(providerOptions); - parser.validate(response, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(response), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Invalid Assertion signature'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -524,7 +526,7 @@ describe('SAML', () => { .replace('[NOTONORAFTER]', notOnOrAfter.toISOString()); const parser = new ResponseParser(providerOptions); - parser.validate(response, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(response), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Invalid Signature'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -547,7 +549,7 @@ describe('SAML', () => { const response = samlResponse.replace('[NOTBEFORE]', notBefore.toISOString()).replace('[NOTONORAFTER]', notOnOrAfter.toISOString()); const parser = new ResponseParser(providerOptions); - parser.validate(response, (err, data, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(response), (err, data, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Invalid Assertion signature'); expect(data).to.not.exist; expect(loggedOut).to.be.false; @@ -562,7 +564,7 @@ describe('SAML', () => { }; const parser = new ResponseParser(providerOptions); - parser.validate(samlResponseValidAssertionSignature, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseValidAssertionSignature), (err, profile, loggedOut) => { // To have a valid signature, we can't change the assertion conditions ¯\_(ツ)_/¯ expect(err).to.be.an('error').that.has.property('message').that.is.equal('NotBefore / NotOnOrAfter assertion failed'); expect(loggedOut).to.be.false; @@ -570,6 +572,21 @@ describe('SAML', () => { }); }); + it('should reject an assertion with a valid signature if the certificate is not configured', () => { + const providerOptions = { + ...serviceProviderOptions, + signatureValidationType: 'Assertion', + }; + + const parser = new ResponseParser(providerOptions); + parser.validate(makeLoginResponseEnvelope(samlResponseValidAssertionSignature), (err, profile, loggedOut) => { + // To have a valid signature, we can't change the assertion conditions ¯\_(ツ)_/¯ + expect(err).to.be.an('error').that.has.property('message').that.is.equal('Unable to validate signature'); + expect(loggedOut).to.be.false; + expect(profile).to.be.null; + }); + }); + it('should accept a document with a valid response signature', () => { const providerOptions = { ...serviceProviderOptions, @@ -578,7 +595,7 @@ describe('SAML', () => { }; const parser = new ResponseParser(providerOptions); - parser.validate(samlResponseValidSignatures, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseValidSignatures), (err, profile, loggedOut) => { // To have a valid signature, we can't change the assertion conditions ¯\_(ツ)_/¯ expect(err).to.be.an('error').that.has.property('message').that.is.equal('NotBefore / NotOnOrAfter assertion failed'); expect(loggedOut).to.be.false; @@ -594,7 +611,7 @@ describe('SAML', () => { }; const parser = new ResponseParser(providerOptions); - parser.validate(samlResponseValidAssertionSignature, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseValidAssertionSignature), (err, profile, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Invalid Signature'); expect(loggedOut).to.be.false; expect(profile).to.be.null; @@ -609,7 +626,7 @@ describe('SAML', () => { }; const parser = new ResponseParser(providerOptions); - parser.validate(samlResponseValidSignatures, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseValidSignatures), (err, profile, loggedOut) => { // To have a valid signature, we can't change the assertion conditions ¯\_(ツ)_/¯ expect(err).to.be.an('error').that.has.property('message').that.is.equal('NotBefore / NotOnOrAfter assertion failed'); expect(loggedOut).to.be.false; @@ -625,7 +642,7 @@ describe('SAML', () => { }; const parser = new ResponseParser(providerOptions); - parser.validate(samlResponseValidAssertionSignature, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseValidAssertionSignature), (err, profile, loggedOut) => { expect(err).to.be.an('error').that.has.property('message').that.is.equal('Invalid Signature'); expect(loggedOut).to.be.false; expect(profile).to.be.null; @@ -640,7 +657,7 @@ describe('SAML', () => { }; const parser = new ResponseParser(providerOptions); - parser.validate(samlResponseValidAssertionSignature, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(samlResponseValidAssertionSignature), (err, profile, loggedOut) => { // To have a valid signature, we can't change the assertion conditions ¯\_(ツ)_/¯ expect(err).to.be.an('error').that.has.property('message').that.is.equal('NotBefore / NotOnOrAfter assertion failed'); expect(loggedOut).to.be.false; @@ -952,7 +969,7 @@ describe('SAML', () => { .replace('[NOTONORAFTER]', notOnOrAfter.toISOString()); const parser = new ResponseParser(serviceProviderOptions); - parser.validate(response, (err, profile, loggedOut) => { + parser.validate(makeLoginResponseEnvelope(response), (err, profile, loggedOut) => { expect(profile).to.be.an('object'); expect(err).to.be.null; expect(loggedOut).to.be.false; diff --git a/packages/core-typings/src/ILoginServiceConfiguration.ts b/packages/core-typings/src/ILoginServiceConfiguration.ts index 74d2841f036dc..76a4779e806e9 100644 --- a/packages/core-typings/src/ILoginServiceConfiguration.ts +++ b/packages/core-typings/src/ILoginServiceConfiguration.ts @@ -88,7 +88,9 @@ export type SAMLConfiguration = { cert: string; algorithm: SAMLSignatureAlgorithm; }; - signatureValidationType: 'All' | 'Response' | 'Assertion' | 'Either'; + signatureValidationType: 'All' | 'Response' | 'Assertion' | 'Either' | 'None'; + validateLogoutRequestSignature: boolean; + validateLogoutResponseSignature: boolean; userDataFieldMap: string; allowedClockDrift: number; channelsAttributeUpdate: boolean; diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index 465ee29d3896b..7137a09eba7ed 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -4619,6 +4619,7 @@ "SAML_Custom_signature_validation_all": "Validate All Signatures", "SAML_Custom_signature_validation_assertion": "Validate Assertion Signature", "SAML_Custom_signature_validation_either": "Validate Either Signature", + "SAML_Custom_signature_validation_none": "Don't Validate Signatures", "SAML_Custom_signature_validation_response": "Validate Response Signature", "SAML_Custom_signature_validation_type": "Signature Validation Type", "SAML_Custom_signature_validation_type_description": "This setting will be ignored if no Custom Certificate is provided.", @@ -4626,6 +4627,8 @@ "SAML_Custom_user_data_custom_fieldmap_description": "Configure how user custom fields are populated from a record in SAML (once found).", "SAML_Custom_user_data_fieldmap": "User Data Field Map", "SAML_Custom_user_data_fieldmap_description": "Configure how user account fields (like email) are populated from a record in SAML (once found). \nAs an example, `{\"name\":\"cn\", \"email\":\"mail\"}` will choose a person's human readable name from the cn attribute, and their email from the mail attribute. \nAvailable fields in Rocket.Chat: `name`, `email` and `username`, everything else will be discarted. \n`{\"email\": \"mail\",\"username\": {\"fieldName\": \"mail\",\"regex\": \"(.*)@.+$\",\"template\": \"user-regex\"}, \"name\": { \"fieldNames\": [\"firstName\", \"lastName\"], \"template\": \"{{firstName}} {{lastName}}\"}, \"{{identifier}}\": \"uid\"}`", + "SAML_Custom_validate_logout_request_signature": "Verify Logout Request Signature", + "SAML_Custom_validate_logout_response_signature": "Verify Logout Response Signature", "SAML_Default_User_Role": "Default User Role", "SAML_Default_User_Role_Description": "You can specify multiple roles, separating them with commas.", "SAML_Description": "Security Assertion Markup Language used for exchanging authentication and authorization data.", diff --git a/packages/model-typings/src/models/IBaseUploadsModel.ts b/packages/model-typings/src/models/IBaseUploadsModel.ts index 1161ab0fe86d9..be8698e2bef84 100644 --- a/packages/model-typings/src/models/IBaseUploadsModel.ts +++ b/packages/model-typings/src/models/IBaseUploadsModel.ts @@ -1,4 +1,4 @@ -import type { IUpload } from '@rocket.chat/core-typings'; +import type { EncryptedContent, IUpload } from '@rocket.chat/core-typings'; import type { DeleteResult, UpdateResult, ClientSession, Document, InsertOneResult, WithId, FindCursor, FindOptions } from 'mongodb'; import type { IBaseModel } from './IBaseModel'; @@ -21,4 +21,12 @@ export interface IBaseUploadsModel extends IBaseModel { updateFileNameById(fileId: string, name: string): Promise; deleteFile(fileId: string, options?: { session?: ClientSession }): Promise; + + findOneByIdAndUserIdAndRoomId(fileId: string, userId: string, rid: string, options?: FindOptions): Promise; + + updateFileMetadata( + fileId: string, + userId: string, + metadata: { name?: string; description?: string; typeGroup?: string; content?: EncryptedContent }, + ): Promise; } diff --git a/packages/models/src/models/BaseUploadModel.ts b/packages/models/src/models/BaseUploadModel.ts index ba4165534c00d..a9a8483c231ee 100644 --- a/packages/models/src/models/BaseUploadModel.ts +++ b/packages/models/src/models/BaseUploadModel.ts @@ -1,4 +1,4 @@ -import type { IUpload } from '@rocket.chat/core-typings'; +import type { EncryptedContent, IUpload } from '@rocket.chat/core-typings'; import type { IBaseUploadsModel } from '@rocket.chat/model-typings'; import type { DeleteResult, @@ -134,4 +134,16 @@ export abstract class BaseUploadModelRaw extends BaseRaw implements IBaseUplo async deleteFile(fileId: string, options?: { session?: ClientSession }): Promise { return this.deleteOne({ _id: fileId }, { session: options?.session }); } + + async findOneByIdAndUserIdAndRoomId(fileId: string, userId: string, rid: string, options?: FindOptions): Promise { + return this.findOne({ _id: fileId, userId, rid }, options); + } + + async updateFileMetadata( + fileId: string, + userId: string, + metadata: { name?: string; description?: string; typeGroup?: string; content?: EncryptedContent }, + ): Promise { + return this.updateOne({ _id: fileId, userId }, { $set: metadata }); + } } diff --git a/packages/models/src/models/OAuthAccessTokens.ts b/packages/models/src/models/OAuthAccessTokens.ts index 5adca5f964e9a..d7d9e77000eed 100644 --- a/packages/models/src/models/OAuthAccessTokens.ts +++ b/packages/models/src/models/OAuthAccessTokens.ts @@ -19,14 +19,14 @@ export class OAuthAccessTokensRaw extends BaseRaw implements } async findOneByAccessToken(accessToken: string, options?: FindOptions): Promise { - if (!accessToken) { + if (typeof accessToken !== 'string' || !accessToken) { return null; } return this.findOne({ accessToken }, options); } async findOneByRefreshToken(refreshToken: string, options?: FindOptions): Promise { - if (!refreshToken) { + if (typeof refreshToken !== 'string' || !refreshToken) { return null; } return this.findOne({ refreshToken }, options); diff --git a/packages/models/src/models/OAuthApps.ts b/packages/models/src/models/OAuthApps.ts index 0f0d412365023..ccac7c15eee16 100644 --- a/packages/models/src/models/OAuthApps.ts +++ b/packages/models/src/models/OAuthApps.ts @@ -28,6 +28,9 @@ export class OAuthAppsRaw extends BaseRaw implements IOAuthAppsModel } findOneActiveByClientId(clientId: string, options?: FindOptions): Promise { + if (typeof clientId !== 'string' || !clientId) { + return Promise.resolve(null); + } return this.findOne( { active: true, @@ -42,6 +45,9 @@ export class OAuthAppsRaw extends BaseRaw implements IOAuthAppsModel clientSecret: string, options?: FindOptions, ): Promise { + if (typeof clientId !== 'string' || !clientId || typeof clientSecret !== 'string' || !clientSecret) { + return Promise.resolve(null); + } return this.findOne( { active: true, diff --git a/packages/models/src/models/OAuthAuthCodes.ts b/packages/models/src/models/OAuthAuthCodes.ts index 852f3ac4cb1ee..30124f488686c 100644 --- a/packages/models/src/models/OAuthAuthCodes.ts +++ b/packages/models/src/models/OAuthAuthCodes.ts @@ -14,6 +14,9 @@ export class OAuthAuthCodesRaw extends BaseRaw implements IOAuth } findOneByAuthCode(authCode: string, options?: FindOptions): Promise { + if (typeof authCode !== 'string' || !authCode) { + return Promise.resolve(null); + } return this.findOne({ authCode }, options); } } diff --git a/packages/models/src/models/OAuthRefreshTokens.ts b/packages/models/src/models/OAuthRefreshTokens.ts index 2693cb61ebe6f..031dc0fa9dd75 100644 --- a/packages/models/src/models/OAuthRefreshTokens.ts +++ b/packages/models/src/models/OAuthRefreshTokens.ts @@ -14,6 +14,9 @@ export class OAuthRefreshTokensRaw extends BaseRaw implement } findOneByRefreshToken(refreshToken: string, options?: FindOptions): Promise { + if (typeof refreshToken !== 'string' || !refreshToken) { + return Promise.resolve(null); + } return this.findOne({ refreshToken }, options); } } diff --git a/yarn.lock b/yarn.lock index d88bc8a06ef6d..7734461cad8a8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10437,7 +10437,7 @@ __metadata: peerDependencies: "@rocket.chat/layout": "*" "@rocket.chat/tools": 0.2.4 - "@rocket.chat/ui-contexts": 28.0.1 + "@rocket.chat/ui-contexts": 28.0.2 "@tanstack/react-query": "*" react: "*" react-hook-form: "*"