Improve login to use ASWebAuthenticationSession with integrated browser #3938

UBaggeler · 2025-12-26T18:13:54Z

This PR improves log-in experience for NextCloud servers by migrating to ASWebAuthenticationSession.

Using in-app system/default browser to restrictions/shortcomings of WKWebView
Setting prefersEphemeralWebBrowserSession makes sure we always start fresh (without any cookies from previous sessions or default browser)
Using callback to keep nc://login redirect handling intact (pre V2 flow)
mTLS supported (can be configured in iOS Settings.app)

Fixes issues with deep-links and passkeys: #3932
Fixes "Your browser is not supported": #3891 #3804

Signed-off-by: Fabian Aggeler <aggeler@ubique.ch>

…e login UI (webview, browser,...) Signed-off-by: Fabian Aggeler <aggeler@ubique.ch>

Signed-off-by: Fabian Aggeler <aggeler@ubique.ch>

mpivchev · 2026-01-06T14:36:18Z

Hi @UBaggeler, thanks for the PR! We will review it soon.

Copilot

Pull request overview

This PR modernizes the Nextcloud iOS app login flow by migrating from WKWebView to ASWebAuthenticationSession, addressing several critical user-facing issues with OAuth 2.0 flows, passkeys, deep links, and browser compatibility warnings.

Key changes:

Replaces WKWebView-based login with ASWebAuthenticationSession to enable passkeys, deep links, and proper browser support
Refactors login polling logic into a dedicated NCLoginFlowPoller class for better separation of concerns
Extracts callback URL parsing into a reusable NCProviderLoginHandler struct

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
iOSClient/Login/NCWebViewLoginProvider.swift	Renamed from NCLoginProvider and removed polling logic, now focuses solely on WKWebView-based fallback login
iOSClient/Login/NCProviderLoginHandler.swift	New utility struct that centralizes callback URL parsing for provider login flows
iOSClient/Login/NCLoginFlowPoller.swift	New class encapsulating login flow V2 polling logic with proper task lifecycle management
iOSClient/Login/NCLogin.swift	Integrated ASWebAuthenticationSession for primary login flow with lifecycle management and error handling
iOSClient/Login/NCLogin.storyboard	Updated view controller reference to renamed NCWebViewLoginProvider class
Brand/Intro/NCIntroViewController.swift	Updated type reference to renamed NCWebViewLoginProvider class
Nextcloud.xcodeproj/project.pbxproj	Added new source files to project build configuration

Comments suppressed due to low confidence (1)

iOSClient/Login/NCWebViewLoginProvider.swift:139

The comparison on line 139 uses the lowercased currentWebViewURLString, but passes the original (non-lowercased) currentWebViewURL to the handler. The handler then lowercases it again for comparison (line 10 of NCProviderLoginHandler). This works, but checking should be done consistently - either both use the original URL or both use lowercased versions to avoid confusion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T16:16:31Z

iOSClient/Login/NCProviderLoginHandler.swift

+            if value.contains("server:") { server = value }
+            if value.contains("user:") { user = value }
+            if value.contains("password:") { password = value }


The parsing logic checks if values contain the prefixes "server:", "user:", "password:" but this could match false positives. For example, if a password contains the string "user:", it could be incorrectly assigned to the user variable. Consider using hasPrefix instead of contains to be more precise, or use a more robust URL component parsing approach.

Suggested change

if value.contains("server:") { server = value }

if value.contains("user:") { user = value }

if value.contains("password:") { password = value }

if value.hasPrefix("/server:") { server = value }

if value.hasPrefix("user:") { user = value }

if value.hasPrefix("password:") { password = value }

Copilot · 2026-01-06T16:16:31Z

iOSClient/Login/NCProviderLoginHandler.swift

+            return nil
+        }
+
+        let serverClean = server.replacingOccurrences(of: "/server:", with: "")


The replacement logic on line 32 only replaces "/server:" but the check on line 23 uses "server:" without the leading slash. This inconsistency could lead to the prefix not being properly removed if the format doesn't match expectations. Ensure the format is consistent or handle both cases.

Suggested change

let serverClean = server.replacingOccurrences(of: "/server:", with: "")

let serverClean = server

.replacingOccurrences(of: "/server:", with: "")

.replacingOccurrences(of: "server:", with: "")

Copilot · 2026-01-06T16:16:31Z

iOSClient/Login/NCLogin.swift

+        createAccount(urlBase: grant.urlBase, user: grant.loginName, password: grant.appPassword)
+    }
+
+    @MainActor


The function handleAuthenticationSessionCompletion is marked as @MainActor but is used as a completion handler for ASWebAuthenticationSession. According to Apple's documentation, completion handlers for ASWebAuthenticationSession are already called on the main thread, so the @MainActor annotation is redundant. While not harmful, it could be removed for clarity.

Suggested change

@MainActor

Copilot · 2026-01-06T16:16:32Z

iOSClient/Login/NCLogin.swift

