* MM-66813 - enhance sso callback metadata
* fix translation order
* Add serverUrl validation, remove non-null assertions
(cherry picked from commit 3735aa6ff8)
Co-authored-by: Pablo Vélez <pablovv2016@gmail.com>
This commit is contained in:
parent
4ef7d5d6fe
commit
37c4de6bb4
5 changed files with 134 additions and 9 deletions
|
|
@ -55,6 +55,18 @@ const SSO = ({
|
|||
const intl = useIntl();
|
||||
const [loginError, setLoginError] = useState<string>('');
|
||||
|
||||
// Validate serverUrl is provided
|
||||
if (!serverUrl) {
|
||||
return (
|
||||
<View
|
||||
nativeID={SecurityManager.getShieldScreenId(componentId, false, true)}
|
||||
style={styles.flex}
|
||||
>
|
||||
<Background theme={theme}/>
|
||||
</View>
|
||||
);
|
||||
}
|
||||
|
||||
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,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<string>('');
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -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<string>('');
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Reference in a new issue