diff --git a/docs/notes-3.7.txt b/docs/notes-3.7.txt index 93c4e4a73e..024dac630f 100644 --- a/docs/notes-3.7.txt +++ b/docs/notes-3.7.txt @@ -369,6 +369,18 @@ Python API Improvements: Password Manager Improvements: - Bitwarden is now a supported password manager. - Keeper Security is also supported. +- The Keeper adapter supports Nested Shared + Folders as well as the classic vault. + Both appear in one list, labeled (Classic) + or (Nested). Add, edit, and delete work + for either type. +- Password manager adapters can declare + optional toggles in their handshake that the + Add Account panel renders as checkboxes. The + Keeper adapter uses this to offer a “Use + classic permission model” choice. Leaving + it unchecked adds to Nested Shared Folders; + checking it adds to the classic vault. Shell Integration Improvements: - xonsh is now a supported shell. diff --git a/pwmplugin/Sources/PasswordManagerProtocol/PasswordManagerProtocol.swift b/pwmplugin/Sources/PasswordManagerProtocol/PasswordManagerProtocol.swift index 57617f5d56..b3b6ec6f1f 100644 --- a/pwmplugin/Sources/PasswordManagerProtocol/PasswordManagerProtocol.swift +++ b/pwmplugin/Sources/PasswordManagerProtocol/PasswordManagerProtocol.swift @@ -33,6 +33,20 @@ public enum PasswordManagerProtocol { } } + public struct AddAccountToggle: Codable { + public var key: String + public var label: String + public var note: String? + public var defaultValue: Bool + + public init(key: String, label: String, note: String?, defaultValue: Bool) { + self.key = key + self.label = label + self.note = note + self.defaultValue = defaultValue + } + } + // A custom setting for your adapter. public struct SettingsField: Codable { public var key: String @@ -79,9 +93,10 @@ public enum PasswordManagerProtocol { public var persistsCredentials: Bool? public var customCommands: [CustomCommand]? public var settingsFields: [SettingsField]? + public var addAccountToggles: [AddAccountToggle]? public init(protocolVersion: Int, name: String, requiresMasterPassword: Bool, canSetPasswords: Bool, userAccounts: [UserAccount]?, needsPathToDatabase: Bool, databaseExtension: String?, needsPathToExecutable: String?, - pathToDatabaseKind: PathKind? = nil, pathToDatabasePrompt: String? = nil, pathToDatabasePlaceholder: String? = nil, masterPasswordLabel: String? = nil, persistsCredentials: Bool? = nil, customCommands: [CustomCommand]? = nil, settingsFields: [SettingsField]? = nil) { + pathToDatabaseKind: PathKind? = nil, pathToDatabasePrompt: String? = nil, pathToDatabasePlaceholder: String? = nil, masterPasswordLabel: String? = nil, persistsCredentials: Bool? = nil, customCommands: [CustomCommand]? = nil, settingsFields: [SettingsField]? = nil, addAccountToggles: [AddAccountToggle]? = nil) { self.protocolVersion = protocolVersion self.name = name self.requiresMasterPassword = requiresMasterPassword @@ -97,6 +112,7 @@ public enum PasswordManagerProtocol { self.persistsCredentials = persistsCredentials self.customCommands = customCommands self.settingsFields = settingsFields + self.addAccountToggles = addAccountToggles } } @@ -164,12 +180,20 @@ public enum PasswordManagerProtocol { public var userName: String public var accountName: String public var hasOTP: Bool - - public init(identifier: AccountIdentifier, userName: String, accountName: String, hasOTP: Bool) { + /// Optional vault/source hint for host display formatting (e.g. "Classic", "Nested"). + /// Must not be baked into `accountName`, which is a stable identity for matching. + public var sourceLabel: String? + + public init(identifier: AccountIdentifier, + userName: String, + accountName: String, + hasOTP: Bool, + sourceLabel: String? = nil) { self.identifier = identifier self.userName = userName self.accountName = accountName self.hasOTP = hasOTP + self.sourceLabel = sourceLabel } } @@ -232,6 +256,8 @@ public enum PasswordManagerProtocol { public var userName: String public var accountName: String public var password: String? + + public var flags: [String: Bool]? } public struct AddAccountResponse: Codable { diff --git a/pwmplugin/Sources/iterm2-keeper-adapter/KeeperAPIErrorFormatting.swift b/pwmplugin/Sources/iterm2-keeper-adapter/KeeperAPIErrorFormatting.swift index 360a41f030..6b14001829 100644 --- a/pwmplugin/Sources/iterm2-keeper-adapter/KeeperAPIErrorFormatting.swift +++ b/pwmplugin/Sources/iterm2-keeper-adapter/KeeperAPIErrorFormatting.swift @@ -17,3 +17,55 @@ func keeperUserFacingPasswordUpdateError(apiDetail: String) -> String { } return apiDetail } + +private let keeperUpdateCredentialsAdvice = + "Update your Service URL and API key in Password Manager settings, and confirm your Keeper Commander server is running." + +private func keeperResponseBodyLooksLikeHTML(_ data: Data) -> Bool { + guard let head = String(data: data.prefix(2048), encoding: .utf8)?.lowercased() else { return false } + return head.contains(" String { + if let detail = keeperHumanReadableError(fromResponseData: data) { + return detail + } + if let body = data, !body.isEmpty, keeperResponseBodyLooksLikeHTML(body) { + return "The Service URL returned an HTML error page (the server may be offline, the URL may be wrong, or a proxy/tunnel may be misconfigured). \(keeperUpdateCredentialsAdvice)" + } + if let code = statusCode { + if code == 401 || code == 403 { + return "API key rejected (HTTP \(code)). \(keeperUpdateCredentialsAdvice)" + } + if code >= 500 { + return "Keeper Commander returned an error (HTTP \(code)). \(keeperUpdateCredentialsAdvice)" + } + return "Unexpected response from Keeper Commander (HTTP \(code)). \(keeperUpdateCredentialsAdvice)" + } + return "Could not reach Keeper Commander. \(keeperUpdateCredentialsAdvice)" +} + +func keeperConnectivityErrorMessage(urlError: Error?) -> String? { + guard let urlError = urlError as? URLError else { return nil } + switch urlError.code { + case .timedOut: + return "Could not reach the Keeper Commander server (request timed out). \(keeperUpdateCredentialsAdvice)" + case .cannotFindHost, .dnsLookupFailed: + return "The Service URL host could not be resolved. \(keeperUpdateCredentialsAdvice)" + case .cannotConnectToHost: + return "The Service URL is unreachable (connection refused). \(keeperUpdateCredentialsAdvice)" + case .notConnectedToInternet: + return "No internet connection. \(keeperUpdateCredentialsAdvice)" + case .networkConnectionLost, .resourceUnavailable: + return "The connection to Keeper Commander was lost. \(keeperUpdateCredentialsAdvice)" + case .secureConnectionFailed, .serverCertificateUntrusted, + .serverCertificateHasBadDate, .serverCertificateNotYetValid, + .serverCertificateHasUnknownRoot, .clientCertificateRejected, + .clientCertificateRequired: + return "TLS handshake with the Service URL failed. \(keeperUpdateCredentialsAdvice)" + case .badURL, .unsupportedURL: + return "The Service URL is malformed. \(keeperUpdateCredentialsAdvice)" + default: + return "Could not reach Keeper Commander (\(urlError.localizedDescription)). \(keeperUpdateCredentialsAdvice)" + } +} diff --git a/pwmplugin/Sources/iterm2-keeper-adapter/KeeperAdapterLog.swift b/pwmplugin/Sources/iterm2-keeper-adapter/KeeperAdapterLog.swift new file mode 100644 index 0000000000..a1a0cd0d04 --- /dev/null +++ b/pwmplugin/Sources/iterm2-keeper-adapter/KeeperAdapterLog.swift @@ -0,0 +1,44 @@ +// Best-effort file log at ~/Library/Logs/iterm2-keeper-adapter.log (stdout is JSON). + +import Foundation + +enum KeeperAdapterLog { + static let defaultURL: URL = { + let logsDir = FileManager.default + .urls(for: .libraryDirectory, in: .userDomainMask) + .first? + .appendingPathComponent("Logs") + let dir = logsDir ?? URL(fileURLWithPath: NSTemporaryDirectory()) + try? FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + return dir.appendingPathComponent("iterm2-keeper-adapter.log") + }() + + static let isEnabled: Bool = { + ProcessInfo.processInfo.environment["ITERM2_KEEPER_ADAPTER_LOG_ENABLED"] == "1" + }() + + private static let formatter: ISO8601DateFormatter = { + let f = ISO8601DateFormatter() + f.formatOptions = [.withInternetDateTime, .withFractionalSeconds] + return f + }() + + // Synchronous so short-lived CLI processes do not drop lines on exit. + private static let lock = NSLock() + + static func write(_ message: String) { + guard isEnabled else { return } + let stamp = formatter.string(from: Date()) + let line = "[\(stamp)] [\(getpid())] \(message)\n" + guard let data = line.data(using: .utf8) else { return } + lock.lock() + defer { lock.unlock() } + if let handle = try? FileHandle(forWritingTo: defaultURL) { + defer { try? handle.close() } + _ = try? handle.seekToEnd() + try? handle.write(contentsOf: data) + } else { + try? data.write(to: defaultURL, options: .atomic) + } + } +} diff --git a/pwmplugin/Sources/iterm2-keeper-adapter/KeeperCommanderClient.swift b/pwmplugin/Sources/iterm2-keeper-adapter/KeeperCommanderClient.swift index c681c77d95..360d73b491 100644 --- a/pwmplugin/Sources/iterm2-keeper-adapter/KeeperCommanderClient.swift +++ b/pwmplugin/Sources/iterm2-keeper-adapter/KeeperCommanderClient.swift @@ -43,8 +43,7 @@ private struct KeeperFolder: Decodable { let flags: String? } -/// Commander `ls -l` often appends connection info after `login @ …`: `https://…`, or database-style -/// `host:port` / IPv4. iTerm2 sends `userName` to the terminal as-is, so we keep only the login segment. +/// Strip URL/host suffix after `login @ …` from Commander list descriptions. private func loginFromKeeperListDisplayDescription(_ raw: String) -> String { let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines) guard let range = trimmed.range(of: " @ ", options: .literal) else { @@ -59,17 +58,14 @@ private func loginFromKeeperListDisplayDescription(_ raw: String) -> String { return trimmed } -/// True when the part after ` @ ` looks like a URL or server address (not part of the login). private func keeperListDescriptionSuffixIsConnectionInfo(_ tail: String) -> Bool { let t = tail.trimmingCharacters(in: .whitespacesAndNewlines) if t.isEmpty { return false } let lower = t.lowercased() if lower.hasPrefix("http://") || lower.hasPrefix("https://") { return true } - // IPv4 or IPv4:port (e.g. 127.0.0.1:3366) if t.range(of: #"^(?:\d{1,3}\.){3}\d{1,3}(:\d+)?$"#, options: .regularExpression) != nil { return true } - // hostname:port (e.g. db.example.com:3306) if t.range(of: #"^[a-zA-Z0-9][a-zA-Z0-9.-]*:\d{1,5}$"#, options: .regularExpression) != nil { return true } @@ -80,32 +76,123 @@ struct KeeperRecord: Decodable { let number: Int? let uid: String? private let record_uid: String? - var effectiveUid: String? { uid ?? record_uid } let type: String? let title: String? let description: String? - /// Present in some Commander JSON list payloads; avoids N+1 `get` calls (which trigger HTTP 429 rate limits). + let name: String? + let details: String? + let source: String? let login: String? let username: String? - - init(number: Int?, uid: String?, record_uid: String?, type: String?, title: String?, description: String?, login: String? = nil, username: String? = nil) { + let record_category: String? + + private let upperUID: String? + private let upperTitle: String? + private let upperType: String? + private let itemType: String? + + enum CodingKeys: String, CodingKey { + case number, uid, record_uid, type, title, description + case name, details, source, login, username, record_category + case upperUID = "UID" + case upperTitle = "Title" + case upperType = "Type" + case itemType = "Item Type" + } + + init(number: Int? = nil, uid: String? = nil, record_uid: String? = nil, + type: String? = nil, title: String? = nil, description: String? = nil, + name: String? = nil, details: String? = nil, source: String? = nil, + login: String? = nil, username: String? = nil, + record_category: String? = nil, + upperUID: String? = nil, upperTitle: String? = nil, + upperType: String? = nil, itemType: String? = nil) { self.number = number self.uid = uid self.record_uid = record_uid self.type = type self.title = title self.description = description + self.name = name + self.details = details + self.source = source self.login = login self.username = username + self.record_category = record_category + self.upperUID = upperUID + self.upperTitle = upperTitle + self.upperType = upperType + self.itemType = itemType + } + + func withRecordCategory(_ category: String) -> KeeperRecord { + KeeperRecord(number: number, uid: uid, record_uid: record_uid, + type: type, title: title, description: description, + name: name, details: details, source: source, + login: login, username: username, record_category: category, + upperUID: upperUID, upperTitle: upperTitle, + upperType: upperType, itemType: itemType) + } + + var effectiveUid: String? { uid ?? record_uid ?? upperUID } + var effectiveType: String? { type ?? upperType } + + var isFolder: Bool { + if let t = effectiveType?.lowercased(), t == "folder" { return true } + if let it = itemType?.lowercased(), it == "folder" { return true } + return false + } + + var displayTitle: String { + for candidate in [title, upperTitle, name] { + if let t = candidate?.trimmingCharacters(in: .whitespacesAndNewlines), !t.isEmpty { return t } + } + return "Untitled" + } + + var detailsDescription: String? { + guard let d = details else { return nil } + guard let range = d.range(of: "Description:", options: .literal) else { return nil } + let raw = d[range.upperBound...].trimmingCharacters(in: .whitespacesAndNewlines) + if raw.isEmpty || raw.caseInsensitiveCompare("None") == .orderedSame { return nil } + return raw } - /// Best-effort login/username for the password manager list without extra API round-trips. var listUserName: String { if let l = login?.trimmingCharacters(in: .whitespacesAndNewlines), !l.isEmpty { return l } if let u = username?.trimmingCharacters(in: .whitespacesAndNewlines), !u.isEmpty { return u } - let d = description?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" - if d.isEmpty { return "" } - return loginFromKeeperListDisplayDescription(d) + for candidate in [description, detailsDescription] { + let d = candidate?.trimmingCharacters(in: .whitespacesAndNewlines) ?? "" + if d.isEmpty { continue } + return loginFromKeeperListDisplayDescription(d) + } + return "" + } + + var sourceLabel: String? { + normalizedKeeperSourceLabel(record_category) ?? normalizedKeeperSourceLabel(source) + } + + var displayTitleWithSource: String { + guard let label = sourceLabel else { return displayTitle } + return "\(displayTitle) (\(label))" + } +} + +private func normalizedKeeperSourceLabel(_ raw: String?) -> String? { + guard let value = raw?.trimmingCharacters(in: .whitespacesAndNewlines), !value.isEmpty else { + return nil + } + switch value.lowercased() { + case "classic", "legacy": + return "Classic" + case "nested", + "nested share folder", + "nested share subfolder", + "nested share subfolders": + return "Nested" + default: + return value } } @@ -128,6 +215,10 @@ enum KeeperClientError: Error, LocalizedError { } final class KeeperCommanderClient { + static let longRequestTimeout: TimeInterval = 300 + static let validationRequestTimeout: TimeInterval = 120 + static let statusPollTimeout: TimeInterval = 30 + let baseURL: URL private let session: URLSession @@ -159,49 +250,61 @@ final class KeeperCommanderClient { Self.v2BaseURL(from: baseURL).appendingPathComponent("result").appendingPathComponent(requestId) } - func executeCommand(apiKey: String, command: String) throws -> Data { + func executeCommand(apiKey: String, + command: String, + timeout: TimeInterval = longRequestTimeout) throws -> Data { let body = try JSONEncoder().encode(KeeperExecuteRequest(command: command)) var request = URLRequest(url: asyncURL()) + request.timeoutInterval = timeout request.httpMethod = "POST" request.setValue("application/json", forHTTPHeaderField: "Content-Type") request.setValue(apiKey, forHTTPHeaderField: "api-key") request.httpBody = body let (data, response, err) = session.synchronousData(for: request) - if let err = err { throw err } + if let err = err { + throw KeeperClientError.message(keeperConnectivityErrorMessage(urlError: err) ?? err.localizedDescription) + } guard let data = data else { throw KeeperClientError.message("No data") } guard let http = response as? HTTPURLResponse, http.statusCode == 202 else { - throw KeeperClientError.message(keeperHumanReadableError(fromResponseData: data) ?? String(data: data, encoding: .utf8) ?? "Unexpected response") + let code = (response as? HTTPURLResponse)?.statusCode + throw KeeperClientError.message(keeperConnectivityErrorMessage(statusCode: code, data: data)) } let queued = try JSONDecoder().decode(KeeperV2QueuedResponse.self, from: data) guard let requestId = queued.request_id, !requestId.isEmpty else { throw KeeperClientError.message("No request_id in v2 response") } - return try pollForResult(apiKey: apiKey, requestId: requestId) + return try pollForResult(apiKey: apiKey, requestId: requestId, totalTimeout: timeout) } - private func pollForResult(apiKey: String, requestId: String) throws -> Data { + private func pollForResult(apiKey: String, + requestId: String, + totalTimeout: TimeInterval) throws -> Data { let interval: TimeInterval = 2 - let deadline = Date().addingTimeInterval(120) + let deadline = Date().addingTimeInterval(totalTimeout) + let maxPolls = Int(totalTimeout / interval) + 30 var pollCount = 0 var consecutiveUnparseable = 0 while true { pollCount += 1 if Date() > deadline { throw KeeperClientError.message("Keeper service v2 request timed out") } - if pollCount > 60 { throw KeeperClientError.message("Keeper service did not complete the request in time") } + if pollCount > maxPolls { throw KeeperClientError.message("Keeper service did not complete the request in time") } var sreq = URLRequest(url: statusURL(requestId: requestId)) + sreq.timeoutInterval = Self.statusPollTimeout sreq.setValue(apiKey, forHTTPHeaderField: "api-key") let (sdata, sresp, serr) = session.synchronousData(for: sreq) - if let serr = serr { throw serr } + if let serr = serr { + throw KeeperClientError.message(keeperConnectivityErrorMessage(urlError: serr) ?? serr.localizedDescription) + } let scode = (sresp as? HTTPURLResponse)?.statusCode ?? 0 if scode != 200 { - throw KeeperClientError.message(keeperHumanReadableError(fromResponseData: sdata) ?? "HTTP \(scode)") + throw KeeperClientError.message(keeperConnectivityErrorMessage(statusCode: scode, data: sdata)) } guard let sdata = sdata, let statusResp = try? JSONDecoder().decode(KeeperV2StatusResponse.self, from: sdata), let status = statusResp.status else { consecutiveUnparseable += 1 if consecutiveUnparseable >= 15 { - throw KeeperClientError.message(keeperHumanReadableError(fromResponseData: sdata) ?? "Invalid status response") + throw KeeperClientError.message(keeperConnectivityErrorMessage(statusCode: scode, data: sdata)) } Thread.sleep(forTimeInterval: interval) continue @@ -210,12 +313,15 @@ final class KeeperCommanderClient { switch status { case "completed": var rreq = URLRequest(url: resultURL(requestId: requestId)) + rreq.timeoutInterval = totalTimeout rreq.setValue(apiKey, forHTTPHeaderField: "api-key") let (rdata, rresp, rerr) = session.synchronousData(for: rreq) - if let rerr = rerr { throw rerr } + if let rerr = rerr { + throw KeeperClientError.message(keeperConnectivityErrorMessage(urlError: rerr) ?? rerr.localizedDescription) + } guard let rdata = rdata, !rdata.isEmpty else { throw KeeperClientError.message("No data") } if let http = rresp as? HTTPURLResponse, http.statusCode != 200 { - throw KeeperClientError.message(keeperHumanReadableError(fromResponseData: rdata) ?? "HTTP \(http.statusCode)") + throw KeeperClientError.message(keeperConnectivityErrorMessage(statusCode: http.statusCode, data: rdata)) } return rdata case "failed", "expired": @@ -343,9 +449,117 @@ private func validatedRecordUID(_ recordUid: String) throws -> String { return uid } -func listAccountsRecords(apiKey: String, client: KeeperCommanderClient) throws -> [PasswordManagerProtocol.Account] { - let data = try client.executeCommand(apiKey: apiKey, command: "ls -R -l") - var records: [KeeperRecord]? +private let keeperEmbeddedRecordUIDRegex = try! NSRegularExpression(pattern: "[A-Za-z0-9_-]{15,}") + +private func extractRecordUID(from text: String) throws -> String { + let trimmed = text.trimmingCharacters(in: .whitespacesAndNewlines) + if let uid = try? validatedRecordUID(trimmed) { + return uid + } + let fullRange = NSRange(trimmed.startIndex..., in: trimmed) + for match in keeperEmbeddedRecordUIDRegex.matches(in: trimmed, range: fullRange) { + guard let range = Range(match.range, in: trimmed) else { continue } + if let uid = try? validatedRecordUID(String(trimmed[range])) { + return uid + } + } + throw KeeperClientError.message("Invalid record identifier.") +} + +private enum KeeperMutationStrategy { + case stopAfterFirstFailure + case tryAllPreservingFirstError +} + +private struct KeeperMutationAttempt { + let label: String + let run: () throws -> Data +} + +private func runKeeperMutationAttempts(logPrefix: String, + uid: String, + attempts: [KeeperMutationAttempt], + strategy: KeeperMutationStrategy, + formatFailure: (Data) -> String) throws { + func attempt(_ step: KeeperMutationAttempt) throws -> (success: Bool, raw: Data) { + KeeperAdapterLog.write("\(logPrefix): trying verb=\(step.label) uid=\(uid)") + let data = try step.run() + KeeperAdapterLog.write("\(logPrefix): \(step.label) response bytes=\(data.count)") + let success = (try? JSONDecoder().decode(KeeperExecuteResponse.self, from: data))?.status == "success" + return (success, data) + } + + var firstFailureResponse: Data? + var firstNetworkError: Error? + for step in attempts { + do { + let result = try attempt(step) + if result.success { + KeeperAdapterLog.write("\(logPrefix): success uid=\(uid) via \(step.label)") + return + } + if firstFailureResponse == nil { + firstFailureResponse = result.raw + } + if strategy == .stopAfterFirstFailure { + break + } + } catch { + KeeperAdapterLog.write("\(logPrefix): \(step.label) executeCommand threw: \(error.localizedDescription)") + if firstNetworkError == nil { + firstNetworkError = error + } + if strategy == .stopAfterFirstFailure { + throw error + } + // tryAll: keep going so classic/nested fallback can still run. + } + } + + if let response = firstFailureResponse { + let raw = formatFailure(response) + KeeperAdapterLog.write("\(logPrefix): FAILED uid=\(uid): \(raw)") + throw KeeperClientError.message(raw) + } + if let error = firstNetworkError { + throw error + } + throw KeeperClientError.message(formatFailure(Data())) +} + +enum KeeperRecordSource: String { + case classic + case nested + + static func fromLabel(_ label: String?) -> KeeperRecordSource? { + switch label?.lowercased() { + case "classic": return .classic + case "nested": return .nested + default: return nil + } + } +} + +struct ParsedAccountIdentifier { + let source: KeeperRecordSource? + let uid: String +} + +/// Accept bare UIDs and legacy `classic:`/`nested:` prefixes from older sessions. +/// Routing stays adapter-internal: mutations try both Commander verbs when needed. +func parseAccountIdentifier(_ raw: String) -> ParsedAccountIdentifier { + let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines) + if let colon = trimmed.firstIndex(of: ":") { + let prefix = String(trimmed[.. [KeeperRecord] { var payloadsToTry: [Data] = [data] if let wrapper = try? JSONDecoder().decode(KeeperV2ResultWrapper.self, from: data), wrapper.status == "success", @@ -367,38 +581,102 @@ func listAccountsRecords(apiKey: String, client: KeeperCommanderClient) throws - parsed["command"] as? String == "ls", let dataObj = parsed["data"] as? [String: Any], let rawRecords = dataObj["records"] as? [[String: Any]], !rawRecords.isEmpty { - records = rawRecords.compactMap { dict -> KeeperRecord? in + let recs = rawRecords.compactMap { dict -> KeeperRecord? in guard let line = dict["title"] as? String else { return nil } return parseLsRecordLine(line) } - if !(records?.isEmpty ?? true) { break } + if !recs.isEmpty { return recs } + } + if let response = try? JSONDecoder().decode(KeeperExecuteResponse.self, from: jsonData), + response.status == "success", let recs = response.data?.records, !recs.isEmpty { + return recs + } + if let responseArray = try? JSONDecoder().decode(KeeperExecuteResponseDataArray.self, from: jsonData), + responseArray.status == "success", let arr = responseArray.data, !arr.isEmpty { + return arr + } + } + return [] +} + +private func taggedAsNested(_ records: [KeeperRecord]) -> [KeeperRecord] { + return records.map { rec in + if rec.sourceLabel != nil { + return rec } - if let response = try? JSONDecoder().decode(KeeperExecuteResponse.self, from: jsonData), response.status == "success", let recs = response.data?.records, !recs.isEmpty { - records = recs - break + return rec.withRecordCategory("Nested") + } +} + +private func taggedAsClassic(_ records: [KeeperRecord]) -> [KeeperRecord] { + return records.map { rec in + if rec.sourceLabel != nil { + return rec } - if let responseArray = try? JSONDecoder().decode(KeeperExecuteResponseDataArray.self, from: jsonData), responseArray.status == "success", let arr = responseArray.data, !arr.isEmpty { - records = arr - break + return rec.withRecordCategory("Classic") + } +} + +func validateApiKey(apiKey: String, client: KeeperCommanderClient) throws { + _ = try client.executeCommand(apiKey: apiKey, + command: "list --format=json", + timeout: KeeperCommanderClient.validationRequestTimeout) +} + +func listAccountsRecords(apiKey: String, + client: KeeperCommanderClient, + syncFirst: Bool = false) throws -> [PasswordManagerProtocol.Account] { + KeeperAdapterLog.write("listAccountsRecords: begin syncFirst=\(syncFirst)") + if syncFirst { + _ = try? client.executeCommand(apiKey: apiKey, command: "sync-down") + KeeperAdapterLog.write("listAccountsRecords: sync-down completed") + } + + let listData = try client.executeCommand(apiKey: apiKey, command: "list --format=json") + let listRecords = taggedAsClassic(parseListingPayload(listData)) + KeeperAdapterLog.write("listAccountsRecords: list returned \(listRecords.count) records") + + var nsfRecords: [KeeperRecord] = [] + if let nsfData = try? client.executeCommand(apiKey: apiKey, command: "nsf-list --records --format=json") { + nsfRecords = taggedAsNested(parseListingPayload(nsfData)) + KeeperAdapterLog.write("listAccountsRecords: nsf-list returned \(nsfRecords.count) records") + } else { + KeeperAdapterLog.write("listAccountsRecords: nsf-list call failed (continuing with `list` results only)") + } + + // Prefer classic `list` over `nsf-list` when the same UID appears in both, so a + // genuine classic record that also surfaces in NSF does not get nested verbs. + var byUid: [String: KeeperRecord] = [:] + var order: [String] = [] + for rec in listRecords { + guard let uid = rec.effectiveUid, !uid.isEmpty else { continue } + if byUid[uid] == nil { order.append(uid) } + byUid[uid] = rec + } + for rec in nsfRecords { + guard let uid = rec.effectiveUid, !uid.isEmpty else { continue } + if byUid[uid] == nil { + order.append(uid) + byUid[uid] = rec } } - guard let recs = records, !recs.isEmpty else { return [] } - // Do not call `get` per record here: each record would add several HTTP calls and Commander - // Service Mode often returns HTTP 429 (Too Many Requests). Usernames come from `ls` output - // (description column and optional login/username fields in JSON). - return recs.compactMap { rec -> PasswordManagerProtocol.Account? in + let merged = order.compactMap { byUid[$0] } + KeeperAdapterLog.write("listAccountsRecords: merged total=\(merged.count) unique UIDs (list=\(listRecords.count), nsf=\(nsfRecords.count))") + + return merged.compactMap { rec -> PasswordManagerProtocol.Account? in + if rec.isFolder { return nil } guard let uid = rec.effectiveUid, !uid.isEmpty else { return nil } - let title = rec.title ?? "Untitled" return PasswordManagerProtocol.Account( identifier: PasswordManagerProtocol.AccountIdentifier(accountID: uid), userName: rec.listUserName, - accountName: title, - hasOTP: false) + accountName: rec.displayTitle, + hasOTP: false, + sourceLabel: rec.sourceLabel) } } func getPassword(apiKey: String, recordUid: String, client: KeeperCommanderClient) throws -> PasswordManagerProtocol.Password { - let uid = try validatedRecordUID(recordUid) + let uid = try validatedRecordUID(parseAccountIdentifier(recordUid).uid) let jsonData = try client.executeCommand(apiKey: apiKey, command: "get \(uid) --format=json") if let exact = passwordFromGetJSONResponse(jsonData) { return PasswordManagerProtocol.Password(password: exact, otp: nil) @@ -411,7 +689,7 @@ func getPassword(apiKey: String, recordUid: String, client: KeeperCommanderClien } func getLogin(apiKey: String, recordUid: String, client: KeeperCommanderClient) throws -> String { - let uid = try validatedRecordUID(recordUid) + let uid = try validatedRecordUID(parseAccountIdentifier(recordUid).uid) let jsonData = try client.executeCommand(apiKey: apiKey, command: "get \(uid) --format=json") if let login = loginFromGetJSONResponse(jsonData) { return login @@ -450,27 +728,39 @@ private func extractPasswordFromRawGet(data: Data) -> String? { return nil } +private func passwordCommandFragment(password: String) -> String { + let b64 = Data(password.utf8).base64EncodedString() + return "password=$BASE64:\(b64)" +} + +private func redactedPasswordFragment(password: String) -> String { + let b64Length = Data(password.utf8).base64EncodedString().count + return "password=$BASE64:<\(b64Length) chars>" +} + func setPassword(apiKey: String, recordUid: String, newPassword: String?, client: KeeperCommanderClient) throws { guard let newPassword = newPassword, !newPassword.isEmpty else { throw KeeperClientError.message("Password field is required.") } - let uid = try validatedRecordUID(recordUid) - let b64 = Data(newPassword.utf8).base64EncodedString() - let cmd = "record-update -r \(uid) password=$BASE64:\(b64)" - let data = try client.executeCommand(apiKey: apiKey, command: cmd) - if let response = try? JSONDecoder().decode(KeeperExecuteResponse.self, from: data), response.status == "success" { - return + let uid = try validatedRecordUID(parseAccountIdentifier(recordUid).uid) + + func makeAttempt(verb: String) -> KeeperMutationAttempt { + KeeperMutationAttempt(label: verb) { + let cmd = "\(verb) --force -r \(uid) \(passwordCommandFragment(password: newPassword))" + KeeperAdapterLog.write("setPassword: issuing verb=\(verb) uid=\(uid) \(redactedPasswordFragment(password: newPassword))") + return try client.executeCommand(apiKey: apiKey, command: cmd) + } } - let raw = keeperHumanReadableError(fromResponseData: data) ?? "Update failed" - throw KeeperClientError.message(keeperUserFacingPasswordUpdateError(apiDetail: raw)) + + // Routing is adapter-internal: always try classic then nested verbs. + try runKeeperMutationAttempts( + logPrefix: "setPassword", + uid: uid, + attempts: [makeAttempt(verb: "record-update"), makeAttempt(verb: "nsf-record-update")], + strategy: .tryAllPreservingFirstError, + formatFailure: { keeperUserFacingPasswordUpdateError(apiDetail: keeperHumanReadableError(fromResponseData: $0) ?? "Update failed") }) } -/// Escape `accountName` / `userName` for `record-add ... --title="..." login="..."`. -/// -/// Commander receives the command as a string and may evaluate it in a shell-like way. Values are -/// wrapped in double quotes; inside those quotes, POSIX-ish shells still treat `\`, `"`, `` ` ``, -/// and `$` specially (command substitution / expansion). We also normalize line breaks and escape -/// `!` for bash `histexpand` edge cases. private func escapeForKeeperDoubleQuotedCommandField(_ s: String) -> String { s .replacingOccurrences(of: "\\", with: "\\\\") @@ -483,36 +773,120 @@ private func escapeForKeeperDoubleQuotedCommandField(_ s: String) -> String { } func deleteRecord(apiKey: String, recordUid: String, client: KeeperCommanderClient) throws { - let uid = try validatedRecordUID(recordUid) - let data = try client.executeCommand(apiKey: apiKey, command: "rm -f \(uid)") - if let response = try? JSONDecoder().decode(KeeperExecuteResponse.self, from: data), response.status == "success" { - return + let uid = try validatedRecordUID(parseAccountIdentifier(recordUid).uid) + + func makeAttempt(cmd: String, label: String) -> KeeperMutationAttempt { + KeeperMutationAttempt(label: label) { + try client.executeCommand(apiKey: apiKey, command: cmd) + } } - throw KeeperClientError.message("Delete failed") + + try runKeeperMutationAttempts( + logPrefix: "deleteRecord", + uid: uid, + attempts: [ + makeAttempt(cmd: "rm -f \(uid)", label: "rm"), + makeAttempt(cmd: "nsf-rm \(uid) -f", label: "nsf-rm"), + ], + strategy: .tryAllPreservingFirstError, + formatFailure: { keeperHumanReadableError(fromResponseData: $0) ?? "Delete failed" }) } -func addRecord(apiKey: String, userName: String, accountName: String, password: String?, client: KeeperCommanderClient) throws -> String { +func addRecord(apiKey: String, + userName: String, + accountName: String, + password: String?, + useClassicPermission: Bool, + client: KeeperCommanderClient) throws -> String { let escapedTitle = escapeForKeeperDoubleQuotedCommandField(accountName) - var cmd = "record-add --record-type=login --title=\"\(escapedTitle)\"" + let verb = useClassicPermission ? "record-add" : "nsf-record-add" + var cmd = "\(verb) --force --record-type=login --title=\"\(escapedTitle)\"" if !userName.isEmpty { let escapedLogin = escapeForKeeperDoubleQuotedCommandField(userName) cmd += " login=\"\(escapedLogin)\"" } if let password = password, !password.isEmpty { - let passwordB64 = Data(password.utf8).base64EncodedString() - cmd += " password=$BASE64:\(passwordB64)" + cmd += " " + passwordCommandFragment(password: password) + } + let loggableCmd: String = { + guard let password = password, !password.isEmpty else { return cmd } + return cmd.replacingOccurrences(of: passwordCommandFragment(password: password), + with: redactedPasswordFragment(password: password)) + }() + KeeperAdapterLog.write("addRecord: verb=\(verb) issuing command=\(loggableCmd)") + let data: Data + do { + data = try client.executeCommand(apiKey: apiKey, command: cmd) + } catch { + KeeperAdapterLog.write("addRecord: \(verb) executeCommand threw: \(error.localizedDescription)") + throw error + } + KeeperAdapterLog.write("addRecord: \(verb) response bytes=\(data.count)") + + struct RecordAddData: Decodable { + let record_uid: String? + let uid: String? + var effectiveUid: String? { record_uid ?? uid } } - let data = try client.executeCommand(apiKey: apiKey, command: cmd) struct RecordAddResponse: Decodable { let status: String? - let data: RecordAddData? - } - struct RecordAddData: Decodable { - let record_uid: String? + let message: String? + // Commander sometimes returns `data` as an object and sometimes as a JSON string. + private let dataObject: RecordAddData? + private let dataString: String? + + enum CodingKeys: String, CodingKey { + case status, message, data + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + status = try container.decodeIfPresent(String.self, forKey: .status) + message = try container.decodeIfPresent(String.self, forKey: .message) + if let obj = try? container.decodeIfPresent(RecordAddData.self, forKey: .data) { + dataObject = obj + dataString = nil + } else if let str = try? container.decodeIfPresent(String.self, forKey: .data) { + dataString = str + if let nested = str.data(using: .utf8), + let obj = try? JSONDecoder().decode(RecordAddData.self, from: nested) { + dataObject = obj + } else { + dataObject = nil + } + } else { + dataObject = nil + dataString = nil + } + } + + var effectiveUidFromData: String? { + if let uid = dataObject?.effectiveUid?.trimmingCharacters(in: .whitespacesAndNewlines), !uid.isEmpty { + return uid + } + if let str = dataString?.trimmingCharacters(in: .whitespacesAndNewlines), !str.isEmpty { + return (try? extractRecordUID(from: str)) + } + return nil + } } + let response = try JSONDecoder().decode(RecordAddResponse.self, from: data) - guard response.status == "success", let uid = response.data?.record_uid, !uid.isEmpty else { + guard response.status == "success" else { + KeeperAdapterLog.write("addRecord: \(verb) failed status=\(response.status ?? "nil")") + throw KeeperClientError.message("Add failed") + } + let uid: String + if let fromData = response.effectiveUidFromData { + uid = try validatedRecordUID(fromData) + } else if let message = response.message?.trimmingCharacters(in: .whitespacesAndNewlines), + !message.isEmpty, + let extracted = try? extractRecordUID(from: message) { + uid = extracted + } else { + KeeperAdapterLog.write("addRecord: \(verb) returned without a valid record_uid") throw KeeperClientError.message("Add failed") } + KeeperAdapterLog.write("addRecord: \(verb) returned uid=\(uid)") return uid } diff --git a/pwmplugin/Sources/iterm2-keeper-adapter/main.swift b/pwmplugin/Sources/iterm2-keeper-adapter/main.swift index a2cb047ab1..b9e9f3f8ed 100644 --- a/pwmplugin/Sources/iterm2-keeper-adapter/main.swift +++ b/pwmplugin/Sources/iterm2-keeper-adapter/main.swift @@ -83,13 +83,13 @@ private func handleHandshake() { isSecret: false, note: "Note: Append /api/v2 in your API URL", persistInKeychain: false), - PasswordManagerProtocol.SettingsField( - key: "apiKey", - label: "API key:", - placeholder: "Enter Keeper Commander API key", - isSecret: true, - note: nil, - persistInKeychain: true), + ], + addAccountToggles: [ + PasswordManagerProtocol.AddAccountToggle( + key: "useClassicPermission", + label: "Use classic permission model", + note: "Limits sharing to basic access levels. Recommended only for compatibility with older workflows.", + defaultValue: false), ]) writeOutput(response) } catch { @@ -107,9 +107,6 @@ private func apiKey(fromHeader header: PasswordManagerProtocol.RequestHeader, if let key = decodeToken(token)?.trimmingCharacters(in: .whitespacesAndNewlines), !key.isEmpty { return key } - if let key = header.settings?["apiKey"]?.trimmingCharacters(in: .whitespacesAndNewlines), !key.isEmpty { - return key - } throw KeeperClientError.message("Invalid or missing API key") } @@ -123,7 +120,7 @@ private func handleLogin() { let baseURL = try extractServiceURL(from: request.header) let key = try apiKey(fromHeader: request.header, token: nil, masterPassword: request.masterPassword) let client = KeeperCommanderClient(baseURL: baseURL) - _ = try listAccountsRecords(apiKey: key, client: client) + try validateApiKey(apiKey: key, client: client) let token = Data(key.utf8).base64EncodedString() writeOutput(LoginResponse(token: token)) } catch { @@ -170,7 +167,9 @@ private func handleGetPassword() { } private func handleSetPassword() { + KeeperAdapterLog.write("handleSetPassword: invoked") guard let data = readStdin() else { + KeeperAdapterLog.write("handleSetPassword: no stdin input, aborting") writeError("No input provided") exit(1) } @@ -180,16 +179,21 @@ private func handleSetPassword() { let apiKey = try apiKey(fromHeader: request.header, token: request.token, masterPassword: nil) let client = KeeperCommanderClient(baseURL: baseURL) let uid = request.accountIdentifier.accountID + KeeperAdapterLog.write("handleSetPassword: uid=\(uid), newPassword.length=\(request.newPassword?.count ?? 0)") try setPassword(apiKey: apiKey, recordUid: uid, newPassword: request.newPassword, client: client) + KeeperAdapterLog.write("handleSetPassword: success uid=\(uid)") writeOutput(SetPasswordResponse()) } catch { + KeeperAdapterLog.write("handleSetPassword: ERROR \(error.localizedDescription)") writeError(error.localizedDescription) exit(1) } } private func handleAddAccount() { + KeeperAdapterLog.write("handleAddAccount: invoked") guard let data = readStdin() else { + KeeperAdapterLog.write("handleAddAccount: no stdin input, aborting") writeError("No input provided") exit(1) } @@ -198,21 +202,28 @@ private func handleAddAccount() { let baseURL = try extractServiceURL(from: request.header) let apiKey = try apiKey(fromHeader: request.header, token: request.token, masterPassword: nil) let client = KeeperCommanderClient(baseURL: baseURL) + let useClassicPermission = request.flags?["useClassicPermission"] ?? false + KeeperAdapterLog.write("handleAddAccount: useClassicPermission=\(useClassicPermission), accountName=\"\(request.accountName)\", userName=\"\(request.userName)\"") let uid = try addRecord( apiKey: apiKey, userName: request.userName, accountName: request.accountName, password: request.password, + useClassicPermission: useClassicPermission, client: client) + KeeperAdapterLog.write("handleAddAccount: success uid=\(uid)") writeOutput(AddAccountResponse(accountIdentifier: PasswordManagerProtocol.AccountIdentifier(accountID: uid))) } catch { + KeeperAdapterLog.write("handleAddAccount: ERROR \(error.localizedDescription)") writeError(error.localizedDescription) exit(1) } } private func handleDeleteAccount() { + KeeperAdapterLog.write("handleDeleteAccount: invoked") guard let data = readStdin() else { + KeeperAdapterLog.write("handleDeleteAccount: no stdin input, aborting") writeError("No input provided") exit(1) } @@ -221,9 +232,12 @@ private func handleDeleteAccount() { let baseURL = try extractServiceURL(from: request.header) let apiKey = try apiKey(fromHeader: request.header, token: request.token, masterPassword: nil) let client = KeeperCommanderClient(baseURL: baseURL) + KeeperAdapterLog.write("handleDeleteAccount: accountID=\(request.accountIdentifier.accountID)") try deleteRecord(apiKey: apiKey, recordUid: request.accountIdentifier.accountID, client: client) + KeeperAdapterLog.write("handleDeleteAccount: success accountID=\(request.accountIdentifier.accountID)") writeOutput(DeleteAccountResponse()) } catch { + KeeperAdapterLog.write("handleDeleteAccount: ERROR \(error.localizedDescription)") writeError(error.localizedDescription) exit(1) } diff --git a/pwmplugin/Tests/iterm2-keeper-adapterTests/IntegrationTests.swift b/pwmplugin/Tests/iterm2-keeper-adapterTests/IntegrationTests.swift index 381c402962..ade7264773 100644 --- a/pwmplugin/Tests/iterm2-keeper-adapterTests/IntegrationTests.swift +++ b/pwmplugin/Tests/iterm2-keeper-adapterTests/IntegrationTests.swift @@ -23,6 +23,16 @@ def _send(handler, code, obj): handler.end_headers() handler.wfile.write(payload) +def _send_html(handler, code, body): + # Returns a tunnel/proxy-style HTML error page so the adapter's + # connectivity error formatter can be exercised from tests. + payload = body.encode("utf-8") + handler.send_response(code) + handler.send_header("Content-Type", "text/html; charset=utf-8") + handler.send_header("Content-Length", str(len(payload))) + handler.end_headers() + handler.wfile.write(payload) + class H(BaseHTTPRequestHandler): def log_message(self, format, *args): return @@ -36,6 +46,11 @@ class H(BaseHTTPRequestHandler): if SCENARIO == "async_error": _send(self, 401, {"error": "bad api key"}) return + if SCENARIO == "html_error": + _send_html(self, 502, + "offline" + "

