Skip to content

[PM-28349] feat: Support https:// redirects for SSO and Duo auth connectors#2814

Merged
matt-livefront merged 4 commits into
mainfrom
matt/PM-28349-https-auth-connectors
Jul 6, 2026
Merged

[PM-28349] feat: Support https:// redirects for SSO and Duo auth connectors#2814
matt-livefront merged 4 commits into
mainfrom
matt/PM-28349-https-auth-connectors

Conversation

@matt-livefront

@matt-livefront matt-livefront commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

PM-28349

📔 Objective

Our mobile authentication connectors currently complete the ASWebAuthenticationSession flow at the end of the SSO and Duo flows using a custom bitwarden:// URL scheme. This adds support for switching this to a HTTPS callback URL for Cloud users (US, EU, and Internal regions)

What changed:

  • Region-gated callback selection (no feature flag). The callback descriptor is now resolved centrally in AuthService from EnvironmentService.region. Cloud regions use an https://{apexHost}/{sso,duo}-callback callback at the region's fixed apex host (US→bitwarden.com, EU→bitwarden.eu, Internal→bitwarden.pw); self-hosted continues to use bitwarden://.
  • New RegionType.internal for Bitwarden's bitwarden.pw (QA/Internal) environments. It is hidden from the region picker and detected automatically by bitwarden.pw host suffix (EnvironmentService.region / EnvironmentURLData.region). The apex host comes from RegionType.authCallbackHost.
  • ASWebAuthenticationSession.Callback.https(host:path:) is used to intercept the https:// redirect for SSO and Duo. This API is iOS 17.4+, and the deployment target is iOS 15.0, so it is @available-gated: Cloud users on iOS < 17.4 transparently fall back to the existing bitwarden:// callback.
  • SSO's OAuth redirect_uri follows the same region/availability resolution so the value sent to the identity provider always matches the callback the app intercepts.
  • Session construction for the SSO, Duo, and WebAuthn-self-hosted flows is consolidated behind a single AuthService factory (the coordinator retains presentation/start). The now-unused callbackUrlScheme parameter is removed from AuthRoute.singleSignOn.

Scope notes:

  • WebAuthn is intentionally excluded. On iOS, Cloud WebAuthn 2FA already uses the native passkey API (ASAuthorizationController), not a connector callback URL; only the self-hosted connector uses webauthn-callback, and self-hosted remains on bitwarden://.

@matt-livefront matt-livefront added ai-review Request a Claude code review t:feature labels Jun 19, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context labels Jun 19, 2026
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the region-gated HTTPS auth-connector callback change for SSO and Duo. The core logic — RegionType(baseURL:) resolution, the new .internal region for bitwarden.pw, the AuthWebSessionCallback/resolveCallback factory, and the iOS 17.4 availability gating — is consistent and well covered by tests. Verified that the redirect_uri sent to the IdP and the callback the ASWebAuthenticationSession matches both flow through resolveCallback, so they stay in sync across every region and OS branch.

Code Review Details

No findings above the reporting threshold.

Notes from the validation pass (no action required):

  • RegionType(baseURL:) anchors the internal-host check with a leading dot (host == "bitwarden.pw" || host.hasSuffix(".bitwarden.pw")), correctly excluding look-alikes like notbitwarden.pw.
  • BillingService.isSelfHosted() now returns false for bitwarden.pw environments (previously .selfHosted). This is a consequence of the new .internal region and is consistent with the PR's intent of treating Internal as a Cloud region.
  • The .internal case is excluded from the region picker via userSelectableCases; no remaining RegionType.allCases consumers are affected.
  • The unreachable custom-scheme else branch inside the .https arm of webAuthenticationSession(...) is intentional for compiler exhaustiveness and documented as such.

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.83193% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.67%. Comparing base (af442d5) to head (8dfce99).
⚠️ Report is 29 commits behind head on main.

Files with missing lines Patch % Lines
...twardenShared/Core/Auth/Services/AuthService.swift 61.90% 24 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2814      +/-   ##
==========================================
- Coverage   80.84%   78.67%   -2.18%     
==========================================
  Files        1022     1146     +124     
  Lines       65053    72422    +7369     
==========================================
+ Hits        52593    56978    +4385     
- Misses      12460    15444    +2984     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@matt-livefront
matt-livefront marked this pull request as ready for review June 19, 2026 20:46
@matt-livefront
matt-livefront requested a review from a team as a code owner June 19, 2026 20:46

@fedemkr fedemkr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, really like the RegionType simplification for base URLs.
Just a small question and it seems that Codecov has caught 24 lines not being tested in AuthService although it's the switch resolveCallback(for: callbackKind) block which I'm not sure if we can test it correctly.

Comment thread BitwardenShared/Core/Auth/Services/AuthService.swift
@matt-livefront

Copy link
Copy Markdown
Collaborator Author

it seems that Codecov has caught 24 lines not being tested in AuthService although it's the switch resolveCallback(for: callbackKind) block which I'm not sure if we can test it correctly.

Correct, there's no way to get at the URL or parameters used to create the ASWebAuthenticationSession unfortunately.

@matt-livefront
matt-livefront merged commit 72273fe into main Jul 6, 2026
42 of 47 checks passed
@matt-livefront
matt-livefront deleted the matt/PM-28349-https-auth-connectors branch July 6, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants