From 022a9211df8b36c690a4576a65c05e228e757f94 Mon Sep 17 00:00:00 2001 From: Gil Shapira Date: Tue, 7 Jan 2025 19:39:06 +0200 Subject: [PATCH 1/4] Cleanup session storage code --- src/session/Storage.swift | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/session/Storage.swift b/src/session/Storage.swift index 2e31090..b1f983d 100644 --- a/src/session/Storage.swift +++ b/src/session/Storage.swift @@ -35,7 +35,7 @@ public class SessionStorage: DescopeSessionStorage { public let projectId: String public let store: Store - private var lastValue: Value? + private var lastSaved: EncodedSession? public init(projectId: String, store: Store = .keychain) { self.projectId = projectId @@ -43,23 +43,23 @@ public class SessionStorage: DescopeSessionStorage { } public func saveSession(_ session: DescopeSession) { - let value = Value(sessionJwt: session.sessionJwt, refreshJwt: session.refreshJwt, user: session.user) - guard value != lastValue else { return } - guard let data = try? JSONEncoder().encode(value) else { return } + let encoded = EncodedSession(sessionJwt: session.sessionJwt, refreshJwt: session.refreshJwt, user: session.user) + guard lastSaved != encoded else { return } + guard let data = try? JSONEncoder().encode(encoded) else { return } try? store.saveItem(key: projectId, data: data) - lastValue = value + lastSaved = encoded } public func loadSession() -> DescopeSession? { guard let data = try? store.loadItem(key: projectId) else { return nil } - guard let value = try? JSONDecoder().decode(Value.self, from: data) else { return nil } - let session = try? DescopeSession(sessionJwt: value.sessionJwt, refreshJwt: value.refreshJwt, user: value.user) - lastValue = value + guard let encoded = try? JSONDecoder().decode(EncodedSession.self, from: data) else { return nil } + let session = try? DescopeSession(sessionJwt: encoded.sessionJwt, refreshJwt: encoded.refreshJwt, user: encoded.user) + lastSaved = encoded return session } public func removeSession() { - lastValue = nil + lastSaved = nil try? store.removeItem(key: projectId) } @@ -80,23 +80,23 @@ public class SessionStorage: DescopeSessionStorage { } /// A helper struct for serializing the ``DescopeSession`` data. - private struct Value: Codable, Equatable { + private struct EncodedSession: Codable, Equatable { var sessionJwt: String var refreshJwt: String var user: DescopeUser } } -public extension SessionStorage.Store { +extension SessionStorage.Store { /// A store that does nothing. - static let none = SessionStorage.Store() - + public static let none = SessionStorage.Store() + /// A store that saves the session data to the keychain. - static let keychain = SessionStorage.KeychainStore() + public static let keychain = SessionStorage.KeychainStore() } -public extension SessionStorage { - class KeychainStore: Store { +extension SessionStorage { + public class KeychainStore: Store { public override func loadItem(key: String) -> Data? { var query = queryForItem(key: key) query[kSecReturnData as String] = true From 0e8aee53e00de09d102bcd0635745d9c3bc2113e Mon Sep 17 00:00:00 2001 From: Gil Shapira Date: Tue, 7 Jan 2025 19:39:34 +0200 Subject: [PATCH 2/4] Don't fail refresh on empty response field --- src/internal/http/DescopeClient.swift | 10 +++++----- test/routes/Auth.swift | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/internal/http/DescopeClient.swift b/src/internal/http/DescopeClient.swift index 361516c..706b89e 100644 --- a/src/internal/http/DescopeClient.swift +++ b/src/internal/http/DescopeClient.swift @@ -437,10 +437,10 @@ final class DescopeClient: HTTPClient, @unchecked Sendable { user?.setCustomAttributes(from: dict) } if sessionJwt == nil || sessionJwt == "" { - sessionJwt = try findTokenCookie(named: sessionCookieName, in: cookies) + sessionJwt = findTokenCookie(named: sessionCookieName, in: cookies) } if refreshJwt == nil || refreshJwt == "" { - refreshJwt = try findTokenCookie(named: refreshCookieName, in: cookies) + refreshJwt = findTokenCookie(named: refreshCookieName, in: cookies) } } } @@ -579,10 +579,10 @@ private extension DeliveryMethod { } } -private func findTokenCookie(named name: String, in cookies: [HTTPCookie]) throws(DescopeError) -> String { +private func findTokenCookie(named name: String, in cookies: [HTTPCookie]) -> String? { // keep only cookies matching the required name let cookies = cookies.filter { name.caseInsensitiveCompare($0.name) == .orderedSame } - guard !cookies.isEmpty else { throw DescopeError.decodeError.with(message: "Missing value for \(name) cookie") } + guard !cookies.isEmpty else { return nil } // try to make a deterministic choice between cookies by looking for the best matching token let tokens = cookies.compactMap { try? Token(jwt: $0.value) }.sorted { a, b in @@ -591,7 +591,7 @@ private func findTokenCookie(named name: String, in cookies: [HTTPCookie]) throw } // try to find the best match by prioritizing the newest non-expired token - guard let token = tokens.first else { throw DescopeError.decodeError.with(message: "Invalid value for \(name) cookie") } + guard let token = tokens.first else { return nil } return token.jwt } diff --git a/test/routes/Auth.swift b/test/routes/Auth.swift index 819df3c..617ec0c 100644 --- a/test/routes/Auth.swift +++ b/test/routes/Auth.swift @@ -52,6 +52,20 @@ class TestAuth: XCTestCase { try checkUser(authResponse.user) } + func testRefresh() async throws { + let descope = DescopeSDK.mock() + + MockHTTP.push(body: authNoRefreshPayload) { request in + XCTAssertEqual(request.httpMethod, "POST") + XCTAssertEqual(request.url?.absoluteString ?? "", "https://api.descope.com/v1/auth/refresh") + XCTAssertEqual(request.allHTTPHeaderFields?["Authorization"], "Bearer projId:foo") + } + + let refreshResponse = try await descope.auth.refreshSession(refreshJwt: "foo") + XCTAssertEqual("bar", refreshResponse.sessionToken.entityId) + XCTAssertNil(refreshResponse.refreshToken) + } + func checkUser(_ user: DescopeUser) throws { XCTAssertEqual("userId", user.userId) XCTAssertFalse(user.isVerifiedPhone) @@ -93,6 +107,15 @@ private let authPayload = """ } """ +private let authNoRefreshPayload = """ +{ + "sessionJwt": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJiYXIiLCJuYW1lIjoiU3dpZnR5IE1jQXBwbGVzIiwiaWF0IjoxNTE2MjM5MDIyLCJpc3MiOiJmb28iLCJleHAiOjE2MDMxNzY2MTQsInBlcm1pc3Npb25zIjpbImQiLCJlIl0sInJvbGVzIjpbInVzZXIiXSwidGVuYW50cyI6eyJ0ZW5hbnQiOnsicGVybWlzc2lvbnMiOlsiYSIsImIiLCJjIl0sInJvbGVzIjpbImFkbWluIl19fX0.LEcNdzkdOXlzxcVNhvlqOIoNwzgYYfcDv1_vzF3awF8", + "refreshJwt": "", + "user": \(userPayload), + "firstSeen": false +} +""" + private let tenantsPayload = """ { "tenants": [ From 935a6816a13a5c8a074870556101b187a8d93245 Mon Sep 17 00:00:00 2001 From: Gil Shapira Date: Tue, 7 Jan 2025 19:40:17 +0200 Subject: [PATCH 3/4] Allow changing keychain accessibility --- src/session/Storage.swift | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/session/Storage.swift b/src/session/Storage.swift index b1f983d..a25b52a 100644 --- a/src/session/Storage.swift +++ b/src/session/Storage.swift @@ -97,6 +97,18 @@ extension SessionStorage.Store { extension SessionStorage { public class KeychainStore: Store { + #if os(iOS) + private let accessibility: String + + public override init() { + self.accessibility = kSecAttrAccessibleAfterFirstUnlock as String + } + + public init(accessibility: String) { + self.accessibility = accessibility + } + #endif + public override func loadItem(key: String) -> Data? { var query = queryForItem(key: key) query[kSecReturnData as String] = true @@ -115,7 +127,7 @@ extension SessionStorage { #if os(macOS) values[kSecAttrAccess as String] = SecAccessCreateWithOwnerAndACL(getuid(), 0, SecAccessOwnerType(kSecUseOnlyUID), nil, nil) #else - values[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlock + values[kSecAttrAccessible as String] = accessibility #endif let query = queryForItem(key: key) From 6f38dbbd6bb92162c2fe0030e3d10fd7b4c6aec2 Mon Sep 17 00:00:00 2001 From: Gil Shapira Date: Tue, 7 Jan 2025 19:40:55 +0200 Subject: [PATCH 4/4] Update session lifecycle --- src/sdk/Config.swift | 2 +- src/sdk/SDK.swift | 6 ++--- src/session/Lifecycle.swift | 44 ++++++++++++++++++++++++++++++------- src/session/Manager.swift | 21 ++++++++++-------- 4 files changed, 52 insertions(+), 21 deletions(-) diff --git a/src/sdk/Config.swift b/src/sdk/Config.swift index d2f4e6b..0010637 100644 --- a/src/sdk/Config.swift +++ b/src/sdk/Config.swift @@ -32,7 +32,7 @@ public struct DescopeConfig { /// This property can be useful to test code that uses the Descope SDK without any /// network requests actually taking place. In most other cases there shouldn't be /// any need to use it. - public var networkClient: DescopeNetworkClient? = nil + public var networkClient: DescopeNetworkClient? } /// The ``DescopeLogger`` class can be used to customize logging functionality in the Descope SDK. diff --git a/src/sdk/SDK.swift b/src/sdk/SDK.swift index 60cd725..2b9f9ad 100644 --- a/src/sdk/SDK.swift +++ b/src/sdk/SDK.swift @@ -81,7 +81,7 @@ public class DescopeSDK { /// override the ``DescopeSDK`` object's default networking client with one that always /// fails, using code such as this (see ``DescopeNetworkClient``): /// - /// let descope = DescopeSDK(projectId: "") { config in + /// let descope = DescopeSDK(projectId: "test") { config in /// config.networkClient = FailingNetworkClient() /// } /// testOTPNetworkError(descope) @@ -143,7 +143,7 @@ public extension DescopeSDK { static let name = "DescopeKit" /// The Descope SDK version - static let version = "0.9.10" + static let version = "0.9.11" } // Internal @@ -151,7 +151,7 @@ public extension DescopeSDK { private extension DescopeSessionManager { convenience init(sdk: DescopeSDK) { let storage = SessionStorage(projectId: sdk.config.projectId) - let lifecycle = SessionLifecycle(auth: sdk.auth) + let lifecycle = SessionLifecycle(auth: sdk.auth, storage: storage, logger: sdk.config.logger) self.init(storage: storage, lifecycle: lifecycle) } } diff --git a/src/session/Lifecycle.swift b/src/session/Lifecycle.swift index 4c38a4d..fde013b 100644 --- a/src/session/Lifecycle.swift +++ b/src/session/Lifecycle.swift @@ -20,32 +20,49 @@ public protocol DescopeSessionLifecycle: AnyObject { /// or if it's already expired. public class SessionLifecycle: DescopeSessionLifecycle { public let auth: DescopeAuth + public let storage: DescopeSessionStorage + public let logger: DescopeLogger? - public init(auth: DescopeAuth) { + public init(auth: DescopeAuth, storage: DescopeSessionStorage, logger: DescopeLogger?) { self.auth = auth + self.storage = storage + self.logger = logger } public var stalenessAllowedInterval: TimeInterval = 60 /* seconds */ - public var stalenessCheckFrequency: TimeInterval = 30 /* seconds */ { + public var periodicCheckFrequency: TimeInterval = 30 /* seconds */ { didSet { - if stalenessCheckFrequency != oldValue { + if periodicCheckFrequency != oldValue { resetTimer() } } } + public var shouldSaveAfterPeriodicRefresh: Bool = true + public var session: DescopeSession? { didSet { if session?.refreshJwt != oldValue?.refreshJwt { resetTimer() } + if let session, session.refreshToken.isExpired { + logger(.debug, "Session has an expired refresh token", session.refreshToken.expiresAt) + } } } public func refreshSessionIfNeeded() async throws -> Bool { guard let current = session, shouldRefresh(current) else { return false } + + logger(.info, "Refreshing session that is about to expire", current.sessionToken.expiresAt.timeIntervalSinceNow) let response = try await auth.refreshSession(refreshJwt: current.refreshJwt) + + guard session?.sessionJwt == current.sessionJwt else { + logger(.info, "Skipping refresh because session has changed in the meantime") + return false + } + session?.updateTokens(with: response) return true } @@ -61,7 +78,7 @@ public class SessionLifecycle: DescopeSessionLifecycle { private var timer: Timer? private func resetTimer() { - if stalenessCheckFrequency > 0, let refreshToken = session?.refreshToken, !refreshToken.isExpired { + if periodicCheckFrequency > 0, let refreshToken = session?.refreshToken, !refreshToken.isExpired { startTimer() } else { stopTimer() @@ -70,9 +87,9 @@ public class SessionLifecycle: DescopeSessionLifecycle { private func startTimer() { timer?.invalidate() - timer = Timer.scheduledTimer(withTimeInterval: stalenessCheckFrequency, repeats: true) { [weak self] timer in + timer = Timer.scheduledTimer(withTimeInterval: periodicCheckFrequency, repeats: true) { [weak self] timer in guard let lifecycle = self else { return timer.invalidate() } - Task { @MainActor in + Task { await lifecycle.periodicRefresh() } } @@ -84,11 +101,22 @@ public class SessionLifecycle: DescopeSessionLifecycle { } private func periodicRefresh() async { + if let refreshToken = session?.refreshToken, refreshToken.isExpired { + logger(.debug, "Stopping periodic refresh for session with expired refresh token") + stopTimer() + return + } + do { - _ = try await refreshSessionIfNeeded() + let refreshed = try await refreshSessionIfNeeded() + if refreshed, shouldSaveAfterPeriodicRefresh, let session { + logger(.debug, "Saving refresh session after periodic refresh") + storage.saveSession(session) + } } catch DescopeError.networkError { - // allow retries on network errors + logger(.debug, "Ignoring network error in periodic refresh") } catch { + logger(.error, "Stopping periodic refresh after failure", error) stopTimer() } } diff --git a/src/session/Manager.swift b/src/session/Manager.swift index e8ab45b..3f321d3 100644 --- a/src/session/Manager.swift +++ b/src/session/Manager.swift @@ -102,7 +102,7 @@ public class DescopeSessionManager { /// each other's saved sessions. public func manageSession(_ session: DescopeSession) { lifecycle.session = session - storage.saveSession(session) + saveSession() } /// Clears any active ``DescopeSession`` from this manager and removes it @@ -122,6 +122,17 @@ public class DescopeSessionManager { storage.removeSession() } + /// Saves the active ``DescopeSession`` to the storage. + /// + /// - Important: There is usually no need to call this method directly. + /// The session is automatically saved when it's refreshed or updated, + /// unless you're using a session manager with custom `stroage` and + /// `lifecycle` objects. + public func saveSession() { + guard let session else { return } + storage.saveSession(session) + } + /// Ensures that the session is valid and refreshes it if needed. /// /// The session manager checks whether there's an active ``DescopeSession`` and if @@ -170,12 +181,4 @@ public class DescopeSessionManager { lifecycle.session?.updateUser(with: user) saveSession() } - - // Internal - - /// Saves the latest session value to the storage. - private func saveSession() { - guard let session else { return } - storage.saveSession(session) - } }