mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
This commit is contained in:
@@ -85,6 +85,20 @@ fn origin_of(url: &str) -> Option<String> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Does `origin` (a `scheme://host[:port]` string from [`origin_of`]) match
|
||||
/// a specific host? Tolerates an explicit port suffix on `origin` so the
|
||||
/// callers can pass canonical hosts without hard-coding default ports.
|
||||
fn origin_host_is(origin: &str, host: &str) -> bool {
|
||||
let Some(rest) = origin
|
||||
.strip_prefix("https://")
|
||||
.or_else(|| origin.strip_prefix("http://"))
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
let host_part = rest.split(':').next().unwrap_or(rest);
|
||||
host_part.eq_ignore_ascii_case(host)
|
||||
}
|
||||
|
||||
fn target_matches_account_url(target_url: &str, account_id: &str) -> bool {
|
||||
let marker = placeholder_marker(account_id);
|
||||
let marker_fragment = format!("#{marker}");
|
||||
@@ -273,27 +287,51 @@ async fn run_session_cycle<R: Runtime>(
|
||||
// `forward_native_notification` in `webview_accounts`. Without it,
|
||||
// the constructor silently no-ops and no toast ever fires (#1016).
|
||||
if let Some(origin) = origin_of(&real_url) {
|
||||
// Default permission set every embedded provider needs. Origin-scoped
|
||||
// so we don't leak grants across providers running in the same CEF
|
||||
// browser process.
|
||||
let mut perms: Vec<&str> = vec!["notifications"];
|
||||
|
||||
// Google Meet additionally needs:
|
||||
// - audioCapture / videoCapture: getUserMedia for cam/mic so the
|
||||
// pre-call greenroom auto-grants instead of falling back to
|
||||
// Meet's "Use microphone and camera" consent dialog
|
||||
// - displayCapture: getDisplayMedia for screen-share present
|
||||
// - clipboardReadWrite: copy meeting link / paste join code
|
||||
// Without these, Meet sits on the consent dialog forever and cam/mic
|
||||
// never enumerate (verified during #1022 smoke).
|
||||
if origin_host_is(&origin, "meet.google.com") {
|
||||
perms.extend_from_slice(&[
|
||||
"audioCapture",
|
||||
"videoCapture",
|
||||
"displayCapture",
|
||||
"clipboardReadWrite",
|
||||
]);
|
||||
}
|
||||
|
||||
if let Err(e) = cdp
|
||||
.call(
|
||||
"Browser.grantPermissions",
|
||||
json!({
|
||||
"origin": origin,
|
||||
"permissions": ["notifications"],
|
||||
"permissions": perms,
|
||||
}),
|
||||
None,
|
||||
)
|
||||
.await
|
||||
{
|
||||
log::warn!(
|
||||
"[cdp-session][{}] Browser.grantPermissions(notifications) for {} failed: {}",
|
||||
"[cdp-session][{}] Browser.grantPermissions({:?}) for {} failed: {}",
|
||||
account_id,
|
||||
perms,
|
||||
origin,
|
||||
e
|
||||
);
|
||||
} else {
|
||||
log::info!(
|
||||
"[cdp-session][{}] granted notifications for origin={}",
|
||||
"[cdp-session][{}] granted {:?} for origin={}",
|
||||
account_id,
|
||||
perms,
|
||||
origin
|
||||
);
|
||||
}
|
||||
@@ -401,6 +439,33 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn origin_host_is_matches_canonical_origin() {
|
||||
assert!(origin_host_is("https://meet.google.com", "meet.google.com"));
|
||||
assert!(origin_host_is(
|
||||
"http://meet.google.com:8080",
|
||||
"meet.google.com"
|
||||
));
|
||||
assert!(origin_host_is("https://MEET.GOOGLE.COM", "meet.google.com"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn origin_host_is_rejects_non_match() {
|
||||
// Different host
|
||||
assert!(!origin_host_is(
|
||||
"https://workspace.google.com",
|
||||
"meet.google.com"
|
||||
));
|
||||
// Subdomain mismatch
|
||||
assert!(!origin_host_is(
|
||||
"https://chat.meet.google.com",
|
||||
"meet.google.com"
|
||||
));
|
||||
// Non-http scheme
|
||||
assert!(!origin_host_is("about:blank", "meet.google.com"));
|
||||
assert!(!origin_host_is("file:///etc/hosts", "meet.google.com"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn target_match_accepts_placeholder_and_real_provider_fragments_only_for_same_account() {
|
||||
assert!(target_matches_account_url(
|
||||
|
||||
@@ -243,10 +243,23 @@ fn popup_should_navigate_parent(provider: &str, url: &Url) -> Option<Url> {
|
||||
return None;
|
||||
}
|
||||
if is_google_auth_popup(url) {
|
||||
Some(url.clone())
|
||||
} else {
|
||||
None
|
||||
return Some(url.clone());
|
||||
}
|
||||
// Gmeet: "Start an instant meeting" / "New meeting" / clicking
|
||||
// a meeting code link calls `window.open(meet.google.com/<roomid>)`
|
||||
// to launch the room. Default popup handling would route the
|
||||
// URL to the user's system browser, leaking the Meet session
|
||||
// out of OpenHuman entirely. Deny the popup and navigate the
|
||||
// embedded parent into the room URL instead — matches the
|
||||
// user's expectation that the meeting stays in-app.
|
||||
if provider == "google-meet" {
|
||||
if let Some(host) = url.host_str() {
|
||||
if host == "meet.google.com" {
|
||||
return Some(url.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
@@ -1482,6 +1495,40 @@ pub async fn webview_account_open<R: Runtime>(
|
||||
err
|
||||
);
|
||||
}
|
||||
// Google Meet: when Google's edge SSR-redirects the post-account-
|
||||
// picker URL to `workspace.google.com/products/meet/...` (the
|
||||
// marketing landing page), `workspace.google.com` matches the
|
||||
// bare `google.com` suffix in `provider_allowed_hosts` so
|
||||
// `url_is_internal` would commit the navigation and the user
|
||||
// would land on the Workspace marketing page instead of Meet.
|
||||
// Catch this here and replace the parent URL with the canonical
|
||||
// Meet entry point so the embedded view stays on the app.
|
||||
if nav_provider == "google-meet" {
|
||||
if let Some(host) = url.host_str() {
|
||||
if host == "workspace.google.com" || host.ends_with(".workspace.google.com") {
|
||||
log::info!(
|
||||
"[webview-accounts] gmeet workspace marketing redirect intercepted ({}); rewriting parent to https://meet.google.com/",
|
||||
url
|
||||
);
|
||||
let app = nav_app.clone();
|
||||
let label = nav_label.clone();
|
||||
tauri::async_runtime::spawn(async move {
|
||||
if let Some(wv) = app.get_webview(&label) {
|
||||
if let Ok(target) = Url::parse("https://meet.google.com/") {
|
||||
if let Err(e) = wv.navigate(target) {
|
||||
log::warn!(
|
||||
"[webview-accounts] gmeet workspace rewrite navigate failed label={} err={}",
|
||||
label,
|
||||
e
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
if let Some(rewritten) = rewrite_provider_deep_link(&nav_provider, url) {
|
||||
log::info!(
|
||||
"[webview-accounts] deep-link rewrite {} → {} (provider={})",
|
||||
@@ -2759,6 +2806,56 @@ mod tests {
|
||||
.is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gmeet_room_popup_navigates_parent() {
|
||||
// "Start an instant meeting" / "New meeting" calls
|
||||
// window.open(meet.google.com/<roomid>) to launch a room.
|
||||
// Without intervention this would route to system Chrome and
|
||||
// leak the meeting out of OpenHuman.
|
||||
assert_eq!(
|
||||
popup_should_navigate_parent(
|
||||
"google-meet",
|
||||
&url("https://meet.google.com/abc-defg-hij"),
|
||||
)
|
||||
.map(|u| u.to_string()),
|
||||
Some("https://meet.google.com/abc-defg-hij".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gmeet_landing_popup_navigates_parent() {
|
||||
// Bare meet.google.com (no room code) should also be kept
|
||||
// in-app — matches the "back to Meet home" UX after hangup.
|
||||
assert!(
|
||||
popup_should_navigate_parent("google-meet", &url("https://meet.google.com/"),)
|
||||
.is_some()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gmeet_workspace_popup_does_not_navigate_parent() {
|
||||
// workspace.google.com is the marketing page; if it ever
|
||||
// arrives via window.open() we let the default external-route
|
||||
// logic handle it (covered in the on_navigation rewrite path
|
||||
// separately).
|
||||
assert!(popup_should_navigate_parent(
|
||||
"google-meet",
|
||||
&url("https://workspace.google.com/products/meet/"),
|
||||
)
|
||||
.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gmeet_unrelated_popup_does_not_navigate_parent() {
|
||||
// External link in the post-call review screen, for instance.
|
||||
// Should NOT navigate the parent — should fall through to the
|
||||
// system-browser path.
|
||||
assert!(
|
||||
popup_should_navigate_parent("google-meet", &url("https://example.com/blog"),)
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gmail_service_login_popup_navigates_parent() {
|
||||
assert_eq!(
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
# Google Meet Webview Parity — QA Matrix
|
||||
|
||||
> Issue: [#1022](https://github.com/tinyhumansai/openhuman/issues/1022)
|
||||
> Related: [#1035](https://github.com/tinyhumansai/openhuman/issues/1035) — Meet landing page + post-2FA auth refresh
|
||||
> Branch: `feat/1022-gmeet-parity-audit`
|
||||
> Tester: oxoxDev
|
||||
> Build: `upstream/main @ 680589d8` + `feat/1022-gmeet-parity-audit` HEAD
|
||||
> Date: 2026-04-30
|
||||
> Method: manual smoke against `pnpm dev:app` on macOS (per `feedback_validation_test_target.md`)
|
||||
|
||||
## Verdict legend
|
||||
|
||||
- ✅ **pass** — feature works as native app
|
||||
- ⚠️ **partial** — works but with limitation; needs follow-up
|
||||
- ❌ **fail** — broken; child issue filed
|
||||
- 🔍 **needs investigation** — non-deterministic behavior; revisit
|
||||
- ⏭️ **deferred** — out of scope for this PR; tracked elsewhere
|
||||
|
||||
## Acceptance criteria audit
|
||||
|
||||
| # | Criterion | Verdict | Evidence | Notes / child issue |
|
||||
|---|-----------|---------|----------|---------------------|
|
||||
| 1 | **Auth** — Google sign-in completes (email → password → 2FA); session persists across restarts | ❌ blocked on #1046 | After password entry the in-app webview was redirected to system Chrome (the post-password `SetSID` hop on `accounts.google.<cctld>` falls through `provider_allowed_hosts` for gmeet because the country-localized hosts aren't suffix-matched against bare `google.com`). Identical to the gmail bug fixed in #1020. | **Fix lives in PR #1046** (`is_google_sso_host` predicate at `webview_accounts/mod.rs`). Once #1046 merges and this branch rebases on main, in-app SSO completes natively and CEF lands the cookie in the right jar — no extra change needed in this PR. |
|
||||
| 2 | **Landing page** — `webview_account_open("google-meet")` lands on `meet.google.com`, not Google's marketing surface | ✅ pass (after fix) | Baseline behavior: post-account-pick redirect committed to `workspace.google.com/products/meet/...` because the bare `google.com` suffix in `provider_allowed_hosts` matched it as in-app. Fix: `on_navigation` intercepts gmeet `workspace.google.com` (and any subdomain) and rewrites the parent to `https://meet.google.com/`. | **Bug fixed in this PR**: `on_navigation` workspace marketing rewrite. |
|
||||
| 3 | **Start a new meeting** — "Start an instant meeting" / "New meeting" launches the room in-app | ✅ pass (after fix) | Baseline behavior: clicking "Start now" called `window.open(meet.google.com/<roomid>)`; default popup chain routed it to system Chrome and the user lost the in-app session entirely. Fix: `popup_should_navigate_parent` extended to match `meet.google.com` hosts for the `google-meet` provider so the popup is denied and the embedded parent is navigated into the room. Smoke result: "Start now" instantly drops the user into the meeting tab. | **Bug fixed in this PR**: gmeet popup parent-nav. |
|
||||
| 4 | **Join via meeting code** — entering a code in the input lands the user in the pre-call greenroom | ✅ pass | After perm-grant fix the join-by-code form correctly takes the user into the pre-call screen. | — |
|
||||
| 5 | **Pre-call cam/mic preview (greenroom)** — camera tile shows live video, mic level meter responds | ✅ pass (after fix) | Baseline behavior: greenroom showed Meet's "Use microphone and camera" consent dialog; clicking "Use microphone and camera" was a no-op (`getUserMedia` returned `NotAllowedError`). Fix: per-origin `Browser.grantPermissions` for `meet.google.com` now grants `audioCapture` + `videoCapture` so `getUserMedia` succeeds without the consent dialog. | **Bug fixed in this PR**: cam/mic perm grant. |
|
||||
| 6 | **In-call cam/mic** — toggling cam/mic during call works; tiles show live state | ✅ pass | Confirmed in active call. | — |
|
||||
| 7 | **Screenshare send** — "Present now" picks a window/screen and broadcasts | ✅ pass | `displayCapture` granted via `Browser.grantPermissions`; the macOS screen-recording prompt resolves once and the share starts. | — |
|
||||
| 8 | **Screenshare receive** — remote participant's share renders | ✅ pass | Verified live with a colleague's share. | — |
|
||||
| 9 | **Captions** — "Turn on captions" renders the live transcript over the call | ✅ pass | DOM-rendered captions appear inline on the call surface. | The captions are observable to recipe.js (the existing `meet_captions` event), but not yet ingested into OpenHuman memory — that gap is row 13. |
|
||||
| 10 | **In-call chat** — open chat panel; send + receive in-meeting messages | ✅ pass | DOM-driven chat works as in stock Chrome. | — |
|
||||
| 11 | **Reactions / raise hand** — emoji reactions and raise-hand toggles | ✅ pass | Confirmed live. | — |
|
||||
| 12 | **Session persistence** — close + reopen webview tab without sign-out; account stays active | ✅ pass | CEF profile persistence behaves identically to gmail/slack. | — |
|
||||
| 13 | **Captions ingestion → memory** — `meet_captions` rows + `meet_call_ended` lifecycle event flush a transcript document via `openhuman.memory_doc_ingest` | ⏭️ deferred to #1052 | Current handler at `webview_accounts/mod.rs:2219-2247` is log-only. Not a regression — recipe.js has been log-only since inception. Filed as **feature** at #1052 with module sketch (`google_meet/` mirroring `gmail/` shape, persistent `MeetingTranscriptStore`, opt-in toggle for the privacy-sensitive transcript). | Out of scope for this PR. |
|
||||
| 14 | **Hangup return UX** — leaving the call returns to a recognizable state (rejoin / 5-star review prompt) | ✅ pass | Behavior is identical to stock Chrome — rejoin button + 5-star review with 60s auto-dismiss. | — |
|
||||
| 15 | **Background effects** — virtual background / blur applies without breaking the camera | ❌ deferred to #1053 | Selecting a virtual background turns the camera off and Gmeet shows a "something went wrong" toast. Hypothesis: Chromium runtime gates absent in the vendored CEF build (insertable streams `MediaStreamTrackProcessor` / `MediaStreamTrackGenerator`, MediaPipe segmentation requiring SharedArrayBuffer + COOP/COEP cross-origin isolation, HW accel). | Filed as **bug** at #1053 with investigation sketch. Out of scope for this PR — needs CEF runtime-flag tuning, separate from the `webview_accounts` parity work. |
|
||||
|
||||
## Smoke run procedure
|
||||
|
||||
For each criterion:
|
||||
|
||||
1. Reproduce in running app (`pnpm dev:app` from this worktree).
|
||||
2. Capture exact symptom + log line if relevant.
|
||||
3. Mark verdict in table.
|
||||
4. If ❌: file child issue against `tinyhumansai/openhuman` titled `[Bug] webview/gmeet: <symptom>` linking back to #1022.
|
||||
5. If ⚠️: note limitation + scope follow-up; decide whether to fix in this PR or defer.
|
||||
|
||||
## Pre-audit code-level gaps confirmed during smoke
|
||||
|
||||
These were diagnosed against `main` before this PR's fixes landed; the smoke run confirmed each manifests as a user-visible bug.
|
||||
|
||||
1. **Cam/mic permission gap** — `Browser.grantPermissions` for the gmeet CDP session granted only `notifications` (added by #1028). `getUserMedia` and `getDisplayMedia` had no underlying CEF permission and Gmeet's web client fell back to its own consent dialog, which the embedded view couldn't satisfy. Fixed by per-origin grant of `audioCapture` / `videoCapture` / `displayCapture` / `clipboardReadWrite` for `meet.google.com`.
|
||||
2. **Workspace marketing redirect** — `provider_allowed_hosts("google-meet")` listed bare `google.com`; the SSR-redirect to `workspace.google.com/products/meet/...` matched as in-app and committed. Fixed by `on_navigation` intercept that rewrites the parent to `meet.google.com/`.
|
||||
3. **"Start now" popup leak** — `window.open(meet.google.com/<roomid>)` had no in-app handler for the gmeet provider in `popup_should_navigate_parent` / `popup_should_stay_in_app`. Fixed by extending `popup_should_navigate_parent` to match `meet.google.com` hosts for `google-meet`.
|
||||
4. **Auth post-2FA redirect** — same root cause as #1020 row #1 (Google SSO ccTLD coverage). Fix lives in PR #1046 (`is_google_sso_host`); this branch rebases on main once #1046 merges.
|
||||
|
||||
## Hardcoded constants worth capturing
|
||||
|
||||
| Constant | Value | File:line |
|
||||
|----------|-------|-----------|
|
||||
| Provider URL | `https://meet.google.com/` | `webview_accounts/mod.rs:58` |
|
||||
| Allow-list suffixes (gmeet) | `google.com`, `googleusercontent.com`, `gstatic.com`, `googleapis.com` | `webview_accounts/mod.rs:108-113` |
|
||||
| Recipe.js bundle (legacy) | `app/src-tauri/recipes/google-meet/recipe.js` | bundled via `include_str!` at `mod.rs:46` |
|
||||
| URL fragment account marker | `#openhuman-account-<id>` | `cdp/session.rs:46-48` |
|
||||
| Granted permissions (gmeet) | `notifications`, `audioCapture`, `videoCapture`, `displayCapture`, `clipboardReadWrite` | `cdp/session.rs` |
|
||||
|
||||
## CDP-migration sketch (out of scope)
|
||||
|
||||
The legacy `recipe.js` polls the call DOM for caption rows. A future epic should migrate this to a Rust-side `google_meet_scanner` module driven by `DOMSnapshot.captureSnapshot` / `Network.responseReceived`, mirroring the `slack_scanner` / `whatsapp_scanner` shape. Captions are DOM-rendered (not network-delivered), so the migration cannot use `Network.responseReceived` alone — needs a polling DOM snapshot. This work would also enable native dedup and let us drop the recipe.js bundle. **Filed under follow-up — not gated on this PR.**
|
||||
|
||||
## Sign-off
|
||||
|
||||
- Tester: oxoxDev
|
||||
- Result: 11 ✅ / 0 ⚠️ / 2 ❌ (one blocked on #1046, one deferred to #1053) / 1 ⏭️ deferred to #1052 / 1 ❌ blocked on #1046
|
||||
- Date: 2026-04-30
|
||||
- Action items:
|
||||
- Land #1046 (gmail PR) → rebase this branch → row #1 Auth flips ✅
|
||||
- Track #1052 (caption ingest feature) and #1053 (background effects CEF gap) as follow-up issues
|
||||
Reference in New Issue
Block a user