From 5e22431d11fafee0af500214e5ef35f9639f3679 Mon Sep 17 00:00:00 2001 From: pdurlej Date: Tue, 12 May 2026 17:45:02 +0200 Subject: [PATCH] fix: preserve token restore error context (#30) Problem - Token-only session restore currently reports every token validation failure as an expired/revoked token, including permission, server, network, and invalid-response failures. Change - Preserves token validation error context for token-only instances. - Maps auth/service/network failures to more specific user-facing restore errors. - Leaves password fallback behavior unchanged for password-based instances. Tests - DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer xcodebuild -quiet -project Forji/Forji.xcodeproj -scheme Forji -destination "platform=iOS Simulator,name=iPhone 17 Pro,OS=26.4.1" build-for-testing -only-testing:ForjiTests/SessionRestoreErrorTests - DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer xcodebuild -quiet -project Forji/Forji.xcodeproj -scheme Forji -destination "platform=iOS Simulator,name=iPhone 17 Pro,OS=26.4.1" test-without-building -only-testing:ForjiTests/SessionRestoreErrorTests Co-authored-by: Piotr Durlej Reviewed-on: https://codeberg.org/secana/Forji/pulls/30 Reviewed-by: secana --- .../Services/AuthenticationService.swift | 91 ++++++++++++++++++- .../ForjiTests/SessionRestoreErrorTests.swift | 67 ++++++++++++++ 2 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 Forji/ForjiTests/SessionRestoreErrorTests.swift diff --git a/Forji/Forji/Services/AuthenticationService.swift b/Forji/Forji/Services/AuthenticationService.swift index 81c0648..05cbb70 100644 --- a/Forji/Forji/Services/AuthenticationService.swift +++ b/Forji/Forji/Services/AuthenticationService.swift @@ -171,7 +171,7 @@ class AuthenticationService { } catch { // Token is invalid/expired — only fall through to password if this is not a token-only instance if useTokenAuth { - throw SessionRestoreError.tokenExpired + throw SessionRestoreError.fromTokenValidationError(error) } // Otherwise fall through to password-based login below } @@ -205,13 +205,100 @@ class AuthenticationService { #endif } -enum SessionRestoreError: LocalizedError { +enum SessionRestoreError: LocalizedError, Equatable { case tokenExpired + case tokenPermissionDenied + case serverUnavailable + case serverNotFound + case networkUnavailable + case certificateError + case invalidServerResponse + case tokenValidationFailedHTTPStatus(Int) + case tokenValidationFailed var errorDescription: String? { switch self { case .tokenExpired: "Your API token is expired or has been revoked. Please edit this instance and enter a new token." + case .tokenPermissionDenied: + "Your API token does not have permission to access this account. Please edit this instance and enter a token with the required scopes." + case .serverUnavailable: + "The Forgejo server returned an error while validating your token. Please try again later." + case .serverNotFound: + "The Forgejo server could not be found. Please check the instance URL." + case .networkUnavailable: + "Forji could not reach the Forgejo server while validating your token. Please check your connection and try again." + case .certificateError: + "Forji could not validate the server certificate. Enable self-signed certificates if this instance uses one." + case .invalidServerResponse: + "The Forgejo server returned an invalid response while validating your token." + case let .tokenValidationFailedHTTPStatus(statusCode): + "Forji could not validate your API token (HTTP \(statusCode)). Please edit this instance or try again later." + case .tokenValidationFailed: + "Forji could not validate your API token. Please edit this instance or try again later." + } + } + + static func fromTokenValidationError(_ error: Error) -> SessionRestoreError { + if let sessionError = error as? SessionRestoreError { + return sessionError + } + if let authError = error as? AuthenticationError { + return fromAuthenticationError(authError) + } + if let serviceError = error as? ServiceError { + return fromServiceError(serviceError) + } + if error is URLError { + return .networkUnavailable + } + return .tokenValidationFailed + } + + private static func fromAuthenticationError(_ error: AuthenticationError) -> SessionRestoreError { + switch error { + case .invalidCredentials, .otpRequired: + .tokenExpired + case .serverNotFound: + .serverNotFound + case .certificateError: + .certificateError + case .invalidResponse: + .invalidServerResponse + case let .unknownError(statusCode): + fromHTTPStatusCode(statusCode) + case .invalidURL: + .serverNotFound + } + } + + private static func fromServiceError(_ error: ServiceError) -> SessionRestoreError { + switch error { + case .invalidURL: + .serverNotFound + case .invalidResponse, .decodingFailed: + .invalidServerResponse + case let .httpError(statusCode, _): + fromHTTPStatusCode(statusCode) + case .noActiveInstance: + .tokenValidationFailed + case .notMergeable, .mergeConflict: + .tokenValidationFailed + } + } + + private static func fromHTTPStatusCode(_ statusCode: Int) -> SessionRestoreError { + switch statusCode { + case 401: + .tokenExpired + case 403: + .tokenPermissionDenied + case 404: + .serverNotFound + case 500 ... 599: + .serverUnavailable + default: + .tokenValidationFailedHTTPStatus(statusCode) } } } diff --git a/Forji/ForjiTests/SessionRestoreErrorTests.swift b/Forji/ForjiTests/SessionRestoreErrorTests.swift new file mode 100644 index 0000000..34bc1ad --- /dev/null +++ b/Forji/ForjiTests/SessionRestoreErrorTests.swift @@ -0,0 +1,67 @@ +import ForgejoKit +import Foundation +import Testing +@testable import Forji + +struct SessionRestoreErrorTests { + + @Test func mapsTokenValidationAuthenticationErrors() { + #expect( + SessionRestoreError.fromTokenValidationError(AuthenticationError.invalidCredentials) + == .tokenExpired + ) + #expect( + SessionRestoreError.fromTokenValidationError(AuthenticationError.unknownError(statusCode: 403)) + == .tokenPermissionDenied + ) + #expect( + SessionRestoreError.fromTokenValidationError(AuthenticationError.serverNotFound) + == .serverNotFound + ) + #expect( + SessionRestoreError.fromTokenValidationError(AuthenticationError.certificateError) + == .certificateError + ) + #expect( + SessionRestoreError.fromTokenValidationError(AuthenticationError.unknownError(statusCode: 502)) + == .serverUnavailable + ) + } + + @Test func mapsTokenValidationServiceErrors() { + #expect( + SessionRestoreError.fromTokenValidationError(ServiceError.httpError(statusCode: 401)) + == .tokenExpired + ) + #expect( + SessionRestoreError.fromTokenValidationError(ServiceError.httpError(statusCode: 403)) + == .tokenPermissionDenied + ) + #expect( + SessionRestoreError.fromTokenValidationError(ServiceError.httpError(statusCode: 503)) + == .serverUnavailable + ) + #expect( + SessionRestoreError.fromTokenValidationError(ServiceError.invalidResponse) + == .invalidServerResponse + ) + } + + @Test func mapsTokenValidationNetworkErrors() { + #expect( + SessionRestoreError.fromTokenValidationError(URLError(.timedOut)) + == .networkUnavailable + ) + } + + @Test func describesPermissionFailureWithoutCallingItExpired() { + #expect( + SessionRestoreError.tokenPermissionDenied.errorDescription? + .contains("does not have permission") == true + ) + #expect( + SessionRestoreError.tokenPermissionDenied.errorDescription? + .contains("expired") == false + ) + } +}