mirror of
https://codeberg.org/secana/Forji.git
synced 2026-06-16 05:13:55 -07:00
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 <pdurlej@users.noreply.github.com> Reviewed-on: https://codeberg.org/secana/Forji/pulls/30 Reviewed-by: secana <secana@noreply.codeberg.org>
This commit is contained in:
pdurlej
secana
parent
db4e54db6b
commit
5e22431d11
2 changed files with 156 additions and 2 deletions
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
67
Forji/ForjiTests/SessionRestoreErrorTests.swift
Normal file
67
Forji/ForjiTests/SessionRestoreErrorTests.swift
Normal file
|
|
@ -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
|
||||
)
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue