Code review (#140)

* Code review * removed context path manipulation * Tests fixed * Changed method signature to correct --------- Co-authored-by: Ionut Manolache <ionut.manolache@okta.com>
auth0 · Jan 8, 2025 · 0ed4950 · 0ed4950
1 parent 0b5f66c
commit 0ed4950
Show file tree

Hide file tree

Showing 15 changed files with 103 additions and 128 deletions.
diff --git a/Guardian/API/APIClient.swift b/Guardian/API/APIClient.swift
@@ -23,17 +23,17 @@
 import Foundation
 
 struct APIClient: API {
-
+    private let path = "appliance-mfa"
     let baseUrl: URL
     let telemetryInfo: Auth0TelemetryInfo?
 
     init(baseUrl: URL, telemetryInfo: Auth0TelemetryInfo? = nil) {
-        self.baseUrl = baseUrl
+        self.baseUrl = baseUrl.appendingPathComponentIfNeeded(path)
         self.telemetryInfo = telemetryInfo
     }
 
     func enroll(withTicket enrollmentTicket: String, identifier: String, name: String, notificationToken: String, verificationKey: VerificationKey) -> Request<Device, Enrollment> {
-        let url = self.baseUrl.appendingPathComponent("api/enroll")
+        let url = baseUrl.appendingPathComponent("api/enroll")
         do {
             let headers = ["Authorization": "Ticket id=\"\(enrollmentTicket)\""]
             guard let jwk = verificationKey.jwk else {
@@ -50,7 +50,7 @@ struct APIClient: API {
 
     func resolve(transaction transactionToken: String, withChallengeResponse challengeResponse: String) -> Request<Transaction, NoContent> {
         let transaction = Transaction(challengeResponse: challengeResponse)
-        let url = self.baseUrl.appendingPathComponent("api/resolve-transaction")
+        let url = baseUrl.appendingPathComponent("api/resolve-transaction")
         return Request.new(method: .post, url: url, headers: ["Authorization": "Bearer \(transactionToken)"], body: transaction, telemetryInfo: self.telemetryInfo)
     }
 
@@ -64,7 +64,7 @@ struct APIClient: API {
         let claims = BasicClaimSet(
             subject: userId,
             issuer: enrollmentId,
-            audience: self.baseUrl.appendingPathComponent(DeviceAPIClient.path).absoluteString,
+            audience: baseUrl.appendingPathComponent(DeviceAPIClient.path).absoluteString,
             expireAt: currentTime.addingTimeInterval(responseExpiration),
             issuedAt: currentTime
         )
@@ -74,3 +74,9 @@ struct APIClient: API {
     }
 
 }
+
+private extension URL {
+    func appendingPathComponentIfNeeded(_ pathComponent: String) -> URL {
+        lastPathComponent == pathComponent ? self : appendingPathComponent(pathComponent)
+    }
+}
diff --git a/Guardian/API/ConsentAPI.swift b/Guardian/API/ConsentAPI.swift
@@ -26,21 +26,19 @@ import Foundation
  `ConsentAPI` lets you retrieve consent objects from auth0's rich-consents API for authentication flows that require additional consent e.g. Client Initiated Backchannel Authentication (CIBA)
 
  ```
- let device: AuthenticationDevice = // the object you obtained when enrolling
  let consent = Guardian
-    .consent(forDomain: "tenant.guardian.auth0.com", device: device)
+    .consent(forDomain: "tenant.region.auth0.com")
  ```
  */
 public protocol ConsentAPI {
     /**
      ```
-     let device: AuthenticationDevice = // the object you obtained when enrolling
      let notification: Notification = // the notification received
      let consentId = notification.transactionLinkingId
 
      Guardian
-        .consent(forDomain: "tenant.guardian.auth0.com", device: device)
-     .fetch(consentId: consentId, notificationToken: notification.transactionToken)
+        .consent(forDomain: "tenant.region.auth0.com")
+     .fetch(consentId: consentId, notificationToken: notification.transactionToken, signingKey: enrollment.signingKey)
      .start { result in
           switch result {
           case .success(let payload):
@@ -54,8 +52,9 @@ public protocol ConsentAPI {
      - parameter consentId: the id of the consent object to fetch, this is obtained from the
                             transaction linking id of the ncoming push notification where relevant
      - parameter transactionToken: the access token obtained from the incoming push notification
+     - parameter signingKey: the private key used to sign Guardian AuthN requests
 
      - returns: a request to execute
      */
-    func fetch(consentId:String, notificationToken: String) -> Request<NoContent, Consent>
+    func fetch(consentId: String, transactionToken: String, signingKey: SigningKey) -> Request<NoContent, Consent>
 }
diff --git a/Guardian/API/ConsentAPIClient.swift b/Guardian/API/ConsentAPIClient.swift
@@ -24,31 +24,27 @@ import Foundation
 import CryptoKit
 
 struct ConsentAPIClient : ConsentAPI {
-    static let path: String = "rich-consents"
+    private let path: String = "rich-consents"
 
     let url:URL
     let telemetryInfo: Auth0TelemetryInfo?
-    let privateKey: SecKey
-    let publicKey: VerificationKey
 
-    init(baseUrl: URL, signingKey:SigningKey, telemetryInfo: Auth0TelemetryInfo? = nil ) {
-        let url = ConsentAPIClient.constructConsentAPIUrl(baseUrl: baseUrl)
+    init(baseConsentUrl: URL, telemetryInfo: Auth0TelemetryInfo? = nil ) {
+        let url = baseConsentUrl.appendingPathComponent(path, isDirectory: false)
         self.url = url
         self.telemetryInfo = telemetryInfo
-        self.privateKey = signingKey.secKey
-        self.publicKey = try! signingKey.verificationKey()
     }
 
-    func fetch(consentId:String, notificationToken: String) -> Request<NoContent, Consent> {
+    func fetch(consentId:String, transactionToken: String, signingKey: SigningKey) -> Request<NoContent, Consent> {
         let consentURL = self.url.appendingPathComponent(consentId)
 
         do {
-            let dpopAssertion = try self.proofOfPossesion(url:consentURL, notificationToken:notificationToken)
+            let dpopAssertion = try self.proofOfPossesion(url: consentURL, transactionToken: transactionToken, signingKey: signingKey)
             return Request.new(
                 method: .get,
                 url: consentURL,
                 headers: [
-                    "Authorization": "MFA-DPoP \(notificationToken)",
+                    "Authorization": "MFA-DPoP \(transactionToken)",
                     "MFA-DPoP": dpopAssertion
                 ],
                 telemetryInfo: self.telemetryInfo
@@ -58,28 +54,14 @@ struct ConsentAPIClient : ConsentAPI {
             return Request(method: .get, url: consentURL, error: error)
         }
     }
-
-    private static func constructConsentAPIUrl(baseUrl: URL) -> URL {
-        var consentAPIUrl = baseUrl
-
-        if baseUrl.lastPathComponent == "appliance-mfa" {
-            consentAPIUrl = baseUrl.deletingLastPathComponent()
-        }
-
-        else if baseUrl.host!.components(separatedBy: ".").contains("guardian") {
-            consentAPIUrl = URL(string: baseUrl.absoluteString.replacingOccurrences(of: "guardian.", with: ""))!
-        }
-
-        return consentAPIUrl.appendingPathComponent(ConsentAPIClient.path, isDirectory: false)
-    }
 
-    private func proofOfPossesion (url: URL, notificationToken:String) throws -> String {
-        guard let jwk = publicKey.jwk else {
+    private func proofOfPossesion (url: URL, transactionToken: String, signingKey: SigningKey) throws -> String {
+        guard let jwk = try signingKey.verificationKey().jwk else {
             throw GuardianError(code: .invalidJWK)
         }
 
         let header = JWT<DPoPClaimSet>.Header(algorithm: .rs256, type: "dpop+jwt", jwk: jwk)
-        let tokenHash = try self.authTokenHash(transactionToken: notificationToken);
+        let tokenHash = try self.authTokenHash(transactionToken: transactionToken);
 
         let claims = DPoPClaimSet(
             httpURI: url.absoluteString,
@@ -88,7 +70,7 @@ struct ConsentAPIClient : ConsentAPI {
             jti: UUID().uuidString,
             issuedAt: Date())
 
-         let jwt = try JWT<DPoPClaimSet>(claimSet: claims, header: header, key: self.privateKey)
+         let jwt = try JWT<DPoPClaimSet>(claimSet: claims, header: header, key: signingKey.secKey)
          return jwt.string
     }
 

diff --git a/Guardian/Guardian.swift b/Guardian/Guardian.swift
@@ -384,45 +384,39 @@ public func notification(from userInfo: [AnyHashable: Any]) -> Notification? {
  Creates an consent manager for a Guardian enrollment
 
  ```
- let device: AuthenticationDevice = // the object you obtained when enrolling
  let consent = Guardian
-    .consent(forDomain: "tenant.guardian.auth0.com", device: device)
+    .consent(forDomain: "<tenant>.<region>.auth0.com")
  ```
 
  - parameter forDomain:     domain or URL of your Guardian server
- - parameter device:        the enrolled device that will be used to handle authentication
  - parameter telemetryInfo:       information about the app, used for internal auth0 analitycs purposes
 
  - returns: an `ConsentAPI` instance
 
  - seealso: Guardian.ConsentAPI
  */
-public func consent(forDomain domain: String, device: AuthenticationDevice, telemetryInfo: Auth0TelemetryInfo? = nil) -> ConsentAPI {
-    return consent(url: url(from: domain)!, device: device, telemetryInfo: telemetryInfo)
+public func consent(forDomain domain: String, telemetryInfo: Auth0TelemetryInfo? = nil) -> ConsentAPI {
+    consent(consentUrl: url(from: domain)!, telemetryInfo: telemetryInfo)
 }
 
 /**
  Creates an consent manager for a Guardian enrollment
 
  ```
- let device: AuthenticationDevice = // the object you obtained when enrolling
  let authenticator = Guardian
-    .consent(url: URL(string: "https://tenant.guardian.auth0.com/")!,
-                    device: device)
+    .consent(url: URL(string: "https://<tenant>.<region>.auth0.com/")!)
  ```
 
  - parameter url:           URL of your Guardian server
- - parameter device:        the enrolled device that will be used to handle authentication
- - parameter telemetryInfo:       information about the app, used for internal auth0 analitycs purposes
+ - parameter telemetryInfo:       information about the app, used for internal auth0 analytics purposes
 
 
  - returns: an `ConsentAPI` instance
 
  - seealso: Guardian.ConsentAPI
  */
-public func consent(url: URL, device: AuthenticationDevice, telemetryInfo: Auth0TelemetryInfo? = nil) -> ConsentAPI {
-    let signingKey = device.signingKey;
-    return ConsentAPIClient(baseUrl: url, signingKey:signingKey, telemetryInfo: telemetryInfo)
+public func consent(consentUrl: URL, telemetryInfo: Auth0TelemetryInfo? = nil) -> ConsentAPI {
+    ConsentAPIClient(baseConsentUrl: consentUrl, telemetryInfo: telemetryInfo)
 }
 
 func url(from domain: String) -> URL? {

diff --git a/GuardianApp/AppDelegate.swift b/GuardianApp/AppDelegate.swift
@@ -27,7 +27,8 @@ import UserNotifications
 @UIApplicationMain
 class AppDelegate: UIResponder, UIApplicationDelegate {
 
-    static let guardianDomain = "guardian-demo.guardian.auth0.com"
+    static let tenantDomain = "https://dev-jo51o3afnh6qac8u.us.auth0.com"
+
     static var pushToken: String? = nil
     var window: UIWindow?
 
@@ -126,7 +127,7 @@ extension AppDelegate: UNUserNotificationCenterDelegate {
                 completionHandler()
             } else { // Guardian allow/reject action
                 Guardian
-                    .authentication(forDomain: AppDelegate.guardianDomain, device: enrollment)
+                    .authentication(forDomain: AppDelegate.tenantDomain, device: enrollment)
                     .handleAction(withIdentifier: identifier, notification: notification)
                     .start { _ in completionHandler() }
             }

diff --git a/GuardianApp/Base.lproj/Main.storyboard b/GuardianApp/Base.lproj/Main.storyboard
@@ -76,7 +76,7 @@
                                 </connections>
                             </button>
                         </subviews>
-                        <color key="backgroundColor" white="1" alpha="1" colorSpace="custom" customColorSpace="calibratedWhite"/>
+                        <color key="backgroundColor" systemColor="systemGray2Color"/>
                         <constraints>
                             <constraint firstItem="wfy-db-euE" firstAttribute="top" secondItem="d7L-C1-tps" secondAttribute="bottom" constant="20" id="3RK-gf-RFG"/>
                             <constraint firstItem="d7L-C1-tps" firstAttribute="trailing" secondItem="8bC-Xf-vdC" secondAttribute="trailingMargin" id="Bnw-A7-uqh"/>
@@ -267,7 +267,7 @@
                                 </subviews>
                             </stackView>
                         </subviews>
-                        <color key="backgroundColor" white="1" alpha="1" colorSpace="calibratedWhite"/>
+                        <color key="backgroundColor" systemColor="systemGray2Color"/>
                         <constraints>
                             <constraint firstItem="QZH-1e-kFg" firstAttribute="leading" secondItem="ayv-cV-wfg" secondAttribute="leadingMargin" id="1h7-fY-JRK"/>
                             <constraint firstItem="ray-6M-hml" firstAttribute="top" secondItem="Ioc-2K-RpH" secondAttribute="bottom" constant="20" id="2qU-b2-quF"/>
@@ -282,10 +282,12 @@
                     </view>
                     <simulatedNavigationBarMetrics key="simulatedTopBarMetrics" prompted="NO"/>
                     <connections>
+                        <outlet property="allowButton" destination="pAt-hB-LdP" id="N7c-Jk-akg"/>
                         <outlet property="bindingMessageLabel" destination="xgY-uW-cgY" id="Mfg-31-j7K"/>
                         <outlet property="browserLabel" destination="xc7-mu-JVG" id="NM3-N2-jnz"/>
                         <outlet property="consentDetailsView" destination="QZH-1e-kFg" id="Bxm-bc-n46"/>
                         <outlet property="dateLabel" destination="GSN-Uh-cn2" id="jhP-xU-RHq"/>
+                        <outlet property="denyButton" destination="TSg-91-JQ7" id="6q4-Me-u6H"/>
                         <outlet property="locationLabel" destination="RKL-g1-tm5" id="eCd-ib-k97"/>
                     </connections>
                 </viewController>
@@ -298,5 +300,8 @@
         <systemColor name="systemBackgroundColor">
             <color white="1" alpha="1" colorSpace="custom" customColorSpace="genericGamma22GrayColorSpace"/>
         </systemColor>
+        <systemColor name="systemGray2Color">
+            <color red="0.68235294120000001" green="0.68235294120000001" blue="0.69803921570000005" alpha="1" colorSpace="custom" customColorSpace="sRGB"/>
+        </systemColor>
     </resources>
 </document>
diff --git a/GuardianApp/NotificationController.swift b/GuardianApp/NotificationController.swift
@@ -31,6 +31,9 @@ class NotificationController: UIViewController {
     @IBOutlet var locationLabel: UILabel!
     @IBOutlet var dateLabel: UILabel!
 
+    @IBOutlet var denyButton: UIButton!
+    @IBOutlet var allowButton: UIButton!
+
     @IBOutlet var consentDetailsView: UIStackView!
     @IBOutlet var bindingMessageLabel: UILabel!
 
@@ -39,7 +42,7 @@ class NotificationController: UIViewController {
             return self.dismiss(animated: true, completion: nil)
         }
         let request = Guardian
-            .authentication(forDomain: AppDelegate.guardianDomain, device: enrollment)
+            .authentication(forDomain: AppDelegate.tenantDomain, device: enrollment)
             .allow(notification: notification)
         debugPrint(request)
         request.start { result in
@@ -60,7 +63,7 @@ class NotificationController: UIViewController {
             return self.dismiss(animated: true, completion: nil)
         }
         let request = Guardian
-            .authentication(forDomain: AppDelegate.guardianDomain, device: enrollment)
+            .authentication(forDomain: AppDelegate.tenantDomain, device: enrollment)
             .reject(notification: notification)
         debugPrint(request)
         request.start { result in
@@ -92,9 +95,12 @@ class NotificationController: UIViewController {
             return
         }
 
+        denyButton.isHidden = true
+        allowButton.isHidden = true
+
         Guardian
-            .consent(forDomain: AppDelegate.guardianDomain, device: enrollment)
-            .fetch(consentId: consentId, notificationToken: notification.transactionToken)
+            .consent(forDomain: AppDelegate.tenantDomain)
+            .fetch(consentId: consentId, transactionToken: notification.transactionToken, signingKey: enrollment.signingKey)
             .start{ [unowned self] result in
                 switch result {
                 case .failure(let cause):
@@ -103,11 +109,12 @@ class NotificationController: UIViewController {
                     updateBindingMessage(bindingMessage: payload.requestedDetails.bindingMessage)
             }
         }
-
     }
 
     func updateBindingMessage(bindingMessage:String) {
         DispatchQueue.main.async { [unowned self] in
+            self.denyButton.isHidden = false
+            self.allowButton.isHidden = false
             self.consentDetailsView.isHidden = false
             self.bindingMessageLabel.text = bindingMessage
         }

diff --git a/GuardianApp/ViewController.swift b/GuardianApp/ViewController.swift
@@ -43,7 +43,7 @@ class ViewController: UIViewController {
     @IBAction func unenrollAction(_ sender: AnyObject) {
         if let enrollment = AppDelegate.state {
             let request = Guardian
-                .api(forDomain: AppDelegate.guardianDomain)
+                .api(forDomain: AppDelegate.tenantDomain)
                 .device(forEnrollmentId: enrollment.identifier, userId: enrollment.userId, signingKey: enrollment.signingKey)
                 .delete()
             debugPrint(request)
@@ -122,7 +122,7 @@ extension ViewController: QRCodeReaderViewControllerDelegate {
                 let verificationKey = try? signingKey.verificationKey() else { return }
 
             let request = Guardian.enroll(
-                forDomain: AppDelegate.guardianDomain,
+                forDomain: AppDelegate.tenantDomain,
                 usingUri: qrCodeValue,
                 notificationToken: AppDelegate.pushToken!,
                 signingKey: signingKey,

diff --git a/GuardianTests/APIClientSpec.swift b/GuardianTests/APIClientSpec.swift
@@ -250,7 +250,7 @@ class APIClientSpec: QuickSpec {
                         isDeleteEnrollment(baseUrl: ValidURL, enrollmentId: ValidEnrollmentId)
                         && hasBearerJWTToken(withSub: ValidUserId,
                                              iss: ValidEnrollmentId,
-                                             aud: ValidURL.appendingPathComponent(DeviceAPIClient.path).absoluteString,
+                                             aud: ValidURL.appendingPathComponent("appliance-mfa").appendingPathComponent(DeviceAPIClient.path).absoluteString,
                                              validFor: ValidBasicJWTDuration),
                     response: { _ in
                         successResponse()
@@ -348,7 +348,7 @@ class APIClientSpec: QuickSpec {
                         isUpdateEnrollment(baseUrl: ValidURL, enrollmentId: ValidEnrollmentId)
                         && hasBearerJWTToken(withSub: ValidUserId,
                                              iss: ValidEnrollmentId,
-                                             aud: ValidURL.appendingPathComponent(DeviceAPIClient.path).absoluteString,
+                                             aud: ValidURL.appendingPathComponent("appliance-mfa").appendingPathComponent(DeviceAPIClient.path).absoluteString,
                                              validFor: ValidBasicJWTDuration),
                     response: { req in
                         let payload = req.a0_payload

diff --git a/GuardianTests/AuthenticationSpec.swift b/GuardianTests/AuthenticationSpec.swift
@@ -541,7 +541,7 @@ func checkJWT(request: URLRequest, accepted: Bool, reason: String? = nil, challe
         let jwt: JWT<GuardianClaimSet> = try? JWT(string: challengeResponse),
         let verified = try? jwt.verify(with: verificationKey.secKey),
         verified == true,
-        jwt.claimSet.audience == "https://tenant.guardian.auth0.com/also/works/in/appliance/api/resolve-transaction",
+        jwt.claimSet.audience == "https://tenant.region.auth0.com/appliance-mfa/api/resolve-transaction",
         jwt.claimSet.subject == challenge,
         jwt.claimSet.issuer == EnrolledDevice.vendorIdentifier,
         jwt.claimSet.issuedAt <= currentTime,