Commit bee5f10a92
Unsigned
Layout: unified · split
octosentry/Account.swift added +55
| @@ -0,0 +1,55 @@ | |||
| 1 | // | ||
| 2 | // Account.swift | ||
| 3 | // octosentry | ||
| 4 | // | ||
| 5 | // A signed-in GitHub identity. The token itself stays in the Keychain; | ||
| 6 | // `keychainAccount` is the item name it lives under. | ||
| 7 | // | ||
| 8 | // Existing single-account installs keep the item they already have — | ||
| 9 | // `KeychainTokenStore.legacyAccount` is a perfectly good name for one | ||
| 10 | // account, and rewriting it at upgrade time would risk stranding a token to | ||
| 11 | // gain nothing. | ||
| 12 | // | ||
| 13 | |||
| 14 | import Foundation | ||
| 15 | |||
| 16 | nonisolated struct Account: Codable, Equatable, Identifiable, Hashable { | ||
| 17 | /// Stable across renames, unlike the login. Zero for an account carried | ||
| 18 | /// over from a single-account install before its login was resolved. | ||
| 19 | var id: Int | ||
| 20 | var login: String | ||
| 21 | var keychainAccount: String | ||
| 22 | var hasRepoScope: Bool | ||
| 23 | |||
| 24 | var displayName: String { | ||
| 25 | login.isEmpty ? "GitHub account" : login | ||
| 26 | } | ||
| 27 | |||
| 28 | /// The account an upgrading install already has a token for. | ||
| 29 | static func legacy() -> Account { | ||
| 30 | Account( | ||
| 31 | id: 0, | ||
| 32 | login: "", | ||
| 33 | keychainAccount: KeychainTokenStore.legacyAccount, | ||
| 34 | hasRepoScope: false | ||
| 35 | ) | ||
| 36 | } | ||
| 37 | |||
| 38 | static func new(id: Int, login: String, hasRepoScope: Bool) -> Account { | ||
| 39 | Account( | ||
| 40 | id: id, | ||
| 41 | login: login, | ||
| 42 | keychainAccount: "account-\(id)", | ||
| 43 | hasRepoScope: hasRepoScope | ||
| 44 | ) | ||
| 45 | } | ||
| 46 | } | ||
| 47 | |||
| 48 | /// A repo on the watch list, and the account whose token can see it. The same | ||
| 49 | /// repo can appear under more than one account; the feed merges those and | ||
| 50 | /// attributes them. | ||
| 51 | nonisolated struct WatchedRepo: Codable, Equatable, Hashable { | ||
| 52 | var fullName: String | ||
| 53 | /// Account id, or 0 for entries carried over from a single-account install. | ||
| 54 | var accountID: Int | ||
| 55 | } | ||
octosentry/AuthStore.swift +136 −20
| @@ -2,9 +2,9 @@ | |||
| 2 | // AuthStore.swift | 2 | // AuthStore.swift |
| 3 | // octosentry | 3 | // octosentry |
| 4 | // | 4 | // |
| 5 | // Drives the device authorization flow and mirrors whether a token is | 5 | // Drives the device authorization flow and tracks which GitHub identities |
| 6 | // currently in the Keychain. Replaces the GITHUB_TOKEN env var dev | 6 | // are signed in. Each account's token lives in its own Keychain item; this |
| 7 | // shortcut (spec §13) with the real v1 auth flow (spec §6). | 7 | // holds only the account list, which is persisted alongside everything else. |
| 8 | // | 8 | // |
| 9 | // Sign-in requests the minimal security_events scope by default. | 9 | // Sign-in requests the minimal security_events scope by default. |
| 10 | // Broader "repo" scope (needed to list repos for the picker, #15) is | 10 | // Broader "repo" scope (needed to list repos for the picker, #15) is |
| @@ -19,17 +19,18 @@ import Observation | |||
| 19 | final class AuthStore { | 19 | final class AuthStore { |
| 20 | private(set) var state: AuthState | 20 | private(set) var state: AuthState |
| 21 | private(set) var errorMessage: String? | 21 | private(set) var errorMessage: String? |
| 22 | private(set) var hasRepoAccess = false | 22 | private(set) var accounts: [Account] = [] |
| 23 | 23 | ||
| 24 | private let client = GitHubDeviceAuthClient() | 24 | private let client = GitHubDeviceAuthClient() |
| 25 | private let persistenceStore = PersistenceStore() | 25 | private let persistenceStore = PersistenceStore() |
| 26 | private var authorizationTask: Task<Void, Never>? | 26 | private var authorizationTask: Task<Void, Never>? |
| 27 | 27 | ||
| 28 | init() { | 28 | init() { |
| 29 | // Before the account list loads, fall back to whether the | ||
| 30 | // single-account Keychain item exists, so an upgrading install isn't | ||
| 31 | // shown a sign-in screen it doesn't need. | ||
| 29 | state = KeychainTokenStore.load() != nil ? .signedIn : .signedOut | 32 | state = KeychainTokenStore.load() != nil ? .signedIn : .signedOut |
| 30 | Task { | 33 | Task { await loadAccounts() } |
| 31 | hasRepoAccess = await persistenceStore.load().hasRepoScope | ||
| 32 | } | ||
| 33 | } | 34 | } |
| 34 | 35 | ||
| 35 | var isSignedIn: Bool { | 36 | var isSignedIn: Bool { |
| @@ -37,10 +38,21 @@ final class AuthStore { | |||
| 37 | return false | 38 | return false |
| 38 | } | 39 | } |
| 39 | 40 | ||
| 41 | /// True when any signed-in account can list repos for the picker. | ||
| 42 | var hasRepoAccess: Bool { | ||
| 43 | accounts.contains(where: \.hasRepoScope) | ||
| 44 | } | ||
| 45 | |||
| 40 | func signIn() { | 46 | func signIn() { |
| 41 | beginAuthorization(scope: GitHubDeviceAuthClient.defaultScope) | 47 | beginAuthorization(scope: GitHubDeviceAuthClient.defaultScope) |
| 42 | } | 48 | } |
| 43 | 49 | ||
| 50 | /// Adds another identity. Same flow as signing in — GitHub decides which | ||
| 51 | /// account authorizes the code. | ||
| 52 | func addAccount() { | ||
| 53 | beginAuthorization(scope: GitHubDeviceAuthClient.defaultScope) | ||
| 54 | } | ||
| 55 | |||
| 44 | /// Re-runs device auth with broader scope so the repo picker can list | 56 | /// Re-runs device auth with broader scope so the repo picker can list |
| 45 | /// repos. Only called explicitly from the repo picker UI, never on | 57 | /// repos. Only called explicitly from the repo picker UI, never on |
| 46 | /// the default sign-in path. | 58 | /// the default sign-in path. |
| @@ -48,12 +60,87 @@ final class AuthStore { | |||
| 48 | beginAuthorization(scope: GitHubDeviceAuthClient.repoAccessScope) | 60 | beginAuthorization(scope: GitHubDeviceAuthClient.repoAccessScope) |
| 49 | } | 61 | } |
| 50 | 62 | ||
| 51 | func signOut() { | 63 | /// Signs out one account, leaving the others alone. |
| 64 | func signOut(_ account: Account) async { | ||
| 65 | KeychainTokenStore.delete(account: account.keychainAccount) | ||
| 66 | |||
| 67 | var persisted = await persistenceStore.load() | ||
| 68 | persisted.accounts.removeAll { $0.keychainAccount == account.keychainAccount } | ||
| 69 | // Its repos can no longer be fetched, so drop them too. | ||
| 70 | persisted.watchedRepos.removeAll { $0.accountID == account.id } | ||
| 71 | await persistenceStore.save(persisted) | ||
| 72 | |||
| 73 | accounts = persisted.accounts | ||
| 74 | state = accounts.isEmpty ? .signedOut : .signedIn | ||
| 75 | } | ||
| 76 | |||
| 77 | func signOutAll() async { | ||
| 78 | for account in accounts { | ||
| 79 | KeychainTokenStore.delete(account: account.keychainAccount) | ||
| 80 | } | ||
| 81 | // Also clear the pre-multi-account item, in case no account row | ||
| 82 | // referenced it. | ||
| 83 | KeychainTokenStore.delete() | ||
| 84 | |||
| 85 | var persisted = await persistenceStore.load() | ||
| 86 | persisted.accounts = [] | ||
| 87 | persisted.watchedRepos = [] | ||
| 88 | await persistenceStore.save(persisted) | ||
| 89 | |||
| 52 | authorizationTask?.cancel() | 90 | authorizationTask?.cancel() |
| 53 | authorizationTask = nil | 91 | authorizationTask = nil |
| 54 | KeychainTokenStore.delete() | 92 | accounts = [] |
| 55 | state = .signedOut | 93 | state = .signedOut |
| 56 | hasRepoAccess = false | 94 | } |
| 95 | |||
| 96 | /// Reconciles the stored account list with the Keychain, and adopts a | ||
| 97 | /// token left by a single-account install. | ||
| 98 | private func loadAccounts() async { | ||
| 99 | var persisted = await persistenceStore.load() | ||
| 100 | |||
| 101 | if persisted.accounts.isEmpty, KeychainTokenStore.load() != nil { | ||
| 102 | var legacy = Account.legacy() | ||
| 103 | legacy.hasRepoScope = persisted.hasRepoScope | ||
| 104 | persisted.accounts = [legacy] | ||
| 105 | await persistenceStore.save(persisted) | ||
| 106 | } | ||
| 107 | |||
| 108 | accounts = persisted.accounts | ||
| 109 | state = accounts.isEmpty ? .signedOut : .signedIn | ||
| 110 | |||
| 111 | await resolveMissingLogins() | ||
| 112 | } | ||
| 113 | |||
| 114 | /// An adopted legacy account has no login until we ask GitHub who it is. | ||
| 115 | private func resolveMissingLogins() async { | ||
| 116 | let unresolved = accounts.filter { $0.login.isEmpty } | ||
| 117 | guard !unresolved.isEmpty else { return } | ||
| 118 | |||
| 119 | var persisted = await persistenceStore.load() | ||
| 120 | var changed = false | ||
| 121 | |||
| 122 | for account in unresolved { | ||
| 123 | guard let token = KeychainTokenStore.load(account: account.keychainAccount), | ||
| 124 | let user = try? await GitHubSecurityAPIClient(token: token).fetchCurrentUser(), | ||
| 125 | let index = persisted.accounts.firstIndex(where: { $0.keychainAccount == account.keychainAccount }) | ||
| 126 | else { continue } | ||
| 127 | |||
| 128 | persisted.accounts[index].login = user.login | ||
| 129 | // Keep the keychain item where it is; only the identity is filled in. | ||
| 130 | if persisted.accounts[index].id == 0 { | ||
| 131 | let oldID = persisted.accounts[index].id | ||
| 132 | persisted.accounts[index].id = user.id | ||
| 133 | for repoIndex in persisted.watchedRepos.indices where persisted.watchedRepos[repoIndex].accountID == oldID { | ||
| 134 | persisted.watchedRepos[repoIndex].accountID = user.id | ||
| 135 | } | ||
| 136 | } | ||
| 137 | changed = true | ||
| 138 | } | ||
| 139 | |||
| 140 | if changed { | ||
| 141 | await persistenceStore.save(persisted) | ||
| 142 | accounts = persisted.accounts | ||
| 143 | } | ||
| 57 | } | 144 | } |
| 58 | 145 | ||
| 59 | private func beginAuthorization(scope: String) { | 146 | private func beginAuthorization(scope: String) { |
| @@ -71,22 +158,51 @@ final class AuthStore { | |||
| 71 | interval: deviceCode.interval, | 158 | interval: deviceCode.interval, |
| 72 | expiresIn: deviceCode.expiresIn | 159 | expiresIn: deviceCode.expiresIn |
| 73 | ) | 160 | ) |
| 74 | try KeychainTokenStore.save(token) | 161 | try await register(token: token, grantedRepoScope: scope.contains("repo")) |
| 75 | |||
| 76 | let grantedRepoScope = scope.contains("repo") | ||
| 77 | var persisted = await persistenceStore.load() | ||
| 78 | persisted.hasRepoScope = grantedRepoScope | ||
| 79 | await persistenceStore.save(persisted) | ||
| 80 | hasRepoAccess = grantedRepoScope | ||
| 81 | |||
| 82 | state = .signedIn | 162 | state = .signedIn |
| 83 | } catch { | 163 | } catch { |
| 84 | errorMessage = (error as? LocalizedError)?.errorDescription ?? error.localizedDescription | 164 | errorMessage = (error as? LocalizedError)?.errorDescription ?? error.localizedDescription |
| 85 | // A failed re-auth (e.g. requestRepoAccess while already | 165 | // A failed re-auth (e.g. requestRepoAccess while already |
| 86 | // signed in) shouldn't sign the user out of their existing | 166 | // signed in) shouldn't sign the user out of their existing |
| 87 | // valid token — only reflect reality from the Keychain. | 167 | // valid token — only reflect reality. |
| 88 | state = KeychainTokenStore.load() != nil ? .signedIn : .signedOut | 168 | state = accounts.isEmpty && KeychainTokenStore.load() == nil ? .signedOut : .signedIn |
| 89 | } | 169 | } |
| 90 | } | 170 | } |
| 91 | } | 171 | } |
| 172 | |||
| 173 | /// Stores a freshly authorized token under its own Keychain item and | ||
| 174 | /// records the account. Re-authorizing an account already present updates | ||
| 175 | /// it in place rather than adding a duplicate. | ||
| 176 | private func register(token: String, grantedRepoScope: Bool) async throws { | ||
| 177 | let user = try await GitHubSecurityAPIClient(token: token).fetchCurrentUser() | ||
| 178 | |||
| 179 | var persisted = await persistenceStore.load() | ||
| 180 | |||
| 181 | if let index = persisted.accounts.firstIndex(where: { $0.id == user.id }) { | ||
| 182 | persisted.accounts[index].login = user.login | ||
| 183 | persisted.accounts[index].hasRepoScope = grantedRepoScope | ||
| 184 | try KeychainTokenStore.save(token, account: persisted.accounts[index].keychainAccount) | ||
| 185 | } else if let index = persisted.accounts.firstIndex(where: { $0.id == 0 }) { | ||
| 186 | // The adopted single-account entry, now identified. | ||
| 187 | let keychainAccount = persisted.accounts[index].keychainAccount | ||
| 188 | persisted.accounts[index] = Account( | ||
| 189 | id: user.id, | ||
| 190 | login: user.login, | ||
| 191 | keychainAccount: keychainAccount, | ||
| 192 | hasRepoScope: grantedRepoScope | ||
| 193 | ) | ||
| 194 | for repoIndex in persisted.watchedRepos.indices where persisted.watchedRepos[repoIndex].accountID == 0 { | ||
| 195 | persisted.watchedRepos[repoIndex].accountID = user.id | ||
| 196 | } | ||
| 197 | try KeychainTokenStore.save(token, account: keychainAccount) | ||
| 198 | } else { | ||
| 199 | let account = Account.new(id: user.id, login: user.login, hasRepoScope: grantedRepoScope) | ||
| 200 | try KeychainTokenStore.save(token, account: account.keychainAccount) | ||
| 201 | persisted.accounts.append(account) | ||
| 202 | } | ||
| 203 | |||
| 204 | persisted.hasRepoScope = persisted.accounts.contains(where: \.hasRepoScope) | ||
| 205 | await persistenceStore.save(persisted) | ||
| 206 | accounts = persisted.accounts | ||
| 207 | } | ||
| 92 | } | 208 | } |
octosentry/GitHubAPIModels.swift +5
| @@ -87,6 +87,11 @@ nonisolated struct SecretScanningAlertDTO: Decodable { | |||
| 87 | } | 87 | } |
| 88 | } | 88 | } |
| 89 | 89 | ||
| 90 | nonisolated struct GitHubUserDTO: Decodable { | ||
| 91 | let id: Int | ||
| 92 | let login: String | ||
| 93 | } | ||
| 94 | |||
| 90 | nonisolated struct GitHubRepoDTO: Decodable { | 95 | nonisolated struct GitHubRepoDTO: Decodable { |
| 91 | let fullName: String | 96 | let fullName: String |
| 92 | 97 | ||
octosentry/GitHubSecurityAPIClient.swift +9
| @@ -103,6 +103,15 @@ actor GitHubSecurityAPIClient { | |||
| 103 | return dtos.map(\.fullName) | 103 | return dtos.map(\.fullName) |
| 104 | } | 104 | } |
| 105 | 105 | ||
| 106 | /// Identifies whose token this is, so accounts can be told apart and | ||
| 107 | /// alerts attributed. Needs no scope beyond a valid user token. | ||
| 108 | func fetchCurrentUser() async throws -> GitHubUserDTO { | ||
| 109 | var components = URLComponents(url: baseURL, resolvingAgainstBaseURL: false)! | ||
| 110 | components.path = "/user" | ||
| 111 | let (data, _) = try await fetchData(url: components.url!) | ||
| 112 | return try decode(data) | ||
| 113 | } | ||
| 114 | |||
| 106 | private func alertsURL(owner: String, repo: String, path: String) -> URL { | 115 | private func alertsURL(owner: String, repo: String, path: String) -> URL { |
| 107 | var components = URLComponents(url: baseURL, resolvingAgainstBaseURL: false)! | 116 | var components = URLComponents(url: baseURL, resolvingAgainstBaseURL: false)! |
| 108 | components.path = "/repos/\(owner)/\(repo)/\(path)" | 117 | components.path = "/repos/\(owner)/\(repo)/\(path)" |
octosentry/KeychainTokenStore.swift +7 −4
| @@ -14,9 +14,12 @@ import Security | |||
| 14 | 14 | ||
| 15 | nonisolated enum KeychainTokenStore { | 15 | nonisolated enum KeychainTokenStore { |
| 16 | private static let service = "net.cleberg.octosentry.github-token" | 16 | private static let service = "net.cleberg.octosentry.github-token" |
| 17 | private static let account = "github-oauth-token" | ||
| 18 | 17 | ||
| 19 | static func save(_ token: String) throws { | 18 | /// The item name used before octosentry supported more than one account. |
| 19 | /// Still the name for that account's token — upgrading does not move it. | ||
| 20 | static let legacyAccount = "github-oauth-token" | ||
| 21 | |||
| 22 | static func save(_ token: String, account: String = legacyAccount) throws { | ||
| 20 | let query: [String: Any] = [ | 23 | let query: [String: Any] = [ |
| 21 | kSecClass as String: kSecClassGenericPassword, | 24 | kSecClass as String: kSecClassGenericPassword, |
| 22 | kSecAttrService as String: service, | 25 | kSecAttrService as String: service, |
| @@ -35,7 +38,7 @@ nonisolated enum KeychainTokenStore { | |||
| 35 | } | 38 | } |
| 36 | } | 39 | } |
| 37 | 40 | ||
| 38 | static func load() -> String? { | 41 | static func load(account: String = legacyAccount) -> String? { |
| 39 | let query: [String: Any] = [ | 42 | let query: [String: Any] = [ |
| 40 | kSecClass as String: kSecClassGenericPassword, | 43 | kSecClass as String: kSecClassGenericPassword, |
| 41 | kSecAttrService as String: service, | 44 | kSecAttrService as String: service, |
| @@ -50,7 +53,7 @@ nonisolated enum KeychainTokenStore { | |||
| 50 | return String(data: data, encoding: .utf8) | 53 | return String(data: data, encoding: .utf8) |
| 51 | } | 54 | } |
| 52 | 55 | ||
| 53 | static func delete() { | 56 | static func delete(account: String = legacyAccount) { |
| 54 | let query: [String: Any] = [ | 57 | let query: [String: Any] = [ |
| 55 | kSecClass as String: kSecClassGenericPassword, | 58 | kSecClass as String: kSecClassGenericPassword, |
| 56 | kSecAttrService as String: service, | 59 | kSecAttrService as String: service, |
octosentry/PersistedState.swift +18 −6
| @@ -2,7 +2,8 @@ | |||
| 2 | // PersistedState.swift | 2 | // PersistedState.swift |
| 3 | // octosentry | 3 | // octosentry |
| 4 | // | 4 | // |
| 5 | // Everything the app remembers across launches: the repo watch list, | 5 | // Everything the app remembers across launches: the signed-in accounts and |
| 6 | // the repo watch list, | ||
| 6 | // local-only seen-state per event, last-fetch timestamp per repo, the | 7 | // local-only seen-state per event, last-fetch timestamp per repo, the |
| 7 | // minimum severity filter, the feed sort order, local triage state, the | 8 | // minimum severity filter, the feed sort order, local triage state, the |
| 8 | // per-repo alert IDs the notifier has already accounted for, and whether the current token | 9 | // per-repo alert IDs the notifier has already accounted for, and whether the current token |
| @@ -15,7 +16,8 @@ | |||
| 15 | import Foundation | 16 | import Foundation |
| 16 | 17 | ||
| 17 | nonisolated struct PersistedState: Codable { | 18 | nonisolated struct PersistedState: Codable { |
| 18 | var watchedRepos: [String] | 19 | var accounts: [Account] |
| 20 | var watchedRepos: [WatchedRepo] | ||
| 19 | var seenEventIDs: Set<String> | 21 | var seenEventIDs: Set<String> |
| 20 | var lastFetchByRepo: [String: Date] | 22 | var lastFetchByRepo: [String: Date] |
| 21 | var minimumSeverity: SecurityEventSeverity | 23 | var minimumSeverity: SecurityEventSeverity |
| @@ -38,11 +40,12 @@ nonisolated struct PersistedState: Codable { | |||
| 38 | 40 | ||
| 39 | enum CodingKeys: String, CodingKey { | 41 | enum CodingKeys: String, CodingKey { |
| 40 | case watchedRepos, seenEventIDs, lastFetchByRepo, minimumSeverity, hasRepoScope, sortOrder | 42 | case watchedRepos, seenEventIDs, lastFetchByRepo, minimumSeverity, hasRepoScope, sortOrder |
| 41 | case notifiedEventIDsByRepo, triage, history | 43 | case notifiedEventIDsByRepo, triage, history, accounts |
| 42 | } | 44 | } |
| 43 | 45 | ||
| 44 | init( | 46 | init( |
| 45 | watchedRepos: [String], | 47 | accounts: [Account] = [], |
| 48 | watchedRepos: [WatchedRepo], | ||
| 46 | seenEventIDs: Set<String>, | 49 | seenEventIDs: Set<String>, |
| 47 | lastFetchByRepo: [String: Date], | 50 | lastFetchByRepo: [String: Date], |
| 48 | minimumSeverity: SecurityEventSeverity, | 51 | minimumSeverity: SecurityEventSeverity, |
| @@ -52,6 +55,7 @@ nonisolated struct PersistedState: Codable { | |||
| 52 | triage: AlertTriage = AlertTriage(), | 55 | triage: AlertTriage = AlertTriage(), |
| 53 | history: AlertHistory = AlertHistory() | 56 | history: AlertHistory = AlertHistory() |
| 54 | ) { | 57 | ) { |
| 58 | self.accounts = accounts | ||
| 55 | self.watchedRepos = watchedRepos | 59 | self.watchedRepos = watchedRepos |
| 56 | self.seenEventIDs = seenEventIDs | 60 | self.seenEventIDs = seenEventIDs |
| 57 | self.lastFetchByRepo = lastFetchByRepo | 61 | self.lastFetchByRepo = lastFetchByRepo |
| @@ -67,7 +71,15 @@ nonisolated struct PersistedState: Codable { | |||
| 67 | // and sortOrder existed still load instead of falling back to .placeholder. | 71 | // and sortOrder existed still load instead of falling back to .placeholder. |
| 68 | init(from decoder: Decoder) throws { | 72 | init(from decoder: Decoder) throws { |
| 69 | let container = try decoder.container(keyedBy: CodingKeys.self) | 73 | let container = try decoder.container(keyedBy: CodingKeys.self) |
| 70 | watchedRepos = try container.decode([String].self, forKey: .watchedRepos) | 74 | accounts = try container.decodeIfPresent([Account].self, forKey: .accounts) ?? [] |
| 75 | // Before multi-account, watchedRepos was a plain [String] belonging to | ||
| 76 | // the one signed-in account. Decode either shape. | ||
| 77 | if let repos = try? container.decode([WatchedRepo].self, forKey: .watchedRepos) { | ||
| 78 | watchedRepos = repos | ||
| 79 | } else { | ||
| 80 | let names = try container.decode([String].self, forKey: .watchedRepos) | ||
| 81 | watchedRepos = names.map { WatchedRepo(fullName: $0, accountID: 0) } | ||
| 82 | } | ||
| 71 | seenEventIDs = try container.decode(Set<String>.self, forKey: .seenEventIDs) | 83 | seenEventIDs = try container.decode(Set<String>.self, forKey: .seenEventIDs) |
| 72 | lastFetchByRepo = try container.decode([String: Date].self, forKey: .lastFetchByRepo) | 84 | lastFetchByRepo = try container.decode([String: Date].self, forKey: .lastFetchByRepo) |
| 73 | minimumSeverity = try container.decode(SecurityEventSeverity.self, forKey: .minimumSeverity) | 85 | minimumSeverity = try container.decode(SecurityEventSeverity.self, forKey: .minimumSeverity) |
| @@ -82,7 +94,7 @@ nonisolated struct PersistedState: Codable { | |||
| 82 | } | 94 | } |
| 83 | 95 | ||
| 84 | static let placeholder = PersistedState( | 96 | static let placeholder = PersistedState( |
| 85 | watchedRepos: ["ccleberg/cleberg.net"], | 97 | watchedRepos: [WatchedRepo(fullName: "ccleberg/cleberg.net", accountID: 0)], |
| 86 | seenEventIDs: [], | 98 | seenEventIDs: [], |
| 87 | lastFetchByRepo: [:], | 99 | lastFetchByRepo: [:], |
| 88 | minimumSeverity: .low | 100 | minimumSeverity: .low |
octosentry/SecurityEvent.swift +3
| @@ -16,4 +16,7 @@ nonisolated struct SecurityEvent: Identifiable, Codable, Sendable { | |||
| 16 | let createdAt: Date | 16 | let createdAt: Date |
| 17 | let updatedAt: Date | 17 | let updatedAt: Date |
| 18 | var seenLocally: Bool | 18 | var seenLocally: Bool |
| 19 | /// Logins of the accounts whose token can see this alert. More than one | ||
| 20 | /// when the same repo is watched under several identities. | ||
| 21 | var accountLogins: [String] = [] | ||
| 19 | } | 22 | } |
octosentry/SecurityEventListView.swift +91 −52
| @@ -300,6 +300,9 @@ struct SecurityEventListView: View { | |||
| 300 | ForEach(store.events) { event in | 300 | ForEach(store.events) { event in |
| 301 | SecurityEventRow( | 301 | SecurityEventRow( |
| 302 | event: event, | 302 | event: event, |
| 303 | attribution: store.showsAttribution(for: event.repoFullName) | ||
| 304 | ? event.accountLogins.sorted().joined(separator: ", ") | ||
| 305 | : nil, | ||
| 303 | isHidden: store.triage.isHidden(event.id, now: .now), | 306 | isHidden: store.triage.isHidden(event.id, now: .now), |
| 304 | snoozedUntil: store.triage.snoozedUntil(event.id, now: .now), | 307 | snoozedUntil: store.triage.snoozedUntil(event.id, now: .now), |
| 305 | onMarkSeen: { Task { await store.markSeen(event.id) } }, | 308 | onMarkSeen: { Task { await store.markSeen(event.id) } }, |
| @@ -321,28 +324,76 @@ private struct RepoManagerView: View { | |||
| 321 | var store: SecurityEventStore | 324 | var store: SecurityEventStore |
| 322 | var authStore: AuthStore | 325 | var authStore: AuthStore |
| 323 | @State private var newRepoText = "" | 326 | @State private var newRepoText = "" |
| 324 | @State private var isBrowsingRepos = false | 327 | @State private var addingToAccountID: Int? |
| 328 | @State private var browsingAccount: Account? | ||
| 325 | @State private var availableRepos: [String] = [] | 329 | @State private var availableRepos: [String] = [] |
| 326 | @State private var isLoadingRepos = false | 330 | @State private var isLoadingRepos = false |
| 327 | @State private var browseErrorMessage: String? | 331 | @State private var browseErrorMessage: String? |
| 328 | 332 | ||
| 329 | var body: some View { | 333 | var body: some View { |
| 330 | VStack(alignment: .leading, spacing: 10) { | 334 | ScrollView { |
| 331 | Text("Watched Repositories") | 335 | VStack(alignment: .leading, spacing: 12) { |
| 332 | .font(.subheadline.weight(.semibold)) | 336 | ForEach(authStore.accounts) { account in |
| 337 | accountSection(account) | ||
| 338 | Divider() | ||
| 339 | } | ||
| 340 | |||
| 341 | Button { | ||
| 342 | authStore.addAccount() | ||
| 343 | } label: { | ||
| 344 | Label("Add another account", systemImage: "person.badge.plus") | ||
| 345 | .font(.caption) | ||
| 346 | } | ||
| 347 | .buttonStyle(.plain) | ||
| 348 | .foregroundStyle(Color.accentColor) | ||
| 349 | |||
| 350 | if let errorMessage = store.watchListErrorMessage { | ||
| 351 | Text(errorMessage) | ||
| 352 | .font(.caption2) | ||
| 353 | .foregroundStyle(.red) | ||
| 354 | } | ||
| 333 | 355 | ||
| 334 | if store.watchedRepos.isEmpty { | 356 | Divider() |
| 335 | Text("No repos watched yet.") | 357 | |
| 336 | .font(.callout) | 358 | Button("Sign Out of All Accounts") { |
| 359 | Task { await authStore.signOutAll() } | ||
| 360 | } | ||
| 361 | .buttonStyle(.plain) | ||
| 362 | .foregroundStyle(.red) | ||
| 363 | } | ||
| 364 | .padding(12) | ||
| 365 | .frame(maxWidth: .infinity, alignment: .leading) | ||
| 366 | } | ||
| 367 | } | ||
| 368 | |||
| 369 | @ViewBuilder | ||
| 370 | private func accountSection(_ account: Account) -> some View { | ||
| 371 | VStack(alignment: .leading, spacing: 8) { | ||
| 372 | HStack { | ||
| 373 | Text(account.displayName) | ||
| 374 | .font(.subheadline.weight(.semibold)) | ||
| 375 | Spacer() | ||
| 376 | Button("Sign out") { | ||
| 377 | Task { await authStore.signOut(account) } | ||
| 378 | } | ||
| 379 | .buttonStyle(.plain) | ||
| 380 | .font(.caption) | ||
| 381 | .foregroundStyle(.red) | ||
| 382 | } | ||
| 383 | |||
| 384 | let repos = store.watchedRepos.filter { $0.accountID == account.id } | ||
| 385 | if repos.isEmpty { | ||
| 386 | Text("No repos watched under this account.") | ||
| 387 | .font(.caption) | ||
| 337 | .foregroundStyle(.secondary) | 388 | .foregroundStyle(.secondary) |
| 338 | } else { | 389 | } else { |
| 339 | ForEach(store.watchedRepos, id: \.self) { repo in | 390 | ForEach(repos, id: \.self) { watched in |
| 340 | HStack { | 391 | HStack { |
| 341 | Text(repo) | 392 | Text(watched.fullName) |
| 342 | .font(.callout) | 393 | .font(.callout) |
| 343 | Spacer() | 394 | Spacer() |
| 344 | Button { | 395 | Button { |
| 345 | Task { await store.removeRepo(repo) } | 396 | Task { await store.removeRepo(watched) } |
| 346 | } label: { | 397 | } label: { |
| 347 | Image(systemName: "minus.circle.fill") | 398 | Image(systemName: "minus.circle.fill") |
| 348 | .foregroundStyle(.red) | 399 | .foregroundStyle(.red) |
| @@ -352,57 +403,41 @@ private struct RepoManagerView: View { | |||
| 352 | } | 403 | } |
| 353 | } | 404 | } |
| 354 | 405 | ||
| 355 | Divider() | 406 | if browsingAccount == account { |
| 356 | 407 | browsingContent(account) | |
| 357 | if isBrowsingRepos { | ||
| 358 | browsingContent | ||
| 359 | } else { | 408 | } else { |
| 360 | HStack { | 409 | HStack { |
| 361 | TextField("owner/repo", text: $newRepoText) | 410 | TextField("owner/repo", text: Binding( |
| 362 | .textFieldStyle(.roundedBorder) | 411 | get: { addingToAccountID == account.id ? newRepoText : "" }, |
| 363 | .onSubmit(addRepo) | 412 | set: { newRepoText = $0; addingToAccountID = account.id } |
| 364 | 413 | )) | |
| 365 | Button("Add", action: addRepo) | 414 | .textFieldStyle(.roundedBorder) |
| 366 | .disabled(newRepoText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) | 415 | .onSubmit { addRepo(to: account) } |
| 416 | |||
| 417 | Button("Add") { addRepo(to: account) } | ||
| 418 | .disabled(addingToAccountID != account.id | ||
| 419 | || newRepoText.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) | ||
| 367 | } | 420 | } |
| 368 | 421 | ||
| 369 | Button(action: startBrowsing) { | 422 | Button { startBrowsing(account) } label: { |
| 370 | Label("Browse your repos", systemImage: "list.bullet") | 423 | Label("Browse repos", systemImage: "list.bullet") |
| 371 | .font(.caption) | 424 | .font(.caption) |
| 372 | } | 425 | } |
| 373 | .buttonStyle(.plain) | 426 | .buttonStyle(.plain) |
| 374 | .foregroundStyle(Color.accentColor) | 427 | .foregroundStyle(Color.accentColor) |
| 375 | } | 428 | } |
| 376 | |||
| 377 | if let errorMessage = store.watchListErrorMessage { | ||
| 378 | Text(errorMessage) | ||
| 379 | .font(.caption2) | ||
| 380 | .foregroundStyle(.red) | ||
| 381 | } | ||
| 382 | |||
| 383 | Spacer() | ||
| 384 | |||
| 385 | Divider() | ||
| 386 | |||
| 387 | Button("Sign Out") { | ||
| 388 | authStore.signOut() | ||
| 389 | } | ||
| 390 | .buttonStyle(.plain) | ||
| 391 | .foregroundStyle(.red) | ||
| 392 | } | 429 | } |
| 393 | .padding(12) | ||
| 394 | .frame(maxWidth: .infinity, alignment: .leading) | ||
| 395 | } | 430 | } |
| 396 | 431 | ||
| 397 | @ViewBuilder | 432 | @ViewBuilder |
| 398 | private var browsingContent: some View { | 433 | private func browsingContent(_ account: Account) -> some View { |
| 399 | VStack(alignment: .leading, spacing: 6) { | 434 | VStack(alignment: .leading, spacing: 6) { |
| 400 | HStack { | 435 | HStack { |
| 401 | Text("Your Repositories") | 436 | Text("Repositories") |
| 402 | .font(.caption.weight(.semibold)) | 437 | .font(.caption.weight(.semibold)) |
| 403 | Spacer() | 438 | Spacer() |
| 404 | Button { | 439 | Button { |
| 405 | isBrowsingRepos = false | 440 | browsingAccount = nil |
| 406 | } label: { | 441 | } label: { |
| 407 | Image(systemName: "xmark.circle") | 442 | Image(systemName: "xmark.circle") |
| 408 | } | 443 | } |
| @@ -418,7 +453,10 @@ private struct RepoManagerView: View { | |||
| 418 | .font(.caption2) | 453 | .font(.caption2) |
| 419 | .foregroundStyle(.red) | 454 | .foregroundStyle(.red) |
| 420 | } else { | 455 | } else { |
| 421 | let selectableRepos = availableRepos.filter { !store.watchedRepos.contains($0) } | 456 | let watched = Set( |
| 457 | store.watchedRepos.filter { $0.accountID == account.id }.map(\.fullName) | ||
| 458 | ) | ||
| 459 | let selectableRepos = availableRepos.filter { !watched.contains($0) } | ||
| 422 | if selectableRepos.isEmpty { | 460 | if selectableRepos.isEmpty { |
| 423 | Text("All visible repos are already watched.") | 461 | Text("All visible repos are already watched.") |
| 424 | .font(.caption2) | 462 | .font(.caption2) |
| @@ -428,8 +466,8 @@ private struct RepoManagerView: View { | |||
| 428 | LazyVStack(alignment: .leading, spacing: 4) { | 466 | LazyVStack(alignment: .leading, spacing: 4) { |
| 429 | ForEach(selectableRepos, id: \.self) { repo in | 467 | ForEach(selectableRepos, id: \.self) { repo in |
| 430 | Button { | 468 | Button { |
| 431 | Task { await store.addRepo(repo) } | 469 | Task { await store.addRepo(repo, accountID: account.id) } |
| 432 | isBrowsingRepos = false | 470 | browsingAccount = nil |
| 433 | } label: { | 471 | } label: { |
| 434 | Text(repo) | 472 | Text(repo) |
| 435 | .font(.callout) | 473 | .font(.callout) |
| @@ -445,17 +483,17 @@ private struct RepoManagerView: View { | |||
| 445 | } | 483 | } |
| 446 | } | 484 | } |
| 447 | 485 | ||
| 448 | private func startBrowsing() { | 486 | private func startBrowsing(_ account: Account) { |
| 449 | guard authStore.hasRepoAccess else { | 487 | guard account.hasRepoScope else { |
| 450 | authStore.requestRepoAccess() | 488 | authStore.requestRepoAccess() |
| 451 | return | 489 | return |
| 452 | } | 490 | } |
| 453 | isBrowsingRepos = true | 491 | browsingAccount = account |
| 454 | isLoadingRepos = true | 492 | isLoadingRepos = true |
| 455 | browseErrorMessage = nil | 493 | browseErrorMessage = nil |
| 456 | Task { | 494 | Task { |
| 457 | do { | 495 | do { |
| 458 | availableRepos = try await store.fetchAccessibleRepos() | 496 | availableRepos = try await store.fetchAccessibleRepos(for: account) |
| 459 | } catch { | 497 | } catch { |
| 460 | browseErrorMessage = (error as? LocalizedError)?.errorDescription ?? error.localizedDescription | 498 | browseErrorMessage = (error as? LocalizedError)?.errorDescription ?? error.localizedDescription |
| 461 | } | 499 | } |
| @@ -463,10 +501,11 @@ private struct RepoManagerView: View { | |||
| 463 | } | 501 | } |
| 464 | } | 502 | } |
| 465 | 503 | ||
| 466 | private func addRepo() { | 504 | private func addRepo(to account: Account) { |
| 467 | let text = newRepoText | 505 | let text = newRepoText |
| 468 | newRepoText = "" | 506 | newRepoText = "" |
| 469 | Task { await store.addRepo(text) } | 507 | addingToAccountID = nil |
| 508 | Task { await store.addRepo(text, accountID: account.id) } | ||
| 470 | } | 509 | } |
| 471 | } | 510 | } |
| 472 | 511 | ||
octosentry/SecurityEventRow.swift +12
| @@ -8,6 +8,9 @@ import SwiftUI | |||
| 8 | 8 | ||
| 9 | struct SecurityEventRow: View { | 9 | struct SecurityEventRow: View { |
| 10 | let event: SecurityEvent | 10 | let event: SecurityEvent |
| 11 | /// Which account(s) this alert came through. Set only when the same repo | ||
| 12 | /// is watched under more than one identity — otherwise it's noise. | ||
| 13 | var attribution: String? | ||
| 11 | var isHidden = false | 14 | var isHidden = false |
| 12 | var snoozedUntil: Date? | 15 | var snoozedUntil: Date? |
| 13 | var onMarkSeen: () -> Void | 16 | var onMarkSeen: () -> Void |
| @@ -45,6 +48,15 @@ struct SecurityEventRow: View { | |||
| 45 | .font(.caption) | 48 | .font(.caption) |
| 46 | .foregroundStyle(.secondary) | 49 | .foregroundStyle(.secondary) |
| 47 | 50 | ||
| 51 | if let attribution { | ||
| 52 | Text(attribution) | ||
| 53 | .font(.caption2) | ||
| 54 | .foregroundStyle(.secondary) | ||
| 55 | .padding(.horizontal, 5) | ||
| 56 | .padding(.vertical, 1) | ||
| 57 | .background(.secondary.opacity(0.15), in: Capsule()) | ||
| 58 | } | ||
| 59 | |||
| 48 | Spacer() | 60 | Spacer() |
| 49 | 61 | ||
| 50 | if let hiddenLabel { | 62 | if let hiddenLabel { |
octosentry/SecurityEventStore.swift +83 −21
| @@ -26,7 +26,7 @@ final class SecurityEventStore { | |||
| 26 | private(set) var minimumSeverity: SecurityEventSeverity = .low | 26 | private(set) var minimumSeverity: SecurityEventSeverity = .low |
| 27 | private(set) var sortOrder: AlertSortOrder = .severity | 27 | private(set) var sortOrder: AlertSortOrder = .severity |
| 28 | private(set) var totalFetchedCount = 0 | 28 | private(set) var totalFetchedCount = 0 |
| 29 | private(set) var watchedRepos: [String] = [] | 29 | private(set) var watchedRepos: [WatchedRepo] = [] |
| 30 | private(set) var watchListErrorMessage: String? | 30 | private(set) var watchListErrorMessage: String? |
| 31 | 31 | ||
| 32 | /// Per-session narrowing, not persisted. Setting it re-derives `events`. | 32 | /// Per-session narrowing, not persisted. Setting it re-derives `events`. |
| @@ -48,6 +48,14 @@ final class SecurityEventStore { | |||
| 48 | Set(rawEvents.map(\.repoFullName)).sorted { $0.localizedCaseInsensitiveCompare($1) == .orderedAscending } | 48 | Set(rawEvents.map(\.repoFullName)).sorted { $0.localizedCaseInsensitiveCompare($1) == .orderedAscending } |
| 49 | } | 49 | } |
| 50 | 50 | ||
| 51 | /// Repo names watched under more than one account — the feed shows | ||
| 52 | /// attribution only for these, since it's noise everywhere else. | ||
| 53 | private var reposWatchedBySeveralAccounts: Set<String> = [] | ||
| 54 | |||
| 55 | func showsAttribution(for repoFullName: String) -> Bool { | ||
| 56 | reposWatchedBySeveralAccounts.contains(repoFullName) | ||
| 57 | } | ||
| 58 | |||
| 51 | /// Alerts held back by the source/repo filter, as opposed to the | 59 | /// Alerts held back by the source/repo filter, as opposed to the |
| 52 | /// severity floor, so the empty state can say which one is hiding them. | 60 | /// severity floor, so the empty state can say which one is hiding them. |
| 53 | var filteredOutCount: Int { | 61 | var filteredOutCount: Int { |
| @@ -76,14 +84,13 @@ final class SecurityEventStore { | |||
| 76 | triage = state.triage | 84 | triage = state.triage |
| 77 | history = state.history | 85 | history = state.history |
| 78 | 86 | ||
| 79 | guard let token = KeychainTokenStore.load() else { | 87 | let accountsByID = Dictionary(uniqueKeysWithValues: state.accounts.map { ($0.id, $0) }) |
| 88 | guard !accountsByID.isEmpty else { | ||
| 80 | errorMessages = [stateLoadFailure, GitHubAPIError.missingToken.errorDescription ?? "Not signed in."] | 89 | errorMessages = [stateLoadFailure, GitHubAPIError.missingToken.errorDescription ?? "Not signed in."] |
| 81 | .compactMap { $0 } | 90 | .compactMap { $0 } |
| 82 | return | 91 | return |
| 83 | } | 92 | } |
| 84 | 93 | ||
| 85 | let client = GitHubSecurityAPIClient(token: token) | ||
| 86 | |||
| 87 | var fetchedEvents: [SecurityEvent] = [] | 94 | var fetchedEvents: [SecurityEvent] = [] |
| 88 | var errors: [String] = [stateLoadFailure].compactMap { $0 } | 95 | var errors: [String] = [stateLoadFailure].compactMap { $0 } |
| 89 | var notices: [String] = [] | 96 | var notices: [String] = [] |
| @@ -92,19 +99,30 @@ final class SecurityEventStore { | |||
| 92 | // alerts look new on the next poll. | 99 | // alerts look new on the next poll. |
| 93 | var fetchedEventsByRepo: [String: [SecurityEvent]] = [:] | 100 | var fetchedEventsByRepo: [String: [SecurityEvent]] = [:] |
| 94 | 101 | ||
| 95 | for repoFullName in state.watchedRepos { | 102 | for watched in state.watchedRepos { |
| 103 | let repoFullName = watched.fullName | ||
| 96 | let parts = repoFullName.split(separator: "/", maxSplits: 1) | 104 | let parts = repoFullName.split(separator: "/", maxSplits: 1) |
| 97 | guard parts.count == 2 else { continue } | 105 | guard parts.count == 2 else { continue } |
| 98 | let owner = String(parts[0]) | 106 | let owner = String(parts[0]) |
| 99 | let repo = String(parts[1]) | 107 | let repo = String(parts[1]) |
| 100 | 108 | ||
| 101 | async let dependabot = fetchSource(label: "\(repoFullName) · Dependabot") { | 109 | guard let account = accountsByID[watched.accountID], |
| 110 | let token = KeychainTokenStore.load(account: account.keychainAccount) else { | ||
| 111 | errors.append("\(repoFullName): no signed-in account can reach this repo.") | ||
| 112 | continue | ||
| 113 | } | ||
| 114 | let client = GitHubSecurityAPIClient(token: token) | ||
| 115 | let label = accountsByID.count > 1 | ||
| 116 | ? "\(repoFullName) (\(account.displayName))" | ||
| 117 | : repoFullName | ||
| 118 | |||
| 119 | async let dependabot = fetchSource(label: "\(label) · Dependabot") { | ||
| 102 | try await client.fetchDependabotAlerts(owner: owner, repo: repo) | 120 | try await client.fetchDependabotAlerts(owner: owner, repo: repo) |
| 103 | } | 121 | } |
| 104 | async let codeScanning = fetchSource(label: "\(repoFullName) · Code scanning") { | 122 | async let codeScanning = fetchSource(label: "\(label) · Code scanning") { |
| 105 | try await client.fetchCodeScanningAlerts(owner: owner, repo: repo) | 123 | try await client.fetchCodeScanningAlerts(owner: owner, repo: repo) |
| 106 | } | 124 | } |
| 107 | async let secretScanning = fetchSource(label: "\(repoFullName) · Secret scanning") { | 125 | async let secretScanning = fetchSource(label: "\(label) · Secret scanning") { |
| 108 | try await client.fetchSecretScanningAlerts(owner: owner, repo: repo) | 126 | try await client.fetchSecretScanningAlerts(owner: owner, repo: repo) |
| 109 | } | 127 | } |
| 110 | 128 | ||
| @@ -114,7 +132,11 @@ final class SecurityEventStore { | |||
| 114 | for outcome in outcomes { | 132 | for outcome in outcomes { |
| 115 | switch outcome { | 133 | switch outcome { |
| 116 | case .events(let sourceEvents): | 134 | case .events(let sourceEvents): |
| 117 | repoEvents += sourceEvents | 135 | repoEvents += sourceEvents.map { event in |
| 136 | var event = event | ||
| 137 | event.accountLogins = [account.displayName] | ||
| 138 | return event | ||
| 139 | } | ||
| 118 | repoSucceeded = true | 140 | repoSucceeded = true |
| 119 | case .unavailable(let label): | 141 | case .unavailable(let label): |
| 120 | notices.append("\(label) alerts aren't available for this repo (disabled, or token lacks that permission).") | 142 | notices.append("\(label) alerts aren't available for this repo (disabled, or token lacks that permission).") |
| @@ -125,10 +147,18 @@ final class SecurityEventStore { | |||
| 125 | fetchedEvents += repoEvents | 147 | fetchedEvents += repoEvents |
| 126 | if repoSucceeded { | 148 | if repoSucceeded { |
| 127 | state.lastFetchByRepo[repoFullName] = Date() | 149 | state.lastFetchByRepo[repoFullName] = Date() |
| 128 | fetchedEventsByRepo[repoFullName] = repoEvents | 150 | fetchedEventsByRepo[repoFullName, default: []] += repoEvents |
| 129 | } | 151 | } |
| 130 | } | 152 | } |
| 131 | 153 | ||
| 154 | // The same alert reached through two identities is one alert; merge | ||
| 155 | // the attributions rather than showing it twice. | ||
| 156 | fetchedEvents = Self.merged(fetchedEvents) | ||
| 157 | for (repoFullName, events) in fetchedEventsByRepo { | ||
| 158 | fetchedEventsByRepo[repoFullName] = Self.merged(events) | ||
| 159 | } | ||
| 160 | reposWatchedBySeveralAccounts = Self.reposWatchedBySeveralAccounts(in: state.watchedRepos) | ||
| 161 | |||
| 132 | rawEvents = fetchedEvents.map { event in | 162 | rawEvents = fetchedEvents.map { event in |
| 133 | var event = event | 163 | var event = event |
| 134 | event.seenLocally = state.seenEventIDs.contains(event.id) | 164 | event.seenLocally = state.seenEventIDs.contains(event.id) |
| @@ -142,7 +172,7 @@ final class SecurityEventStore { | |||
| 142 | 172 | ||
| 143 | // Only prune against a complete picture: if a repo failed this round | 173 | // Only prune against a complete picture: if a repo failed this round |
| 144 | // its alerts are missing, and pruning would forget they were hidden. | 174 | // its alerts are missing, and pruning would forget they were hidden. |
| 145 | if fetchedEventsByRepo.count == state.watchedRepos.count { | 175 | if fetchedEventsByRepo.count == Set(state.watchedRepos.map(\.fullName)).count { |
| 146 | let now = Date() | 176 | let now = Date() |
| 147 | state.triage = state.triage.pruned( | 177 | state.triage = state.triage.pruned( |
| 148 | presentEventIDs: Set(fetchedEvents.map(\.id)), | 178 | presentEventIDs: Set(fetchedEvents.map(\.id)), |
| @@ -164,7 +194,7 @@ final class SecurityEventStore { | |||
| 164 | ) | 194 | ) |
| 165 | state.notifiedEventIDsByRepo = AlertDiff.updatedBaseline( | 195 | state.notifiedEventIDsByRepo = AlertDiff.updatedBaseline( |
| 166 | from: fetchedEventsByRepo, | 196 | from: fetchedEventsByRepo, |
| 167 | watchedRepos: state.watchedRepos, | 197 | watchedRepos: state.watchedRepos.map(\.fullName), |
| 168 | previous: state.notifiedEventIDsByRepo | 198 | previous: state.notifiedEventIDsByRepo |
| 169 | ) | 199 | ) |
| 170 | await persistenceStore.save(state) | 200 | await persistenceStore.save(state) |
| @@ -190,7 +220,7 @@ final class SecurityEventStore { | |||
| 190 | await persistenceStore.save(state) | 220 | await persistenceStore.save(state) |
| 191 | } | 221 | } |
| 192 | 222 | ||
| 193 | func addRepo(_ input: String) async { | 223 | func addRepo(_ input: String, accountID: Int) async { |
| 194 | watchListErrorMessage = nil | 224 | watchListErrorMessage = nil |
| 195 | let trimmed = input.trimmingCharacters(in: .whitespacesAndNewlines) | 225 | let trimmed = input.trimmingCharacters(in: .whitespacesAndNewlines) |
| 196 | let parts = trimmed.split(separator: "/", omittingEmptySubsequences: true) | 226 | let parts = trimmed.split(separator: "/", omittingEmptySubsequences: true) |
| @@ -205,14 +235,15 @@ final class SecurityEventStore { | |||
| 205 | 235 | ||
| 206 | var state = await persistenceStore.load() | 236 | var state = await persistenceStore.load() |
| 207 | // GitHub owner/repo names are case-insensitive, so treat entries | 237 | // GitHub owner/repo names are case-insensitive, so treat entries |
| 208 | // that differ only in case as the same watched repo. | 238 | // that differ only in case as the same watched repo. The same repo |
| 239 | // under a different account is a separate entry on purpose. | ||
| 209 | guard !state.watchedRepos.contains(where: { | 240 | guard !state.watchedRepos.contains(where: { |
| 210 | $0.caseInsensitiveCompare(repoFullName) == .orderedSame | 241 | $0.accountID == accountID && $0.fullName.caseInsensitiveCompare(repoFullName) == .orderedSame |
| 211 | }) else { | 242 | }) else { |
| 212 | watchListErrorMessage = "\(repoFullName) is already watched." | 243 | watchListErrorMessage = "\(repoFullName) is already watched." |
| 213 | return | 244 | return |
| 214 | } | 245 | } |
| 215 | state.watchedRepos.append(repoFullName) | 246 | state.watchedRepos.append(WatchedRepo(fullName: repoFullName, accountID: accountID)) |
| 216 | await persistenceStore.save(state) | 247 | await persistenceStore.save(state) |
| 217 | watchedRepos = state.watchedRepos | 248 | watchedRepos = state.watchedRepos |
| 218 | 249 | ||
| @@ -222,8 +253,8 @@ final class SecurityEventStore { | |||
| 222 | /// Lists repos the current token can see, for the repo picker (#15). | 253 | /// Lists repos the current token can see, for the repo picker (#15). |
| 223 | /// Requires broader repo-access scope — throws if the token only has | 254 | /// Requires broader repo-access scope — throws if the token only has |
| 224 | /// the default security_events scope. | 255 | /// the default security_events scope. |
| 225 | func fetchAccessibleRepos() async throws -> [String] { | 256 | func fetchAccessibleRepos(for account: Account) async throws -> [String] { |
| 226 | guard let token = KeychainTokenStore.load() else { | 257 | guard let token = KeychainTokenStore.load(account: account.keychainAccount) else { |
| 227 | throw GitHubAPIError.missingToken | 258 | throw GitHubAPIError.missingToken |
| 228 | } | 259 | } |
| 229 | return try await GitHubSecurityAPIClient(token: token).fetchAccessibleRepos() | 260 | return try await GitHubSecurityAPIClient(token: token).fetchAccessibleRepos() |
| @@ -265,10 +296,12 @@ final class SecurityEventStore { | |||
| 265 | applyFilters() | 296 | applyFilters() |
| 266 | } | 297 | } |
| 267 | 298 | ||
| 268 | func removeRepo(_ repoFullName: String) async { | 299 | func removeRepo(_ watched: WatchedRepo) async { |
| 269 | var state = await persistenceStore.load() | 300 | var state = await persistenceStore.load() |
| 270 | state.watchedRepos.removeAll { $0 == repoFullName } | 301 | state.watchedRepos.removeAll { $0 == watched } |
| 271 | state.lastFetchByRepo.removeValue(forKey: repoFullName) | 302 | if !state.watchedRepos.contains(where: { $0.fullName == watched.fullName }) { |
| 303 | state.lastFetchByRepo.removeValue(forKey: watched.fullName) | ||
| 304 | } | ||
| 272 | await persistenceStore.save(state) | 305 | await persistenceStore.save(state) |
| 273 | watchedRepos = state.watchedRepos | 306 | watchedRepos = state.watchedRepos |
| 274 | 307 | ||
| @@ -298,6 +331,35 @@ final class SecurityEventStore { | |||
| 298 | events = sortOrder.sorted(filter.apply(to: admitted)) | 331 | events = sortOrder.sorted(filter.apply(to: admitted)) |
| 299 | } | 332 | } |
| 300 | 333 | ||
| 334 | /// Collapses alerts that arrived through more than one account, keeping | ||
| 335 | /// one row and unioning the attributions. Input order is preserved. | ||
| 336 | nonisolated static func merged(_ events: [SecurityEvent]) -> [SecurityEvent] { | ||
| 337 | var order: [String] = [] | ||
| 338 | var byID: [String: SecurityEvent] = [:] | ||
| 339 | |||
| 340 | for event in events { | ||
| 341 | if var existing = byID[event.id] { | ||
| 342 | for login in event.accountLogins where !existing.accountLogins.contains(login) { | ||
| 343 | existing.accountLogins.append(login) | ||
| 344 | } | ||
| 345 | byID[event.id] = existing | ||
| 346 | } else { | ||
| 347 | order.append(event.id) | ||
| 348 | byID[event.id] = event | ||
| 349 | } | ||
| 350 | } | ||
| 351 | |||
| 352 | return order.compactMap { byID[$0] } | ||
| 353 | } | ||
| 354 | |||
| 355 | nonisolated static func reposWatchedBySeveralAccounts(in watched: [WatchedRepo]) -> Set<String> { | ||
| 356 | var accountsByRepo: [String: Set<Int>] = [:] | ||
| 357 | for repo in watched { | ||
| 358 | accountsByRepo[repo.fullName, default: []].insert(repo.accountID) | ||
| 359 | } | ||
| 360 | return Set(accountsByRepo.filter { $0.value.count > 1 }.keys) | ||
| 361 | } | ||
| 362 | |||
| 301 | private enum SourceOutcome { | 363 | private enum SourceOutcome { |
| 302 | case events([SecurityEvent]) | 364 | case events([SecurityEvent]) |
| 303 | case unavailable(label: String) | 365 | case unavailable(label: String) |
octosentryTests/AccountTests.swift added +131
| @@ -0,0 +1,131 @@ | |||
| 1 | // | ||
| 2 | // AccountTests.swift | ||
| 3 | // octosentryTests | ||
| 4 | // | ||
| 5 | |||
| 6 | import Foundation | ||
| 7 | import Testing | ||
| 8 | @testable import octosentry | ||
| 9 | |||
| 10 | struct AccountTests { | ||
| 11 | |||
| 12 | // An install that already has a token must keep using the Keychain item | ||
| 13 | // it's in — rewriting it at upgrade time risks stranding the token. | ||
| 14 | @Test func theLegacyAccountPointsAtTheExistingKeychainItem() { | ||
| 15 | let account = Account.legacy() | ||
| 16 | |||
| 17 | #expect(account.keychainAccount == KeychainTokenStore.legacyAccount) | ||
| 18 | #expect(account.id == 0) | ||
| 19 | #expect(account.login.isEmpty) | ||
| 20 | } | ||
| 21 | |||
| 22 | @Test func newAccountsGetTheirOwnKeychainItem() { | ||
| 23 | let first = Account.new(id: 1, login: "octocat", hasRepoScope: false) | ||
| 24 | let second = Account.new(id: 2, login: "hubot", hasRepoScope: true) | ||
| 25 | |||
| 26 | #expect(first.keychainAccount != second.keychainAccount) | ||
| 27 | #expect(first.keychainAccount != KeychainTokenStore.legacyAccount) | ||
| 28 | #expect(second.hasRepoScope) | ||
| 29 | } | ||
| 30 | |||
| 31 | @Test func displayNameFallsBackBeforeTheLoginIsKnown() { | ||
| 32 | #expect(Account.legacy().displayName == "GitHub account") | ||
| 33 | #expect(Account.new(id: 1, login: "octocat", hasRepoScope: false).displayName == "octocat") | ||
| 34 | } | ||
| 35 | |||
| 36 | @Test func roundTripsThroughCodable() throws { | ||
| 37 | let account = Account.new(id: 7, login: "octocat", hasRepoScope: true) | ||
| 38 | let decoded = try JSONDecoder().decode(Account.self, from: try JSONEncoder().encode(account)) | ||
| 39 | |||
| 40 | #expect(decoded == account) | ||
| 41 | } | ||
| 42 | |||
| 43 | @Test func watchedRepoRoundTripsThroughCodable() throws { | ||
| 44 | let repo = WatchedRepo(fullName: "octocat/hello-world", accountID: 7) | ||
| 45 | let decoded = try JSONDecoder().decode(WatchedRepo.self, from: try JSONEncoder().encode(repo)) | ||
| 46 | |||
| 47 | #expect(decoded == repo) | ||
| 48 | } | ||
| 49 | } | ||
| 50 | |||
| 51 | struct MultiAccountFeedTests { | ||
| 52 | |||
| 53 | private func event(_ id: String, repo: String = "octocat/hello-world", logins: [String]) -> SecurityEvent { | ||
| 54 | var event = TestEvents.event(id: id, repo: repo) | ||
| 55 | event.accountLogins = logins | ||
| 56 | return event | ||
| 57 | } | ||
| 58 | |||
| 59 | // MARK: - Merging | ||
| 60 | |||
| 61 | // The same alert reached through two identities is one alert. | ||
| 62 | @Test func duplicateAlertsCollapseAndUnionTheirAttributions() { | ||
| 63 | let merged = SecurityEventStore.merged([ | ||
| 64 | event("a", logins: ["octocat"]), | ||
| 65 | event("a", logins: ["hubot"]), | ||
| 66 | ]) | ||
| 67 | |||
| 68 | #expect(merged.count == 1) | ||
| 69 | #expect(merged[0].accountLogins.sorted() == ["hubot", "octocat"]) | ||
| 70 | } | ||
| 71 | |||
| 72 | @Test func mergingLeavesDistinctAlertsAlone() { | ||
| 73 | let merged = SecurityEventStore.merged([ | ||
| 74 | event("a", logins: ["octocat"]), | ||
| 75 | event("b", logins: ["octocat"]), | ||
| 76 | ]) | ||
| 77 | |||
| 78 | #expect(merged.map(\.id) == ["a", "b"]) | ||
| 79 | } | ||
| 80 | |||
| 81 | @Test func mergingPreservesInputOrder() { | ||
| 82 | let merged = SecurityEventStore.merged([ | ||
| 83 | event("b", logins: ["octocat"]), | ||
| 84 | event("a", logins: ["hubot"]), | ||
| 85 | event("b", logins: ["hubot"]), | ||
| 86 | ]) | ||
| 87 | |||
| 88 | #expect(merged.map(\.id) == ["b", "a"]) | ||
| 89 | } | ||
| 90 | |||
| 91 | @Test func mergingDoesNotRepeatTheSameLogin() { | ||
| 92 | let merged = SecurityEventStore.merged([ | ||
| 93 | event("a", logins: ["octocat"]), | ||
| 94 | event("a", logins: ["octocat"]), | ||
| 95 | ]) | ||
| 96 | |||
| 97 | #expect(merged[0].accountLogins == ["octocat"]) | ||
| 98 | } | ||
| 99 | |||
| 100 | @Test func mergingAnEmptyFeedIsEmpty() { | ||
| 101 | #expect(SecurityEventStore.merged([]).isEmpty) | ||
| 102 | } | ||
| 103 | |||
| 104 | // MARK: - Attribution | ||
| 105 | |||
| 106 | // Attribution is noise unless a repo is genuinely reachable two ways. | ||
| 107 | @Test func onlyReposWatchedUnderSeveralAccountsAreAttributed() { | ||
| 108 | let watched = [ | ||
| 109 | WatchedRepo(fullName: "octocat/shared", accountID: 1), | ||
| 110 | WatchedRepo(fullName: "octocat/shared", accountID: 2), | ||
| 111 | WatchedRepo(fullName: "octocat/solo", accountID: 1), | ||
| 112 | ] | ||
| 113 | |||
| 114 | let attributed = SecurityEventStore.reposWatchedBySeveralAccounts(in: watched) | ||
| 115 | |||
| 116 | #expect(attributed == ["octocat/shared"]) | ||
| 117 | } | ||
| 118 | |||
| 119 | @Test func aRepoListedTwiceUnderOneAccountIsNotAttributed() { | ||
| 120 | let watched = [ | ||
| 121 | WatchedRepo(fullName: "octocat/solo", accountID: 1), | ||
| 122 | WatchedRepo(fullName: "octocat/solo", accountID: 1), | ||
| 123 | ] | ||
| 124 | |||
| 125 | #expect(SecurityEventStore.reposWatchedBySeveralAccounts(in: watched).isEmpty) | ||
| 126 | } | ||
| 127 | |||
| 128 | @Test func anEmptyWatchListAttributesNothing() { | ||
| 129 | #expect(SecurityEventStore.reposWatchedBySeveralAccounts(in: []).isEmpty) | ||
| 130 | } | ||
| 131 | } | ||
octosentryTests/PersistedStateTests.swift +10 −3
| @@ -27,7 +27,11 @@ struct PersistedStateTests { | |||
| 27 | @Test func roundTripsEveryField() throws { | 27 | @Test func roundTripsEveryField() throws { |
| 28 | let fetchedAt = Date(timeIntervalSince1970: 1_785_000_000) | 28 | let fetchedAt = Date(timeIntervalSince1970: 1_785_000_000) |
| 29 | let original = PersistedState( | 29 | let original = PersistedState( |
| 30 | watchedRepos: ["octocat/hello-world", "octocat/spoon-knife"], | 30 | accounts: [Account.new(id: 42, login: "octocat", hasRepoScope: true)], |
| 31 | watchedRepos: [ | ||
| 32 | WatchedRepo(fullName: "octocat/hello-world", accountID: 42), | ||
| 33 | WatchedRepo(fullName: "octocat/spoon-knife", accountID: 42), | ||
| 34 | ], | ||
| 31 | seenEventIDs: ["dependabot-octocat/hello-world-1", "codeScanning-octocat/spoon-knife-7"], | 35 | seenEventIDs: ["dependabot-octocat/hello-world-1", "codeScanning-octocat/spoon-knife-7"], |
| 32 | lastFetchByRepo: ["octocat/hello-world": fetchedAt], | 36 | lastFetchByRepo: ["octocat/hello-world": fetchedAt], |
| 33 | minimumSeverity: .high, | 37 | minimumSeverity: .high, |
| @@ -48,6 +52,7 @@ struct PersistedStateTests { | |||
| 48 | ) | 52 | ) |
| 49 | 53 | ||
| 50 | #expect(decoded.watchedRepos == original.watchedRepos) | 54 | #expect(decoded.watchedRepos == original.watchedRepos) |
| 55 | #expect(decoded.accounts == original.accounts) | ||
| 51 | #expect(decoded.seenEventIDs == original.seenEventIDs) | 56 | #expect(decoded.seenEventIDs == original.seenEventIDs) |
| 52 | #expect(decoded.lastFetchByRepo == original.lastFetchByRepo) | 57 | #expect(decoded.lastFetchByRepo == original.lastFetchByRepo) |
| 53 | #expect(decoded.minimumSeverity == original.minimumSeverity) | 58 | #expect(decoded.minimumSeverity == original.minimumSeverity) |
| @@ -65,7 +70,7 @@ struct PersistedStateTests { | |||
| 65 | 70 | ||
| 66 | #expect(Set(object.keys) == [ | 71 | #expect(Set(object.keys) == [ |
| 67 | "watchedRepos", "seenEventIDs", "lastFetchByRepo", "minimumSeverity", "hasRepoScope", "sortOrder", | 72 | "watchedRepos", "seenEventIDs", "lastFetchByRepo", "minimumSeverity", "hasRepoScope", "sortOrder", |
| 68 | "triage", "history", | 73 | "triage", "history", "accounts", |
| 69 | ]) | 74 | ]) |
| 70 | // notifiedEventIDsByRepo is optional and nil on the placeholder, so it | 75 | // notifiedEventIDsByRepo is optional and nil on the placeholder, so it |
| 71 | // encodes to nothing rather than a null. | 76 | // encodes to nothing rather than a null. |
| @@ -85,7 +90,9 @@ struct PersistedStateTests { | |||
| 85 | 90 | ||
| 86 | let state = try Self.decoder.decode(PersistedState.self, from: Data(legacy.utf8)) | 91 | let state = try Self.decoder.decode(PersistedState.self, from: Data(legacy.utf8)) |
| 87 | 92 | ||
| 88 | #expect(state.watchedRepos == ["octocat/hello-world"]) | 93 | // The pre-multi-account shape was a plain [String]. |
| 94 | #expect(state.watchedRepos == [WatchedRepo(fullName: "octocat/hello-world", accountID: 0)]) | ||
| 95 | #expect(state.accounts.isEmpty) | ||
| 89 | #expect(state.seenEventIDs == ["dependabot-octocat/hello-world-1"]) | 96 | #expect(state.seenEventIDs == ["dependabot-octocat/hello-world-1"]) |
| 90 | #expect(state.minimumSeverity == .medium) | 97 | #expect(state.minimumSeverity == .medium) |
| 91 | #expect(state.hasRepoScope == false) | 98 | #expect(state.hasRepoScope == false) |
octosentryTests/PersistenceStoreTests.swift +1 −1
| @@ -34,7 +34,7 @@ struct PersistenceStoreTests { | |||
| 34 | defer { try? FileManager.default.removeItem(at: directory) } | 34 | defer { try? FileManager.default.removeItem(at: directory) } |
| 35 | 35 | ||
| 36 | let saved = PersistedState( | 36 | let saved = PersistedState( |
| 37 | watchedRepos: ["octocat/hello-world"], | 37 | watchedRepos: [WatchedRepo(fullName: "octocat/hello-world", accountID: 42)], |
| 38 | seenEventIDs: ["dependabot-octocat/hello-world-1"], | 38 | seenEventIDs: ["dependabot-octocat/hello-world-1"], |
| 39 | lastFetchByRepo: ["octocat/hello-world": Date(timeIntervalSince1970: 1_785_000_000)], | 39 | lastFetchByRepo: ["octocat/hello-world": Date(timeIntervalSince1970: 1_785_000_000)], |
| 40 | minimumSeverity: .high, | 40 | minimumSeverity: .high, |