From 19628af09cf5085500d1e3c5038375ade56ba6cb Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 16 Sep 2026 12:40:55 -0700 Subject: [PATCH] fix(identities): send unlink provider as a query param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings -> Connected accounts -> Disconnect always failed with 400 {"error":"provider is required"}. `unlinkIdentity` sent the provider in a camelCase JSON body via `deleteCamel`, but DELETE /api/user/identities reads it from the query string (`searchParams.get('provider')`) and never parses a body, so the value was always null. Switch to the bodyless `delete(_:)` with `?provider=`. The value is percent-encoded with `.urlQueryAllowed` minus `+&=?#/`: that set permits the sub-delimiters, and the backend reads params through `URLSearchParams`, which decodes a literal `+` as a space. Drop `providerId` — the route never reads it and deletes by `{userId, provider}`, removing every identity stored under that provider value. That is recorded on the method, not worked around here. Tests cover the query parameter, an empty body, the verb and path, the mastodon:instance provider form, delimiter encoding, and the 401/403/404 paths. The four new assertions fail against the previous body-based call. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017bss5MgZa7Jvj2m9zdaUd1 --- InterlinedList/Services/APIClient.swift | 24 +++++-- .../Services/APIClientTransport.swift | 5 +- .../Views/LinkedIdentitiesView.swift | 2 +- .../APIClientIdentitiesTests.swift | 66 ++++++++++++++++--- 4 files changed, 81 insertions(+), 16 deletions(-) diff --git a/InterlinedList/Services/APIClient.swift b/InterlinedList/Services/APIClient.swift index 589c422..1fb82b6 100644 --- a/InterlinedList/Services/APIClient.swift +++ b/InterlinedList/Services/APIClient.swift @@ -169,10 +169,26 @@ final class APIClient { return response.identities ?? [] } - func unlinkIdentity(provider: String, providerId: String) async throws { - struct Body: Encodable { let provider: String; let providerId: String } - try await deleteCamel("/api/user/identities", body: Body(provider: provider, providerId: providerId)) - } + /// `DELETE /api/user/identities?provider=…` — the route reads `provider` from + /// the **query string** and never parses a body, so a JSON payload here answers + /// `400 "provider is required"`. It also deletes by `{userId, provider}` alone, + /// which removes *every* identity stored under that provider value (an account + /// with two rows for the same provider loses both); the identity's own id is + /// not read at all. + func unlinkIdentity(provider: String) async throws { + let encoded = provider.addingPercentEncoding(withAllowedCharacters: Self.queryValueAllowed) ?? provider + try await delete("/api/user/identities?provider=\(encoded)") + } + + /// `.urlQueryAllowed` permits the sub-delimiters `+ & = ? # /` — legal + /// *somewhere* in a query string, but inside a single value they end it or + /// split it into another parameter. `+` matters most: the backend reads params + /// through `URLSearchParams`, which decodes a literal `+` as a space. + private static let queryValueAllowed: CharacterSet = { + var allowed = CharacterSet.urlQueryAllowed + allowed.remove(charactersIn: "+&=?#/") + return allowed + }() func verifyIdentity(provider: String, providerId: String) async throws { struct Body: Encodable { let provider: String; let providerId: String } diff --git a/InterlinedList/Services/APIClientTransport.swift b/InterlinedList/Services/APIClientTransport.swift index 43fb7ee..02e8678 100644 --- a/InterlinedList/Services/APIClientTransport.swift +++ b/InterlinedList/Services/APIClientTransport.swift @@ -135,8 +135,9 @@ extension APIClient { return try decoder.decode(T.self, from: data) } - /// DELETE with a camelCase JSON body — a few routes identify their target in - /// the body rather than the path (identity unlink, push unregister). + /// DELETE with a camelCase JSON body — for routes that identify their target in + /// the body rather than the path (push unregister). Check the route first: a + /// body sent to a route that reads the query string is silently ignored. func deleteCamel(_ path: String, body: B) async throws { var request = try jsonRequest(path, method: "DELETE") request.httpBody = try camelCaseEncoder.encode(body) diff --git a/InterlinedList/Views/LinkedIdentitiesView.swift b/InterlinedList/Views/LinkedIdentitiesView.swift index 074500a..a869c63 100644 --- a/InterlinedList/Views/LinkedIdentitiesView.swift +++ b/InterlinedList/Views/LinkedIdentitiesView.swift @@ -272,7 +272,7 @@ struct LinkedIdentitiesView: View { errorMessage = nil pendingUnlink = nil do { - try await APIClient.shared.unlinkIdentity(provider: identity.provider, providerId: identity.id) + try await APIClient.shared.unlinkIdentity(provider: identity.provider) await load() } catch APIError.status(401) { authState.handleUnauthorized() diff --git a/InterlinedListTests/APIClientTests/APIClientIdentitiesTests.swift b/InterlinedListTests/APIClientTests/APIClientIdentitiesTests.swift index 80c7a77..01b22bf 100644 --- a/InterlinedListTests/APIClientTests/APIClientIdentitiesTests.swift +++ b/InterlinedListTests/APIClientTests/APIClientIdentitiesTests.swift @@ -65,33 +65,81 @@ final class APIClientIdentitiesTests: XCTestCase { // MARK: unlinkIdentity - func test_unlinkIdentity_sendsDeleteWithBody() async throws { + func test_unlinkIdentity_sendsDeleteToIdentitiesPath() async throws { session.stub(data: Data(), statusCode: 204) - try await sut.unlinkIdentity(provider: "github", providerId: "abc-123") - XCTAssertEqual(session.lastRequest?.url?.path, "/api/user/identities") + try await sut.unlinkIdentity(provider: "github") XCTAssertEqual(session.lastRequest?.httpMethod, "DELETE") - let body = String(data: session.lastRequest?.httpBody ?? Data(), encoding: .utf8) ?? "" - XCTAssertTrue(body.contains("\"provider\":\"github\"")) - XCTAssertTrue(body.contains("\"providerId\":\"abc-123\""), - "Body must use camelCase providerId. Got: \(body)") + XCTAssertEqual(session.lastRequest?.url?.path, "/api/user/identities") + } + + func test_unlinkIdentity_sendsProviderAsQueryParameter() async throws { + session.stub(data: Data(), statusCode: 204) + try await sut.unlinkIdentity(provider: "github") + XCTAssertEqual(session.lastRequest?.url?.query, "provider=github") + } + + func test_unlinkIdentity_sendsEmptyBody() async throws { + // The route reads the query string and never parses a body; a JSON payload + // here is what produced 400 "provider is required". + session.stub(data: Data(), statusCode: 204) + try await sut.unlinkIdentity(provider: "github") + XCTAssertNil(session.lastRequest?.httpBody) + } + + func test_unlinkIdentity_mastodonProvider_keepsInstanceSuffix() async throws { + session.stub(data: Data(), statusCode: 204) + try await sut.unlinkIdentity(provider: "mastodon:techhub.social") + let value = URLComponents(url: try XCTUnwrap(session.lastRequest?.url), resolvingAgainstBaseURL: false)? + .queryItems?.first(where: { $0.name == "provider" })?.value + XCTAssertEqual(value, "mastodon:techhub.social") + } + + func test_unlinkIdentity_providerWithQueryDelimiters_percentEncodesThem() async throws { + session.stub(data: Data(), statusCode: 204) + try await sut.unlinkIdentity(provider: "mastodon:a+b&c=d") + let query = try XCTUnwrap(session.lastRequest?.url?.query) + XCTAssertEqual(query, "provider=mastodon:a%2Bb%26c%3Dd") + let value = URLComponents(url: try XCTUnwrap(session.lastRequest?.url), resolvingAgainstBaseURL: false)? + .queryItems?.first(where: { $0.name == "provider" })?.value + XCTAssertEqual(value, "mastodon:a+b&c=d") } func test_unlinkIdentity_sendsBearerToken() async throws { session.stub(data: Data(), statusCode: 204) - try await sut.unlinkIdentity(provider: "github", providerId: "x") + try await sut.unlinkIdentity(provider: "github") XCTAssertEqual(session.lastRequest?.value(forHTTPHeaderField: "Authorization"), "Bearer tok") } func test_unlinkIdentity_403_throws() async throws { session.stub(data: Data(), statusCode: 403) do { - try await sut.unlinkIdentity(provider: "github", providerId: "x") + try await sut.unlinkIdentity(provider: "github") XCTFail("Expected throw") } catch APIError.status(let code) { XCTAssertEqual(code, 403) } } + func test_unlinkIdentity_401_throws() async throws { + session.stub(data: Data(), statusCode: 401) + do { + try await sut.unlinkIdentity(provider: "github") + XCTFail("Expected throw") + } catch APIError.status(let code) { + XCTAssertEqual(code, 401) + } + } + + func test_unlinkIdentity_404_surfacesRouteMessage() async throws { + session.stub(json: #"{"error":"Identity not found"}"#, statusCode: 404) + do { + try await sut.unlinkIdentity(provider: "github") + XCTFail("Expected throw") + } catch APIError.server(let message) { + XCTAssertEqual(message, "Identity not found") + } + } + // MARK: verifyIdentity func test_verifyIdentity_sendsCorrectPath() async throws {