From 2d6a5d509774b8099aef316d87fcf676191c1176 Mon Sep 17 00:00:00 2001 From: JG Heithcock Date: Mon, 29 Sep 2025 14:29:37 -0700 Subject: [PATCH] Mobile-side for MM-65084 (#9115) * Mobile fix for MM-65084 * Changing test/setup.ts to use a deterministic fill This avoids the ci issue about parenthesis and is more clear that this is just a fixed sequence for testing, similar to randomUUID above. * Add setBearerToken and setCSRFToken to Client definition * Use setClientCredentials and memoize createPkceBundle * Restoring the preauthSecret back to the Client constructors This came out of a response to MM-65085: Support Pre Shared Password on server connect where preauthSecret was added in the buildConfig. Claude (correctly imo) identified this as now redundant and so removed it but it is valid to keep it as well. In any case, putting it back to be consistent with ClientTracking and ClientBase. * Rename PKCE to SAML based terminology, similar to server * Fix lint issue with too many blank lines at eof * Removing plain on mobile side --------- Co-authored-by: Mattermost Build --- app/actions/remote/session.ts | 46 ++++++++++++++++ app/client/rest/index.ts | 5 +- app/client/rest/users.ts | 19 +++++++ app/managers/network_manager.ts | 2 + app/screens/sso/index.tsx | 12 +++- app/screens/sso/sso.test.tsx | 1 + app/screens/sso/sso_authentication.tsx | 29 ++++++++-- ...o_authentication_with_external_browser.tsx | 28 ++++++++-- app/utils/saml_challenge.ts | 55 +++++++++++++++++++ package-lock.json | 6 ++ package.json | 1 + scripts/pre-commit.sh | 2 +- test/setup.ts | 5 ++ types/vendor/js-sha256.d.ts | 15 +++++ 14 files changed, 212 insertions(+), 14 deletions(-) create mode 100644 app/utils/saml_challenge.ts create mode 100644 types/vendor/js-sha256.d.ts diff --git a/app/actions/remote/session.ts b/app/actions/remote/session.ts index 40d426e13..ef3e33744 100644 --- a/app/actions/remote/session.ts +++ b/app/actions/remote/session.ts @@ -19,6 +19,7 @@ import EphemeralStore from '@store/ephemeral_store'; import {getFullErrorMessage, isErrorWithStatusCode, isErrorWithUrl} from '@utils/errors'; import {logWarning, logError, logDebug} from '@utils/log'; import {scheduleExpiredNotification} from '@utils/notification'; +import {type SAMLChallenge} from '@utils/saml_challenge'; import {getCSRFFromCookie} from '@utils/security'; import {loginEntry} from './entry'; @@ -345,6 +346,51 @@ export const ssoLogin = async (serverUrl: string, serverDisplayName: string, ser } }; +export const ssoLoginWithCodeExchange = async (serverUrl: string, serverDisplayName: string, serverIdentifier: string, loginCode: string, samlChallenge: Pick, preauthSecret?: string): Promise => { + const database = DatabaseManager.appDatabase?.database; + if (!database) { + return {error: 'App database not found', failed: true}; + } + + try { + const client = NetworkManager.getClient(serverUrl); + const {token, csrf} = await client.exchangeSsoLoginCode(loginCode, samlChallenge.codeVerifier, samlChallenge.state); + + client.setClientCredentials(token, preauthSecret); + client.setCSRFToken(csrf); + + const server = await DatabaseManager.createServerDatabase({ + config: { + dbName: serverUrl, + serverUrl, + identifier: serverIdentifier, + displayName: serverDisplayName, + }, + }); + const user = await client.getMe(); + await server?.operator.handleUsers({users: [user], prepareRecordsOnly: false}); + await server?.operator.handleSystem({ + systems: [{ + id: Database.SYSTEM_IDENTIFIERS.CURRENT_USER_ID, + value: user.id, + }], + prepareRecordsOnly: false, + }); + } catch (error) { + logDebug('error on ssoLoginWithCodeExchange', getFullErrorMessage(error)); + return {error, failed: true}; + } + + try { + await addPushProxyVerificationStateFromLogin(serverUrl); + const {error} = await loginEntry({serverUrl}); + await DatabaseManager.setActiveServerDatabase(serverUrl); + return {error, failed: false}; + } catch (error) { + return {error, failed: false}; + } +}; + export async function findSession(serverUrl: string, sessions: Session[]) { try { const {database} = DatabaseManager.getServerDatabaseAndOperator(serverUrl); diff --git a/app/client/rest/index.ts b/app/client/rest/index.ts index ce3a68252..eaa2dc5dd 100644 --- a/app/client/rest/index.ts +++ b/app/client/rest/index.ts @@ -51,7 +51,10 @@ interface Client extends ClientBase, ClientNPSMix, ClientCustomAttributesMix, ClientPlaybooksMix -{} +{ + setClientCredentials: (token: string, preauthSecret?: string) => void; + setCSRFToken: (csrfToken: string) => void; +} class Client extends mix(ClientBase).with( ClientApps, diff --git a/app/client/rest/users.ts b/app/client/rest/users.ts index cb2aea9e1..edc55ef77 100644 --- a/app/client/rest/users.ts +++ b/app/client/rest/users.ts @@ -46,6 +46,7 @@ export interface ClientUsersMix { updateCustomStatus: (customStatus: UserCustomStatus) => Promise<{status: string}>; unsetCustomStatus: () => Promise<{status: string}>; removeRecentCustomStatus: (customStatus: UserCustomStatus) => Promise<{status: string}>; + exchangeSsoLoginCode: (loginCode: string, codeVerifier: string, state: string) => Promise<{token: string; csrf: string}>; } const ClientUsers = >(superclass: TBase) => class extends superclass { @@ -395,6 +396,24 @@ const ClientUsers = >(superclass: TBase) = {method: 'post', body: customStatus}, ); }; + + exchangeSsoLoginCode = async (loginCode: string, codeVerifier: string, state: string) => { + const body = { + login_code: loginCode, + code_verifier: codeVerifier, + state, + }; + + // Intentionally no-cache + const resp = await this.doFetch( + `${this.getUsersRoute()}/login/sso/code-exchange`, + {method: 'post', body, headers: {'Cache-Control': 'no-store'}}, + false, + ); + + // Expected shape: { token: string, csrf: string } + return resp?.data || resp; + }; }; export default ClientUsers; diff --git a/app/managers/network_manager.ts b/app/managers/network_manager.ts index 61dde731d..6e8d1286a 100644 --- a/app/managers/network_manager.ts +++ b/app/managers/network_manager.ts @@ -105,6 +105,8 @@ class NetworkManagerSingleton { try { const {client} = await getOrCreateAPIClient(serverUrl, config, this.clientErrorEventHandler); const csrfToken = await getCSRFFromCookie(serverUrl); + + // Pass preauthSecret explicitly to constructor to match ClientBase behavior this.clients[serverUrl] = new Client(client, serverUrl, bearerToken, csrfToken, preauthSecret); } catch (error) { throw new ClientError(serverUrl, { diff --git a/app/screens/sso/index.tsx b/app/screens/sso/index.tsx index 7968ca540..0e4215e38 100644 --- a/app/screens/sso/index.tsx +++ b/app/screens/sso/index.tsx @@ -6,7 +6,7 @@ import {StyleSheet, View} from 'react-native'; import Animated from 'react-native-reanimated'; import {SafeAreaView} from 'react-native-safe-area-context'; -import {ssoLogin} from '@actions/remote/session'; +import {ssoLogin, ssoLoginWithCodeExchange} from '@actions/remote/session'; import {Screens, Sso} from '@constants'; import useAndroidHardwareBackHandler from '@hooks/android_back_handler'; import useNavButtonPressed from '@hooks/navigation_button_pressed'; @@ -98,6 +98,15 @@ const SSO = ({ goToHome(result.error); }; + const doSSOCodeExchange = async (loginCode: string, samlChallenge: {codeVerifier: string; state: string}) => { + const result: LoginActionResponse = await ssoLoginWithCodeExchange(serverUrl!, serverDisplayName, config.DiagnosticId!, loginCode, samlChallenge, serverPreauthSecret); + if (result?.error && result.failed) { + onLoadEndError(result.error); + return; + } + goToHome(result.error); + }; + const goToHome = (error?: unknown) => { const hasError = launchError || Boolean(error); resetToHome({extra, launchError: hasError, launchType, serverUrl}); @@ -126,6 +135,7 @@ const SSO = ({ const props = { doSSOLogin, + doSSOCodeExchange, loginError, loginUrl, setLoginError, diff --git a/app/screens/sso/sso.test.tsx b/app/screens/sso/sso.test.tsx index 3a1f0a8a7..d933ab3f1 100644 --- a/app/screens/sso/sso.test.tsx +++ b/app/screens/sso/sso.test.tsx @@ -19,6 +19,7 @@ describe('SSO with redirect url', () => { const baseProps = { customUrlScheme: LocalConfig.AuthUrlSchemeDev, doSSOLogin: jest.fn(), + doSSOCodeExchange: jest.fn(), intl: {}, loginError: '', loginUrl: '', diff --git a/app/screens/sso/sso_authentication.tsx b/app/screens/sso/sso_authentication.tsx index ef7fe859b..ce3aea86b 100644 --- a/app/screens/sso/sso_authentication.tsx +++ b/app/screens/sso/sso_authentication.tsx @@ -3,13 +3,14 @@ import {openAuthSessionAsync} from 'expo-web-browser'; import qs from 'querystringify'; -import React, {useEffect, useState} from 'react'; +import React, {useCallback, useEffect, useMemo, useState} from 'react'; import {useIntl} from 'react-intl'; import {Linking, Platform, StyleSheet, View, type EventSubscription} from 'react-native'; import urlParse from 'url-parse'; import {Sso} from '@constants'; import {isBetaApp} from '@utils/general'; +import {createSamlChallenge} from '@utils/saml_challenge'; import AuthError from './components/auth_error'; import AuthRedirect from './components/auth_redirect'; @@ -17,6 +18,7 @@ import AuthSuccess from './components/auth_success'; interface SSOAuthenticationProps { doSSOLogin: (bearerToken: string, csrfToken: string) => void; + doSSOCodeExchange: (loginCode: string, samlChallenge: {codeVerifier: string; state: string}) => void; loginError: string; loginUrl: string; setLoginError: (value: string) => void; @@ -30,7 +32,7 @@ const style = StyleSheet.create({ }, }); -const SSOAuthentication = ({doSSOLogin, loginError, loginUrl, setLoginError, theme}: SSOAuthenticationProps) => { +const SSOAuthentication = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, setLoginError, theme}: SSOAuthenticationProps) => { const [error, setError] = useState(''); const [loginSuccess, setLoginSuccess] = useState(false); const intl = useIntl(); @@ -40,7 +42,8 @@ const SSOAuthentication = ({doSSOLogin, loginError, loginUrl, setLoginError, the } const redirectUrl = customUrlScheme + 'callback'; - const init = async (resetErrors = true) => { + const samlChallenge = useMemo(() => createSamlChallenge(), []); + const init = useCallback(async (resetErrors = true) => { setLoginSuccess(false); if (resetErrors !== false) { setError(''); @@ -50,12 +53,22 @@ const SSOAuthentication = ({doSSOLogin, loginError, loginUrl, setLoginError, the const query: Record = { ...parsedUrl.query, redirect_to: redirectUrl, + state: samlChallenge.state, + code_challenge: samlChallenge.codeChallenge, + code_challenge_method: samlChallenge.method, }; parsedUrl.set('query', qs.stringify(query)); const url = parsedUrl.toString(); const result = await openAuthSessionAsync(url, null, {preferEphemeralSession: true, createTask: false}); if ('url' in result && result.url) { const resultUrl = urlParse(result.url, true); + const loginCode = resultUrl.query?.login_code as string | undefined; + if (loginCode) { + // Prefer code exchange when available + setLoginSuccess(true); + doSSOCodeExchange(loginCode, {codeVerifier: samlChallenge.codeVerifier, state: samlChallenge.state}); + return; + } const bearerToken = resultUrl.query?.MMAUTHTOKEN; const csrfToken = resultUrl.query?.MMCSRF; if (bearerToken && csrfToken) { @@ -70,7 +83,7 @@ const SSOAuthentication = ({doSSOLogin, loginError, loginUrl, setLoginError, the }), ); } - }; + }, [doSSOCodeExchange, doSSOLogin, intl, loginUrl, samlChallenge, redirectUrl, setLoginError]); useEffect(() => { let listener: EventSubscription | null = null; @@ -80,6 +93,12 @@ const SSOAuthentication = ({doSSOLogin, loginError, loginUrl, setLoginError, the setError(''); if (url && url.startsWith(redirectUrl)) { const parsedUrl = urlParse(url, true); + const loginCode = parsedUrl.query?.login_code as string | undefined; + if (loginCode) { + setLoginSuccess(true); + doSSOCodeExchange(loginCode, {codeVerifier: samlChallenge.codeVerifier, state: samlChallenge.state}); + return; + } const bearerToken = parsedUrl.query?.MMAUTHTOKEN; const csrfToken = parsedUrl.query?.MMCSRF; if (bearerToken && csrfToken) { @@ -107,7 +126,7 @@ const SSOAuthentication = ({doSSOLogin, loginError, loginUrl, setLoginError, the clearTimeout(timeout); listener?.remove(); }; - }, []); + }, [doSSOCodeExchange, doSSOLogin, init, intl, samlChallenge, redirectUrl]); 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 62f810e29..7ea3b0d4c 100644 --- a/app/screens/sso/sso_authentication_with_external_browser.tsx +++ b/app/screens/sso/sso_authentication_with_external_browser.tsx @@ -2,7 +2,7 @@ // See LICENSE.txt for license information. import qs from 'querystringify'; -import React, {useEffect, useState} from 'react'; +import React, {useCallback, useEffect, useMemo, useState} from 'react'; import {useIntl} from 'react-intl'; import {Linking, Platform, View} from 'react-native'; import urlParse from 'url-parse'; @@ -10,6 +10,7 @@ import urlParse from 'url-parse'; import {Sso} from '@constants'; import {isErrorWithMessage} from '@utils/errors'; 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'; @@ -20,6 +21,7 @@ import AuthSuccess from './components/auth_success'; interface SSOWithRedirectURLProps { doSSOLogin: (bearerToken: string, csrfToken: string) => void; + doSSOCodeExchange: (loginCode: string, samlChallenge: {codeVerifier: string; state: string}) => void; loginError: string; loginUrl: string; setLoginError: (value: string) => void; @@ -57,7 +59,7 @@ const getStyleSheet = makeStyleSheetFromTheme((theme: Theme) => { }; }); -const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, loginError, loginUrl, setLoginError, theme}: SSOWithRedirectURLProps) => { +const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, doSSOCodeExchange, loginError, loginUrl, setLoginError, theme}: SSOWithRedirectURLProps) => { const [error, setError] = useState(''); const [loginSuccess, setLoginSuccess] = useState(false); const style = getStyleSheet(theme); @@ -68,7 +70,8 @@ const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, loginError, loginUrl, } const redirectUrl = customUrlScheme + 'callback'; - const init = (resetErrors = true) => { + const samlChallenge = useMemo(() => createSamlChallenge(), []); + const init = useCallback((resetErrors = true) => { setLoginSuccess(false); if (resetErrors !== false) { setError(''); @@ -78,6 +81,9 @@ const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, loginError, loginUrl, const query: Record = { ...parsedUrl.query, redirect_to: redirectUrl, + state: samlChallenge.state, + code_challenge: samlChallenge.codeChallenge, + code_challenge_method: samlChallenge.method, }; parsedUrl.set('query', qs.stringify(query)); const url = parsedUrl.toString(); @@ -101,12 +107,19 @@ const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, loginError, loginUrl, }; tryOpenURL(url, onError); - }; + }, [intl, loginUrl, redirectUrl, samlChallenge, setLoginError]); useEffect(() => { + const startedRef = {current: false}; const onURLChange = ({url}: { url: string }) => { if (url && url.startsWith(redirectUrl)) { const parsedUrl = urlParse(url, true); + const loginCode = parsedUrl.query?.login_code as string | undefined; + if (loginCode) { + setLoginSuccess(true); + doSSOCodeExchange(loginCode, {codeVerifier: samlChallenge.codeVerifier, state: samlChallenge.state}); + return; + } const bearerToken = parsedUrl.query?.MMAUTHTOKEN; const csrfToken = parsedUrl.query?.MMCSRF; if (bearerToken && csrfToken) { @@ -126,13 +139,16 @@ const SSOAuthenticationWithExternalBrowser = ({doSSOLogin, loginError, loginUrl, const listener = Linking.addEventListener('url', onURLChange); const timeout = setTimeout(() => { - init(false); + if (!startedRef.current) { + startedRef.current = true; + init(false); + } }, 1000); return () => { listener.remove(); clearTimeout(timeout); }; - }, []); + }, [doSSOCodeExchange, doSSOLogin, init, intl, samlChallenge, redirectUrl]); let content; if (loginSuccess) { diff --git a/app/utils/saml_challenge.ts b/app/utils/saml_challenge.ts new file mode 100644 index 000000000..10fec6e1f --- /dev/null +++ b/app/utils/saml_challenge.ts @@ -0,0 +1,55 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +// SAML mobile code-exchange challenge helpers (modeled after RFC 7636) + +import base64 from 'base-64'; +import {getRandomValues, randomUUID} from 'expo-crypto'; +import {sha256} from 'js-sha256'; + +function getRandomBytes(length: number): Uint8Array { + const bytes = new Uint8Array(length); + getRandomValues(bytes); + return bytes; +} + +function bytesToBase64Url(bytes: Uint8Array): string { + let binary = ''; + for (let i = 0; i < bytes.length; i++) { + binary += String.fromCharCode(bytes[i]); + } + const b64 = base64.encode(binary); + return b64.replace(/\+/g, '-').replace(/\//g, '_').split('=').join(''); +} + +export function generateState(): string { + // Use UUID for state; allowed challenge chars include '-' + return randomUUID(); +} + +export function generateCodeVerifier(length = 64): string { + // Allowed characters are ALPHA / DIGIT / "-" / "." / "_" / "~" + // We generate random bytes and base64url encode without padding which fits the charset + const bytes = getRandomBytes(length); + return bytesToBase64Url(bytes); +} + +export function computeS256CodeChallenge(verifier: string): string { + const hashArrayBuffer = sha256.arrayBuffer(verifier) as ArrayBuffer; + const bytes = new Uint8Array(hashArrayBuffer); + return bytesToBase64Url(bytes); +} + +export type SAMLChallenge = { + state: string; + codeVerifier: string; + codeChallenge: string; + method: 'S256'; +}; + +export function createSamlChallenge(): SAMLChallenge { + const state = generateState(); + const codeVerifier = generateCodeVerifier(64); + const codeChallenge = computeS256CodeChallenge(codeVerifier); + return {state, codeVerifier, codeChallenge, method: 'S256'}; +} diff --git a/package-lock.json b/package-lock.json index f6d5d5ab5..63b7377bf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -64,6 +64,7 @@ "fflate": "0.8.2", "fuse.js": "7.1.0", "html-entities": "2.6.0", + "js-sha256": "0.11.1", "mime-db": "1.54.0", "moment-timezone": "0.5.48", "node-html-parser": "7.0.1", @@ -16012,6 +16013,11 @@ "integrity": "sha512-bF7vcQxbODoGK1imE2P9GS9aw4zD0Sd+Hni68IMZLj7zRnquH7dXUmMw9hDI5S/Jzt7q+IyTXN0rSg2GI0IKhQ==", "license": "MIT" }, + "node_modules/js-sha256": { + "version": "0.11.1", + "resolved": "https://registry.npmjs.org/js-sha256/-/js-sha256-0.11.1.tgz", + "integrity": "sha512-o6WSo/LUvY2uC4j7mO50a2ms7E/EAdbP0swigLV+nzHKTTaYnaLIWJ02VdXrsJX0vGedDESQnLsOekr94ryfjg==" + }, "node_modules/js-tokens": { "version": "4.0.0", "license": "MIT" diff --git a/package.json b/package.json index 02fdd2ab4..b852e6d45 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,7 @@ "fflate": "0.8.2", "fuse.js": "7.1.0", "html-entities": "2.6.0", + "js-sha256": "0.11.1", "mime-db": "1.54.0", "moment-timezone": "0.5.48", "node-html-parser": "7.0.1", diff --git a/scripts/pre-commit.sh b/scripts/pre-commit.sh index 6c37dfc1c..1f2785f6c 100755 --- a/scripts/pre-commit.sh +++ b/scripts/pre-commit.sh @@ -3,7 +3,7 @@ jsfiles=$(git diff --cached --name-only --diff-filter=ACM | grep -E '\.js$|\.ts$|\.tsx$') exit_code=0 -if [ -z "jsfiles" ]; then +if [ -z "$jsfiles" ]; then exit 0 fi diff --git a/test/setup.ts b/test/setup.ts index 5f18bb291..ed6749d70 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -35,6 +35,11 @@ jest.mock('expo-application', () => { jest.mock('expo-crypto', () => ({ randomUUID: jest.fn(() => '12345678-1234-1234-1234-1234567890ab'), + getRandomValues: jest.fn((arr: Uint8Array) => { + // deterministic non-zero bytes for tests + arr.fill(0x7b); + return arr; + }), })); jest.mock('expo-device', () => { diff --git a/types/vendor/js-sha256.d.ts b/types/vendor/js-sha256.d.ts new file mode 100644 index 000000000..7a404e5dc --- /dev/null +++ b/types/vendor/js-sha256.d.ts @@ -0,0 +1,15 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. +declare module 'js-sha256' { + export interface Sha256Fn { + (message: string | ArrayBuffer | Uint8Array): string; + array(message: string | ArrayBuffer | Uint8Array): number[]; + arrayBuffer(message: string | ArrayBuffer | Uint8Array): ArrayBuffer; + hex(message: string | ArrayBuffer | Uint8Array): string; + } + + const sha256: Sha256Fn; + export default sha256; + export {sha256}; +} +