Enhance logging and repair flow for FPSSO (#49524)

**Related issue:** Resolves # # Checklist for submitter If some of the following don't apply, delete the relevant line. Unreleased bug so no changes file - [ ] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. ## Testing - [x] QA'd all new/changed functionality manually  ## Summary by CodeRabbit * **Bug Fixes** * Improved device and user registration reliability with clearer validation and error handling. * Added fallback username resolution when registration details are incomplete. * Registration now appropriately requests user interaction when required. * **Diagnostics** * Added structured logging for network failures, invalid responses, missing credentials, configuration errors, and registration outcomes.
2026-07-20 08:28:53 -04:00
parent 6c5d7334e3
commit 4762dba089
4 changed files with 86 additions and 13 deletions
@@ -13,6 +13,7 @@
 // to support a web view

 import Foundation
+import os
 import Security

 extension AuthenticationViewController {
@@ -23,35 +24,59 @@ extension AuthenticationViewController {
    // assertion. Fleet always publishes an encryption key, so the caller treats
    // nil as fatal rather than proceeding with password encryption disabled.
    func loginRequestEncryptionKey(jwksURL: URL) async -> SecKey? {
-        guard let (data, resp) = try? await URLSession.shared.data(from: jwksURL),
-              let http = resp as? HTTPURLResponse,
-              (200...299).contains(http.statusCode),
-              let jwks = try? JSONDecoder().decode(JWKSet.self, from: data)
-        else { return nil }
+        let data: Data
+        do {
+            let (body, resp) = try await URLSession.shared.data(from: jwksURL)
+            guard let http = resp as? HTTPURLResponse,
+                  (200...299).contains(http.statusCode) else {
+                let status = (resp as? HTTPURLResponse)?.statusCode ?? -1
+                logger.error("loginRequestEncryptionKey: JWKS fetch returned HTTP \(status, privacy: .public)")
+                return nil
+            }
+            data = body
+        } catch {
+            logger.error("loginRequestEncryptionKey: JWKS fetch failed: \(String(describing: error), privacy: .public)")
+            return nil
+        }
+        guard let jwks = try? JSONDecoder().decode(JWKSet.self, from: data) else {
+            logger.error("loginRequestEncryptionKey: JWKS decode failed")
+            return nil
+        }

        for jwk in jwks.keys where jwk.use == "enc" {
            if let key = jwk.ecPublicSecKey() {
                return key
            }
        }
+        logger.error("loginRequestEncryptionKey: no usable enc key in JWKS")
        return nil
    }

    // postDeviceRegistration POSTs the registration payload to Fleet and
    // returns true on a 2xx response.
    func postDeviceRegistration(payload: [String: String]) async -> Bool {
-        guard let endpoint = registrationEndpointURL else { return false }
+        guard let endpoint = registrationEndpointURL else {
+            logger.error("postDeviceRegistration: no registration endpoint URL")
+            return false
+        }
        var req = URLRequest(url: endpoint)
        req.httpMethod = "POST"
        req.setValue("application/x-www-form-urlencoded",
                     forHTTPHeaderField: "Content-Type")
        let items = payload.map { URLQueryItem(name: $0.key, value: $0.value) }
        req.httpBody = formURLEncodedBody(items)
-        guard let (_, resp) = try? await URLSession.shared.data(for: req),
-              let http = resp as? HTTPURLResponse else {
+        do {
+            let (_, resp) = try await URLSession.shared.data(for: req)
+            guard let http = resp as? HTTPURLResponse else {
+                logger.error("postDeviceRegistration: non-HTTP response")
+                return false
+            }
+            logger.log("postDeviceRegistration: HTTP \(http.statusCode, privacy: .public)")
+            return (200...299).contains(http.statusCode)
+        } catch {
+            logger.error("postDeviceRegistration: request failed: \(String(describing: error), privacy: .public)")
            return false
        }
-        return (200...299).contains(http.statusCode)
    }

    // formURLEncodedBody serializes query items as an x-www-form-urlencoded
@@ -12,6 +12,7 @@ import AuthenticationServices
 import CryptoKit
 import Foundation
 import IOKit
+import os
 import Security

@available(macOS 14.0, *)
@@ -23,13 +24,24 @@ extension AuthenticationViewController:
        options: ASAuthorizationProviderExtensionRequestOptions,
        completion: @escaping (ASAuthorizationProviderExtensionRegistrationResult) -> Void
    ) {
+        logger.log("beginDeviceRegistration options=\(options.rawValue, privacy: .public)")
        self.loginManager = loginManager
+        // A repair means the framework is recovering from bad registration
+        // state; minting fresh keys keeps the keychain, the rebuilt device
+        // configuration, and the server in lockstep instead of re-registering
+        // handles that may be part of the broken state.
+        if options.contains(.registrationRepair) {
+            logger.log("beginDeviceRegistration: repair requested, resetting device keys")
+            loginManager.resetDeviceKeys()
+        }
        guard let signKey = loginManager.key(for: .sharedDeviceSigning),
              let encKey = loginManager.key(for: .sharedDeviceEncryption) else {
+            logger.error("beginDeviceRegistration: shared device keys unavailable")
            completion(.failed)
            return
        }
        guard let registrationToken = loginManager.registrationToken, !registrationToken.isEmpty else {
+            logger.error("beginDeviceRegistration: no registration token in profile")
            completion(.failed)
            return
        }
@@ -42,6 +54,7 @@ extension AuthenticationViewController:
            do {
                try await self.applyLoginConfiguration(loginManager)
            } catch {
+                logger.error("beginDeviceRegistration: applyLoginConfiguration failed: \(String(describing: error), privacy: .public)")
                completion(.failed)
                return
            }
@@ -55,10 +68,12 @@ extension AuthenticationViewController:
            let requiredFields = ["device_signing_key", "device_encryption_key",
                                  "signing_key_id", "encryption_key_id"]
            guard requiredFields.allSatisfy({ !(payload[$0] ?? "").isEmpty }) else {
+                logger.error("beginDeviceRegistration: key export failed, refusing incomplete payload")
                completion(.failed)
                return
            }
            let ok = await self.postDeviceRegistration(payload: payload)
+            logger.log("beginDeviceRegistration: completing with \(ok ? "success" : "failed", privacy: .public)")
            completion(ok ? .success : .failed)
        }
    }
@@ -70,26 +85,46 @@ extension AuthenticationViewController:
        options: ASAuthorizationProviderExtensionRequestOptions,
        completion: @escaping (ASAuthorizationProviderExtensionRegistrationResult) -> Void
    ) {
+        logger.log("beginUserRegistration options=\(options.rawValue, privacy: .public) method=\(method.rawValue, privacy: .public) hasUserName=\(userName?.isEmpty == false, privacy: .public)")
        // Persist the user login configuration. Without this the framework
        // reports "no user configuration for user" and never finishes binding
        // the PSSO user to the local account, so the unlock-key/SecureToken
        // setup stays incomplete and key unwrap fails at login ("previous
        // password required"). For password mode saving the config is all the
        // extension needs to do.
-        guard let userName, !userName.isEmpty else {
-            completion(.failed)
+        //
+        // Background repair runs can arrive without a userName; the user login
+        // configuration saved by the original registration still names the
+        // registered user, so fall back to it rather than failing the whole
+        // registration cycle.
+        let userNameParam = userName?.isEmpty == false ? userName : nil
+        guard let resolvedUserName = userNameParam ?? loginManager.userLoginConfiguration?.loginUserName,
+              !resolvedUserName.isEmpty else {
+            if options.contains(.userInteractionEnabled) {
+                logger.error("beginUserRegistration: no user name available")
+                completion(.failed)
+            } else {
+                logger.log("beginUserRegistration: no user name and interaction disabled, deferring to UI retry")
+                completion(.userInterfaceRequired)
+            }
            return
        }
-        let config = ASAuthorizationProviderExtensionUserLoginConfiguration(loginUserName: userName)
+        let config = ASAuthorizationProviderExtensionUserLoginConfiguration(loginUserName: resolvedUserName)
        do {
            try loginManager.saveUserLoginConfiguration(config)
        } catch {
+            logger.error("beginUserRegistration: saveUserLoginConfiguration failed: \(String(describing: error), privacy: .public)")
            completion(.failed)
            return
        }
+        logger.log("beginUserRegistration: user login configuration saved")
        completion(.success)
    }

+    func registrationDidComplete() {
+        logger.log("registrationDidComplete")
+    }
+
    func protocolVersion() -> ASAuthorizationProviderExtensionPlatformSSOProtocolVersion {
        .version2_0
    }
@@ -10,6 +10,7 @@ import AuthenticationServices
 import CryptoKit
 import Foundation
 import IOKit
+import os
 import Security

@available(macOS 14.0, *)
@@ -83,7 +84,10 @@ extension AuthenticationViewController {
              let base = URL(string: baseString),
              let host = base.host,
              base.scheme?.lowercased() == "https"
-        else { throw NSError(domain: "FleetPSSO", code: -1) }
+        else {
+            logger.error("applyLoginConfiguration: missing or non-HTTPS BaseURL in profile ExtensionData")
+            throw NSError(domain: "FleetPSSO", code: -1)
+        }
        let cfg = ASAuthorizationProviderExtensionLoginConfiguration(
            clientID: Bundle.main.bundleIdentifier ?? "",
            issuer: host,
@@ -98,6 +102,7 @@ extension AuthenticationViewController {
        cfg.keyEndpointURL = pssoEndpointURL(base, "token")
        self.registrationEndpointURL = pssoEndpointURL(base, "registration")
        guard let encryptionKey = await loginRequestEncryptionKey(jwksURL: pssoEndpointURL(base, "jwks")) else {
+            logger.error("applyLoginConfiguration: failed to load login request encryption key")
            throw NSError(domain: "FleetPSSO", code: -2)
        }
        cfg.loginRequestEncryptionPublicKey = encryptionKey
@@ -9,6 +9,14 @@

 import AuthenticationServices
 import Cocoa
+import os
+
+// Registration runs headless (Setup Assistant, background repairs), so the
+// unified log is the only visibility into which step failed. Dynamic values
+// are private-by-default; annotate non-sensitive ones .public and never log
+// the registration token or key material.
+let logger = Logger(subsystem: "com.fleetdm.fleet-desktop.pssoextension",
+                    category: "psso")

 final class AuthenticationViewController: NSViewController,
    ASAuthorizationProviderExtensionAuthorizationRequestHandler {