fix: guard that prevents edit of accounts to add the same account

This commit is contained in:
Stefan Hausotte 2026-06-02 18:20:12 +02:00
parent 89a0cd0bb2
commit 8648f40c07
2 changed files with 121 additions and 6 deletions

View file

@ -263,15 +263,24 @@ struct InstanceFormView: View {
case .add:
try await performLogin()
// Resolve the account identity after login: token auth learns the
// real username from the server, so it isn't known before this point.
let normalizedURL = ForgejoClient.normalizeServerURL(serverURL)
let resolvedUsername =
authMode == .token ? (authService.currentUser?.login ?? username) : username
// Two accounts sharing a server+username collide on the same sourceKey,
// which used to crash merged overviews. Reject the duplicate up front.
if rejectDuplicateAccount(serverURL: normalizedURL, username: resolvedUsername) {
return
}
if isDefault {
for inst in instances where inst.isDefault {
inst.isDefault = false
}
}
let normalizedURL = ForgejoClient.normalizeServerURL(serverURL)
let resolvedUsername =
authMode == .token ? (authService.currentUser?.login ?? username) : username
let instance = ForgejoInstance(
serverURL: normalizedURL,
username: resolvedUsername,
@ -289,16 +298,30 @@ struct InstanceFormView: View {
authService.currentInstance = instance
case let .edit(instance):
// Resolve the would-be identity before mutating the instance: token
// auth may change the username, and credentials auth may too.
var resolvedUsername = instance.username
if authMode == .token, !apiToken.isEmpty {
try await performLogin()
instance.username = authService.currentUser?.login ?? username
resolvedUsername = authService.currentUser?.login ?? username
} else if authMode == .credentials, !password.isEmpty {
try await performLogin()
instance.username = username
resolvedUsername = username
}
let normalizedURL = ForgejoClient.normalizeServerURL(serverURL)
// Editing must not produce a server+username that already belongs to
// another account, since that collides on the same sourceKey.
if rejectDuplicateAccount(
serverURL: normalizedURL, username: resolvedUsername, excluding: instance,
) {
return
}
instance.username = resolvedUsername
instance.name = name
instance.serverURL = ForgejoClient.normalizeServerURL(serverURL)
instance.serverURL = normalizedURL
instance.allowSelfSignedCertificates = allowSelfSignedCertificates
instance.useTokenAuth = authMode == .token
@ -328,6 +351,36 @@ struct InstanceFormView: View {
isLoading = false
}
}
/// Sets the duplicate-account error and returns true if another stored account
/// already uses this server+username. `excluding` skips the instance being edited.
private func rejectDuplicateAccount(
serverURL normalizedURL: String,
username: String,
excluding: ForgejoInstance? = nil,
) -> Bool {
guard Self.accountAlreadyExists(
serverURL: normalizedURL, username: username, in: instances, excluding: excluding,
) else { return false }
errorMessage = "An account for \"\(username)\" on \(normalizedURL) already exists."
showError = true
isLoading = false
return true
}
/// True if `instances` already contains an account with this server+username,
/// ignoring `excluding` (the instance being edited). Accounts that share a
/// server+username collide on the same sourceKey, which crashes merged overviews.
static func accountAlreadyExists(
serverURL normalizedURL: String,
username: String,
in instances: [ForgejoInstance],
excluding: ForgejoInstance? = nil,
) -> Bool {
instances.contains { inst in
inst !== excluding && inst.serverURL == normalizedURL && inst.username == username
}
}
}
#if DEBUG

View file

@ -0,0 +1,62 @@
import Foundation
import Testing
@testable import Forji
@MainActor
struct InstanceFormViewTests {
private func makeInstance(
server: String = "https://a.com",
username: String = "user",
name: String = "Account",
) -> ForgejoInstance {
ForgejoInstance(serverURL: server, username: username, name: name)
}
// MARK: - accountAlreadyExists
@Test func detectsDuplicateServerAndUsername() {
let existing = [makeInstance(server: "https://a.com", username: "user", name: "Work")]
#expect(InstanceFormView.accountAlreadyExists(
serverURL: "https://a.com", username: "user", in: existing,
))
}
@Test func differentServerIsNotDuplicate() {
let existing = [makeInstance(server: "https://a.com", username: "user")]
#expect(!InstanceFormView.accountAlreadyExists(
serverURL: "https://b.com", username: "user", in: existing,
))
}
@Test func differentUsernameIsNotDuplicate() {
let existing = [makeInstance(server: "https://a.com", username: "user")]
#expect(!InstanceFormView.accountAlreadyExists(
serverURL: "https://a.com", username: "other", in: existing,
))
}
@Test func emptyListIsNeverDuplicate() {
#expect(!InstanceFormView.accountAlreadyExists(
serverURL: "https://a.com", username: "user", in: [],
))
}
// MARK: - excluding (edit path)
@Test func editingInstanceWithoutChangingIdentityIsAllowed() {
// Editing only the name on an existing account must not flag itself as a duplicate.
let instance = makeInstance(server: "https://a.com", username: "user", name: "Old")
#expect(!InstanceFormView.accountAlreadyExists(
serverURL: "https://a.com", username: "user", in: [instance], excluding: instance,
))
}
@Test func editingIntoAnotherAccountsIdentityIsRejected() {
// Two distinct accounts; editing the second to match the first must be caught.
let first = makeInstance(server: "https://a.com", username: "user", name: "First")
let second = makeInstance(server: "https://b.com", username: "user", name: "Second")
#expect(InstanceFormView.accountAlreadyExists(
serverURL: "https://a.com", username: "user", in: [first, second], excluding: second,
))
}
}