ionic-team / capacitor

Build cross-platform Native Progressive Web Apps for iOS, Android, and the Web ⚡️
https://capacitorjs.com
MIT License
12.14k stars 1.01k forks source link

bug: For iOS, a cookie received with HttpOnly set is added to document.cookie #6885

Open nickredding opened 1 year ago

nickredding commented 1 year ago

Bug Report

Capacitor Version

   Capacitor Doctor

Latest Dependencies:

  @capacitor/cli: 5.3.0
  @capacitor/core: 5.3.0
  @capacitor/android: 5.3.0
  @capacitor/ios: 5.3.0

Installed Dependencies:

  @capacitor/cli: 5.3.0
  @capacitor/core: 5.3.0
  @capacitor/android: 5.3.0
  @capacitor/ios: 5.3.0

Platform(s)

iOS

Current Behavior

A cookie received with HttpOnly set is added to document.cookie, contrary to the spec for this parameter

Expected Behavior

The cookie should not be seen in document.cookie

Code Reproduction

See https://github.com/nickredding/bug6792 which has been extended to demonstrate this problem.

Other Technical Details

nickredding commented 1 year ago

The fix is to modify setCookiesFromResponse in HttpRequestHandler and getServerUrl in CapacitorCookieManager as follows

// fixed to not split the cookie at expires=<day>, and process domain attribute properly
// this resolves https://github.com/ionic-team/capacitor/issues/6821
// fixed to not add HttpOnly cookies to document.cookie
// this resolves https://github.com/ionic-team/capacitor/issues/6885
// this also avoids https://github.com/ionic-team/capacitor/issues/6882
// (not adding the cookie to document.cookie if the domain attribute is set)
public static func setCookiesFromResponse(_ response: HTTPURLResponse, _ config: InstanceConfiguration?) {
    let scanExpires = "=(Sun|Mon|Tue|Wed|Thu|Fri|Sat),"
    let restoreExpires = "=(Sun|Mon|Tue|Wed|Thu|Fri|Sat) "
    let headers = response.allHeaderFields
    if let cookies = headers["Set-Cookie"] as? String {
        let scannedCookies = cookies.replacingOccurrences(of: scanExpires, with: "=$1 ", options: .regularExpression, range: nil)
        for cookie in scannedCookies.components(separatedBy: ",") {
            let restoredCookie = cookie.replacingOccurrences(of: restoreExpires, with: "=$1,", options: .regularExpression, range: nil)
            let domainComponents = restoredCookie.lowercased().components(separatedBy: "domain=")
            if domainComponents.count > 1 {
                CapacitorCookieManager(config).setCookie(
                    domainComponents[1].components(separatedBy: ";")[0],
                    restoredCookie
                )
            }
            if restoredCookie.lowercased().range(of: "(httponly;)|(httponly$)", options: .regularExpression) == nil {
                // the secure attribute must be removed to set the cookie in document.cookie via the default capacitor url
                // as well, the domain attribute (if present) must be set to the default capacitor host
                let localDomain = CapacitorCookieManager(config).getServerUrl("")!.host
                var localDomainCookie = restoredCookie.replacingOccurrences(of: "; *secure", with: "", options: .regularExpression)
                if domainComponents.count > 1 {
                    localDomainCookie = localDomainCookie.replacingOccurrences(of: domainComponents[1].components(separatedBy: ";")[0], with: localDomain!)
                }
                CapacitorCookieManager(config).setCookie(localDomain!, localDomainCookie)
            }
        }
    }
    CapacitorCookieManager(config).syncCookiesToWebView()
}

// part of the fix for https://github.com/ionic-team/capacitor/issues/6821
public func getServerUrl(_ urlString: String?) -> URL? {
    guard let urlString = urlString else {
        return getServerUrl()
    }

    if urlString.isEmpty {
        return getServerUrl()
    }

    // should return "https://\(urlString)" not "http://\(urlString)" in case isUrlSanitized returns false
    let validUrlString = (isUrlSanitized(urlString)) ? urlString : "https://\(urlString)"

    guard let url = URL(string: validUrlString) else {
        return getServerUrl()
    }

    return url
}