From 66f68d2e182a9b15cd015b30a18199ed246df5b7 Mon Sep 17 00:00:00 2001 From: Lukas Foldyna Date: Wed, 3 Sep 2025 16:09:33 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat(auth):=20implement=20proper=20?= =?UTF-8?q?OAuth2=20refresh=20token=20flow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add dedicated refreshToken() method to Api class for standard OAuth2 token refresh - Update refreshTokenIfPossible() to first attempt refresh token flow before fallback - Make TokenResponse.refreshToken optional to handle refresh responses correctly - Update ApiRequest protocol to support authorization header control - Add authorization parameter to all ApiRequest initializers - Fix test implementations to match updated ApiRequest protocol The implementation now follows OAuth2 best practices: 1. First attempts to refresh using the refresh_token grant type 2. Falls back to credential-based login only if refresh fails 3. Properly preserves device ID and stamp during refresh 🤖 Generated with Claude Code Co-Authored-By: Claude --- KiaMaps/App/ApiExtensions.swift | 49 ++++++++++--- KiaMaps/Core/Api/Api.swift | 19 ++++- KiaMaps/Core/Api/ApiRequest.swift | 69 +++++++++++++------ .../Api/Models/AuthenticationModels.swift | 2 +- KiaTests/AuthenticationTests.swift | 15 ++-- KiaTests/UIComponentMockDataTests.swift | 6 +- 6 files changed, 119 insertions(+), 41 deletions(-) diff --git a/KiaMaps/App/ApiExtensions.swift b/KiaMaps/App/ApiExtensions.swift index da1075c..1615414 100644 --- a/KiaMaps/App/ApiExtensions.swift +++ b/KiaMaps/App/ApiExtensions.swift @@ -41,23 +41,52 @@ extension Api { } } - /// Attempts to refresh the token using stored credentials + /// Attempts to refresh the token using stored refresh token or fallback to credentials /// - returns: True if token was successfully refreshed, false otherwise private func refreshTokenIfPossible() async -> Bool { - // Check if we have stored login credentials - guard let storedCredentials = LoginCredentialManager.retrieveCredentials() else { - logInfo("No stored credentials available for token refresh", category: .auth) - return false - } - // Check if current token is actually expired before attempting refresh if let currentAuth = authorization, !isTokenExpired(currentAuth) { logDebug("Current token is not expired, no refresh needed", category: .auth) return true } + // First, try to refresh using the refresh token if available + if let currentAuth = authorization { + logInfo("Attempting to refresh token using refresh token", category: .auth) + + do { + let tokenResponse = try await refreshToken(currentAuth.refreshToken) + + // Create new authorization data with refreshed tokens + let newAuthData = AuthorizationData( + stamp: currentAuth.stamp, // Keep existing stamp + deviceId: currentAuth.deviceId, // Keep existing device ID + accessToken: tokenResponse.accessToken, + expiresIn: tokenResponse.expiresIn, + refreshToken: tokenResponse.refreshToken ?? currentAuth.refreshToken, + isCcuCCS2Supported: currentAuth.isCcuCCS2Supported + ) + + // Store the new authorization data + Authorization.store(data: newAuthData) + self.authorization = newAuthData + + logInfo("Successfully refreshed token using refresh token", category: .auth) + return true + } catch { + logWarning("Refresh token failed: \(error.localizedDescription). Falling back to credential login", category: .auth) + // Continue to fallback method below + } + } + + // Fallback: Try to refresh using stored login credentials + guard let storedCredentials = LoginCredentialManager.retrieveCredentials() else { + logInfo("No stored credentials available for fallback token refresh", category: .auth) + return false + } + do { - logInfo("Attempting to login with stored credentials", category: .auth) + logInfo("Attempting fallback login with stored credentials", category: .auth) let newAuthData = try await login( username: storedCredentials.username, password: storedCredentials.password @@ -67,11 +96,11 @@ extension Api { Authorization.store(data: newAuthData) self.authorization = newAuthData - logInfo("Successfully refreshed token", category: .auth) + logInfo("Successfully refreshed token using credential fallback", category: .auth) return true } catch { - logError("Failed to refresh token with error: \(error.localizedDescription)", category: .auth) + logError("Failed to refresh token with fallback login: \(error.localizedDescription)", category: .auth) // If login fails, clear the stored credentials as they might be invalid if error is ApiError { diff --git a/KiaMaps/Core/Api/Api.swift b/KiaMaps/Core/Api/Api.swift index d9fa57e..c793760 100644 --- a/KiaMaps/Core/Api/Api.swift +++ b/KiaMaps/Core/Api/Api.swift @@ -161,7 +161,7 @@ class Api { deviceId: deviceId, accessToken: tokenResponse.accessToken, expiresIn: tokenResponse.expiresIn, - refreshToken: tokenResponse.refreshToken, + refreshToken: tokenResponse.refreshToken ?? "", isCcuCCS2Supported: true ) @@ -564,6 +564,23 @@ extension Api { ).data() } + /// Login - Refresh token + func refreshToken(_ refreshToken: String) async throws -> TokenResponse { + let form: [String: String] = [ + "client_id": configuration.serviceId, + "client_secret": "secret", // TODO: something generated + "grant_type": "refresh_token", + "refresh_token": refreshToken, + ] + + return try await provider.request( + with: .post, + endpoint: .loginToken, + form: form, + authorization: false + ).data() + } + /// Register device and retrieve device ID for push notifications /// - Parameter stamp: Authorization stamp for device registration /// - Returns: Unique device ID for this installation diff --git a/KiaMaps/Core/Api/ApiRequest.swift b/KiaMaps/Core/Api/ApiRequest.swift index 0203299..c045ce1 100644 --- a/KiaMaps/Core/Api/ApiRequest.swift +++ b/KiaMaps/Core/Api/ApiRequest.swift @@ -132,6 +132,7 @@ protocol ApiRequest { /// - headers: HTTP headers /// - encodable: Codable object to encode as JSON body /// - timeout: Request timeout in seconds + /// - authorization: If we should set authorization header /// - Throws: Encoding errors if encodable cannot be serialized init( caller: ApiCaller, @@ -140,7 +141,8 @@ protocol ApiRequest { queryItems: [URLQueryItem], headers: Headers, encodable: Encodable, - timeout: TimeInterval + timeout: TimeInterval, + authorization: Bool ) throws /// Initializer for requests with raw Data body @@ -152,6 +154,7 @@ protocol ApiRequest { /// - headers: HTTP headers /// - body: Raw data for request body /// - timeout: Request timeout in seconds + /// - authorization: If we should set authorization header init( caller: ApiCaller, method: ApiMethod?, @@ -159,7 +162,8 @@ protocol ApiRequest { queryItems: [URLQueryItem], headers: Headers, body: Data?, - timeout: TimeInterval + timeout: TimeInterval, + authorization: Bool ) /// Initializer for form-encoded requests @@ -171,6 +175,7 @@ protocol ApiRequest { /// - headers: HTTP headers /// - form: Form data dictionary /// - timeout: Request timeout in seconds + /// - authorization: If we should set authorization header init( caller: ApiCaller, method: ApiMethod?, @@ -178,7 +183,8 @@ protocol ApiRequest { queryItems: [URLQueryItem], headers: Headers, form: Form, - timeout: TimeInterval + timeout: TimeInterval, + authorization: Bool ) /// The configured URLRequest ready for execution @@ -291,6 +297,8 @@ struct ApiRequestImpl: ApiRequest { let body: Data? /// Request timeout in seconds let timeout: TimeInterval + /// If we should set authorization header + let authorization: Bool /// Character set used for form data encoding private static let formCharset: CharacterSet = { @@ -310,7 +318,8 @@ struct ApiRequestImpl: ApiRequest { queryItems: [URLQueryItem], headers: Headers, encodable: Encodable, - timeout: TimeInterval + timeout: TimeInterval, + authorization: Bool ) throws { var headers = headers if headers["Content-type"] == nil { @@ -326,6 +335,7 @@ struct ApiRequestImpl: ApiRequest { self.headers = headers body = try JSONEncoders.default.encode(encodable) self.timeout = timeout + self.authorization = authorization } init( @@ -335,7 +345,8 @@ struct ApiRequestImpl: ApiRequest { queryItems: [URLQueryItem], headers: Headers, body: Data?, - timeout: TimeInterval + timeout: TimeInterval, + authorization: Bool ) { var headers = headers if headers["Content-type"] == nil { @@ -351,16 +362,17 @@ struct ApiRequestImpl: ApiRequest { self.headers = headers self.body = body self.timeout = timeout + self.authorization = authorization } - init( - caller: ApiCaller, - method: ApiMethod?, - endpoint: ApiEndpoint, - queryItems: [URLQueryItem], - headers: Headers, - form: Form, - timeout: TimeInterval + init(caller: ApiCaller, + method: ApiMethod?, + endpoint: ApiEndpoint, + queryItems: [URLQueryItem], + headers: Headers, + form: Form, + timeout: TimeInterval, + authorization: Bool ) { var headers = Self.commonFormHeaders headers["User-Agent"] = caller.configuration.userAgent @@ -378,6 +390,7 @@ struct ApiRequestImpl: ApiRequest { self.headers = headers body = formData self.timeout = timeout + self.authorization = authorization } var urlRequest: URLRequest { @@ -389,7 +402,7 @@ struct ApiRequestImpl: ApiRequest { var request = URLRequest(url: url, cachePolicy: .reloadIgnoringCacheData, timeoutInterval: timeout) request.httpMethod = method.rawValue var headers = self.headers - if let authorization = caller.authorization { + if let authorization = caller.authorization, self.authorization { for (key, value) in authorization.authorizatioHeaders(for: caller.configuration) { headers[key] = value } @@ -550,6 +563,7 @@ class ApiRequestProvider: NSObject { /// - headers: HTTP headers /// - encodable: Object to encode as JSON body /// - timeout: Request timeout + /// - authorization: If authorization header should be set /// - Returns: Configured API request /// - Throws: Encoding errors func request( @@ -558,7 +572,8 @@ class ApiRequestProvider: NSObject { queryItems: [URLQueryItem] = [], headers: ApiRequest.Headers = [:], encodable: Encodable, - timeout: TimeInterval = ApiDefaultTimeout + timeout: TimeInterval = ApiDefaultTimeout, + authorization: Bool = true ) throws -> ApiRequest { try requestType.init( caller: caller, @@ -567,7 +582,8 @@ class ApiRequestProvider: NSObject { queryItems: queryItems, headers: headers, encodable: encodable, - timeout: timeout + timeout: timeout, + authorization: authorization ) } @@ -579,6 +595,7 @@ class ApiRequestProvider: NSObject { /// - headers: HTTP headers /// - body: Raw body data /// - timeout: Request timeout + /// - authorization: If authorization header should be set /// - Returns: Configured API request func request( with method: ApiMethod? = nil, @@ -586,7 +603,8 @@ class ApiRequestProvider: NSObject { queryItems: [URLQueryItem] = [], headers: ApiRequest.Headers = [:], body: Data? = nil, - timeout: TimeInterval = ApiDefaultTimeout + timeout: TimeInterval = ApiDefaultTimeout, + authorization: Bool = true ) -> ApiRequest { requestType.init( caller: caller, @@ -595,7 +613,8 @@ class ApiRequestProvider: NSObject { queryItems: queryItems, headers: headers, body: body, - timeout: timeout + timeout: timeout, + authorization: authorization ) } @@ -607,6 +626,7 @@ class ApiRequestProvider: NSObject { /// - headers: HTTP headers /// - string: String to encode as UTF-8 body /// - timeout: Request timeout + /// - authorization: If authorization header should be set /// - Returns: Configured API request func request( with method: ApiMethod? = nil, @@ -614,7 +634,8 @@ class ApiRequestProvider: NSObject { queryItems: [URLQueryItem] = [], headers: ApiRequest.Headers = [:], string: String, - timeout: TimeInterval = ApiDefaultTimeout + timeout: TimeInterval = ApiDefaultTimeout, + authorization: Bool = true ) -> ApiRequest { requestType.init( caller: caller, @@ -623,7 +644,8 @@ class ApiRequestProvider: NSObject { queryItems: queryItems, headers: headers, body: string.data(using: .utf8), - timeout: timeout + timeout: timeout, + authorization: authorization ) } @@ -635,6 +657,7 @@ class ApiRequestProvider: NSObject { /// - headers: HTTP headers /// - form: Form data dictionary /// - timeout: Request timeout + /// - authorization: If authorization header should be set /// - Returns: Configured API request func request( with method: ApiMethod? = nil, @@ -642,7 +665,8 @@ class ApiRequestProvider: NSObject { queryItems: [URLQueryItem] = [], headers: ApiRequest.Headers = [:], form: ApiRequest.Form, - timeout: TimeInterval = ApiDefaultTimeout + timeout: TimeInterval = ApiDefaultTimeout, + authorization: Bool = true ) -> ApiRequest { requestType.init( caller: caller, @@ -651,7 +675,8 @@ class ApiRequestProvider: NSObject { queryItems: queryItems, headers: headers, form: form, - timeout: timeout + timeout: timeout, + authorization: authorization ) } diff --git a/KiaMaps/Core/Api/Models/AuthenticationModels.swift b/KiaMaps/Core/Api/Models/AuthenticationModels.swift index e7fd60b..57923ac 100644 --- a/KiaMaps/Core/Api/Models/AuthenticationModels.swift +++ b/KiaMaps/Core/Api/Models/AuthenticationModels.swift @@ -97,7 +97,7 @@ struct TokenResponse: Codable { let scope: String? let connector: [String: ConnectorTokenInfo]? let accessToken: String - let refreshToken: String + let refreshToken: String? let idToken: String? let tokenType: String let expiresIn: Int diff --git a/KiaTests/AuthenticationTests.swift b/KiaTests/AuthenticationTests.swift index 5576ecc..f1506e9 100644 --- a/KiaTests/AuthenticationTests.swift +++ b/KiaTests/AuthenticationTests.swift @@ -485,6 +485,7 @@ struct MockApiRequest: ApiRequest { let headers: Headers let body: Data? let timeout: TimeInterval + let authorization: Bool private static let formCharset: CharacterSet = { var charset = CharacterSet.alphanumerics @@ -502,7 +503,8 @@ struct MockApiRequest: ApiRequest { queryItems: [URLQueryItem], headers: Headers, encodable: Encodable, - timeout: TimeInterval + timeout: TimeInterval, + authorization: Bool ) throws { var headers = headers if headers["Content-type"] == nil { @@ -518,6 +520,7 @@ struct MockApiRequest: ApiRequest { self.headers = headers body = try JSONEncoders.default.encode(encodable) self.timeout = timeout + self.authorization = authorization } init( @@ -527,7 +530,8 @@ struct MockApiRequest: ApiRequest { queryItems: [URLQueryItem], headers: Headers, body: Data?, - timeout: TimeInterval + timeout: TimeInterval, + authorization: Bool ) { var headers = headers if headers["Content-type"] == nil { @@ -543,6 +547,7 @@ struct MockApiRequest: ApiRequest { self.headers = headers self.body = body self.timeout = timeout + self.authorization = authorization } init( @@ -552,7 +557,8 @@ struct MockApiRequest: ApiRequest { queryItems: [URLQueryItem], headers: Headers, form: Form, - timeout: TimeInterval + timeout: TimeInterval, + authorization: Bool ) { var headers = Self.commonFormHeaders headers["User-Agent"] = caller.configuration.userAgent @@ -570,6 +576,7 @@ struct MockApiRequest: ApiRequest { self.headers = headers body = formData self.timeout = timeout + self.authorization = authorization } var urlRequest: URLRequest { @@ -581,7 +588,7 @@ struct MockApiRequest: ApiRequest { var request = URLRequest(url: url, cachePolicy: .reloadIgnoringCacheData, timeoutInterval: timeout) request.httpMethod = method.rawValue var headers = self.headers - if let authorization = caller.authorization { + if let authorization = caller.authorization, self.authorization { for (key, value) in authorization.authorizatioHeaders(for: caller.configuration) { headers[key] = value } diff --git a/KiaTests/UIComponentMockDataTests.swift b/KiaTests/UIComponentMockDataTests.swift index 9cefb04..d7a8a34 100644 --- a/KiaTests/UIComponentMockDataTests.swift +++ b/KiaTests/UIComponentMockDataTests.swift @@ -248,8 +248,8 @@ final class UIComponentMockDataTests: XCTestCase { func testVehicleSilhouetteViewWithMockData() { let VehicleState = MockVehicleData.preconditioning - let view = VehicleSilhouetteView(VehicleState: VehicleState) - + let view = VehicleSilhouetteView(vehicleState: VehicleState) + // View should be created successfully XCTAssertNotNil(view) @@ -259,7 +259,7 @@ final class UIComponentMockDataTests: XCTestCase { func testVehicleSilhouetteViewDoorStates() { let VehicleState = MockVehicleData.standard - let view = VehicleSilhouetteView(VehicleState: VehicleState) + let view = VehicleSilhouetteView(vehicleState: VehicleState) XCTAssertNotNil(view)