wip(oauth): add safety timeout and debug logging for popup handling
- Add 30s safety timeout for stuck OAuth popups - Add window.opener debug script injection for popups - Add debug logging for related webview creation - Refactor OAuth auto-close with better URL detection order
- commit
- a5bf0cb18d962eb91cebc15c9251429ff96e4610
- author
- brice · · 2026-01-14 08:45 UTC
- parent
- 74af521
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +97 -21
M internal/infrastructure/webkit/factory.go +19 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | ||
| 7 | 7 | "github.com/bnema/dumber/internal/application/port" | |
| 8 | 8 | "github.com/bnema/dumber/internal/logging" | |
| 9 | + | wk "github.com/bnema/puregotk-webkit/webkit" | |
| 9 | 10 | ) | |
| 10 | 11 | ||
| 11 | 12 | // WebViewFactory creates WebView instances for the application. | |
| @@ -114,6 +115,24 @@ func (f *WebViewFactory) CreateRelated(ctx context.Context, parentID port.WebVie | |||
| 114 | 115 | log.Warn().Err(err).Uint64("id", uint64(wv.ID())).Msg("failed to attach frontend to related webview") | |
| 115 | 116 | } | |
| 116 | 117 | ||
| 118 | + | // DEBUG: Inject script to check window.opener status | |
| 119 | + | openerDebugScript := wk.NewUserScript( | |
| 120 | + | `console.log('[DUMBER DEBUG] window.opener:', window.opener); | |
| 121 | + | console.log('[DUMBER DEBUG] window.opener type:', typeof window.opener); | |
| 122 | + | if (window.opener) { | |
| 123 | + | console.log('[DUMBER DEBUG] window.opener.location:', window.opener.location?.href || 'BLOCKED'); | |
| 124 | + | console.log('[DUMBER DEBUG] can postMessage:', typeof window.opener.postMessage); | |
| 125 | + | }`, | |
| 126 | + | wk.UserContentInjectAllFramesValue, | |
| 127 | + | wk.UserScriptInjectAtDocumentStartValue, | |
| 128 | + | nil, | |
| 129 | + | nil, | |
| 130 | + | ) | |
| 131 | + | if openerDebugScript != nil { | |
| 132 | + | wv.ucm.AddScript(openerDebugScript) | |
| 133 | + | log.Debug().Msg("injected window.opener debug script into popup") | |
| 134 | + | } | |
| 135 | + | ||
| 117 | 136 | // Apply content filters if configured | |
| 118 | 137 | if f.filterApplier != nil { | |
| 119 | 138 | f.filterApplier.ApplyTo(ctx, wv.ucm) | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | 6 | ||||
| 7 | "github.com/bnema/dumber/internal/application/port" | 7 | "github.com/bnema/dumber/internal/application/port" | ||
| 8 | "github.com/bnema/dumber/internal/logging" | 8 | "github.com/bnema/dumber/internal/logging" | ||
| 9 | + | wk "github.com/bnema/puregotk-webkit/webkit" | |||
| 9 | ) | 10 | ) | ||
| 10 | 11 | ||||
| 11 | // WebViewFactory creates WebView instances for the application. | 12 | // WebViewFactory creates WebView instances for the application. | ||
| @@ -114,6 +115,24 @@ func (f *WebViewFactory) CreateRelated(ctx context.Context, parentID port.WebVie | |||||
| 114 | log.Warn().Err(err).Uint64("id", uint64(wv.ID())).Msg("failed to attach frontend to related webview") | 115 | log.Warn().Err(err).Uint64("id", uint64(wv.ID())).Msg("failed to attach frontend to related webview") | ||
| 115 | } | 116 | } | ||
| 116 | 117 | ||||
| 118 | + | // DEBUG: Inject script to check window.opener status | |||
| 119 | + | openerDebugScript := wk.NewUserScript( | |||
| 120 | + | `console.log('[DUMBER DEBUG] window.opener:', window.opener); | |||
| 121 | + | console.log('[DUMBER DEBUG] window.opener type:', typeof window.opener); | |||
| 122 | + | if (window.opener) { | |||
| 123 | + | console.log('[DUMBER DEBUG] window.opener.location:', window.opener.location?.href || 'BLOCKED'); | |||
| 124 | + | console.log('[DUMBER DEBUG] can postMessage:', typeof window.opener.postMessage); | |||
| 125 | + | }`, | |||
| 126 | + | wk.UserContentInjectAllFramesValue, | |||
| 127 | + | wk.UserScriptInjectAtDocumentStartValue, | |||
| 128 | + | nil, | |||
| 129 | + | nil, | |||
| 130 | + | ) | |||
| 131 | + | if openerDebugScript != nil { | |||
| 132 | + | wv.ucm.AddScript(openerDebugScript) | |||
| 133 | + | log.Debug().Msg("injected window.opener debug script into popup") | |||
| 134 | + | } | |||
| 135 | + | ||||
| 117 | // Apply content filters if configured | 136 | // Apply content filters if configured | ||
| 118 | if f.filterApplier != nil { | 137 | if f.filterApplier != nil { | ||
| 119 | f.filterApplier.ApplyTo(ctx, wv.ucm) | 138 | f.filterApplier.ApplyTo(ctx, wv.ucm) | ||
M internal/infrastructure/webkit/webview.go +15 -0
| @@ -327,11 +327,20 @@ func NewWebViewWithRelated(ctx context.Context, parent *WebView, settings *Setti | |||
| 327 | 327 | return nil, fmt.Errorf("parent webview %d is destroyed", parent.id) | |
| 328 | 328 | } | |
| 329 | 329 | ||
| 330 | + | log.Debug(). | |
| 331 | + | Uint64("parent_ptr", uint64(parent.inner.GoPointer())). | |
| 332 | + | Msg("creating related webview with parent pointer") | |
| 333 | + | ||
| 330 | 334 | inner := webkit.NewWebViewWithRelatedView(parent.inner) | |
| 331 | 335 | if inner == nil { | |
| 332 | 336 | return nil, fmt.Errorf("failed to create related webkit webview") | |
| 333 | 337 | } | |
| 334 | 338 | ||
| 339 | + | log.Debug(). | |
| 340 | + | Uint64("new_ptr", uint64(inner.GoPointer())). | |
| 341 | + | Uint64("new_widget_ptr", uint64(inner.Widget.GoPointer())). | |
| 342 | + | Msg("related webview created, checking pointers") | |
| 343 | + | ||
| 335 | 344 | wv := &WebView{ | |
| 336 | 345 | inner: inner, | |
| 337 | 346 | isRelated: true, // Shares web process with parent - must not terminate process on destroy | |
| @@ -446,6 +455,12 @@ func (wv *WebView) connectCreateSignal() { | |||
| 446 | 455 | return gtk.Widget{} // Block popup | |
| 447 | 456 | } | |
| 448 | 457 | ||
| 458 | + | // DEBUG: Log the pointer being returned | |
| 459 | + | wv.logger.Debug(). | |
| 460 | + | Uint64("returning_ptr", uint64(newWV.inner.GoPointer())). | |
| 461 | + | Uint64("returning_widget_ptr", uint64(newWV.inner.Widget.GoPointer())). | |
| 462 | + | Msg("create signal returning webview widget") | |
| 463 | + | ||
| 449 | 464 | return newWV.inner.Widget | |
| 450 | 465 | } | |
| 451 | 466 | sigID := wv.inner.ConnectCreate(&createCb) | |
| @@ -327,11 +327,20 @@ func NewWebViewWithRelated(ctx context.Context, parent *WebView, settings *Setti | |||||
| 327 | return nil, fmt.Errorf("parent webview %d is destroyed", parent.id) | 327 | return nil, fmt.Errorf("parent webview %d is destroyed", parent.id) | ||
| 328 | } | 328 | } | ||
| 329 | 329 | ||||
| 330 | + | log.Debug(). | |||
| 331 | + | Uint64("parent_ptr", uint64(parent.inner.GoPointer())). | |||
| 332 | + | Msg("creating related webview with parent pointer") | |||
| 333 | + | ||||
| 330 | inner := webkit.NewWebViewWithRelatedView(parent.inner) | 334 | inner := webkit.NewWebViewWithRelatedView(parent.inner) | ||
| 331 | if inner == nil { | 335 | if inner == nil { | ||
| 332 | return nil, fmt.Errorf("failed to create related webkit webview") | 336 | return nil, fmt.Errorf("failed to create related webkit webview") | ||
| 333 | } | 337 | } | ||
| 334 | 338 | ||||
| 339 | + | log.Debug(). | |||
| 340 | + | Uint64("new_ptr", uint64(inner.GoPointer())). | |||
| 341 | + | Uint64("new_widget_ptr", uint64(inner.Widget.GoPointer())). | |||
| 342 | + | Msg("related webview created, checking pointers") | |||
| 343 | + | ||||
| 335 | wv := &WebView{ | 344 | wv := &WebView{ | ||
| 336 | inner: inner, | 345 | inner: inner, | ||
| 337 | isRelated: true, // Shares web process with parent - must not terminate process on destroy | 346 | isRelated: true, // Shares web process with parent - must not terminate process on destroy | ||
| @@ -446,6 +455,12 @@ func (wv *WebView) connectCreateSignal() { | |||||
| 446 | return gtk.Widget{} // Block popup | 455 | return gtk.Widget{} // Block popup | ||
| 447 | } | 456 | } | ||
| 448 | 457 | ||||
| 458 | + | // DEBUG: Log the pointer being returned | |||
| 459 | + | wv.logger.Debug(). | |||
| 460 | + | Uint64("returning_ptr", uint64(newWV.inner.GoPointer())). | |||
| 461 | + | Uint64("returning_widget_ptr", uint64(newWV.inner.Widget.GoPointer())). | |||
| 462 | + | Msg("create signal returning webview widget") | |||
| 463 | + | ||||
| 449 | return newWV.inner.Widget | 464 | return newWV.inner.Widget | ||
| 450 | } | 465 | } | ||
| 451 | sigID := wv.inner.ConnectCreate(&createCb) | 466 | sigID := wv.inner.ConnectCreate(&createCb) | ||
M internal/ui/coordinator/content.go +63 -20
| @@ -21,7 +21,10 @@ import ( | |||
| 21 | 21 | "github.com/jwijenbergh/puregotk/v4/gdk" | |
| 22 | 22 | ) | |
| 23 | 23 | ||
| 24 | - | const aboutBlankURI = "about:blank" | |
| 24 | + | const ( | |
| 25 | + | aboutBlankURI = "about:blank" | |
| 26 | + | logURLMaxLen = 80 | |
| 27 | + | ) | |
| 25 | 28 | ||
| 26 | 29 | // ContentCoordinator manages WebView lifecycle, title tracking, and content attachment. | |
| 27 | 30 | type ContentCoordinator struct { | |
| @@ -1289,17 +1292,29 @@ func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID | |||
| 1289 | 1292 | popupPane.URI = pending.TargetURI | |
| 1290 | 1293 | ||
| 1291 | 1294 | // Enable OAuth auto-close if configured | |
| 1292 | - | if c.popupConfig != nil && c.popupConfig.OAuthAutoClose && IsOAuthURL(pending.TargetURI) { | |
| 1293 | - | popupPane.AutoClose = true | |
| 1294 | - | c.setupOAuthAutoClose(ctx, paneID, pending.WebView) | |
| 1295 | - | } | |
| 1295 | + | hasConfig := c.popupConfig != nil | |
| 1296 | + | oauthEnabled := hasConfig && c.popupConfig.OAuthAutoClose | |
| 1297 | + | isOAuth := IsOAuthURL(pending.TargetURI) | |
| 1298 | + | log.Debug(). | |
| 1299 | + | Bool("has_config", hasConfig). | |
| 1300 | + | Bool("oauth_enabled", oauthEnabled). | |
| 1301 | + | Bool("is_oauth", isOAuth). | |
| 1302 | + | Str("uri", logging.TruncateURL(pending.TargetURI, logURLMaxLen)). | |
| 1303 | + | Msg("popup OAuth check") | |
| 1296 | 1304 | ||
| 1297 | 1305 | // Register WebView in our map | |
| 1298 | 1306 | c.webViews[paneID] = pending.WebView | |
| 1299 | 1307 | ||
| 1300 | - | // Setup standard callbacks | |
| 1308 | + | // Setup standard callbacks first | |
| 1301 | 1309 | c.setupWebViewCallbacks(ctx, paneID, pending.WebView) | |
| 1302 | 1310 | ||
| 1311 | + | // Setup OAuth auto-close AFTER standard callbacks so we can wrap them | |
| 1312 | + | if hasConfig && oauthEnabled && isOAuth { | |
| 1313 | + | popupPane.AutoClose = true | |
| 1314 | + | c.setupOAuthAutoClose(ctx, paneID, pending.WebView) | |
| 1315 | + | log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup") | |
| 1316 | + | } | |
| 1317 | + | ||
| 1303 | 1318 | // Request insertion into workspace | |
| 1304 | 1319 | if c.onInsertPopup != nil { | |
| 1305 | 1320 | popupInput := InsertPopupInput{ | |
| @@ -1426,9 +1441,44 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||
| 1426 | 1441 | } | |
| 1427 | 1442 | ||
| 1428 | 1443 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | |
| 1444 | + | // It uses URL pattern detection for standard OAuth callbacks (code=, access_token=, etc.) | |
| 1445 | + | // For providers using postMessage (like Google Sign-In), we rely on the provider calling | |
| 1446 | + | // window.close() which triggers WebKit's close signal. | |
| 1447 | + | // A long safety timeout (30s) catches popups that get stuck. | |
| 1429 | 1448 | func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 1430 | 1449 | log := logging.FromContext(ctx) | |
| 1431 | 1450 | ||
| 1451 | + | // Safety timeout - only triggers if popup gets stuck (provider should close via window.close) | |
| 1452 | + | var safetyTimer *time.Timer | |
| 1453 | + | var safetyTimerMu sync.Mutex | |
| 1454 | + | const oauthSafetyTimeout = 30 * time.Second | |
| 1455 | + | ||
| 1456 | + | startSafetyTimer := func() { | |
| 1457 | + | safetyTimerMu.Lock() | |
| 1458 | + | defer safetyTimerMu.Unlock() | |
| 1459 | + | if safetyTimer != nil { | |
| 1460 | + | safetyTimer.Stop() | |
| 1461 | + | } | |
| 1462 | + | safetyTimer = time.AfterFunc(oauthSafetyTimeout, func() { | |
| 1463 | + | if wv != nil && !wv.IsDestroyed() { | |
| 1464 | + | log.Warn().Str("pane", string(paneID)).Msg("oauth safety timeout, closing stuck popup") | |
| 1465 | + | wv.Close() | |
| 1466 | + | } | |
| 1467 | + | }) | |
| 1468 | + | } | |
| 1469 | + | ||
| 1470 | + | cancelSafetyTimer := func() { | |
| 1471 | + | safetyTimerMu.Lock() | |
| 1472 | + | defer safetyTimerMu.Unlock() | |
| 1473 | + | if safetyTimer != nil { | |
| 1474 | + | safetyTimer.Stop() | |
| 1475 | + | safetyTimer = nil | |
| 1476 | + | } | |
| 1477 | + | } | |
| 1478 | + | ||
| 1479 | + | // Start safety timer immediately | |
| 1480 | + | startSafetyTimer() | |
| 1481 | + | ||
| 1432 | 1482 | // Wrap the existing OnURIChanged to also check for OAuth callbacks | |
| 1433 | 1483 | originalURIChanged := wv.OnURIChanged | |
| 1434 | 1484 | wv.OnURIChanged = func(uri string) { | |
| @@ -1437,39 +1487,32 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent | |||
| 1437 | 1487 | originalURIChanged(uri) | |
| 1438 | 1488 | } | |
| 1439 | 1489 | ||
| 1440 | - | // Check for OAuth callback | |
| 1490 | + | // Check for OAuth callback (URL-based detection) | |
| 1441 | 1491 | if ShouldAutoClose(uri) { | |
| 1442 | - | log.Info(). | |
| 1443 | - | Str("pane_id", string(paneID)). | |
| 1444 | - | Str("uri", uri). | |
| 1445 | - | Msg("OAuth callback detected, auto-closing popup") | |
| 1446 | - | ||
| 1447 | - | // Trigger close after a short delay to let the page process | |
| 1492 | + | cancelSafetyTimer() | |
| 1493 | + | log.Info().Str("pane", string(paneID)).Msg("oauth callback detected, closing") | |
| 1448 | 1494 | go func() { | |
| 1449 | 1495 | time.Sleep(500 * time.Millisecond) | |
| 1450 | 1496 | if wv != nil && !wv.IsDestroyed() { | |
| 1451 | - | // This will trigger handlePopupClose | |
| 1452 | 1497 | wv.Close() | |
| 1453 | 1498 | } | |
| 1454 | 1499 | }() | |
| 1455 | 1500 | } | |
| 1456 | 1501 | } | |
| 1457 | 1502 | ||
| 1458 | - | // Also check on load committed for immediate redirects | |
| 1503 | + | // Monitor load events for URL-based detection | |
| 1459 | 1504 | originalLoadChanged := wv.OnLoadChanged | |
| 1460 | 1505 | wv.OnLoadChanged = func(event webkit.LoadEvent) { | |
| 1461 | 1506 | if originalLoadChanged != nil { | |
| 1462 | 1507 | originalLoadChanged(event) | |
| 1463 | 1508 | } | |
| 1464 | 1509 | ||
| 1510 | + | // Check URL on commit for immediate detection | |
| 1465 | 1511 | if event == webkit.LoadCommitted { | |
| 1466 | 1512 | uri := wv.URI() | |
| 1467 | 1513 | if ShouldAutoClose(uri) { | |
| 1468 | - | log.Info(). | |
| 1469 | - | Str("pane_id", string(paneID)). | |
| 1470 | - | Str("uri", uri). | |
| 1471 | - | Msg("OAuth callback detected on load, auto-closing popup") | |
| 1472 | - | ||
| 1514 | + | cancelSafetyTimer() | |
| 1515 | + | log.Info().Str("pane", string(paneID)).Msg("oauth callback on load, closing") | |
| 1473 | 1516 | go func() { | |
| 1474 | 1517 | time.Sleep(500 * time.Millisecond) | |
| 1475 | 1518 | if wv != nil && !wv.IsDestroyed() { | |
| @@ -21,7 +21,10 @@ import ( | |||||
| 21 | "github.com/jwijenbergh/puregotk/v4/gdk" | 21 | "github.com/jwijenbergh/puregotk/v4/gdk" | ||
| 22 | ) | 22 | ) | ||
| 23 | 23 | ||||
| 24 | - | const aboutBlankURI = "about:blank" | 24 | + | const ( |
| 25 | + | aboutBlankURI = "about:blank" | |||
| 26 | + | logURLMaxLen = 80 | |||
| 27 | + | ) | |||
| 25 | 28 | ||||
| 26 | // ContentCoordinator manages WebView lifecycle, title tracking, and content attachment. | 29 | // ContentCoordinator manages WebView lifecycle, title tracking, and content attachment. | ||
| 27 | type ContentCoordinator struct { | 30 | type ContentCoordinator struct { | ||
| @@ -1289,17 +1292,29 @@ func (c *ContentCoordinator) handlePopupReadyToShow(ctx context.Context, popupID | |||||
| 1289 | popupPane.URI = pending.TargetURI | 1292 | popupPane.URI = pending.TargetURI | ||
| 1290 | 1293 | ||||
| 1291 | // Enable OAuth auto-close if configured | 1294 | // Enable OAuth auto-close if configured | ||
| 1292 | - | if c.popupConfig != nil && c.popupConfig.OAuthAutoClose && IsOAuthURL(pending.TargetURI) { | 1295 | + | hasConfig := c.popupConfig != nil |
| 1293 | - | popupPane.AutoClose = true | 1296 | + | oauthEnabled := hasConfig && c.popupConfig.OAuthAutoClose |
| 1294 | - | c.setupOAuthAutoClose(ctx, paneID, pending.WebView) | 1297 | + | isOAuth := IsOAuthURL(pending.TargetURI) |
| 1295 | - | } | 1298 | + | log.Debug(). |
| 1299 | + | Bool("has_config", hasConfig). | |||
| 1300 | + | Bool("oauth_enabled", oauthEnabled). | |||
| 1301 | + | Bool("is_oauth", isOAuth). | |||
| 1302 | + | Str("uri", logging.TruncateURL(pending.TargetURI, logURLMaxLen)). | |||
| 1303 | + | Msg("popup OAuth check") | |||
| 1296 | 1304 | ||||
| 1297 | // Register WebView in our map | 1305 | // Register WebView in our map | ||
| 1298 | c.webViews[paneID] = pending.WebView | 1306 | c.webViews[paneID] = pending.WebView | ||
| 1299 | 1307 | ||||
| 1300 | - | // Setup standard callbacks | 1308 | + | // Setup standard callbacks first |
| 1301 | c.setupWebViewCallbacks(ctx, paneID, pending.WebView) | 1309 | c.setupWebViewCallbacks(ctx, paneID, pending.WebView) | ||
| 1302 | 1310 | ||||
| 1311 | + | // Setup OAuth auto-close AFTER standard callbacks so we can wrap them | |||
| 1312 | + | if hasConfig && oauthEnabled && isOAuth { | |||
| 1313 | + | popupPane.AutoClose = true | |||
| 1314 | + | c.setupOAuthAutoClose(ctx, paneID, pending.WebView) | |||
| 1315 | + | log.Debug().Str("pane_id", string(paneID)).Msg("OAuth auto-close enabled for popup") | |||
| 1316 | + | } | |||
| 1317 | + | ||||
| 1303 | // Request insertion into workspace | 1318 | // Request insertion into workspace | ||
| 1304 | if c.onInsertPopup != nil { | 1319 | if c.onInsertPopup != nil { | ||
| 1305 | popupInput := InsertPopupInput{ | 1320 | popupInput := InsertPopupInput{ | ||
| @@ -1426,9 +1441,44 @@ func (c *ContentCoordinator) setupWebViewCallbacks(ctx context.Context, paneID e | |||||
| 1426 | } | 1441 | } | ||
| 1427 | 1442 | ||||
| 1428 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | 1443 | // setupOAuthAutoClose monitors the popup for OAuth callback URLs and auto-closes. | ||
| 1444 | + | // It uses URL pattern detection for standard OAuth callbacks (code=, access_token=, etc.) | |||
| 1445 | + | // For providers using postMessage (like Google Sign-In), we rely on the provider calling | |||
| 1446 | + | // window.close() which triggers WebKit's close signal. | |||
| 1447 | + | // A long safety timeout (30s) catches popups that get stuck. | |||
| 1429 | func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | 1448 | func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | ||
| 1430 | log := logging.FromContext(ctx) | 1449 | log := logging.FromContext(ctx) | ||
| 1431 | 1450 | ||||
| 1451 | + | // Safety timeout - only triggers if popup gets stuck (provider should close via window.close) | |||
| 1452 | + | var safetyTimer *time.Timer | |||
| 1453 | + | var safetyTimerMu sync.Mutex | |||
| 1454 | + | const oauthSafetyTimeout = 30 * time.Second | |||
| 1455 | + | ||||
| 1456 | + | startSafetyTimer := func() { | |||
| 1457 | + | safetyTimerMu.Lock() | |||
| 1458 | + | defer safetyTimerMu.Unlock() | |||
| 1459 | + | if safetyTimer != nil { | |||
| 1460 | + | safetyTimer.Stop() | |||
| 1461 | + | } | |||
| 1462 | + | safetyTimer = time.AfterFunc(oauthSafetyTimeout, func() { | |||
| 1463 | + | if wv != nil && !wv.IsDestroyed() { | |||
| 1464 | + | log.Warn().Str("pane", string(paneID)).Msg("oauth safety timeout, closing stuck popup") | |||
| 1465 | + | wv.Close() | |||
| 1466 | + | } | |||
| 1467 | + | }) | |||
| 1468 | + | } | |||
| 1469 | + | ||||
| 1470 | + | cancelSafetyTimer := func() { | |||
| 1471 | + | safetyTimerMu.Lock() | |||
| 1472 | + | defer safetyTimerMu.Unlock() | |||
| 1473 | + | if safetyTimer != nil { | |||
| 1474 | + | safetyTimer.Stop() | |||
| 1475 | + | safetyTimer = nil | |||
| 1476 | + | } | |||
| 1477 | + | } | |||
| 1478 | + | ||||
| 1479 | + | // Start safety timer immediately | |||
| 1480 | + | startSafetyTimer() | |||
| 1481 | + | ||||
| 1432 | // Wrap the existing OnURIChanged to also check for OAuth callbacks | 1482 | // Wrap the existing OnURIChanged to also check for OAuth callbacks | ||
| 1433 | originalURIChanged := wv.OnURIChanged | 1483 | originalURIChanged := wv.OnURIChanged | ||
| 1434 | wv.OnURIChanged = func(uri string) { | 1484 | wv.OnURIChanged = func(uri string) { | ||
| @@ -1437,39 +1487,32 @@ func (c *ContentCoordinator) setupOAuthAutoClose(ctx context.Context, paneID ent | |||||
| 1437 | originalURIChanged(uri) | 1487 | originalURIChanged(uri) | ||
| 1438 | } | 1488 | } | ||
| 1439 | 1489 | ||||
| 1440 | - | // Check for OAuth callback | 1490 | + | // Check for OAuth callback (URL-based detection) |
| 1441 | if ShouldAutoClose(uri) { | 1491 | if ShouldAutoClose(uri) { | ||
| 1442 | - | log.Info(). | 1492 | + | cancelSafetyTimer() |
| 1443 | - | Str("pane_id", string(paneID)). | 1493 | + | log.Info().Str("pane", string(paneID)).Msg("oauth callback detected, closing") |
| 1444 | - | Str("uri", uri). | |||
| 1445 | - | Msg("OAuth callback detected, auto-closing popup") | |||
| 1446 | - | ||||
| 1447 | - | // Trigger close after a short delay to let the page process | |||
| 1448 | go func() { | 1494 | go func() { | ||
| 1449 | time.Sleep(500 * time.Millisecond) | 1495 | time.Sleep(500 * time.Millisecond) | ||
| 1450 | if wv != nil && !wv.IsDestroyed() { | 1496 | if wv != nil && !wv.IsDestroyed() { | ||
| 1451 | - | // This will trigger handlePopupClose | |||
| 1452 | wv.Close() | 1497 | wv.Close() | ||
| 1453 | } | 1498 | } | ||
| 1454 | }() | 1499 | }() | ||
| 1455 | } | 1500 | } | ||
| 1456 | } | 1501 | } | ||
| 1457 | 1502 | ||||
| 1458 | - | // Also check on load committed for immediate redirects | 1503 | + | // Monitor load events for URL-based detection |
| 1459 | originalLoadChanged := wv.OnLoadChanged | 1504 | originalLoadChanged := wv.OnLoadChanged | ||
| 1460 | wv.OnLoadChanged = func(event webkit.LoadEvent) { | 1505 | wv.OnLoadChanged = func(event webkit.LoadEvent) { | ||
| 1461 | if originalLoadChanged != nil { | 1506 | if originalLoadChanged != nil { | ||
| 1462 | originalLoadChanged(event) | 1507 | originalLoadChanged(event) | ||
| 1463 | } | 1508 | } | ||
| 1464 | 1509 | ||||
| 1510 | + | // Check URL on commit for immediate detection | |||
| 1465 | if event == webkit.LoadCommitted { | 1511 | if event == webkit.LoadCommitted { | ||
| 1466 | uri := wv.URI() | 1512 | uri := wv.URI() | ||
| 1467 | if ShouldAutoClose(uri) { | 1513 | if ShouldAutoClose(uri) { | ||
| 1468 | - | log.Info(). | 1514 | + | cancelSafetyTimer() |
| 1469 | - | Str("pane_id", string(paneID)). | 1515 | + | log.Info().Str("pane", string(paneID)).Msg("oauth callback on load, closing") |
| 1470 | - | Str("uri", uri). | |||
| 1471 | - | Msg("OAuth callback detected on load, auto-closing popup") | |||
| 1472 | - | ||||
| 1473 | go func() { | 1516 | go func() { | ||
| 1474 | time.Sleep(500 * time.Millisecond) | 1517 | time.Sleep(500 * time.Millisecond) | ||
| 1475 | if wv != nil && !wv.IsDestroyed() { | 1518 | if wv != nil && !wv.IsDestroyed() { | ||
M internal/ui/coordinator/oauth.go +0 -1
| @@ -101,7 +101,6 @@ func IsOAuthCallback(url string) bool { | |||
| 101 | 101 | // - Errors: error=, error_description= | |
| 102 | 102 | // - Various OAuth providers (Google, GitHub, Auth0, etc.) | |
| 103 | 103 | func ShouldAutoClose(url string) bool { | |
| 104 | - | // Only auto-close on callback URLs that have OAuth response params | |
| 105 | 104 | return IsOAuthCallback(url) | |
| 106 | 105 | } | |
| 107 | 106 | ||
| @@ -101,7 +101,6 @@ func IsOAuthCallback(url string) bool { | |||||
| 101 | // - Errors: error=, error_description= | 101 | // - Errors: error=, error_description= | ||
| 102 | // - Various OAuth providers (Google, GitHub, Auth0, etc.) | 102 | // - Various OAuth providers (Google, GitHub, Auth0, etc.) | ||
| 103 | func ShouldAutoClose(url string) bool { | 103 | func ShouldAutoClose(url string) bool { | ||
| 104 | - | // Only auto-close on callback URLs that have OAuth response params | |||
| 105 | return IsOAuthCallback(url) | 104 | return IsOAuthCallback(url) | ||
| 106 | } | 105 | } | ||
| 107 | 106 | ||||