From 4762dba089af3b65e57e90d4ef7736f310463f7f Mon Sep 17 00:00:00 2001 From: Jordan Montgomery Date: Mon, 20 Jul 2026 08:28:53 -0400 Subject: [PATCH] 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. --- ...henticationViewController+Networking.swift | 43 +++++++++++++++---- .../AuthenticationViewController+PSSO.swift | 41 ++++++++++++++++-- .../AuthenticationViewController+Shared.swift | 7 ++- .../AuthenticationViewController.swift | 8 ++++ 4 files changed, 86 insertions(+), 13 deletions(-) diff --git a/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Networking.swift b/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Networking.swift index 9f6ea073fb..a86f23facf 100644 --- a/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Networking.swift +++ b/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Networking.swift @@ -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 diff --git a/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+PSSO.swift b/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+PSSO.swift index da47454461..49c5b1dedf 100644 --- a/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+PSSO.swift +++ b/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+PSSO.swift @@ -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 } diff --git a/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Shared.swift b/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Shared.swift index 52e12b3dad..dde55020d7 100644 --- a/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Shared.swift +++ b/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Shared.swift @@ -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 diff --git a/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController.swift b/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController.swift index c9826649e9..1c89596cd2 100644 --- a/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController.swift +++ b/apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController.swift @@ -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 {