From d91f497f2fb6a547601b524ae9910c3ab37ff987 Mon Sep 17 00:00:00 2001 From: "S. B. | Software Developer" <87614560+SB2318@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:21:26 +0530 Subject: [PATCH 1/6] feat: enhance relay state handling for multi-step IdPs in SAMLUtils --- apps/meteor/server/lib/saml/lib/Utils.ts | 29 +++++++++++++++---- .../unit/server/lib/saml/server.tests.ts | 12 ++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/apps/meteor/server/lib/saml/lib/Utils.ts b/apps/meteor/server/lib/saml/lib/Utils.ts index efaf3490c072d..2b6ca129631d9 100644 --- a/apps/meteor/server/lib/saml/lib/Utils.ts +++ b/apps/meteor/server/lib/saml/lib/Utils.ts @@ -1,3 +1,4 @@ +import { Buffer } from 'node:buffer'; import crypto from 'node:crypto'; import { EventEmitter } from 'node:events'; import zlib from 'node:zlib'; @@ -164,13 +165,31 @@ export class SAMLUtils { return {}; } - if (relayState.startsWith('provider=') && relayState.includes('&loginClient=')) { - const params = new URLSearchParams(relayState); + let decodedState = relayState; + try { + if (decodedState.includes('%3D') || decodedState.includes('%26')) { + decodedState = decodeURIComponent(decodedState); + } + } catch (err) { + this.log({ msg: 'Failed to decode relay state', err }); + } + + // If the relay state contains a loginClient, it will be in the format of a query string, so we need to parse it + // relayState.startsWith('provider=') deliberately because of how Identity Providers handle RelayState parameters during multi-step MFA flows + // relied on provider= being at index 0 of the string. During multi-step MFA, IdPs frequently modify the RelayState string in ways that break startsWith: + // - Okta adds a prefix to the RelayState string, so it no longer starts with provider= + // - Azure AD adds a suffix to the RelayState string, so it no longer starts with provider= + // - PingFederate adds a prefix to the RelayState string, so it no longer starts with provider= + // Not AI generated: The following check is a workaround to handle these cases by checking for the presence of &loginClient= in the decodedState string, which indicates that the relay state contains a loginClient parameter. + if (decodedState.includes('&loginClient=')) { + const params = new URLSearchParams(decodedState); + + // If the provider is not present in the relay state, we will use the decodedState as the provider value + // provider is not omitted from extraction, it is parsed dynamically using URLSearchParams: const provider = params.get('provider') ?? undefined; const loginClient = params.get('loginClient'); - return { - provider, + provider: provider || decodedState, loginClient: this.isSupportedLoginClient(loginClient) ? loginClient : undefined, }; } @@ -198,7 +217,7 @@ export class SAMLUtils { public static async inflateXml(deflatedXml: Buffer): Promise> { return new Promise((resolve, reject) => { - zlib.inflateRaw(deflatedXml, (err, inflatedXml) => { + zlib.inflateRaw(deflatedXml, (err: Error | null, inflatedXml: Buffer) => { if (err) { this.log({ msg: 'Error while inflating.', err }); return reject(err); diff --git a/apps/meteor/tests/unit/server/lib/saml/server.tests.ts b/apps/meteor/tests/unit/server/lib/saml/server.tests.ts index a2dc7f3bbf9a4..d6e8807f30324 100644 --- a/apps/meteor/tests/unit/server/lib/saml/server.tests.ts +++ b/apps/meteor/tests/unit/server/lib/saml/server.tests.ts @@ -1160,6 +1160,18 @@ describe('SAML', () => { const encoded = SAMLUtils.encodeAuthorizeRelayState('a b&c=d', 'mobile'); expect(SAMLUtils.decodeAuthorizeRelayState(encoded)).to.be.deep.equal({ provider: 'a b&c=d', loginClient: 'mobile' }); }); + + it('should handle URL-encoded RelayState strings from multi-step IdPs', () => { + const encoded = encodeURIComponent('provider=test-sp&loginClient=mobile'); + expect(SAMLUtils.decodeAuthorizeRelayState(encoded)).to.be.deep.equal({ provider: 'test-sp', loginClient: 'mobile' }); + }); + + it('should handle reordered RelayState parameters', () => { + expect(SAMLUtils.decodeAuthorizeRelayState('loginClient=mobile&provider=test-sp')).to.be.deep.equal({ + provider: 'test-sp', + loginClient: 'mobile', + }); + }); }); }); From 517513393b6798daa67f59fb1347ccbfbe3239a2 Mon Sep 17 00:00:00 2001 From: "S. B. | Software Developer" <87614560+SB2318@users.noreply.github.com> Date: Fri, 11 Sep 2026 23:09:05 +0530 Subject: [PATCH 2/6] fix: resolve SAML MFA deep-link callback issue on mobile devices --- .../saml-mfa-deeplink-relaystate-fix.md | 9 ++++ apps/meteor/server/lib/saml/lib/Utils.ts | 48 +++++++++---------- 2 files changed, 32 insertions(+), 25 deletions(-) create mode 100644 .changeset/saml-mfa-deeplink-relaystate-fix.md diff --git a/.changeset/saml-mfa-deeplink-relaystate-fix.md b/.changeset/saml-mfa-deeplink-relaystate-fix.md new file mode 100644 index 0000000000000..60ae867b5d3a0 --- /dev/null +++ b/.changeset/saml-mfa-deeplink-relaystate-fix.md @@ -0,0 +1,9 @@ +--- +'@rocket.chat/meteor': patch +--- + +Fixes SAML + IdP MFA deep-link callback failure on mobile devices introduced in 8.8.0. + +When an Identity Provider (IdP) enforces multi-step 2FA/MFA challenges (such as Okta Verify, Azure AD Conditional Access, Duo, or PingIdentity), the SAML `RelayState` parameter containing the `loginClient` context was not correctly decoded. IdPs may URL-encode the `RelayState` value or reorder its query parameters across MFA redirect hops. As a result, `loginClient` was silently lost, causing the server to omit the `&loginClient=mobile` segment from the post-authentication redirect. The mobile app never received the `rocketchat://auth` deep-link callback and remained stuck on the login screen. + +The fix updates `SAMLUtils.decodeAuthorizeRelayState` to handle position-independent parameter parsing and safely attempt `decodeURIComponent` only when the string contains no literal separators, preserving provider values that legitimately contain `&` or `=` characters. diff --git a/apps/meteor/server/lib/saml/lib/Utils.ts b/apps/meteor/server/lib/saml/lib/Utils.ts index 2b6ca129631d9..0e88f885d3600 100644 --- a/apps/meteor/server/lib/saml/lib/Utils.ts +++ b/apps/meteor/server/lib/saml/lib/Utils.ts @@ -165,33 +165,31 @@ export class SAMLUtils { return {}; } - let decodedState = relayState; - try { - if (decodedState.includes('%3D') || decodedState.includes('%26')) { - decodedState = decodeURIComponent(decodedState); - } - } catch (err) { - this.log({ msg: 'Failed to decode relay state', err }); - } + // CASE 1: relayState is a simple string, which is the provider name - // If the relay state contains a loginClient, it will be in the format of a query string, so we need to parse it - // relayState.startsWith('provider=') deliberately because of how Identity Providers handle RelayState parameters during multi-step MFA flows - // relied on provider= being at index 0 of the string. During multi-step MFA, IdPs frequently modify the RelayState string in ways that break startsWith: - // - Okta adds a prefix to the RelayState string, so it no longer starts with provider= - // - Azure AD adds a suffix to the RelayState string, so it no longer starts with provider= - // - PingFederate adds a prefix to the RelayState string, so it no longer starts with provider= - // Not AI generated: The following check is a workaround to handle these cases by checking for the presence of &loginClient= in the decodedState string, which indicates that the relay state contains a loginClient parameter. - if (decodedState.includes('&loginClient=')) { - const params = new URLSearchParams(decodedState); - - // If the provider is not present in the relay state, we will use the decodedState as the provider value - // provider is not omitted from extraction, it is parsed dynamically using URLSearchParams: + if (relayState.includes('loginClient=') || relayState.includes('provider=')) { + const params = new URLSearchParams(relayState); const provider = params.get('provider') ?? undefined; - const loginClient = params.get('loginClient'); - return { - provider: provider || decodedState, - loginClient: this.isSupportedLoginClient(loginClient) ? loginClient : undefined, - }; + const loginClient = params.get('loginClient') ?? undefined; + if (provider && this.isSupportedLoginClient(loginClient)) { + return { provider, loginClient }; + } + } + // CASE 2: relayState is a query string, which contains the provider name and the loginClient + + if (!relayState.includes('&') && !relayState.includes('=') && relayState.includes('%')) { + try { + const decoded = decodeURIComponent(relayState); + const params = new URLSearchParams(decoded); + const provider = params.get('provider') ?? undefined; + const loginClient = params.get('loginClient') ?? undefined; + if (provider && this.isSupportedLoginClient(loginClient)) { + return { provider, loginClient }; + } + } catch (err) { + // Fallback to treating as plain provider string below + this.log({ msg: 'Failed to decode relay state', err }); + } } return { provider: relayState }; From ab00fed838f9cbd7de4ea606cac8848fb8b6ed5d Mon Sep 17 00:00:00 2001 From: "S. B. | Software Developer" <87614560+SB2318@users.noreply.github.com> Date: Fri, 11 Sep 2026 23:40:57 +0530 Subject: [PATCH 3/6] fix: improve RelayState decoding logic for SAML multi-step IdPs --- apps/meteor/server/lib/saml/lib/Utils.ts | 55 ++++++++++++++----- .../unit/server/lib/saml/server.tests.ts | 16 +++++- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/apps/meteor/server/lib/saml/lib/Utils.ts b/apps/meteor/server/lib/saml/lib/Utils.ts index 0e88f885d3600..8d83de714dab9 100644 --- a/apps/meteor/server/lib/saml/lib/Utils.ts +++ b/apps/meteor/server/lib/saml/lib/Utils.ts @@ -165,29 +165,58 @@ export class SAMLUtils { return {}; } - // CASE 1: relayState is a simple string, which is the provider name + // CASE 1: Compound RelayState in the format produced by encodeAuthorizeRelayState. + // 'tenant&provider=other&loginClient=mobile' — will never begin with 'provider='. - if (relayState.includes('loginClient=') || relayState.includes('provider=')) { + if (relayState.startsWith('provider=') && relayState.includes('&loginClient=')) { const params = new URLSearchParams(relayState); const provider = params.get('provider') ?? undefined; - const loginClient = params.get('loginClient') ?? undefined; - if (provider && this.isSupportedLoginClient(loginClient)) { - return { provider, loginClient }; - } + const loginClient = params.get('loginClient'); + return { + provider, + loginClient: this.isSupportedLoginClient(loginClient) ? loginClient : undefined, + }; } - // CASE 2: relayState is a query string, which contains the provider name and the loginClient + // CASE 2: The IdP wrapped the entire RelayState in encodeURIComponent, producing a + // single opaque blob with no literal separators (e.g. 'provider%3Dtest-sp%26loginClient%3Dmobile'). + if (!relayState.includes('&') && !relayState.includes('=') && relayState.includes('%')) { try { const decoded = decodeURIComponent(relayState); - const params = new URLSearchParams(decoded); - const provider = params.get('provider') ?? undefined; - const loginClient = params.get('loginClient') ?? undefined; - if (provider && this.isSupportedLoginClient(loginClient)) { - return { provider, loginClient }; + if (decoded.startsWith('provider=') && decoded.includes('&loginClient=')) { + const params = new URLSearchParams(decoded); + const provider = params.get('provider') ?? undefined; + const loginClient = params.get('loginClient'); + if (provider && this.isSupportedLoginClient(loginClient)) { + return { provider, loginClient }; + } + } + } catch (err) { + + this.log({ msg: 'Failed to decode relay state', err }); + } + } + + // CASE 3: The IdP encoded the '=' signs within each segment but kept literal '&' + // as the outer separator (e.g. 'provider%3Dtest-sp&loginClient%3Dmobile'). + + if ((relayState.includes('%3D') || relayState.includes('%3d')) && relayState.includes('&')) { + try { + const rebuilt = relayState + .split('&') + .map((s) => decodeURIComponent(s)) + .join('&'); + if (rebuilt.startsWith('provider=') && rebuilt.includes('&loginClient=')) { + const params = new URLSearchParams(rebuilt); + const provider = params.get('provider') ?? undefined; + const loginClient = params.get('loginClient'); + if (provider && this.isSupportedLoginClient(loginClient)) { + return { provider, loginClient }; + } } } catch (err) { - // Fallback to treating as plain provider string below + this.log({ msg: 'Failed to decode relay state', err }); } } diff --git a/apps/meteor/tests/unit/server/lib/saml/server.tests.ts b/apps/meteor/tests/unit/server/lib/saml/server.tests.ts index d6e8807f30324..2eec57b2ad79d 100644 --- a/apps/meteor/tests/unit/server/lib/saml/server.tests.ts +++ b/apps/meteor/tests/unit/server/lib/saml/server.tests.ts @@ -1161,17 +1161,27 @@ describe('SAML', () => { expect(SAMLUtils.decodeAuthorizeRelayState(encoded)).to.be.deep.equal({ provider: 'a b&c=d', loginClient: 'mobile' }); }); - it('should handle URL-encoded RelayState strings from multi-step IdPs', () => { + it('should handle fully URL-encoded RelayState from multi-step IdPs', () => { + // IdP wrapped the entire RelayState in encodeURIComponent const encoded = encodeURIComponent('provider=test-sp&loginClient=mobile'); expect(SAMLUtils.decodeAuthorizeRelayState(encoded)).to.be.deep.equal({ provider: 'test-sp', loginClient: 'mobile' }); }); - it('should handle reordered RelayState parameters', () => { - expect(SAMLUtils.decodeAuthorizeRelayState('loginClient=mobile&provider=test-sp')).to.be.deep.equal({ + it('should handle mixed-encoded RelayState where only "=" is encoded but "&" is literal', () => { + + expect(SAMLUtils.decodeAuthorizeRelayState('provider%3Dtest-sp&loginClient%3Dmobile')).to.be.deep.equal({ provider: 'test-sp', loginClient: 'mobile', }); }); + + it('should not misread a raw provider name that contains query-like substrings', () => { + // A provider named 'tenant&provider=other&loginClient=mobile' must not be + // parsed as a compound RelayState — it does not start with 'provider='. + expect(SAMLUtils.decodeAuthorizeRelayState('tenant&provider=other&loginClient=mobile')).to.be.deep.equal({ + provider: 'tenant&provider=other&loginClient=mobile', + }); + }); }); }); From fa7b8c298c020ef9c963042d862838bac9281d8e Mon Sep 17 00:00:00 2001 From: "S. B. | Software Developer" <87614560+SB2318@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:46:08 +0530 Subject: [PATCH 4/6] fix: enhance RelayState decoding logic for improved handling of SAML parameters --- apps/meteor/server/lib/saml/lib/Utils.ts | 62 +++++++++++++----------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/apps/meteor/server/lib/saml/lib/Utils.ts b/apps/meteor/server/lib/saml/lib/Utils.ts index 8d83de714dab9..73a1b200e0fd8 100644 --- a/apps/meteor/server/lib/saml/lib/Utils.ts +++ b/apps/meteor/server/lib/saml/lib/Utils.ts @@ -160,63 +160,71 @@ export class SAMLUtils { return new URLSearchParams({ provider, loginClient }).toString(); } + + public static decodeAuthorizeRelayState(relayState?: string | null): { provider?: string; loginClient?: 'desktop' | 'mobile' } { if (!relayState) { return {}; } // CASE 1: Compound RelayState in the format produced by encodeAuthorizeRelayState. - // 'tenant&provider=other&loginClient=mobile' — will never begin with 'provider='. - - if (relayState.startsWith('provider=') && relayState.includes('&loginClient=')) { + // Parameter order is irrelevant: + // 'provider=test-sp&loginClient=mobile' + // 'loginClient=mobile&provider=test-sp' + if (relayState.includes('loginClient=')) { const params = new URLSearchParams(relayState); const provider = params.get('provider') ?? undefined; const loginClient = params.get('loginClient'); + return { provider, loginClient: this.isSupportedLoginClient(loginClient) ? loginClient : undefined, }; } - // CASE 2: The IdP wrapped the entire RelayState in encodeURIComponent, producing a - // single opaque blob with no literal separators (e.g. 'provider%3Dtest-sp%26loginClient%3Dmobile'). - - if (!relayState.includes('&') && !relayState.includes('=') && relayState.includes('%')) { + // CASE 2: The IdP URL-encoded the complete RelayState. + // Example: + // 'provider%3Dtest-sp%26loginClient%3Dmobile' + if (relayState.includes('%3D') || relayState.includes('%3d')) { try { const decoded = decodeURIComponent(relayState); - if (decoded.startsWith('provider=') && decoded.includes('&loginClient=')) { - const params = new URLSearchParams(decoded); - const provider = params.get('provider') ?? undefined; - const loginClient = params.get('loginClient'); - if (provider && this.isSupportedLoginClient(loginClient)) { - return { provider, loginClient }; - } + const params = new URLSearchParams(decoded); + + const provider = params.get('provider') ?? undefined; + const loginClient = params.get('loginClient'); + + if (provider && this.isSupportedLoginClient(loginClient)) { + return { provider, loginClient }; } } catch (err) { - this.log({ msg: 'Failed to decode relay state', err }); } } - // CASE 3: The IdP encoded the '=' signs within each segment but kept literal '&' - // as the outer separator (e.g. 'provider%3Dtest-sp&loginClient%3Dmobile'). - + // CASE 3: The IdP encoded the '=' separators within each segment + // but kept '&' as the outer separator. + // + // Example: + // 'provider%3Dtest-sp&loginClient%3Dmobile' + // + // Decode only the '=' separator instead of decoding the whole segment. + // This prevents an encoded '%26' inside a value from becoming a new + // query separator. if ((relayState.includes('%3D') || relayState.includes('%3d')) && relayState.includes('&')) { try { const rebuilt = relayState .split('&') - .map((s) => decodeURIComponent(s)) + .map((segment) => segment.replace(/%3D/gi, '=')) .join('&'); - if (rebuilt.startsWith('provider=') && rebuilt.includes('&loginClient=')) { - const params = new URLSearchParams(rebuilt); - const provider = params.get('provider') ?? undefined; - const loginClient = params.get('loginClient'); - if (provider && this.isSupportedLoginClient(loginClient)) { - return { provider, loginClient }; - } + + const params = new URLSearchParams(rebuilt); + const provider = params.get('provider') ?? undefined; + const loginClient = params.get('loginClient'); + + if (provider && this.isSupportedLoginClient(loginClient)) { + return { provider, loginClient }; } } catch (err) { - this.log({ msg: 'Failed to decode relay state', err }); } } From 7116addd70f74b41c31a5ed2a08161e5064f0c5d Mon Sep 17 00:00:00 2001 From: "S. B. | Software Developer" <87614560+SB2318@users.noreply.github.com> Date: Sat, 12 Sep 2026 01:03:10 +0530 Subject: [PATCH 5/6] ---- --- apps/meteor/server/lib/saml/lib/Utils.ts | 64 +++++++++++++++++++++++- 1 file changed, 62 insertions(+), 2 deletions(-) diff --git a/apps/meteor/server/lib/saml/lib/Utils.ts b/apps/meteor/server/lib/saml/lib/Utils.ts index 73a1b200e0fd8..0fd2bb362727d 100644 --- a/apps/meteor/server/lib/saml/lib/Utils.ts +++ b/apps/meteor/server/lib/saml/lib/Utils.ts @@ -160,7 +160,67 @@ export class SAMLUtils { return new URLSearchParams({ provider, loginClient }).toString(); } - + // public static decodeAuthorizeRelayState(relayState?: string | null): { provider?: string; loginClient?: 'desktop' | 'mobile' } { + // if (!relayState) { + // return {}; + // } + + // // CASE 1: Compound RelayState in the format produced by encodeAuthorizeRelayState. + // // 'tenant&provider=other&loginClient=mobile' — will never begin with 'provider='. + + // if (relayState.includes('loginClient=')) { + // const params = new URLSearchParams(relayState); + // const provider = params.get('provider') ?? undefined; + // const loginClient = params.get('loginClient'); + // return { + // provider, + // loginClient: this.isSupportedLoginClient(loginClient) ? loginClient : undefined, + // }; + // } + + // // CASE 2: The IdP wrapped the entire RelayState in encodeURIComponent, producing a + // // single opaque blob with no literal separators (e.g. 'provider%3Dtest-sp%26loginClient%3Dmobile'). + + // if (!relayState.includes('&') && !relayState.includes('=') && relayState.includes('%')) { + // try { + // const decoded = decodeURIComponent(relayState); + // if (decoded.startsWith('provider=') && decoded.includes('&loginClient=')) { + // const params = new URLSearchParams(decoded); + // const provider = params.get('provider') ?? undefined; + // const loginClient = params.get('loginClient'); + // if (provider && this.isSupportedLoginClient(loginClient)) { + // return { provider, loginClient }; + // } + // } + // } catch (err) { + // this.log({ msg: 'Failed to decode relay state', err }); + // } + // } + + // // CASE 3: The IdP encoded the '=' signs within each segment but kept literal '&' + // // as the outer separator (e.g. 'provider%3Dtest-sp&loginClient%3Dmobile'). + + // if ((relayState.includes('%3D') || relayState.includes('%3d')) && relayState.includes('&')) { + // try { + // const rebuilt = relayState + // .split('&') + // .map((s) => s.replace(/%3D/i, '=')) + // .join('&'); + + // const params = new URLSearchParams(rebuilt); + // const provider = params.get('provider') ?? undefined; + // const loginClient = params.get('loginClient'); + + // if (provider && this.isSupportedLoginClient(loginClient)) { + // return { provider, loginClient }; + // } + // } catch (err) { + // this.log({ msg: 'Failed to decode relay state', err }); + // } + // } + + // return { provider: relayState }; + // } public static decodeAuthorizeRelayState(relayState?: string | null): { provider?: string; loginClient?: 'desktop' | 'mobile' } { if (!relayState) { @@ -185,7 +245,7 @@ export class SAMLUtils { // CASE 2: The IdP URL-encoded the complete RelayState. // Example: // 'provider%3Dtest-sp%26loginClient%3Dmobile' - if (relayState.includes('%3D') || relayState.includes('%3d')) { + if (relayState.includes('%3D') || relayState.includes('%3d') && !relayState.includes('&')) { try { const decoded = decodeURIComponent(relayState); const params = new URLSearchParams(decoded); From 0a9870259a74cbeca9cc604bbc1759122aff535a Mon Sep 17 00:00:00 2001 From: "S. B. | Software Developer" <87614560+SB2318@users.noreply.github.com> Date: Sat, 12 Sep 2026 01:14:35 +0530 Subject: [PATCH 6/6] Update apps/meteor/server/lib/saml/lib/Utils.ts Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- apps/meteor/server/lib/saml/lib/Utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/server/lib/saml/lib/Utils.ts b/apps/meteor/server/lib/saml/lib/Utils.ts index 0fd2bb362727d..9768306b7baef 100644 --- a/apps/meteor/server/lib/saml/lib/Utils.ts +++ b/apps/meteor/server/lib/saml/lib/Utils.ts @@ -245,7 +245,7 @@ export class SAMLUtils { // CASE 2: The IdP URL-encoded the complete RelayState. // Example: // 'provider%3Dtest-sp%26loginClient%3Dmobile' - if (relayState.includes('%3D') || relayState.includes('%3d') && !relayState.includes('&')) { + if ((relayState.includes('%3D') || relayState.includes('%3d')) && !relayState.includes('&')) { try { const decoded = decodeURIComponent(relayState); const params = new URLSearchParams(decoded);