Skip to content

Commit 4309f5c

Browse files
authored
PGAND-410 Resolve oauth session fixation on iOS (#272)
* update login flow with PKCE * fix MockGuardianAPI
1 parent ee7e380 commit 4309f5c

File tree

11 files changed

+166
-98
lines changed

11 files changed

+166
-98
lines changed

Diff for: FirefoxPrivateNetworkVPN.xcodeproj/project.pbxproj

+6
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,8 @@
244244
D54700E82491D90C00FA40A2 /* NotificationViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = D54700E72491D90C00FA40A2 /* NotificationViewController.swift */; };
245245
D54700EB2491D90C00FA40A2 /* MainInterface.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = D54700E92491D90C00FA40A2 /* MainInterface.storyboard */; };
246246
D54700EF2491D90C00FA40A2 /* NotificationContentExtension.appex in Embed App Extensions */ = {isa = PBXBuildFile; fileRef = D54700E12491D90C00FA40A2 /* NotificationContentExtension.appex */; settings = {ATTRIBUTES = (RemoveHeadersOnCopy, ); }; };
247+
D54D31DD2519F33500D6ED57 /* PKCECodeGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = D54D31DC2519F33500D6ED57 /* PKCECodeGenerator.swift */; };
248+
D54D31DE2519F33500D6ED57 /* PKCECodeGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = D54D31DC2519F33500D6ED57 /* PKCECodeGenerator.swift */; };
247249
D5D349B3249A73FB00DD9F50 /* error_noSignal.png in Resources */ = {isa = PBXBuildFile; fileRef = D5D349B1249A73FA00DD9F50 /* error_noSignal.png */; };
248250
D5D349B4249A73FB00DD9F50 /* error_unstable.png in Resources */ = {isa = PBXBuildFile; fileRef = D5D349B2249A73FA00DD9F50 /* error_unstable.png */; };
249251
D5D349B6249B703F00DD9F50 /* AppExtensionUserDefaults.swift in Sources */ = {isa = PBXBuildFile; fileRef = D5D349B5249B703F00DD9F50 /* AppExtensionUserDefaults.swift */; };
@@ -593,6 +595,7 @@
593595
D54700E72491D90C00FA40A2 /* NotificationViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationViewController.swift; sourceTree = "<group>"; };
594596
D54700EA2491D90C00FA40A2 /* Base */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; name = Base; path = Base.lproj/MainInterface.storyboard; sourceTree = "<group>"; };
595597
D54700EC2491D90C00FA40A2 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
598+
D54D31DC2519F33500D6ED57 /* PKCECodeGenerator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PKCECodeGenerator.swift; sourceTree = "<group>"; };
596599
D5D349B1249A73FA00DD9F50 /* error_noSignal.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = error_noSignal.png; sourceTree = "<group>"; };
597600
D5D349B2249A73FA00DD9F50 /* error_unstable.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = error_unstable.png; sourceTree = "<group>"; };
598601
D5D349B5249B703F00DD9F50 /* AppExtensionUserDefaults.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppExtensionUserDefaults.swift; sourceTree = "<group>"; };
@@ -1134,6 +1137,7 @@
11341137
F4056031239E9F31000A93CC /* KeychainStored.swift */,
11351138
F43DFC0A234D0B5C003EA64A /* TunnelConfigurationBuilder.swift */,
11361139
F4108050242CDAB300973421 /* NetworkingUtilities.swift */,
1140+
D54D31DC2519F33500D6ED57 /* PKCECodeGenerator.swift */,
11371141
);
11381142
path = Utilities;
11391143
sourceTree = "<group>";
@@ -2256,6 +2260,7 @@
22562260
F4E908D32371CAAD00EB9FE6 /* NavigationCoordinating.swift in Sources */,
22572261
F4E909092371CAAD00EB9FE6 /* AccountInformationCell.swift in Sources */,
22582262
F42DE7C82395A64A000B79C3 /* CarouselViewController.swift in Sources */,
2263+
D54D31DD2519F33500D6ED57 /* PKCECodeGenerator.swift in Sources */,
22592264
F45D1D1023873B8A0084B5DE /* CarouselPageViewController.swift in Sources */,
22602265
F4A7FA24237C5C960034AF6C /* Account.swift in Sources */,
22612266
F4E908D52371CAAD00EB9FE6 /* TunnelManaging.swift in Sources */,
@@ -2404,6 +2409,7 @@
24042409
D541182F24C8358500628DF6 /* NavigationCoordinating.swift in Sources */,
24052410
D541183024C8358500628DF6 /* AccountInformationCell.swift in Sources */,
24062411
D541183124C8358500628DF6 /* CarouselViewController.swift in Sources */,
2412+
D54D31DE2519F33500D6ED57 /* PKCECodeGenerator.swift in Sources */,
24072413
D541183224C8358500628DF6 /* CarouselPageViewController.swift in Sources */,
24082414
D541183324C8358500628DF6 /* Account.swift in Sources */,
24092415
D541183424C8358500628DF6 /* TunnelManaging.swift in Sources */,

Diff for: FirefoxPrivateNetworkVPN/AppDelegate.swift

+10
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,16 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
6565
UIDevice.current.userInterfaceIdiom == .pad ? .all : [.portrait, .portraitUpsideDown]
6666
}
6767

68+
func application(_ app: UIApplication, open url: URL, options: [UIApplication.OpenURLOptionsKey: Any] = [:]) -> Bool {
69+
if let sourceApplication = options[.sourceApplication] as? String,
70+
sourceApplication == "com.apple.SafariViewService" {
71+
NotificationCenter.default.post(name: .callbackURLNotification, object: nil,
72+
userInfo: ["callbackURL": url])
73+
return true
74+
}
75+
return false
76+
}
77+
6878
func applicationDidEnterBackground(_ application: UIApplication) {
6979
dependencyManager?.connectionHealthMonitor.stop()
7080
dependencyManager?.heartbeatMonitor.stop()

Diff for: FirefoxPrivateNetworkVPN/Networking/GuardianAPI.swift

+15-15
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,6 @@ class GuardianAPI: NetworkRequesting {
2020
}
2121

2222
// MARK: -
23-
func initiateUserLogin(completion: @escaping (Result<LoginCheckpointModel, GuardianAPIError>) -> Void) {
24-
let urlRequest = GuardianURLRequest.urlRequest(request: .login, type: .POST)
25-
networkLayer.fire(urlRequest: urlRequest) { result in
26-
DispatchQueue.main.async {
27-
completion(result.decode(to: LoginCheckpointModel.self))
28-
}
29-
}
30-
}
31-
3223
func accountInfo(token: String, completion: @escaping (Result<User, GuardianAPIError>) -> Void) {
3324
let urlRequest = GuardianURLRequest.urlRequest(request: .account, type: .GET, httpHeaderParams: headers(with: token))
3425
networkLayer.fire(urlRequest: urlRequest) { result in
@@ -38,8 +29,14 @@ class GuardianAPI: NetworkRequesting {
3829
}
3930
}
4031

41-
func verify(urlString: String, completion: @escaping (Result<VerifyResponse, GuardianAPIError>) -> Void) {
42-
let urlRequest = GuardianURLRequest.urlRequest(with: urlString, type: .GET)
32+
func verify(code: String, codeVerifier: String, completion: @escaping (Result<VerifyResponse, GuardianAPIError>) -> Void) {
33+
let body: [String: Any] = ["code": code, "code_verifier": codeVerifier]
34+
guard let data = try? JSONSerialization.data(withJSONObject: body) else {
35+
completion(.failure(.couldNotEncodeData))
36+
return
37+
}
38+
39+
let urlRequest = GuardianURLRequest.urlRequest(request: .verify, type: .POST, httpHeaderParams: headers(), body: data)
4340
networkLayer.fire(urlRequest: urlRequest) { result in
4441
DispatchQueue.main.async {
4542
completion(result.decode(to: VerifyResponse.self))
@@ -106,9 +103,12 @@ class GuardianAPI: NetworkRequesting {
106103
}
107104

108105
// MARK: - Utils
109-
private func headers(with token: String) -> [String: String] {
110-
return ["Authorization": "Bearer \(token)",
111-
"Content-Type": "application/json",
112-
"User-Agent": userAgentInfo]
106+
private func headers(with token: String = "") -> [String: String] {
107+
var headers = ["Content-Type": "application/json",
108+
"User-Agent": userAgentInfo]
109+
if !token.isEmpty {
110+
headers["Authorization"] = "Bearer \(token)"
111+
}
112+
return headers
113113
}
114114
}

Diff for: FirefoxPrivateNetworkVPN/Networking/GuardianURLRequest.swift

+23-12
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,13 @@ struct GuardianURLRequest {
3131
return urlRequest(with: urlString, type: type, queryParameters: queryParameters, httpHeaderParams: httpHeaderParams, body: body)
3232
}
3333

34-
static func urlRequest(with urlString: String,
35-
type: HttpMethod,
36-
queryParameters: [String: String]? = nil,
37-
httpHeaderParams: [String: String]? = nil,
38-
body: Data? = nil) -> URLRequest {
39-
var urlComponent = URLComponents(string: urlString)!
40-
if let queryParameters = queryParameters {
41-
let queryItems = queryParameters.map { URLQueryItem(name: $0.key, value: $0.value) }
42-
urlComponent.queryItems = queryItems
43-
}
44-
45-
var urlRequest = URLRequest(url: urlComponent.url!)
34+
static private func urlRequest(with urlString: String,
35+
type: HttpMethod,
36+
queryParameters: [String: String]? = nil,
37+
httpHeaderParams: [String: String]? = nil,
38+
body: Data? = nil) -> URLRequest {
39+
let url = generateURL(urlString: urlString, queryParameters: queryParameters)
40+
var urlRequest = URLRequest(url: url!)
4641
if let httpHeaderParams = httpHeaderParams {
4742
httpHeaderParams.forEach {
4843
urlRequest.setValue($0.value, forHTTPHeaderField: $0.key)
@@ -57,4 +52,20 @@ struct GuardianURLRequest {
5752

5853
return urlRequest
5954
}
55+
56+
static private func generateURL(urlString: String, queryParameters: [String: String]?) -> URL? {
57+
var urlComponent = URLComponents(string: urlString)!
58+
if let queryParameters = queryParameters {
59+
let queryItems = queryParameters.map { URLQueryItem(name: $0.key, value: $0.value) }
60+
urlComponent.queryItems = queryItems
61+
}
62+
63+
return urlComponent.url
64+
}
65+
66+
static func pkceLoginURL(codeChallenge: String) -> URL {
67+
let urlString = "\(GuardianURLRequest.baseURL)\(GuardianURLRequestPath.login.endpoint)"
68+
let queryParameters = ["code_challenge": codeChallenge, "code_challenge_method": "S256"]
69+
return generateURL(urlString: urlString, queryParameters: queryParameters)!
70+
}
6071
}

Diff for: FirefoxPrivateNetworkVPN/Networking/GuardianURLRequestPath.swift

+5-4
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
enum GuardianURLRequestPath {
1313
case login
14-
case verify(String)
14+
case verify
1515
case retrieveServers
1616
case account
1717
case addDevice
@@ -20,11 +20,12 @@ enum GuardianURLRequestPath {
2020

2121
var endpoint: String {
2222
let prefix = "api/v1/vpn/"
23+
let v2Prefix = "api/v2/vpn/"
2324
switch self {
2425
case .login:
25-
return prefix + "login"
26-
case .verify(let token):
27-
return prefix + "login/verify/" + token
26+
return v2Prefix + "login/ios"
27+
case .verify:
28+
return v2Prefix + "login/verify"
2829
case .retrieveServers:
2930
return prefix + "servers"
3031
case .account:

Diff for: FirefoxPrivateNetworkVPN/Project Files/Bridging-Header.h

+1
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,6 @@
1818
#include "ringlogger.h"
1919
#include "key.h"
2020
#import "SimplePing.h"
21+
#import "CommonCrypto/CommonCrypto.h"
2122

2223
#endif /* Guardian_Bridging_Header_h */

Diff for: FirefoxPrivateNetworkVPN/Project Files/Info.plist

+13
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,19 @@
7070
<string>APPL</string>
7171
<key>CFBundleShortVersionString</key>
7272
<string>$(MARKETING_VERSION)</string>
73+
<key>CFBundleURLTypes</key>
74+
<array>
75+
<dict>
76+
<key>CFBundleTypeRole</key>
77+
<string>Editor</string>
78+
<key>CFBundleURLName</key>
79+
<string>org.mozilla.ios.FirefoxVPN</string>
80+
<key>CFBundleURLSchemes</key>
81+
<array>
82+
<string>mozilla-vpn</string>
83+
</array>
84+
</dict>
85+
</array>
7386
<key>CFBundleVersion</key>
7487
<string>$(CURRENT_PROJECT_VERSION)</string>
7588
<key>ITSAppUsesNonExemptEncryption</key>

Diff for: FirefoxPrivateNetworkVPN/Protocols/NetworkRequesting.swift

+1-2
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,8 @@
1212
import Foundation
1313

1414
protocol NetworkRequesting {
15-
func initiateUserLogin(completion: @escaping (Result<LoginCheckpointModel, GuardianAPIError>) -> Void)
1615
func accountInfo(token: String, completion: @escaping (Result<User, GuardianAPIError>) -> Void)
17-
func verify(urlString: String, completion: @escaping (Result<VerifyResponse, GuardianAPIError>) -> Void)
16+
func verify(code: String, codeVerifier: String, completion: @escaping (Result<VerifyResponse, GuardianAPIError>) -> Void)
1817
func availableServers(with token: String, completion: @escaping (Result<[VPNCountry], GuardianAPIError>) -> Void)
1918
func addDevice(with token: String, body: [String: Any], completion: @escaping (Result<Device, GuardianAPIError>) -> Void)
2019
func removeDevice(with token: String, deviceKey: String, completion: @escaping (Result<Void, GuardianAPIError>) -> Void)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
//
2+
// PKCECodeGenerator
3+
// FirefoxPrivateNetworkVPN
4+
//
5+
// This Source Code Form is subject to the terms of the Mozilla Public
6+
// License, v. 2.0. If a copy of the MPL was not distributed with this
7+
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
8+
//
9+
// Copyright © 2020 Mozilla Corporation.
10+
//
11+
12+
import Foundation
13+
14+
struct PKCECodeGenerator {
15+
static var generateCode: (codeChallenge: String, codeVerifier: String) {
16+
let codeVerifier = generateCodeVerifier()
17+
let codeChallenge = base64UrlEncode(sha256(string: codeVerifier))
18+
return (codeChallenge, codeVerifier)
19+
}
20+
21+
private static func generateCodeVerifier() -> String {
22+
var buffer = [UInt8](repeating: 0, count: 32)
23+
_ = SecRandomCopyBytes(kSecRandomDefault, buffer.count, &buffer)
24+
return base64UrlEncode(Data(buffer))
25+
}
26+
27+
private static func base64UrlEncode(_ data: Data?) -> String {
28+
guard let data = data else { return "" }
29+
return data.base64EncodedString()
30+
.replacingOccurrences(of: "+", with: "-")
31+
.replacingOccurrences(of: "/", with: "_")
32+
}
33+
34+
private static func sha256(string: String) -> Data? {
35+
guard let data = string.data(using: .utf8) else { return nil }
36+
var hash = [UInt8](repeating: 0, count: Int(CC_SHA256_DIGEST_LENGTH))
37+
data.withUnsafeBytes {
38+
_ = CC_SHA256($0.baseAddress, CC_LONG(data.count), &hash)
39+
}
40+
return Data(hash)
41+
}
42+
}

Diff for: FirefoxPrivateNetworkVPN/ViewControllers/LoginViewController.swift

+49-56
Original file line numberDiff line numberDiff line change
@@ -12,88 +12,81 @@
1212
import UIKit
1313
import SafariServices
1414

15+
extension Notification.Name {
16+
static let callbackURLNotification = Notification.Name("callbackURL")
17+
}
18+
1519
class LoginViewController: UIViewController, Navigating {
1620
static var navigableItem: NavigableItem = .login
1721

1822
private let guardianAPI = DependencyManager.shared.guardianAPI
1923
private var safariViewController: SFSafariViewController?
20-
private var verificationURL: URL?
21-
private var verifyTimer: Timer?
22-
private var isVerifying = false
24+
private let accountManager = DependencyManager.shared.accountManager
25+
private let PKCECode: (String, String) = PKCECodeGenerator.generateCode
2326

2427
init() {
2528
super.init(nibName: nil, bundle: nil)
26-
guardianAPI.initiateUserLogin { [weak self] result in
27-
switch result {
28-
case .success(let checkpointModel):
29-
guard let loginURL = checkpointModel.loginUrl else { return }
30-
self?.verificationURL = checkpointModel.verificationUrl
31-
let safariViewController = SFSafariViewController(url: loginURL)
32-
DispatchQueue.main.async {
33-
self?.addChild(safariViewController)
34-
self?.view.addSubview(safariViewController.view)
35-
safariViewController.view.frame = self?.view.bounds ?? CGRect.zero
36-
safariViewController.view.autoresizingMask = [.flexibleWidth, .flexibleHeight]
37-
safariViewController.didMove(toParent: self)
38-
}
39-
safariViewController.delegate = self
40-
self?.safariViewController = safariViewController
41-
case .failure(let error):
42-
let loginError = error.getLoginError()
43-
let context: NavigableContext = loginError == .maxDevicesReached ? .maxDevicesReached : .error(loginError)
44-
self?.navigate(to: .landing, context: context)
45-
}
46-
}
47-
}
48-
49-
deinit {
50-
verifyTimer?.invalidate()
5129
}
5230

5331
required init?(coder: NSCoder) {
5432
fatalError("init(coder:) has not been implemented")
5533
}
5634

57-
@objc private func verify() {
58-
guard let verificationURL = verificationURL else { return }
59-
if isVerifying { return }
60-
isVerifying = true
35+
override func viewDidLoad() {
36+
super.viewDidLoad()
6137

62-
guardianAPI.verify(urlString: verificationURL.absoluteString) { [weak self] result in
63-
guard let self = self else { return }
38+
let safariViewController = SFSafariViewController(url: GuardianURLRequest.pkceLoginURL(codeChallenge: PKCECode.0))
39+
addChild(safariViewController)
40+
view.addSubview(safariViewController.view)
41+
safariViewController.view.frame = self.view.bounds
42+
safariViewController.view.autoresizingMask = [.flexibleWidth, .flexibleHeight]
43+
safariViewController.didMove(toParent: self)
44+
safariViewController.delegate = self
45+
self.safariViewController = safariViewController
46+
47+
NotificationCenter.default.addObserver(self, selector: #selector(handleCallback), name: .callbackURLNotification, object: nil)
48+
}
6449

50+
@objc private func handleCallback(notification: Notification) {
51+
guard let url = notification.userInfo?["callbackURL"] as? URL,
52+
let queryItems = URLComponents(string: url.absoluteString)?.queryItems,
53+
let code = queryItems.first(where: { $0.name == "code" })?.value else {
54+
navigate(to: .landing)
55+
return
56+
}
57+
verify(code: code)
58+
}
59+
60+
private func verify(code: String) {
61+
guardianAPI.verify(code: code, codeVerifier: PKCECode.1) { [weak self] result in
62+
guard let self = self else { return }
6563
switch result {
6664
case .success(let verification):
67-
DependencyManager.shared.accountManager.login(with: verification) { loginResult in
68-
self.isVerifying = false
69-
self.verifyTimer?.invalidate()
70-
switch loginResult {
71-
case .success:
72-
self.navigate(to: .home)
73-
case .failure(let error):
74-
Logger.global?.log(message: "Authentication Error: \(error)")
75-
let context: NavigableContext = error == .maxDevicesReached ? .maxDevicesReached : .error(error)
76-
self.navigate(to: .landing, context: context)
77-
}
78-
}
79-
case .failure:
80-
self.isVerifying = false
81-
return
65+
self.login(verification: verification)
66+
case .failure(let error):
67+
self.navigate(to: .landing, context: .error(error))
8268
}
8369
}
8470
}
85-
}
8671

87-
// MARK: - SFSafariViewControllerDelegate
88-
extension LoginViewController: SFSafariViewControllerDelegate {
89-
func safariViewController(_ controller: SFSafariViewController, didCompleteInitialLoad didLoadSuccessfully: Bool) {
90-
if didLoadSuccessfully && verifyTimer == nil {
91-
verifyTimer = Timer.scheduledTimer(timeInterval: 3, target: self, selector: #selector(verify), userInfo: nil, repeats: true)
72+
private func login(verification: VerifyResponse) {
73+
accountManager.login(with: verification) { [weak self] loginResult in
74+
guard let self = self else { return }
75+
switch loginResult {
76+
case .success:
77+
self.navigate(to: .home)
78+
case .failure(let error):
79+
Logger.global?.log(message: "Authentication Error: \(error)")
80+
let context: NavigableContext = error == .maxDevicesReached ? .maxDevicesReached : .error(error)
81+
self.navigate(to: .landing, context: context)
82+
}
9283
}
9384
}
85+
}
9486

87+
// MARK: - SFSafariViewControllerDelegate
88+
extension LoginViewController: SFSafariViewControllerDelegate {
9589
func safariViewControllerDidFinish(_ controller: SFSafariViewController) {
96-
self.verifyTimer?.invalidate()
9790
navigate(to: .landing)
9891
}
9992
}

0 commit comments

Comments
 (0)