+        Task {
+            await handleLoginGrant(NCLoginGrant(urlBase: providerGrant.urlBase, loginName: providerGrant.user, appPassword: providerGrant.password))


There's a potential race condition where the polling task could complete and call handleLoginGrant (line 384) at the same time as the authentication session completion handler calls it (line 442). Both paths could execute simultaneously if the user completes the login in the browser right when polling succeeds. Consider adding a flag or synchronization mechanism to ensure the account is only created once.

Copilot · 2026-01-06T16:16:32Z

iOSClient/Login/NCLoginFlowPoller.swift

+
+struct NCLoginGrant {
+    let urlBase: String
+    let loginName: String


Variable naming inconsistency: the struct NCLoginGrant uses loginName (line 9) while the tuple returned by NCProviderLoginHandler.handle uses just user (line 9 of NCProviderLoginHandler.swift). When converting between them (line 442 of NCLogin.swift), it's converted to loginName. Consider using consistent naming across both types for better code clarity.

Copilot · 2026-01-06T16:16:32Z

iOSClient/Login/NCLogin.swift

+    }
+
+    private func startASWebAuthenticationSession(loginURL: URL, token: String, endpoint: String) {
+        let callbackScheme = URL(string: NCBrandOptions.shared.webLoginAutenticationProtocol)?.scheme ?? loginURL.scheme


The fallback to loginURL.scheme on line 389 may not be appropriate. If NCBrandOptions.shared.webLoginAutenticationProtocol cannot be parsed as a URL, using the loginURL scheme (likely "https") as the callback scheme will cause the authentication session to fail since the app won't receive callbacks on the https scheme. Consider logging this case or using a more explicit error handling approach.

Suggested change

let callbackScheme = URL(string: NCBrandOptions.shared.webLoginAutenticationProtocol)?.scheme ?? loginURL.scheme

guard

let callbackURL = URL(string: NCBrandOptions.shared.webLoginAutenticationProtocol),

let callbackScheme = callbackURL.scheme,

!callbackScheme.isEmpty

else {

authenticationSession = nil

loginFlowPoller.cancel()

let error = NKError(errorCode: NCGlobal.shared.errorInternalError, errorDescription: "_error_something_wrong_")

NCContentPresenter().showError(error: error, priority: .max)

return

}

Copilot · 2026-01-06T16:16:33Z

iOSClient/Login/NCProviderLoginHandler.swift

+        let callbackURLString = callbackURL.absoluteString.lowercased()
+        let protocolPrefix = NCBrandOptions.shared.webLoginAutenticationProtocol.lowercased()
+
+        guard callbackURLString.hasPrefix(protocolPrefix), callbackURLString.contains("login") else {
+            return nil
+        }


The callback URL's absolute string is converted to lowercase for comparison, but the original URL (not lowercased) is used for parsing in line 20. This inconsistency could lead to issues if the parsing logic on line 20 expects case sensitivity. Consider using the original URL consistently, or only lowercase the protocol prefix for the prefix check.

Suggested change

let callbackURLString = callbackURL.absoluteString.lowercased()

let protocolPrefix = NCBrandOptions.shared.webLoginAutenticationProtocol.lowercased()

guard callbackURLString.hasPrefix(protocolPrefix), callbackURLString.contains("login") else {

return nil

}

let callbackURLString = callbackURL.absoluteString

let protocolPrefix = NCBrandOptions.shared.webLoginAutenticationProtocol

guard callbackURLString.range(of: protocolPrefix, options: [.caseInsensitive, .anchored]) != nil,

callbackURLString.range(of: "login", options: .caseInsensitive) != nil else {

return nil

Copilot · 2026-01-06T16:16:33Z

iOSClient/Login/NCLogin.swift

    func onBack() {
        loginButton.isEnabled = true
        loginButton.hideSpinnerAndShowButton()
+        loginFlowPoller.cancel()


The onBack() function cancels both the login flow poller and authentication session, but it doesn't clean up the state in NCLogin like viewDidDisappear does. If the user navigates back from the WebView login provider, the authentication session and poller references in NCLogin remain active. Consider adding similar cleanup logic here or consolidating the cleanup into a shared method.

Suggested change

loginFlowPoller.cancel()

loginFlowPoller.cancel()

loginFlowPoller = nil

Copilot · 2026-01-06T16:16:34Z

iOSClient/Login/NCLogin.swift

+        if session.start() {
+            startLoginFlowPolling(token: token, endpoint: endpoint)
+        } else {
+            authenticationSession = nil
+            loginFlowPoller.cancel()
+            let error = NKError(errorCode: NCGlobal.shared.errorInternalError, errorDescription: "_error_something_wrong_")
+            NCContentPresenter().showError(error: error, priority: .max)
+        }


When session.start() returns false (line 400), it indicates the session failed to start. However, the error shown to the user is a generic "error_something_wrong" message. Consider logging or providing more specific error information about why the session failed to start, as this could help with debugging issues related to callback URL schemes or system restrictions.

Copilot · 2026-01-06T16:16:34Z

iOSClient/Login/NCProviderLoginHandler.swift

+        let callbackURLString = callbackURL.absoluteString.lowercased()
+        let protocolPrefix = NCBrandOptions.shared.webLoginAutenticationProtocol.lowercased()
+
+        guard callbackURLString.hasPrefix(protocolPrefix), callbackURLString.contains("login") else {
+            return nil
+        }
+
+        var server: String = ""
+        var user: String = ""
+        var password: String = ""
+        let keyValue = callbackURL.path.components(separatedBy: "&")
+
+        for value in keyValue {
+            if value.contains("server:") { server = value }
+            if value.contains("user:") { user = value }
+            if value.contains("password:") { password = value }
+        }
+
+        guard !server.isEmpty, !user.isEmpty, !password.isEmpty else {
+            return nil
+        }
+
+        let serverClean = server.replacingOccurrences(of: "/server:", with: "")
+        let username = user.replacingOccurrences(of: "user:", with: "").replacingOccurrences(of: "+", with: " ")
+        let passwordClean = password.replacingOccurrences(of: "password:", with: "")
+
+        return (serverClean, username, passwordClean)


The login flow here relies on a custom URL scheme (configured via webLoginAutenticationProtocol, e.g. "nc://") that carries the server URL, username, and app password directly in the deep-link and parses them from callbackURL. On iOS, custom URL schemes are not exclusive to a single app, so any malicious app that registers the same scheme (e.g. nc) can intercept these callback URLs from Safari or ASWebAuthenticationSession and exfiltrate the cleartext credentials, leading to full account compromise. To harden this, avoid embedding credentials in the deep-link and switch to an HTTPS/universal-link based callback that returns only a short-lived authorization code or flow token, which the app then exchanges over TLS for credentials, ensuring only this app can complete the flow.

Suggested change

let callbackURLString = callbackURL.absoluteString.lowercased()

let protocolPrefix = NCBrandOptions.shared.webLoginAutenticationProtocol.lowercased()

guard callbackURLString.hasPrefix(protocolPrefix), callbackURLString.contains("login") else {

return nil

}

var server: String = ""

var user: String = ""

var password: String = ""

let keyValue = callbackURL.path.components(separatedBy: "&")

for value in keyValue {

if value.contains("server:") { server = value }

if value.contains("user:") { user = value }

if value.contains("password:") { password = value }

}

guard !server.isEmpty, !user.isEmpty, !password.isEmpty else {

return nil

}

let serverClean = server.replacingOccurrences(of: "/server:", with: "")

let username = user.replacingOccurrences(of: "user:", with: "").replacingOccurrences(of: "+", with: " ")

let passwordClean = password.replacingOccurrences(of: "password:", with: "")

return (serverClean, username, passwordClean)

// Expect an HTTPS / universal-link callback and avoid custom URL schemes

let protocolPrefix = NCBrandOptions.shared.webLoginAutenticationProtocol.lowercased()

let callbackURLString = callbackURL.absoluteString.lowercased()

// Ensure the callback originates from the expected base URL and contains "login"

guard callbackURLString.hasPrefix(protocolPrefix), callbackURLString.contains("login") else {

return nil

}

// Parse query items instead of embedding credentials in the path

guard let components = URLComponents(url: callbackURL, resolvingAgainstBaseURL: false) else {

return nil

}

let queryItems = components.queryItems ?? []

func value(for name: String) -> String? {

return queryItems.first(where: { $0.name == name })?.value

}

guard

let server = value(for: "server"),

let user = value(for: "user"),

let code = value(for: "code")

else {

return nil

}

// The third tuple element is now a short-lived authorization code, not a long-lived password.

return (server, user, code)

UBaggeler added 4 commits December 26, 2025 18:47

Rename NCLoginProvider to NCWebViewLoginProvider

2149dba

Signed-off-by: Fabian Aggeler <aggeler@ubique.ch>

Move polling of login flow v2 to NCLogin to make it independent of th…

ca83d0a

…e login UI (webview, browser,...) Signed-off-by: Fabian Aggeler <aggeler@ubique.ch>

Extract nc://login handling

a931ab9

Signed-off-by: Fabian Aggeler <aggeler@ubique.ch>

Use ASWebAuthenticationSession for login

1d716cb

Signed-off-by: Fabian Aggeler <aggeler@ubique.ch>

mpivchev requested review from Copilot, marinofaggiana and mpivchev January 6, 2026 14:36

Copilot started reviewing on behalf of mpivchev January 6, 2026 15:34 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

		Task {
		await handleLoginGrant(NCLoginGrant(urlBase: providerGrant.urlBase, loginName: providerGrant.user, appPassword: providerGrant.password))

	loginFlowPoller.cancel()
	loginFlowPoller.cancel()
	loginFlowPoller = nil

Uh oh!

Improve login to use ASWebAuthenticationSession with integrated browser #3938

Are you sure you want to change the base?

Improve login to use ASWebAuthenticationSession with integrated browser #3938

Conversation

UBaggeler commented Dec 26, 2025

Uh oh!

mpivchev commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants