Skip to content

Rework SwitchBrowser Auth Tab integration to provider-based strategy (Common Option 1B), Fixes AB#3561426#3092

Closed
Copilot wants to merge 3 commits into
devfrom
copilot/ab-3561426-rework-auth-tab-strategy
Closed

Rework SwitchBrowser Auth Tab integration to provider-based strategy (Common Option 1B), Fixes AB#3561426#3092
Copilot wants to merge 3 commits into
devfrom
copilot/ab-3561426-rework-auth-tab-strategy

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 22, 2026

This reworks the previous Auth Tab approach in Common to Option 1B: Common no longer owns Auth Tab implementation details tied to Browser 1.9.0 APIs, and instead uses a broker-registered strategy provider. SwitchBrowserActivity now selects Auth Tab via provider hooks with a safe Custom Tabs fallback.

  • Provider-based Auth Tab registration in Common

    • Added AuthTabStrategyProvider as a singleton registry for broker-supplied Auth Tab behavior.
    • Exposes:
      • register(factory, isSupported)
      • isAuthTabSupported(context, browserPackage)
      • createStrategy(activity, onResult)
      • isAvailable()
    • Keeps Common decoupled from Browser 1.9.0-only APIs.
  • Strategy abstraction for browser launch

    • Added BrowserLaunchStrategy interface (launch, handlesCancellationOnResume, cleanup).
    • Added CustomTabsLaunchStrategy implementation wrapping existing CustomTabsManager behavior.
  • SwitchBrowserActivity strategy selection refactor

    • Replaced direct Auth Tab class usage with AuthTabStrategyProvider.
    • Selection logic now:
      1. Check Auth Tab flight + provider availability + provider support check.
      2. Request provider strategy.
      3. Fallback to CustomTabsLaunchStrategy if unavailable/unsupported/null.
    • Removed direct coupling to AuthTabManager / AuthTabLaunchStrategy.
  • Targeted test updates

    • Added AuthTabStrategyProviderTest for registration/availability/support/creation behavior.
    • Added SwitchBrowserActivityTest coverage for provider-driven strategy selection and fallback paths.
    • Added CustomTabsLaunchStrategyTest for cancellation-handling contract and cleanup safety.
val shouldUseAuthTab = isAuthTabFlightEnabled() &&
    AuthTabStrategyProvider.isAvailable() &&
    AuthTabStrategyProvider.isAuthTabSupported(this, browserPackageName)

launchStrategy = if (shouldUseAuthTab) {
    AuthTabStrategyProvider.createStrategy(this, ::handleSwitchBrowserResume)
        ?: CustomTabsLaunchStrategy(this)
} else {
    CustomTabsLaunchStrategy(this)
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dl.google.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=1024m -XX:+HeapDumpOnOutOfMemoryError -Xmx3072m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant (dns block)
  • identitydivision.pkgs.visualstudio.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=1024m -XX:+HeapDumpOnOutOfMemoryError -Xmx3072m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

[AuthTab] Implement strategy pattern + AuthTabStrategyProvider in Common (Option 1B)

Fixes AB#3561426

IMPORTANT: This is a REWORK of PR #3078

There is an existing open PR #3078 on branch copilot/ab-3561426-implement-authtabmanager that implemented the original Option 1 (all Auth Tab code in Common). That approach is no longer valid because Common uses �rowser:1.7.0 and Auth Tab requires �rowser:1.9.0. Bumping Common to 1.9.0 would cascade to MSAL, OneAuth, ADAL.

The PR needs to be reworked to Option 1B: keep only the Common-side code, remove Auth Tab implementation classes.

What to REMOVE from the existing PR

  • DELETE AuthTabManager.kt from common/common/src/main/java/.../ui/browser/ — this class imports AuthTabIntent from browser 1.9.0 which Common cannot use
  • DELETE AuthTabLaunchStrategy.kt from common/common/src/main/java/.../providers/oauth2/ — this class uses AuthTabManager
  • DELETE AuthTabManagerTest.kt from common/common/src/test/java/.../ui/browser/
  • DELETE AuthTabLaunchStrategyTest.kt from common/common/src/test/java/.../providers/oauth2/

What to ADD

  • ADD AuthTabStrategyProvider.kt in common/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/:
    • Kotlin object (singleton)
    • Has
      egister(factory, isSupported) method — Broker will call this at init time
    • Has isAuthTabSupported(context, browserPackage) — delegates to registered checker, returns false if not registered
    • Has createStrategy(activity, onResult) — delegates to registered factory, returns null if not registered
    • Has isAvailable() — returns true if factory is registered
    • NO imports from �ndroidx.browser:browser:1.9.0 — only uses �ndroid.content.Context, �ndroid.os.Bundle, �ndroidx.fragment.app.FragmentActivity

What to KEEP (with updates)

  • KEEP BrowserLaunchStrategy.kt — interface with launch(), handlesCancellationOnResume(), cleanup(). Unchanged.
  • KEEP CustomTabsLaunchStrategy.kt — implements BrowserLaunchStrategy, wraps existing CustomTabsManager logic. Unchanged.
  • KEEP SwitchBrowserActivity.kt — BUT update the strategy selection in onCreate():
    • Replace direct AuthTabManager.isSupported() calls with AuthTabStrategyProvider.isAuthTabSupported()
    • Replace direct AuthTabLaunchStrategy(this, ...) construction with AuthTabStrategyProvider.createStrategy(this, ...)
    • Fallback to CustomTabsLaunchStrategy(this) if provider returns null or is not available
    • Remove any imports of AuthTabManager or AuthTabLaunchStrategy
  • KEEP CustomTabsLaunchStrategyTest.kt — unchanged
  • UPDATE SwitchBrowserActivityTest.kt — mock AuthTabStrategyProvider instead of AuthTabManager

Key Constraint

Common must NOT import any �rowser:1.9.0 APIs. No AuthTabIntent, no CustomTabsClient.isAuthTabSupported(). These APIs will live in Broker (separate PBI AB#3561427).

Acceptance Criteria

  1. AuthTabManager.kt and AuthTabLaunchStrategy.kt are removed from common module
  2. AuthTabStrategyProvider.kt is added as a singleton registry with no browser 1.9.0 imports
  3. SwitchBrowserActivity.kt uses AuthTabStrategyProvider for strategy selection
  4. All existing tests pass (update mocks to use AuthTabStrategyProvider)
  5. Code compiles with �rowser:1.7.0 (no 1.9.0 dependency)
  6. Follow .github/copilot-instructions.md strictly — new files in Kotlin, KDoc comments, use Logger class

@github-actions
Copy link
Copy Markdown

✅ Work item link check complete. Description contains link AB#3561426 to an Azure Boards work item.

1 similar comment
@github-actions
Copy link
Copy Markdown

✅ Work item link check complete. Description contains link AB#3561426 to an Azure Boards work item.

@github-actions github-actions Bot changed the title [WIP] Rework Auth Tab implementation to strategy pattern in Common [WIP] Rework Auth Tab implementation to strategy pattern in Common, Fixes AB#3561426 Apr 22, 2026
Copilot AI changed the title [WIP] Rework Auth Tab implementation to strategy pattern in Common, Fixes AB#3561426 Rework SwitchBrowser Auth Tab integration to provider-based strategy (Common Option 1B) Apr 22, 2026
Copilot AI requested a review from p3dr0rv April 22, 2026 00:15
@github-actions github-actions Bot changed the title Rework SwitchBrowser Auth Tab integration to provider-based strategy (Common Option 1B) Rework SwitchBrowser Auth Tab integration to provider-based strategy (Common Option 1B), Fixes AB#3561426 Apr 22, 2026
@p3dr0rv
Copy link
Copy Markdown
Contributor

p3dr0rv commented Apr 22, 2026

Closing duplicate - keeping PR #3093 as the canonical PR for AB#3561426 (Option 1B rework)

@p3dr0rv p3dr0rv closed this Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants