From ddf2ce63b34ade5e975b669f8a84e2a10ef6cbf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Romero?= Date: Tue, 13 Oct 2026 21:03:18 +0000 Subject: [PATCH] chore: validate Android OAuth configuration --- .github/workflows/quality.yml | 1 + crates/flectar-mail-core/src/lib.rs | 22 +++ .../flectar-mail-core/src/oauth/providers.rs | 150 ++++++++++++++++-- .../flectar-mail-core/tests/oauth_settings.rs | 88 ++++++++-- platform/android/TESTING.md | 36 +++++ scripts/build-android-release.sh | 9 +- scripts/build-android-test-apk.sh | 12 +- scripts/test-android-oauth.py | 129 +++++++++++++++ scripts/verify-android-oauth.py | 48 ++++++ 9 files changed, 451 insertions(+), 44 deletions(-) create mode 100644 scripts/test-android-oauth.py create mode 100644 scripts/verify-android-oauth.py diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 462f77c8..750f0ccf 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -90,6 +90,7 @@ jobs: python3 scripts/test-release.py python3 scripts/test-pdf-packaging.py python3 scripts/test-android-jni.py + python3 scripts/test-android-oauth.py - name: Validate Android workspace lockfile run: cargo metadata --manifest-path platform/android/Cargo.toml --locked --format-version 1 > /dev/null - name: Reject unused direct dependencies diff --git a/crates/flectar-mail-core/src/lib.rs b/crates/flectar-mail-core/src/lib.rs index d724b3f5..f759685f 100644 --- a/crates/flectar-mail-core/src/lib.rs +++ b/crates/flectar-mail-core/src/lib.rs @@ -6935,6 +6935,20 @@ impl Core { .write(move |conn| { let tx = conn.transaction()?; let previous = repo::settings::get(&tx)?; + // Allow unrelated settings to be saved even if an older build + // persisted invalid OAuth keys; validate any newly supplied ID. + if settings.google_client_id != previous.google_client_id { + oauth::providers::validate_custom_client_id( + Provider::Gmail, + &settings.google_client_id, + )?; + } + if settings.ms_client_id != previous.ms_client_id { + oauth::providers::validate_custom_client_id( + Provider::Microsoft, + &settings.ms_client_id, + )?; + } repo::contacts::advance_learning_boundaries( &tx, !previous.collect_outgoing_contacts && settings.collect_outgoing_contacts, @@ -7148,6 +7162,14 @@ impl Core { google: Option<(String, String)>, microsoft: Option, ) -> Result { + // Reject a bad registration before either provider is written. Empty + // IDs remain the explicit way to clear a custom registration. + if let Some((id, _)) = &google { + oauth::providers::validate_custom_client_id(Provider::Gmail, id)?; + } + if let Some(id) = µsoft { + oauth::providers::validate_custom_client_id(Provider::Microsoft, id)?; + } self.db .write(move |conn| { let mut settings = repo::settings::get(conn)?; diff --git a/crates/flectar-mail-core/src/oauth/providers.rs b/crates/flectar-mail-core/src/oauth/providers.rs index da093849..cd72f4e5 100644 --- a/crates/flectar-mail-core/src/oauth/providers.rs +++ b/crates/flectar-mail-core/src/oauth/providers.rs @@ -63,6 +63,66 @@ pub fn for_provider(p: Provider) -> Option<&'static OAuthProviderConfig> { } } +/// Validate local syntax, not the existence or consent status of a provider +/// registration. Microsoft application IDs are non-nil, hyphenated GUIDs. +pub fn validate_client_id(provider: Provider, client_id: &str) -> Result<()> { + let id = client_id.trim(); + let valid = match provider { + Provider::Microsoft => { + let bytes = id.as_bytes(); + bytes.len() == 36 + && bytes.iter().enumerate().all(|(index, byte)| { + if matches!(index, 8 | 13 | 18 | 23) { + *byte == b'-' + } else { + byte.is_ascii_hexdigit() + } + }) + && id != "00000000-0000-0000-0000-000000000000" + } + // Google client IDs are opaque. Reject our former build placeholder + // without imposing an undocumented format on existing registrations. + Provider::Gmail => { + !id.is_empty() + && !id.eq_ignore_ascii_case("test.apps.googleusercontent.com") + && !id.chars().any(|ch| ch.is_whitespace() || ch.is_control()) + } + Provider::Imap => false, + }; + if valid { + Ok(()) + } else { + Err(CoreError::Auth(match provider { + Provider::Microsoft => "Enter the Microsoft Application (client) ID from your app registration in Sign-in settings; it must be a nonzero GUID, not a placeholder.".into(), + Provider::Gmail => "Enter your Google OAuth client ID in Sign-in settings; a test placeholder cannot be used for sign-in.".into(), + Provider::Imap => "provider does not use oauth".into(), + })) + } +} + +pub(crate) fn validate_custom_client_id(provider: Provider, client_id: &str) -> Result<()> { + if client_id.trim().is_empty() { + // Clearing a custom registration restores the bundled default. + Ok(()) + } else { + validate_client_id(provider, client_id) + } +} + +fn checked_credentials( + provider: Provider, + client_id: &str, + client_secret: Option<&str>, +) -> Result<(String, Option)> { + validate_client_id(provider, client_id)?; + let secret = client_secret + .filter(|_| provider == Provider::Gmail) + .map(str::trim) + .filter(|secret| !secret.is_empty()) + .map(str::to_owned); + Ok((client_id.trim().to_owned(), secret)) +} + /// Client credentials the user entered in Settings. Refreshed on startup and /// whenever settings change. An explicit user registration overrides the app /// registration bundled by the build. @@ -109,15 +169,9 @@ fn bundled_credentials(provider: Provider) -> Option<(String, Option)> { Provider::Microsoft => (option_env!("FLECTAR_MICROSOFT_DESKTOP_CLIENT_ID"), None), Provider::Imap => return None, }; - let client_id = client_id?.trim(); - if client_id.is_empty() { - return None; - } - let client_secret = client_secret - .map(str::trim) - .filter(|secret| !secret.is_empty()) - .map(str::to_owned); - Some((client_id.to_owned(), client_secret)) + // Invalid build-time defaults must not enable the sign-in buttons. Runtime + // or user configuration may still provide a real registration. + checked_credentials(provider, client_id?, client_secret).ok() } pub fn has_bundled_credentials(provider: Provider) -> bool { @@ -134,7 +188,8 @@ pub fn resolve_credentials(provider: Provider) -> Result<(String, Option .unwrap_or_else(std::sync::PoisonError::into_inner) .get(&provider) { - return Ok(entry.clone()); + // An invalid saved override must not silently switch registrations. + return checked_credentials(provider, &entry.0, entry.1.as_deref()); } if let Some(entry) = bundled_credentials(provider) { return Ok(entry); @@ -157,7 +212,7 @@ pub fn resolve_credentials(provider: Provider) -> Result<(String, Option let secret = client_secret_env .and_then(|name| std::env::var(name).ok()) .filter(|secret| !secret.trim().is_empty()); - return Ok((id, secret)); + return checked_credentials(provider, &id, secret.as_deref()); } Err(CoreError::Auth(format!( "no OAuth app configured for {}: use custom OAuth credentials in Settings → General, or build with {}", @@ -185,16 +240,72 @@ fn bundled_client_id_env(provider: Provider) -> &'static str { mod tests { use super::*; + #[test] + fn rejects_nil_and_malformed_microsoft_app_ids() { + for id in [ + "", + " ", + "00000000-0000-0000-0000-000000000000", + "00000000000000000000000000000000", + "ms-id", + "11111111-2222-4333-8444-55555555555z", + "111111112222-4333-8444-555555555555", + "{11111111-2222-4333-8444-555555555555}", + "11111111-2222-4333-8444-555555555555?x=1", + ] { + assert!( + validate_client_id(Provider::Microsoft, id).is_err(), + "{id:?}" + ); + assert!(checked_credentials(Provider::Microsoft, id, None).is_err()); + } + for id in [ + "11111111-2222-4333-8444-555555555555", + "ABCDEF01-2345-4678-9ABC-DEF012345678", + " abcdef01-2345-4678-9abc-def012345678 ", + ] { + let (actual, secret) = + checked_credentials(Provider::Microsoft, id, Some("ignored")).unwrap(); + assert_eq!(actual, id.trim()); + assert!(secret.is_none()); + } + assert!(validate_custom_client_id(Provider::Microsoft, " ").is_ok()); + } + + #[test] + fn rejects_legacy_google_placeholder_and_preserves_opaque_ids() { + for id in [ + "", + "test.apps.googleusercontent.com", + " TEST.apps.googleusercontent.com ", + "id with spaces", + ] { + assert!(checked_credentials(Provider::Gmail, id, None).is_err()); + } + assert_eq!( + checked_credentials(Provider::Gmail, " custom-google-id ", Some(" secret ")).unwrap(), + ("custom-google-id".into(), Some("secret".into())) + ); + } + #[test] fn resolve_uses_configured_values_and_clears() { // Explicit settings always win, including in a release build that has // an official registration compiled into it. - set_configured(Provider::Microsoft, " ms-id ", ""); + set_configured( + Provider::Microsoft, + " 11111111-2222-4333-8444-555555555555 ", + "", + ); let (id, secret) = resolve_credentials(Provider::Microsoft).unwrap(); - assert_eq!(id, "ms-id"); + assert_eq!(id, "11111111-2222-4333-8444-555555555555"); assert_eq!(secret, None); - set_configured(Provider::Microsoft, "ms-id", " ignored-secret "); + set_configured( + Provider::Microsoft, + "11111111-2222-4333-8444-555555555555", + " ignored-secret ", + ); let (_, secret) = resolve_credentials(Provider::Microsoft).unwrap(); assert_eq!(secret, None); @@ -203,6 +314,15 @@ mod tests { assert_eq!(secret.as_deref(), Some("s3cret")); set_configured(Provider::Gmail, "", ""); + // Even if a valid default is bundled, an invalid explicit override + // cannot authorize under that default's different app registration. + set_configured( + Provider::Microsoft, + "00000000-0000-0000-0000-000000000000", + "", + ); + assert!(resolve_credentials(Provider::Microsoft).is_err()); + // Empty id clears the custom registration. A bundled registration may // then become active, so only verify that the custom value is gone. set_configured(Provider::Microsoft, "", "whatever"); @@ -210,7 +330,7 @@ mod tests { resolve_credentials(Provider::Microsoft) .ok() .map(|entry| entry.0), - Some("ms-id".to_owned()) + Some("11111111-2222-4333-8444-555555555555".to_owned()) ); } diff --git a/crates/flectar-mail-core/tests/oauth_settings.rs b/crates/flectar-mail-core/tests/oauth_settings.rs index 3f0c83ae..543b0976 100644 --- a/crates/flectar-mail-core/tests/oauth_settings.rs +++ b/crates/flectar-mail-core/tests/oauth_settings.rs @@ -10,7 +10,7 @@ async fn oauth_keys_persist_clear_and_remain_unchanged_after_failed_save() { let mut settings = core.get_settings().await.unwrap(); settings.google_client_id = "custom-google-id".into(); settings.google_client_secret = "custom-google-secret".into(); - settings.ms_client_id = "custom-ms-id".into(); + settings.ms_client_id = "11111111-2222-4333-8444-555555555555".into(); core.set_settings(settings.clone()).await.unwrap(); let saved = core .set_oauth_apps( @@ -18,12 +18,12 @@ async fn oauth_keys_persist_clear_and_remain_unchanged_after_failed_save() { " custom-google-id ".into(), " custom-google-secret ".into(), )), - Some(" custom-ms-id ".into()), + Some(" 11111111-2222-4333-8444-555555555555 ".into()), ) .await .unwrap(); assert_eq!(saved.google_client_id, "custom-google-id"); - assert_eq!(saved.ms_client_id, "custom-ms-id"); + assert_eq!(saved.ms_client_id, "11111111-2222-4333-8444-555555555555"); assert_eq!( providers::resolve_credentials(Provider::Gmail).unwrap(), ( @@ -33,11 +33,43 @@ async fn oauth_keys_persist_clear_and_remain_unchanged_after_failed_save() { ); assert_eq!( providers::resolve_credentials(Provider::Microsoft).unwrap(), - ("custom-ms-id".into(), None) + ("11111111-2222-4333-8444-555555555555".into(), None) + ); + + // Invalid user input must not persist or partially update the other provider. + for id in ["00000000-0000-0000-0000-000000000000", "not-an-app-id"] { + assert!( + core.set_oauth_apps( + Some(("must-not-be-saved".into(), "secret".into())), + Some(id.into()), + ) + .await + .is_err() + ); + let stored = core.get_settings().await.unwrap(); + assert_eq!(stored.google_client_id, "custom-google-id"); + assert_eq!(stored.ms_client_id, "11111111-2222-4333-8444-555555555555"); + let mut invalid_settings = stored; + invalid_settings.ms_client_id = id.into(); + assert!(core.set_settings(invalid_settings).await.is_err()); + assert_eq!( + providers::resolve_credentials(Provider::Microsoft) + .unwrap() + .0, + "11111111-2222-4333-8444-555555555555" + ); + } + assert!( + core.set_oauth_apps( + Some(("test.apps.googleusercontent.com".into(), String::new())), + None, + ) + .await + .is_err() ); drop(core); - let core = Core::start(paths).await.unwrap(); + let core = Core::start(paths.clone()).await.unwrap(); assert_eq!( core.get_settings().await.unwrap().google_client_id, "custom-google-id" @@ -60,7 +92,7 @@ async fn oauth_keys_persist_clear_and_remain_unchanged_after_failed_save() { assert!( core.set_oauth_apps( Some(("must-not-become-active".into(), "wrong-secret".into())), - Some("must-not-become-active".into()), + Some("44444444-5555-4666-8777-888888888888".into()), ) .await .is_err() @@ -69,7 +101,7 @@ async fn oauth_keys_persist_clear_and_remain_unchanged_after_failed_save() { providers::resolve_credentials(Provider::Microsoft) .unwrap() .0, - "custom-ms-id" + "11111111-2222-4333-8444-555555555555" ); assert_eq!( providers::resolve_credentials(Provider::Gmail).unwrap().0, @@ -97,7 +129,7 @@ async fn oauth_keys_persist_clear_and_remain_unchanged_after_failed_save() { .unwrap(); assert!(saved.google_client_secret.is_empty()); assert_eq!(saved.theme, "dark"); - assert_eq!(saved.ms_client_id, "custom-ms-id"); + assert_eq!(saved.ms_client_id, "11111111-2222-4333-8444-555555555555"); assert_eq!( providers::resolve_credentials(Provider::Gmail).ok(), defaults @@ -107,7 +139,7 @@ async fn oauth_keys_persist_clear_and_remain_unchanged_after_failed_save() { providers::resolve_credentials(Provider::Microsoft) .unwrap() .0, - "custom-ms-id" + "11111111-2222-4333-8444-555555555555" ); // A busy writer must suspend the save, leaving the async executor free. let db = core.db.clone(); @@ -125,7 +157,7 @@ async fn oauth_keys_persist_clear_and_remain_unchanged_after_failed_save() { entered_rx.await.unwrap(); let mut save = Box::pin(core.set_oauth_apps( Some(("delayed-google".into(), "secret".into())), - Some("delayed-microsoft".into()), + Some("22222222-3333-4444-8555-666666666666".into()), )); assert!( tokio::time::timeout(std::time::Duration::from_millis(25), &mut save) @@ -135,20 +167,50 @@ async fn oauth_keys_persist_clear_and_remain_unchanged_after_failed_save() { release_tx.send(()).unwrap(); let saved = save.await.unwrap(); assert_eq!(saved.google_client_id, "delayed-google"); - assert_eq!(saved.ms_client_id, "delayed-microsoft"); + assert_eq!(saved.ms_client_id, "22222222-3333-4444-8555-666666666666"); blocked_writer.await.unwrap(); // Retry both providers after the write failure; both must be saved together. core.set_oauth_apps( Some(("retry-google".into(), "retry-secret".into())), - Some("retry-microsoft".into()), + Some("33333333-4444-4555-8666-777777777777".into()), ) .await .unwrap(); let saved = core.get_settings().await.unwrap(); assert_eq!(saved.google_client_id, "retry-google"); - assert_eq!(saved.ms_client_id, "retry-microsoft"); + assert_eq!(saved.ms_client_id, "33333333-4444-4555-8666-777777777777"); core.set_oauth_apps(Some((String::new(), String::new())), Some(String::new())) .await .unwrap(); + + // Emulate configuration persisted by an older version, bypassing new input + // validation. Startup must remain usable and must never launch a browser + // with the placeholder from issue #65. + core.db + .write(|conn| { + let mut settings = flectar_mail_core::db::repo::settings::get(conn)?; + settings.ms_client_id = "00000000-0000-0000-0000-000000000000".into(); + flectar_mail_core::db::repo::settings::set(conn, &settings) + }) + .await + .unwrap(); + drop(core); + let core = Core::start(paths).await.unwrap(); + assert!(providers::resolve_credentials(Provider::Microsoft).is_err()); + let result = flectar_mail_core::oauth::flow::authorize(Provider::Microsoft, |_| { + panic!("invalid OAuth registration must never open the browser") + }) + .await; + assert!(result.is_err()); + let mut settings = core.get_settings().await.unwrap(); + settings.theme = "light".into(); + core.set_settings(settings).await.unwrap(); + core.set_oauth_apps(None, Some("11111111-2222-4333-8444-555555555555".into())) + .await + .unwrap(); + assert!(providers::resolve_credentials(Provider::Microsoft).is_ok()); + core.set_oauth_apps(None, Some(String::new())) + .await + .unwrap(); } diff --git a/platform/android/TESTING.md b/platform/android/TESTING.md index 882b0f3d..e9372656 100644 --- a/platform/android/TESTING.md +++ b/platform/android/TESTING.md @@ -68,3 +68,39 @@ those callbacks use the same library-loading path. Record the APK version, Android version and test results before closing #64. If Google authorization still fails, preserve the provider's error separately from JNI linkage failures; never include access tokens or mailbox data in logs. + +## OAuth configuration and issue #65 + +Preview builds may omit either `FLECTAR_GOOGLE_ANDROID_CLIENT_ID` or +`FLECTAR_MICROSOFT_ANDROID_CLIENT_ID`. No placeholder is substituted: that +provider stays unavailable until a valid custom registration is saved through +Sign-in settings. A supplied invalid ID fails the build before compilation. +Production builds require both IDs and verify that the configured Microsoft +redirect matches the APK signing certificate. + +Microsoft client IDs must be the nonzero Application (client) ID from Entra app +registration, in hyphenated GUID form. The app validates bundled, custom and +runtime configuration before starting authorization. Existing invalid saved +IDs remain editable, and unrelated settings remain usable. Invalid custom +input is rejected without changing either provider's saved registration. + +For working Outlook sign-in, register the Android package `com.flectar.mail` +and the signing certificate of the APK being installed. Obtain the certificate +hash and exact redirect with `scripts/android-signing-identities.sh `. +The running app derives its redirect from its installed signing certificate; +setting a build variable alone does not register that redirect with Microsoft. +The app uses the `common` authority for work/school and personal Microsoft +accounts, so choose the corresponding supported account types in Entra. This +is a public client using authorization code with PKCE; do not add a client secret. + +References: [Microsoft app registration](https://learn.microsoft.com/en-us/entra/identity-platform/quickstart-register-app), +[Android client configuration](https://learn.microsoft.com/en-us/entra/msal/android/msal-configuration), +and [Android signing redirects](https://learn.microsoft.com/en-us/entra/msal/android/frequently-asked-questions). + +Run `python3 scripts/test-android-oauth.py` for the build configuration regression +tests. Run `cargo test -p flectar-mail-core --locked --test oauth_settings` for +saved configuration, atomic rejection, and the no-browser-on-invalid-ID check. +Local syntax validation cannot establish that an ID is registered, that its +redirect is registered, or that a tenant permits consent. Check real sign-in +using the release registration and signing certificate before promising +built-in Outlook availability. diff --git a/scripts/build-android-release.sh b/scripts/build-android-release.sh index 61afb62e..501ea0d8 100755 --- a/scripts/build-android-release.sh +++ b/scripts/build-android-release.sh @@ -29,14 +29,7 @@ if [[ ! -f "$FLECTAR_ANDROID_KEYSTORE" ]]; then printf 'Production keystore not found: %s\n' "$FLECTAR_ANDROID_KEYSTORE" >&2 exit 1 fi -if [[ "$FLECTAR_GOOGLE_ANDROID_CLIENT_ID" != *.apps.googleusercontent.com || "$FLECTAR_GOOGLE_ANDROID_CLIENT_ID" == test.* ]]; then - printf 'FLECTAR_GOOGLE_ANDROID_CLIENT_ID is not a production Android OAuth client ID.\n' >&2 - exit 1 -fi -if [[ ! "$FLECTAR_MICROSOFT_ANDROID_CLIENT_ID" =~ ^[0-9A-Fa-f-]{36}$ || "$FLECTAR_MICROSOFT_ANDROID_CLIENT_ID" == 00000000-0000-0000-0000-000000000000 ]]; then - printf 'FLECTAR_MICROSOFT_ANDROID_CLIENT_ID is not a production Entra application ID.\n' >&2 - exit 1 -fi +python3 "$project_dir/scripts/verify-android-oauth.py" --require-all if [[ ! "$FLECTAR_MICROSOFT_ANDROID_REDIRECT_URI" =~ ^msauth://com\.flectar\.mail/.+ ]]; then printf 'Microsoft redirect must be msauth://com.flectar.mail/.\n' >&2 exit 1 diff --git a/scripts/build-android-test-apk.sh b/scripts/build-android-test-apk.sh index fa2c7361..ded59d6e 100755 --- a/scripts/build-android-test-apk.sh +++ b/scripts/build-android-test-apk.sh @@ -6,6 +6,10 @@ target_dir="$project_dir/target" android_sdk="${ANDROID_HOME:-${ANDROID_SDK_ROOT:-}}" android_ndk="${ANDROID_NDK_ROOT:-${ANDROID_NDK_HOME:-}}" +# Missing registrations are supported: the app shows Sign-in settings instead +# of sending a placeholder client ID to a provider. Reject invalid supplied IDs. +python3 "$project_dir/scripts/verify-android-oauth.py" + if [[ -z "$android_sdk" || ! -d "$android_sdk" ]]; then printf 'Android SDK not found. Set ANDROID_HOME to your Android SDK directory.\n' >&2 exit 1 @@ -68,14 +72,6 @@ fi export ANDROID_HOME="$android_sdk" export CARGO_TARGET_DIR="$target_dir" -# Test builds have deterministic provider placeholders so static packaging and -# device startup can be exercised without borrowing production registrations. -# Provider sign-in intentionally remains unavailable until the release-owner -# values documented in ANDROID_FUNCTIONAL_APP_TODO.md are supplied. -export FLECTAR_GOOGLE_ANDROID_CLIENT_ID="${FLECTAR_GOOGLE_ANDROID_CLIENT_ID:-test.apps.googleusercontent.com}" -export FLECTAR_MICROSOFT_ANDROID_CLIENT_ID="${FLECTAR_MICROSOFT_ANDROID_CLIENT_ID:-00000000-0000-0000-0000-000000000000}" -export FLECTAR_MICROSOFT_ANDROID_REDIRECT_URI="${FLECTAR_MICROSOFT_ANDROID_REDIRECT_URI:-msauth://com.flectar.mail/test-signature-hash}" - cargo build \ --locked \ --manifest-path "$project_dir/platform/android/Cargo.toml" \ diff --git a/scripts/test-android-oauth.py b/scripts/test-android-oauth.py new file mode 100644 index 00000000..80c6b616 --- /dev/null +++ b/scripts/test-android-oauth.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +"""Validate Android OAuth configuration and the actual build-script environment.""" +import importlib.util +import json +import os +from pathlib import Path +import shutil +import subprocess +import sys +import tempfile +import unittest + +sys.dont_write_bytecode = True +SCRIPTS = Path(__file__).resolve().parent +spec = importlib.util.spec_from_file_location("oauth", SCRIPTS / "verify-android-oauth.py") +oauth = importlib.util.module_from_spec(spec) +spec.loader.exec_module(oauth) +GOOGLE = "FLECTAR_GOOGLE_ANDROID_CLIENT_ID" +MICROSOFT = "FLECTAR_MICROSOFT_ANDROID_CLIENT_ID" +REDIRECT = "FLECTAR_MICROSOFT_ANDROID_REDIRECT_URI" +VALID = { + GOOGLE: "123456789-fixture.apps.googleusercontent.com", + MICROSOFT: "abcdef01-2345-4678-9abc-def012345678", +} + + +class AndroidOAuthTests(unittest.TestCase): + def test_optional_providers_and_production_requirements(self): + for values in ({}, {GOOGLE: "", MICROSOFT: " "}, {GOOGLE: VALID[GOOGLE]}, {MICROSOFT: VALID[MICROSOFT]}): + oauth.verify(values) + with self.assertRaises(ValueError): + oauth.verify(values, require_all=True) + oauth.verify(VALID, require_all=True) + oauth.verify({GOOGLE: VALID[GOOGLE], MICROSOFT: " " + VALID[MICROSOFT].upper() + " "}) + + def test_rejects_placeholders_and_malformed_ids_without_echoing_values(self): + for key, value in ( + (MICROSOFT, "00000000-0000-0000-0000-000000000000"), + (MICROSOFT, "------------------------------------"), + (MICROSOFT, "a" * 36), + (MICROSOFT, VALID[MICROSOFT] + "?unexpected=value"), + (MICROSOFT, "invalid-app-id"), + (GOOGLE, "test.apps.googleusercontent.com"), + (GOOGLE, "TEST.apps.googleusercontent.com"), + (GOOGLE, "wrong-suffix"), + ): + with self.subTest(key=key, value=value): + with self.assertRaises(ValueError) as error: + oauth.verify({key: value}) + self.assertIn(key, str(error.exception)) + self.assertNotIn(value, str(error.exception)) + + def run_build(self, script, configuration): + # Run the real shell entry points. Replace Cargo with a recorder that + # stops before compilation; no SDK downloads, signing or network calls. + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + (root / "scripts").mkdir() + for name in (script, "verify-android-oauth.py"): + shutil.copyfile(SCRIPTS / name, root / "scripts" / name) + sdk = root / "sdk" + sdk.mkdir() + toolchain = root / "ndk/toolchains/llvm/prebuilt/fixture/bin" + toolchain.mkdir(parents=True) + binaries = root / "bin" + binaries.mkdir() + for path in (toolchain / "aarch64-linux-android26-clang", binaries / "gradle", binaries / "keytool"): + path.write_text("#!/bin/sh\nexit 99\n") + path.chmod(0o755) + capture = root / "cargo-environment.json" + cargo = binaries / "cargo" + cargo.write_text( + "#!/usr/bin/env python3\nimport os,json,pathlib\n" + "pathlib.Path(os.environ['OAUTH_CAPTURE']).write_text(json.dumps({" + "key:os.environ.get(key) for key in " + repr((GOOGLE, MICROSOFT, REDIRECT)) + "}))\n" + "raise SystemExit(73)\n" + ) + cargo.chmod(0o755) + keystore = root / "test.keystore" + keystore.touch() + env = {key: value for key, value in os.environ.items() if not key.startswith("FLECTAR_")} + env.update({ + "PATH": str(binaries) + os.pathsep + env.get("PATH", ""), + "ANDROID_HOME": str(sdk), + "ANDROID_NDK_ROOT": str(root / "ndk"), + "GRADLE_CMD": str(binaries / "gradle"), + "XDG_DATA_HOME": str(root / "data"), + "FLECTAR_ANDROID_TEST_KEYSTORE": str(keystore), + "FLECTAR_ANDROID_KEYSTORE": str(keystore), + "FLECTAR_ANDROID_KEYSTORE_PASSWORD": "fixture", + "FLECTAR_ANDROID_KEY_ALIAS": "fixture", + "FLECTAR_ANDROID_KEY_PASSWORD": "fixture", + "FLECTAR_ANDROID_VERSION_CODE": "1", + "FLECTAR_ANDROID_VERSION_NAME": "0.1.0", + "OAUTH_CAPTURE": str(capture), + **configuration, + }) + result = subprocess.run(["bash", str(root / "scripts" / script)], + env=env, capture_output=True, text=True, timeout=15) + recorded = json.loads(capture.read_text()) if capture.exists() else None + return result, recorded + + def test_preview_never_injects_placeholder_defaults(self): + for values in ({}, {GOOGLE: "", MICROSOFT: "", REDIRECT: ""}, VALID): + with self.subTest(configuration=values): + result, recorded = self.run_build("build-android-test-apk.sh", values) + self.assertEqual(result.returncode, 73, result.stdout + result.stderr) + self.assertEqual(recorded, {key: values.get(key) for key in (GOOGLE, MICROSOFT, REDIRECT)}) + + def test_preview_rejects_invalid_ids_before_compilation(self): + for values in ({MICROSOFT: "00000000-0000-0000-0000-000000000000"}, {GOOGLE: "test.apps.googleusercontent.com"}): + result, recorded = self.run_build("build-android-test-apk.sh", values) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIsNone(recorded) + self.assertIn("not a valid", result.stderr) + + def test_production_requires_real_ids_before_compilation(self): + values = {**VALID, REDIRECT: "msauth://com.flectar.mail/fixture-signature"} + result, recorded = self.run_build("build-android-release.sh", values) + self.assertEqual(result.returncode, 73, result.stdout + result.stderr) + self.assertEqual(recorded, values) + for invalid in ("", "00000000-0000-0000-0000-000000000000", "a" * 36): + result, recorded = self.run_build("build-android-release.sh", {**values, MICROSOFT: invalid}) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIsNone(recorded) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/verify-android-oauth.py b/scripts/verify-android-oauth.py new file mode 100644 index 00000000..829bd420 --- /dev/null +++ b/scripts/verify-android-oauth.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python3 +"""Reject invalid bundled Android OAuth IDs without printing configuration values. + +Preview APKs may omit either provider. Production builds require both. Local +validation cannot establish that an app is registered or that consent is allowed. +""" +import argparse +import os +import re +import sys + + +def verify(environment, require_all=False): + for provider, variable in ( + ("Google", "FLECTAR_GOOGLE_ANDROID_CLIENT_ID"), + ("Microsoft", "FLECTAR_MICROSOFT_ANDROID_CLIENT_ID"), + ): + value = environment.get(variable, "").strip() + if not value: + if require_all: + raise ValueError(f"Production Android build requires {variable}.") + continue + if provider == "Microsoft": + valid = ( + re.fullmatch(r"[0-9a-fA-F]{8}(?:-[0-9a-fA-F]{4}){3}-[0-9a-fA-F]{12}", value) + and value != "00000000-0000-0000-0000-000000000000" + ) + else: + valid = ( + re.fullmatch(r"[^\s]+\.apps\.googleusercontent\.com", value) + and not value.lower().startswith("test.") + ) + if not valid: + raise ValueError( + f"{variable} is not a valid {provider} application ID. " + "Use a real app registration, or omit it for a preview without bundled sign-in." + ) + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--require-all", action="store_true") + args = parser.parse_args() + try: + verify(os.environ, args.require_all) + except ValueError as error: + print(error, file=sys.stderr) + sys.exit(1)