From 37c4de6bb4e8946f16e47b2b20a6d014d4d4e62b Mon Sep 17 00:00:00 2001 From: Mattermost Build Date: Tue, 17 Feb 2026 08:21:33 +0200 Subject: [PATCH] MM-66813 - enhance sso callback metadata (#9422) (#9522) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * MM-66813 - enhance sso callback metadata * fix translation order * Add serverUrl validation, remove non-null assertions (cherry picked from commit 3735aa6ff80e59ab884e14e092c00fb25357371e) Co-authored-by: Pablo Vélez --- app/screens/sso/index.tsx | 19 ++++++-- app/screens/sso/sso.test.tsx | 47 +++++++++++++++++++ app/screens/sso/sso_authentication.tsx | 45 ++++++++++++++++-- ...o_authentication_with_external_browser.tsx | 31 ++++++++++-- assets/base/i18n/en.json | 1 + 5 files changed, 134 insertions(+), 9 deletions(-) diff --git a/app/screens/sso/index.tsx b/app/screens/sso/index.tsx index 1a05ab074..e1bcfe5e9 100644 --- a/app/screens/sso/index.tsx +++ b/app/screens/sso/index.tsx @@ -55,6 +55,18 @@ const SSO = ({ const intl = useIntl(); const [loginError, setLoginError] = useState(''); + // Validate serverUrl is provided + if (!serverUrl) { + return ( + + + + ); + } + let loginUrl = ''; let shouldUseNativeEntra = false; @@ -109,7 +121,7 @@ const SSO = ({ }, [intl]); const doSSOLogin = async (bearerToken: string, csrfToken: string) => { - const result: LoginActionResponse = await ssoLogin(serverUrl!, serverDisplayName, config.DiagnosticId!, bearerToken, csrfToken, serverPreauthSecret); + const result: LoginActionResponse = await ssoLogin(serverUrl, serverDisplayName, config.DiagnosticId!, bearerToken, csrfToken, serverPreauthSecret); if (result?.error && result.failed) { onLoadEndError(result.error); return; @@ -118,7 +130,7 @@ const SSO = ({ }; const doSSOCodeExchange = async (loginCode: string, samlChallenge: {codeVerifier: string; state: string}) => { - const result: LoginActionResponse = await ssoLoginWithCodeExchange(serverUrl!, serverDisplayName, config.DiagnosticId!, loginCode, samlChallenge, serverPreauthSecret); + const result: LoginActionResponse = await ssoLoginWithCodeExchange(serverUrl, serverDisplayName, config.DiagnosticId!, loginCode, samlChallenge, serverPreauthSecret); if (result?.error && result.failed) { onLoadEndError(result.error); return; @@ -133,7 +145,7 @@ const SSO = ({ const doEntraLogin = useCallback(async () => { const result = await nativeEntraLogin( - serverUrl!, + serverUrl, serverDisplayName, config.DiagnosticId!, config.IntuneScope!, @@ -173,6 +185,7 @@ const SSO = ({ doSSOCodeExchange, loginError, loginUrl, + serverUrl, setLoginError, theme, }; diff --git a/app/screens/sso/sso.test.tsx b/app/screens/sso/sso.test.tsx index d933ab3f1..7dbb0adbd 100644 --- a/app/screens/sso/sso.test.tsx +++ b/app/screens/sso/sso.test.tsx @@ -12,6 +12,7 @@ import SSOAuthentication from './sso_authentication'; jest.mock('@utils/url', () => { return { tryOpenURL: () => null, + sanitizeUrl: (url: string) => url.replace(/\/+$/, '').toLowerCase(), }; }); @@ -23,6 +24,7 @@ describe('SSO with redirect url', () => { intl: {}, loginError: '', loginUrl: '', + serverUrl: 'https://example.mattermost.com', setLoginError: jest.fn(), theme: Preferences.THEMES.denim, }; @@ -49,3 +51,48 @@ describe('SSO with redirect url', () => { expect(browser).toBeUndefined(); }); }); + +describe('Server origin verification', () => { + // Test the URL normalization logic used in server origin verification + const {sanitizeUrl} = jest.requireMock('@utils/url'); + + test('should normalize URLs by removing trailing slashes', () => { + expect(sanitizeUrl('https://example.com/')).toBe('https://example.com'); + expect(sanitizeUrl('https://example.com')).toBe('https://example.com'); + }); + + test('should normalize URLs to lowercase', () => { + expect(sanitizeUrl('https://EXAMPLE.COM')).toBe('https://example.com'); + expect(sanitizeUrl('https://Example.Mattermost.Com')).toBe('https://example.mattermost.com'); + }); + + test('should match identical normalized URLs', () => { + const serverUrl = 'https://example.mattermost.com'; + const srvParam = 'https://example.mattermost.com'; + expect(sanitizeUrl(serverUrl)).toBe(sanitizeUrl(srvParam)); + }); + + test('should match URLs with different casing', () => { + const serverUrl = 'https://example.mattermost.com'; + const srvParam = 'https://EXAMPLE.MATTERMOST.COM'; + expect(sanitizeUrl(serverUrl)).toBe(sanitizeUrl(srvParam)); + }); + + test('should match URLs with trailing slash differences', () => { + const serverUrl = 'https://example.mattermost.com'; + const srvParam = 'https://example.mattermost.com/'; + expect(sanitizeUrl(serverUrl)).toBe(sanitizeUrl(srvParam)); + }); + + test('should not match different server URLs', () => { + const serverUrl = 'https://legitimate.mattermost.com'; + const srvParam = 'https://malicious.attacker.com'; + expect(sanitizeUrl(serverUrl)).not.toBe(sanitizeUrl(srvParam)); + }); + + test('should not match when server URL contains attacker URL as substring', () => { + const serverUrl = 'https://legitimate.mattermost.com'; + const srvParam = 'https://legitimate.mattermost.com.attacker.com'; + expect(sanitizeUrl(serverUrl)).not.toBe(sanitizeUrl(srvParam)); + }); +}); diff --git a/app/screens/sso/sso_authentication.tsx b/app/screens/sso/sso_authentication.tsx index ce3aea86b..cd11043c4 100644 --- a/app/screens/sso/sso_authentication.tsx +++ b/app/screens/sso/sso_authentication.tsx @@ -11,6 +11,7 @@ import urlParse from 'url-parse'; import {Sso} from '@constants'; import {isBetaApp} from '@utils/general'; import {createSamlChallenge} from '@utils/saml_challenge'; +import {sanitizeUrl} from '@utils/url'; import AuthError from './components/auth_error'; import AuthRedirect from './components/auth_redirect'; @@ -21,6 +22,7 @@ interface SSOAuthenticationProps { doSSOCodeExchange: (loginCode: string, samlChallenge: {codeVerifier: string; state: string}) => void; loginError: string; loginUrl: string; + serverUrl: string; setLoginError: (value: string) => void; theme: Theme; } @@ -32,7 +34,7 @@ const style = StyleSheet.create({ }, }); -const SSOAuthentication = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, setLoginError, theme}: SSOAuthenticationProps) => { +const SSOAuthentication = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, serverUrl, setLoginError, theme}: SSOAuthenticationProps) => { const [error, setError] = useState(''); const [loginSuccess, setLoginSuccess] = useState(false); const intl = useIntl(); @@ -43,6 +45,17 @@ const SSOAuthentication = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, const redirectUrl = customUrlScheme + 'callback'; const samlChallenge = useMemo(() => createSamlChallenge(), []); + + // Verify that the srv parameter from the callback matches the expected server + const verifyServerOrigin = useCallback((srvParam: string | undefined): boolean => { + if (!srvParam) { + // Old servers don't send srv parameter - allow for backwards compatibility + return true; + } + const normalizedExpected = sanitizeUrl(serverUrl); + const normalizedActual = sanitizeUrl(srvParam); + return normalizedExpected === normalizedActual; + }, [serverUrl]); const init = useCallback(async (resetErrors = true) => { setLoginSuccess(false); if (resetErrors !== false) { @@ -62,6 +75,19 @@ const SSOAuthentication = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, const result = await openAuthSessionAsync(url, null, {preferEphemeralSession: true, createTask: false}); if ('url' in result && result.url) { const resultUrl = urlParse(result.url, true); + const srvParam = resultUrl.query?.srv as string | undefined; + + // Verify server origin before accepting credentials + if (!verifyServerOrigin(srvParam)) { + setError( + intl.formatMessage({ + id: 'mobile.oauth.server_mismatch', + defaultMessage: 'Login failed: Unable to complete authentication with this server. Please try again.', + }), + ); + return; + } + const loginCode = resultUrl.query?.login_code as string | undefined; if (loginCode) { // Prefer code exchange when available @@ -83,7 +109,7 @@ const SSOAuthentication = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, }), ); } - }, [doSSOCodeExchange, doSSOLogin, intl, loginUrl, samlChallenge, redirectUrl, setLoginError]); + }, [doSSOCodeExchange, doSSOLogin, intl, loginUrl, samlChallenge, redirectUrl, setLoginError, verifyServerOrigin]); useEffect(() => { let listener: EventSubscription | null = null; @@ -93,6 +119,19 @@ const SSOAuthentication = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, setError(''); if (url && url.startsWith(redirectUrl)) { const parsedUrl = urlParse(url, true); + const srvParam = parsedUrl.query?.srv as string | undefined; + + // Verify server origin before accepting credentials + if (!verifyServerOrigin(srvParam)) { + setError( + intl.formatMessage({ + id: 'mobile.oauth.server_mismatch', + defaultMessage: 'Login failed: Unable to complete authentication with this server. Please try again.', + }), + ); + return; + } + const loginCode = parsedUrl.query?.login_code as string | undefined; if (loginCode) { setLoginSuccess(true); @@ -126,7 +165,7 @@ const SSOAuthentication = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, clearTimeout(timeout); listener?.remove(); }; - }, [doSSOCodeExchange, doSSOLogin, init, intl, samlChallenge, redirectUrl]); + }, [doSSOCodeExchange, doSSOLogin, init, intl, samlChallenge, redirectUrl, verifyServerOrigin]); let content; if (loginSuccess) { diff --git a/app/screens/sso/sso_authentication_with_external_browser.tsx b/app/screens/sso/sso_authentication_with_external_browser.tsx index 7ea3b0d4c..89a5b873c 100644 --- a/app/screens/sso/sso_authentication_with_external_browser.tsx +++ b/app/screens/sso/sso_authentication_with_external_browser.tsx @@ -13,7 +13,7 @@ import {isBetaApp} from '@utils/general'; import {createSamlChallenge} from '@utils/saml_challenge'; import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme'; import {typography} from '@utils/typography'; -import {tryOpenURL} from '@utils/url'; +import {sanitizeUrl, tryOpenURL} from '@utils/url'; import AuthError from './components/auth_error'; import AuthRedirect from './components/auth_redirect'; @@ -24,6 +24,7 @@ interface SSOWithRedirectURLProps { doSSOCodeExchange: (loginCode: string, samlChallenge: {codeVerifier: string; state: string}) => void; loginError: string; loginUrl: string; + serverUrl: string; setLoginError: (value: string) => void; theme: Theme; } @@ -59,7 +60,7 @@ const getStyleSheet = makeStyleSheetFromTheme((theme: Theme) => { }; }); -const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, setLoginError, theme}: SSOWithRedirectURLProps) => { +const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, serverUrl, setLoginError, theme}: SSOWithRedirectURLProps) => { const [error, setError] = useState(''); const [loginSuccess, setLoginSuccess] = useState(false); const style = getStyleSheet(theme); @@ -71,6 +72,17 @@ const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, doSSOCodeExchange, lo const redirectUrl = customUrlScheme + 'callback'; const samlChallenge = useMemo(() => createSamlChallenge(), []); + + // Verify that the srv parameter from the callback matches the expected server + const verifyServerOrigin = useCallback((srvParam: string | undefined): boolean => { + if (!srvParam) { + // Old servers don't send srv parameter - allow for backwards compatibility + return true; + } + const normalizedExpected = sanitizeUrl(serverUrl); + const normalizedActual = sanitizeUrl(srvParam); + return normalizedExpected === normalizedActual; + }, [serverUrl]); const init = useCallback((resetErrors = true) => { setLoginSuccess(false); if (resetErrors !== false) { @@ -114,6 +126,19 @@ const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, doSSOCodeExchange, lo const onURLChange = ({url}: { url: string }) => { if (url && url.startsWith(redirectUrl)) { const parsedUrl = urlParse(url, true); + const srvParam = parsedUrl.query?.srv as string | undefined; + + // Verify server origin before accepting credentials + if (!verifyServerOrigin(srvParam)) { + setError( + intl.formatMessage({ + id: 'mobile.oauth.server_mismatch', + defaultMessage: 'Login failed: Unable to complete authentication with this server. Please try again.', + }), + ); + return; + } + const loginCode = parsedUrl.query?.login_code as string | undefined; if (loginCode) { setLoginSuccess(true); @@ -148,7 +173,7 @@ const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, doSSOCodeExchange, lo listener.remove(); clearTimeout(timeout); }; - }, [doSSOCodeExchange, doSSOLogin, init, intl, samlChallenge, redirectUrl]); + }, [doSSOCodeExchange, doSSOLogin, init, intl, samlChallenge, redirectUrl, verifyServerOrigin]); let content; if (loginSuccess) { diff --git a/assets/base/i18n/en.json b/assets/base/i18n/en.json index 7b3b01ea0..a3fad3c23 100644 --- a/assets/base/i18n/en.json +++ b/assets/base/i18n/en.json @@ -796,6 +796,7 @@ "mobile.oauth.failed_to_login": "Your login attempt failed. Please try again.", "mobile.oauth.failed_to_open_link": "The link failed to open. Please try again.", "mobile.oauth.failed_to_open_link_no_browser": "The link failed to open. Please verify that a browser is installed on the device.", + "mobile.oauth.server_mismatch": "Login failed: Unable to complete authentication with this server. Please try again.", "mobile.oauth.something_wrong.okButton": "OK", "mobile.oauth.success.description": "Signing in now, just a moment...", "mobile.oauth.success.title": "Authentication successful",