Endpoint offline

The tunnel is not running.

") + return req_id = str(len(REQUESTS) + 1) REQUESTS[req_id] = cmd _send(self, 202, {"request_id": req_id, "status": "queued"}) @@ -52,11 +67,46 @@ class H(BaseHTTPRequestHandler): if "/result/" in self.path: req_id = self.path.rsplit("/", 1)[-1] cmd = REQUESTS.get(req_id, "") - if cmd.startswith("ls "): - result = {"command": "ls", "data": {"records": [ - {"title": "1 UIDAAAAAAAAAAAAAA login Web user@example.com @ https://testbook.com"}, - {"title": "2 UID1234567890123 login Mysql hborase@keepersecurity.com @ 127.0.0.1:3366"} - ]}} + if cmd.startswith("nsf-list"): + nsf_data = [ + {"Item Type": "Folder", "Parent/Folder": "", + "Title": "NewDemoFolder", "Type": "folder", + "UID": "FOLDERUID123456789"}, + {"Item Type": "Record", "Parent/Folder": "NewDemoFolder", + "Title": "DemoNsfRecord", "Type": "login", + "UID": "UIDNSF1234567890"}, + ] + if SCENARIO == "duplicate_uid": + nsf_data.append( + {"Item Type": "Record", "Parent/Folder": "NewDemoFolder", + "Title": "DupShared", "Type": "login", + "UID": "UIDDUP12345678901"}) + result = { + "command": "nsf-list --records", + "status": "success", + "data": nsf_data, + } + _send(self, 200, {"status": "success", "result": json.dumps(result)}) + return + if cmd.startswith("list "): + list_data = [ + {"description": "user@example.com @ https://testbook.com", + "record_category": "Classic", "record_uid": "UIDAAAAAAAAAAAAAA", + "shared": True, "title": "Web", "type": "login"}, + {"description": "hborase@keepersecurity.com @ 127.0.0.1:3366", + "record_category": "Classic", "record_uid": "UID1234567890123", + "shared": True, "title": "Mysql", "type": "login"}, + ] + if SCENARIO == "duplicate_uid": + list_data.append( + {"description": "dup@example.com", + "record_category": "Classic", "record_uid": "UIDDUP12345678901", + "shared": True, "title": "DupShared", "type": "login"}) + result = { + "command": "list", + "status": "success", + "data": list_data, + } _send(self, 200, {"status": "success", "result": json.dumps(result)}) return if " --format=json" in cmd and cmd.startswith("get "): @@ -65,16 +115,24 @@ class H(BaseHTTPRequestHandler): if " --format=password" in cmd and cmd.startswith("get "): _send(self, 200, "pw-123") return - if cmd.startswith("record-update "): + if cmd.startswith("record-update ") or cmd.startswith("nsf-record-update "): if SCENARIO == "set_password_error": _send(self, 200, {"error": "password invalid"}) else: _send(self, 200, {"status": "success"}) return + if cmd.startswith("nsf-record-add "): + _send(self, 200, { + "command": "nsf-record-add", + "data": None, + "message": "Record NSFUID123456789 was added successfully.", + "status": "success", + }) + return if cmd.startswith("record-add "): _send(self, 200, {"status": "success", "data": {"record_uid": "NEWUID1234567890"}}) return - if cmd.startswith("rm -f "): + if cmd.startswith("rm -f ") or cmd.startswith("nsf-rm "): _send(self, 200, {"status": "success"}) return if cmd == "sync-down": @@ -128,6 +186,8 @@ server.serve_forever() let process = Process() process.executableURL = adapterURL process.arguments = [subcommand] + let env = ProcessInfo.processInfo.environment + process.environment = env let stdin = Pipe() let output = Pipe() @@ -165,6 +225,30 @@ server.serve_forever() XCTAssertEqual(json?["name"] as? String, "Keeper Security") } + func testHandshakeDeclaresClassicPermissionToggle() throws { + let input = #"{"iTermVersion":"3.5.0","minProtocolVersion":0,"maxProtocolVersion":0}"# + let result = try run("handshake", input: input) + XCTAssertEqual(result.status, 0, result.output) + let json = try decodeJSON(result.output) + let toggles = try XCTUnwrap(json["addAccountToggles"] as? [[String: Any]]) + XCTAssertEqual(toggles.count, 1) + XCTAssertEqual(toggles[0]["key"] as? String, "useClassicPermission") + XCTAssertEqual(toggles[0]["label"] as? String, "Use classic permission model") + XCTAssertEqual(toggles[0]["defaultValue"] as? Bool, false) + } + + func testHandshakeDeclaresServiceURLOnlyInSettings() throws { + let input = #"{"iTermVersion":"3.5.0","minProtocolVersion":0,"maxProtocolVersion":0}"# + let result = try run("handshake", input: input) + XCTAssertEqual(result.status, 0, result.output) + let json = try decodeJSON(result.output) + let fields = try XCTUnwrap(json["settingsFields"] as? [[String: Any]]) + XCTAssertEqual(fields.count, 1) + XCTAssertEqual(fields[0]["key"] as? String, "serviceURL") + XCTAssertEqual(json["masterPasswordLabel"] as? String, "API key") + XCTAssertEqual(json["requiresMasterPassword"] as? Bool, true) + } + func testHandshakeRejectsNegativeProtocol() throws { let input = #"{"iTermVersion":"3.5.0","minProtocolVersion":0,"maxProtocolVersion":-1}"# let result = try run("handshake", input: input) @@ -190,9 +274,47 @@ server.serve_forever() XCTAssertEqual(result.status, 0, result.output) let json = try decodeJSON(result.output) let accounts = try XCTUnwrap(json["accounts"] as? [[String: Any]]) - XCTAssertEqual(accounts.count, 2) + XCTAssertEqual(accounts.count, 3) + XCTAssertEqual(accounts[0]["accountName"] as? String, "Web") + XCTAssertEqual(accounts[0]["sourceLabel"] as? String, "Classic") XCTAssertEqual(accounts[0]["userName"] as? String, "user@example.com") + XCTAssertEqual((accounts[0]["identifier"] as? [String: Any])?["accountID"] as? String, + "UIDAAAAAAAAAAAAAA") + XCTAssertEqual(accounts[1]["accountName"] as? String, "Mysql") + XCTAssertEqual(accounts[1]["sourceLabel"] as? String, "Classic") XCTAssertEqual(accounts[1]["userName"] as? String, "hborase@keepersecurity.com") + XCTAssertEqual((accounts[1]["identifier"] as? [String: Any])?["accountID"] as? String, + "UID1234567890123") + XCTAssertEqual(accounts[2]["accountName"] as? String, "DemoNsfRecord") + XCTAssertEqual(accounts[2]["sourceLabel"] as? String, "Nested") + XCTAssertEqual(accounts[2]["userName"] as? String, "") + XCTAssertEqual((accounts[2]["identifier"] as? [String: Any])?["accountID"] as? String, + "UIDNSF1234567890") + } + + func testListAccountsExcludesNsfFolders() throws { + let server = try MockKeeperServer() + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())"}"# + let result = try run("list-accounts", input: input) + XCTAssertEqual(result.status, 0, result.output) + let json = try decodeJSON(result.output) + let accounts = try XCTUnwrap(json["accounts"] as? [[String: Any]]) + XCTAssertEqual(accounts.count, 3) + XCTAssertFalse(accounts.contains { ($0["accountName"] as? String)?.contains("NewDemoFolder") == true }) + } + + func testListAccountsPrefersListForDuplicateUid() throws { + let server = try MockKeeperServer(scenario: "duplicate_uid") + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())"}"# + let result = try run("list-accounts", input: input) + XCTAssertEqual(result.status, 0, result.output) + let json = try decodeJSON(result.output) + let accounts = try XCTUnwrap(json["accounts"] as? [[String: Any]]) + let dup = try XCTUnwrap(accounts.first { account in + ((account["identifier"] as? [String: Any])?["accountID"] as? String) == "UIDDUP12345678901" + }) + XCTAssertEqual(dup["accountName"] as? String, "DupShared") + XCTAssertEqual(dup["sourceLabel"] as? String, "Classic") } func testGetPasswordSuccess() throws { @@ -211,9 +333,9 @@ server.serve_forever() XCTAssertEqual(result.status, 0, result.output) } - func testAddAccountSuccess() throws { + func testAddAccountClassicPermissionUsesRecordAdd() throws { let server = try MockKeeperServer() - let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","userName":"user@example.com","accountName":"Example","password":"new-pass"}"# + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","userName":"user@example.com","accountName":"Example","password":"new-pass","flags":{"useClassicPermission":true}}"# let result = try run("add-account", input: input) XCTAssertEqual(result.status, 0, result.output) let json = try decodeJSON(result.output) @@ -221,6 +343,40 @@ server.serve_forever() XCTAssertEqual(accountIdentifier["accountID"] as? String, "NEWUID1234567890") } + func testAddAccountNestedSharedFoldersUsesNsfRecordAdd() throws { + let server = try MockKeeperServer() + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","userName":"user@example.com","accountName":"Example","password":"new-pass","flags":{"useClassicPermission":false}}"# + let result = try run("add-account", input: input) + XCTAssertEqual(result.status, 0, result.output) + let json = try decodeJSON(result.output) + let accountIdentifier = try XCTUnwrap(json["accountIdentifier"] as? [String: Any]) + XCTAssertEqual(accountIdentifier["accountID"] as? String, "NSFUID123456789") + } + + func testAddAccountDefaultsToNestedSharedFolders() throws { + let server = try MockKeeperServer() + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","userName":"user@example.com","accountName":"Example","password":"new-pass"}"# + let result = try run("add-account", input: input) + XCTAssertEqual(result.status, 0, result.output) + let json = try decodeJSON(result.output) + let accountIdentifier = try XCTUnwrap(json["accountIdentifier"] as? [String: Any]) + XCTAssertEqual(accountIdentifier["accountID"] as? String, "NSFUID123456789") + } + + func testSetPasswordOnNestedRecordUsesNsfRecordUpdate() throws { + let server = try MockKeeperServer() + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","accountIdentifier":{"accountID":"nested:UIDNSF1234567890"},"newPassword":"new-pass"}"# + let result = try run("set-password", input: input) + XCTAssertEqual(result.status, 0, result.output) + } + + func testSetPasswordOnClassicRecordUsesRecordUpdate() throws { + let server = try MockKeeperServer() + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","accountIdentifier":{"accountID":"classic:UID1234567890123"},"newPassword":"new-pass"}"# + let result = try run("set-password", input: input) + XCTAssertEqual(result.status, 0, result.output) + } + func testDeleteAccountSuccess() throws { let server = try MockKeeperServer() let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","accountIdentifier":{"accountID":"UID1234567890123"}}"# @@ -228,6 +384,20 @@ server.serve_forever() XCTAssertEqual(result.status, 0, result.output) } + func testDeleteAccountOnClassicRecordUsesRm() throws { + let server = try MockKeeperServer() + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","accountIdentifier":{"accountID":"classic:UID1234567890123"}}"# + let result = try run("delete-account", input: input) + XCTAssertEqual(result.status, 0, result.output) + } + + func testDeleteAccountOnNestedRecordUsesNsfRm() throws { + let server = try MockKeeperServer() + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","accountIdentifier":{"accountID":"nested:UIDNSF1234567890"}}"# + let result = try run("delete-account", input: input) + XCTAssertEqual(result.status, 0, result.output) + } + func testKeeperSyncDownSuccess() throws { let server = try MockKeeperServer() let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"token":"\#(token())","commandName":"sync-down"}"# @@ -253,6 +423,34 @@ server.serve_forever() XCTAssertEqual(json["error"] as? String, "bad api key") } + func testLoginHTMLErrorPageReturnsActionableMessage() throws { + let server = try MockKeeperServer(scenario: "html_error") + let input = #"{"header":\#(header(server.baseURL)),"userAccountID":null,"masterPassword":"api-key"}"# + let result = try run("login", input: input) + XCTAssertNotEqual(result.status, 0, result.output) + let json = try decodeJSON(result.output) + let err = (json["error"] as? String) ?? "" + XCTAssertTrue(err.contains("HTML error page"), "Expected HTML-detection prefix, got: \(err)") + XCTAssertTrue(err.contains("Update your Service URL and API key"), "Expected actionable advice, got: \(err)") + XCTAssertFalse(err.contains(" ()) { + standardAdd(configuration, + userName: userName, + accountName: accountName, + password: password, + flags: flags, + context: context, + completion: completion) + } + func resetErrors() { // Clear the session token to allow retry. When persistsCredentials is true, // the saved masterPassword will auto-login without prompting on the next attempt. @@ -1016,9 +1055,10 @@ extension AdapterPasswordDataSource: AdapterCapabilities { userAccountID: self.userAccountID, token: self.authToken, commandName: name) - self.runAdapterCommand(name, request: request) { (result: Result) in + self.runAdapterCommand(name, request: request) { [weak self] (result: Result) in switch result { case .success(let response): + self?.invalidateListAccountsCache() completion(response.message, nil) case .failure(let error): completion(nil, error) @@ -1032,6 +1072,9 @@ extension AdapterPasswordDataSource: AdapterCapabilities { } @objc func setSettingsValue(_ value: String, forKey key: String) { + guard handshakeInfo?.settingsFields?.contains(where: { $0.key == key }) == true else { return } storeSettingsValue(value, forKey: key) + // Non-secret settings (e.g. service URL) can change reachability; clear the session. + authToken = nil } } diff --git a/sources/PasswordManager/CommandLinePasswordDataSource.swift b/sources/PasswordManager/CommandLinePasswordDataSource.swift index de62f769e5..cc6ac23daa 100644 --- a/sources/PasswordManager/CommandLinePasswordDataSource.swift +++ b/sources/PasswordManager/CommandLinePasswordDataSource.swift @@ -20,9 +20,17 @@ class CommandLineProvidedAccount: NSObject, PasswordManagerAccount { let userName: String let hasOTP: Bool let sendOTP: Bool + let sourceLabel: String? var displayString: String { - return "\(accountName)\u{2002}—\u{2002}\(userName)" + return "\(formattedAccountName)\u{2002}—\u{2002}\(userName)" + } + + var formattedAccountName: String { + if let source = sourceLabel?.trimmingCharacters(in: .whitespacesAndNewlines), !source.isEmpty { + return "\(accountName) (\(source))" + } + return accountName } func fetchPassword(context: RecipeExecutionContext, _ completion: @escaping (String?, String?, Error?) -> ()) { @@ -56,7 +64,9 @@ class CommandLineProvidedAccount: NSObject, PasswordManagerAccount { } func matches(filter: String) -> Bool { - return accountName.containsCaseInsensitive(filter) || userName.containsCaseInsensitive(filter) + return accountName.containsCaseInsensitive(filter) + || userName.containsCaseInsensitive(filter) + || formattedAccountName.containsCaseInsensitive(filter) } init(identifier: String, @@ -64,12 +74,14 @@ class CommandLineProvidedAccount: NSObject, PasswordManagerAccount { userName: String, hasOTP: Bool, sendOTP: Bool, + sourceLabel: String? = nil, configuration: CommandLinePasswordDataSource.Configuration) { self.identifier = identifier self.accountName = accountName self.userName = userName self.hasOTP = hasOTP - self.sendOTP = sendOTP; + self.sendOTP = sendOTP + self.sourceLabel = sourceLabel self.configuration = configuration } } @@ -903,6 +915,21 @@ class CommandLinePasswordDataSource: NSObject { let accountName: String let hasOTP: Bool let sendOTP: Bool + let sourceLabel: String? + + init(identifier: AccountIdentifier, + userName: String, + accountName: String, + hasOTP: Bool, + sendOTP: Bool, + sourceLabel: String? = nil) { + self.identifier = identifier + self.userName = userName + self.accountName = accountName + self.hasOTP = hasOTP + self.sendOTP = sendOTP + self.sourceLabel = sourceLabel + } } struct SetPasswordRequest { @@ -914,6 +941,17 @@ class CommandLinePasswordDataSource: NSObject { let userName: String let accountName: String let password: String + let flags: [String: Bool] + + init(userName: String, + accountName: String, + password: String, + flags: [String: Bool] = [:]) { + self.userName = userName + self.accountName = accountName + self.password = password + self.flags = flags + } } struct Password { @@ -944,6 +982,7 @@ class CommandLinePasswordDataSource: NSObject { userName: account.userName, hasOTP: account.hasOTP, sendOTP: account.sendOTP, + sourceLabel: account.sourceLabel, configuration: configuration) } completion(accounts, nil) @@ -954,11 +993,13 @@ class CommandLinePasswordDataSource: NSObject { userName: String, accountName: String, password: String, + flags: [String: Bool] = [:], context: RecipeExecutionContext, completion: @escaping (PasswordManagerAccount?, Error?) -> ()) { let inputs = AddRequest(userName: userName, accountName: accountName, - password: password) + password: password, + flags: flags) configuration.addAccountRecipe.transformAsync(context: context, inputs: inputs) { accountIdentifier, maybeError in configuration.listAccountsRecipe.invalidateRecipe() if let error = maybeError { diff --git a/sources/PasswordManager/PasswordManagerDataSource.swift b/sources/PasswordManager/PasswordManagerDataSource.swift index bac077740f..51f608b36a 100644 --- a/sources/PasswordManager/PasswordManagerDataSource.swift +++ b/sources/PasswordManager/PasswordManagerDataSource.swift @@ -14,6 +14,8 @@ protocol PasswordManagerAccount: AnyObject { @objc var displayString: String { get } @objc var hasOTP: Bool { get } @objc var sendOTP: Bool { get } + /// Optional vault/source hint (e.g. "Classic", "Nested") for host label formatting. + @objc optional var sourceLabel: String? { get } @objc(fetchPasswordWithContext:completion:) func fetchPassword(context: RecipeExecutionContext, @@ -43,6 +45,19 @@ protocol PasswordManagerDataSource: AnyObject { password: String, context: RecipeExecutionContext, completion: @escaping (PasswordManagerAccount?, Error?) -> ()) + + /// Optional Add Account checkboxes declared by the data source. Default is nil. + @objc optional var addAccountToggleDescriptions: [[String: Any]]? { get } + + /// Flags-carrying add. Data sources without toggles should forward to the plain add and ignore flags. + @objc(addUserName:accountName:password:flags:context:completion:) + optional func add(userName: String, + accountName: String, + password: String, + flags: [String: Bool], + context: RecipeExecutionContext, + completion: @escaping (PasswordManagerAccount?, Error?) -> ()) + func resetErrors() func reload(_ completion: () -> ()) func consolidateAvailabilityChecks(_ block: () -> ()) diff --git a/sources/PasswordManager/iTermPasswordManager.xib b/sources/PasswordManager/iTermPasswordManager.xib index d2766cd5f9..ef4e71a38a 100644 --- a/sources/PasswordManager/iTermPasswordManager.xib +++ b/sources/PasswordManager/iTermPasswordManager.xib @@ -18,6 +18,10 @@ + + + + @@ -28,6 +32,8 @@ + + @@ -337,7 +343,7 @@ CA - + @@ -346,11 +352,11 @@ CA - + + + + + + + + + + + diff --git a/sources/PasswordManager/iTermPasswordManagerWindowController.m b/sources/PasswordManager/iTermPasswordManagerWindowController.m index 97b0c29d4c..6f820038e3 100644 --- a/sources/PasswordManager/iTermPasswordManagerWindowController.m +++ b/sources/PasswordManager/iTermPasswordManagerWindowController.m @@ -25,6 +25,37 @@ // Looks nice and is unlikely to be used already static NSString *const iTermPasswordManagerAccountNameUserNameSeparator = @"\u2002—\u2002"; NSString *const iTermPasswordManagerDidLoadAccounts = @"iTermPasswordManagerDidLoadAccounts"; +static const CGFloat kNewAccountPanelWidth = 330; +static const CGFloat kNewAccountPanelHeightWithoutToggle = 136; +static const CGFloat kNewAccountPanelHeightWithToggle = 196; + +static void iTermFixViewY(NSView *view, CGFloat y) { + if (view == nil) { + return; + } + view.autoresizingMask = NSViewNotSizable; + NSRect frame = view.frame; + frame.origin.y = y; + view.frame = frame; +} + +static void iTermSetNewAccountPanelContentHeight(NSPanel *panel, CGFloat height) { + NSRect frame = panel.frame; + NSRect contentRect = [panel contentRectForFrameRect:frame]; + contentRect.size.width = kNewAccountPanelWidth; + contentRect.size.height = height; + frame = [panel frameRectForContentRect:contentRect]; + [panel setFrame:frame display:NO]; + NSView *contentView = panel.contentView; + if (contentView != nil) { + NSRect cvFrame = contentView.frame; + cvFrame.size.width = kNewAccountPanelWidth; + cvFrame.size.height = height; + cvFrame.origin = NSZeroPoint; + contentView.frame = cvFrame; + [contentView layoutSubtreeIfNeeded]; + } +} typedef NS_ENUM(NSUInteger, iTermPasswordManagerReload) { iTermPasswordManagerReloadUnlimited, @@ -75,6 +106,12 @@ @implementation iTermPasswordManagerWindowController { IBOutlet NSTextField *_newAccount; IBOutlet NSButton *_newAccountOkButton; IBOutlet NSSecureTextField *_newAccountPassword; + IBOutlet NSTextField *_newAccountLabel; + IBOutlet NSTextField *_newUserNameLabel; + IBOutlet NSTextField *_newPasswordLabel; + IBOutlet NSButton *_generatePasswordButton; + IBOutlet NSButton *_addAccountToggleCheckbox; + IBOutlet NSTextField *_addAccountToggleLabel; IBOutlet NSView *_scrim; IBOutlet NSProgressIndicator *_progressIndicator; @@ -611,12 +648,95 @@ - (IBAction)add:(id)sender { _newAccountPassword.stringValue = @""; _newAccountPassword.placeholderString = nil; } + [self configureFirstAddAccountToggle]; [self.window beginSheet:_newAccountPanel completionHandler:^(NSModalResponse response){ [NSApp stopModal]; }]; + // beginSheet can reset subview frames via autoresizing; apply layout again. + [self configureFirstAddAccountToggle]; [NSApp runModalForWindow:_newAccountPanel]; } +- (NSDictionary *)firstAddAccountToggleDescription { + id ds = self.currentDataSource; + if (![(id)ds respondsToSelector:@selector(addAccountToggleDescriptions)]) { + return nil; + } + NSArray *toggles = ds.addAccountToggleDescriptions; + if (toggles.count == 0) { + return nil; + } + if (toggles.count > 1) { + DLog(@"Data source declared %@ add-account toggles; only the first is rendered.", @(toggles.count)); + } + return toggles.firstObject; +} + +- (void)configureNewAccountPanelFieldLayoutShowingToggle:(BOOL)showingToggle { + const CGFloat fieldHeight = 27; + const CGFloat labelOffset = 3; + const CGFloat topMargin = 34; + const CGFloat toggleGap = 5; + const CGFloat panelHeight = showingToggle ? kNewAccountPanelHeightWithToggle + : kNewAccountPanelHeightWithoutToggle; + + CGFloat y = panelHeight - topMargin; + iTermFixViewY(_newAccount, y); + iTermFixViewY(_newAccountLabel, y + labelOffset); + y -= fieldHeight; + + iTermFixViewY(_newUserName, y); + iTermFixViewY(_newUserNameLabel, y + labelOffset); + y -= fieldHeight; + + iTermFixViewY(_newAccountPassword, y); + iTermFixViewY(_newPasswordLabel, y + labelOffset); + iTermFixViewY(_generatePasswordButton, y - labelOffset); + + if (!showingToggle) { + return; + } + y -= fieldHeight + toggleGap; + iTermFixViewY(_addAccountToggleCheckbox, y); + y -= fieldHeight; + iTermFixViewY(_addAccountToggleLabel, y); +} + +- (void)configureFirstAddAccountToggle { + NSDictionary *toggle = [self firstAddAccountToggleDescription]; + const BOOL hidden = (toggle == nil); + _addAccountToggleCheckbox.hidden = hidden; + _addAccountToggleLabel.hidden = hidden; + const CGFloat height = hidden ? kNewAccountPanelHeightWithoutToggle + : kNewAccountPanelHeightWithToggle; + iTermSetNewAccountPanelContentHeight(_newAccountPanel, height); + [self configureNewAccountPanelFieldLayoutShowingToggle:!hidden]; + if (hidden) { + return; + } + NSString *label = toggle[@"label"]; + NSString *note = toggle[@"note"]; + NSNumber *defaultValue = toggle[@"defaultValue"]; + _addAccountToggleCheckbox.title = label.length > 0 ? label : @""; + _addAccountToggleLabel.stringValue = note.length > 0 ? note : @""; + _addAccountToggleCheckbox.state = (defaultValue.boolValue + ? NSControlStateValueOn + : NSControlStateValueOff); +} + +- (NSDictionary *)collectAddAccountFlags { + NSDictionary *toggle = [self firstAddAccountToggleDescription]; + if (toggle == nil) { + return nil; + } + NSString *key = toggle[@"key"]; + if (key.length == 0) { + return nil; + } + const BOOL on = (_addAccountToggleCheckbox.state == NSControlStateValueOn); + return @{ key: @(on) }; +} + - (IBAction)cancelNewAccount:(id)sender { _newPassword.stringValue = @""; _newUserName.stringValue = @""; @@ -639,16 +759,33 @@ - (IBAction)reallyAdd:(id)sender { } __weak __typeof(self) weakSelf = self; const NSInteger cancelCount = [self incrBusy]; - [[self currentDataSource] addUserName:_newUserName.stringValue ?: @"" - accountName:_newAccount.stringValue ?: @"" - password:_newPassword.stringValue ?: @"" - context:self.recipeExecutionContext - completion:^(id _Nullable newAccount, NSError * _Nullable error) { + NSString *userName = _newUserName.stringValue ?: @""; + NSString *accountName = _newAccount.stringValue ?: @""; + NSString *password = _newPassword.stringValue ?: @""; + void (^onComplete)(id, NSError *) = ^(id _Nullable newAccount, + NSError * _Nullable error) { [weakSelf ifCancelCountUnchanged:cancelCount perform:^{ [weakSelf didAddAccount:newAccount withError:error]; [weakSelf decrBusy]; }]; - }]; + }; + id currentDataSource = self.currentDataSource; + NSDictionary *flags = [self collectAddAccountFlags]; + SEL flagsSel = @selector(addUserName:accountName:password:flags:context:completion:); + if (flags != nil && [(id)currentDataSource respondsToSelector:flagsSel]) { + [currentDataSource addUserName:userName + accountName:accountName + password:password + flags:flags + context:self.recipeExecutionContext + completion:onComplete]; + } else { + [currentDataSource addUserName:userName + accountName:accountName + password:password + context:self.recipeExecutionContext + completion:onComplete]; + } _newPassword.stringValue = @""; _newUserName.stringValue = @""; _newAccount.stringValue = @""; @@ -1560,7 +1697,35 @@ - (NSString *)accountNameForRow:(NSInteger)rowIndex { if (rowIndex < 0 || rowIndex >= _entries.count) { return nil; } - return _entries[rowIndex].accountName; + id entry = _entries[rowIndex]; + NSString *name = entry.accountName; + NSString *source = nil; + if ([(id)entry respondsToSelector:@selector(sourceLabel)]) { + source = entry.sourceLabel; + } + if (source.length > 0) { + return [NSString stringWithFormat:@"%@ (%@)", name, source]; + } + return name; +} + +- (NSString *)accountNameByStrippingSourceSuffix:(NSString *)name + fromAccount:(id)account { + if (name.length == 0) { + return name; + } + NSString *source = nil; + if ([(id)account respondsToSelector:@selector(sourceLabel)]) { + source = account.sourceLabel; + } + if (source.length == 0) { + return name; + } + NSString *suffix = [NSString stringWithFormat:@" (%@)", source]; + if ([name hasSuffix:suffix]) { + return [name substringToIndex:name.length - suffix.length]; + } + return name; } - (NSString *)userNameForRow:(NSInteger)rowIndex { @@ -1689,7 +1854,7 @@ - (void)tableView:(NSTableView *)aTableView NSString *userName = entry.userName; NSString *accountName = entry.accountName; if (aTableColumn == _accountNameColumn) { - accountName = anObject; + accountName = [self accountNameByStrippingSourceSuffix:anObject fromAccount:entry]; } else if (aTableColumn == _userNameColumn) { userName = anObject; }