Skip to content

Commit

Permalink
Minor session management bug fixes (#82)
Browse files Browse the repository at this point in the history
* Fix session being saved to storage even when not refreshed

* Fix handling of failures in periodic refresh

* Minor documentation fixes
  • Loading branch information
shilgapira authored Jan 1, 2025
1 parent d30ee14 commit ec9524f
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 24 deletions.
2 changes: 2 additions & 0 deletions src/docs/DescopeKit.docc/Documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ on the [Descope website](https://descope.com).

- ``DescopeFlow``
- ``DescopeFlowState``
- ``DescopeFlowHook``
- ``DescopeFlowView``
- ``DescopeFlowViewDelegate``
- ``DescopeFlowViewController``
Expand All @@ -64,6 +65,7 @@ on the [Descope website](https://descope.com).
- ``UpdateOptions``
- ``DeliveryMethod``
- ``OAuthProvider``
- ``RevokeType``

### Development

Expand Down
6 changes: 3 additions & 3 deletions src/flows/FlowHook.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import WebKit
/// ```
///
/// You can also implement your own hooks by subclassing ``DescopeFlowHook`` and
/// overriding the ``execute(coordinator:)`` method.
/// overriding the ``execute(event:coordinator:)`` method.
@MainActor
open class DescopeFlowHook {

Expand Down Expand Up @@ -222,7 +222,7 @@ extension DescopeFlowHook {
/// }
/// ```
///
/// - Parameter setup: A closure that receives the `UIScrollView` instance as its only parameter.
/// - Parameter closure: A closure that receives the `UIScrollView` instance as its only parameter.
///
/// - Returns: A ``DescopeFlowHook`` object that can be added to the ``DescopeFlow/hooks`` array.
public static func setupScrollView(_ closure: @escaping (UIScrollView) -> Void) -> DescopeFlowHook {
Expand All @@ -233,7 +233,7 @@ extension DescopeFlowHook {
/// Creates a hook that will run the provided closure when the flow is started
/// on the `WKWebView` used to display it.
///
/// - Parameter setup: A closure that receives the `WKWebView` instance as its only parameter.
/// - Parameter closure: A closure that receives the `WKWebView` instance as its only parameter.
///
/// - Returns: A ``DescopeFlowHook`` object that can be added to the ``DescopeFlow/hooks`` array.
public static func setupWebView(_ closure: @escaping (WKWebView) -> Void) -> DescopeFlowHook {
Expand Down
4 changes: 0 additions & 4 deletions src/internal/others/FlowsV0/FlowsV0Runner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,6 @@ public class DescopeFlowRunner {
/// The host application is expected to intercept this URL via Universal Links and
/// resume the running flow with it.
///
/// You can do this by first getting a reference to the current running flow from
/// the ``DescopeFlow/current`` property and then calling the ``resume(with:)`` method
/// with the URL from the Universal Link.
///
/// @main
/// struct MyApp: App {
/// // ...
Expand Down
1 change: 1 addition & 0 deletions src/sdk/Callbacks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public extension DescopeAuth {
/// hasn't already been selected.
/// - tenantIds: Provide a non-empty array of tenant IDs and set `dct` to `false`
/// to request a specific list of tenants for the user.
/// - refreshJwt: the `refreshJwt` from an active ``DescopeSession``.
///
/// - Returns: A list of one or more ``DescopeTenant`` values.
func tenants(dct: Bool, tenantIds: [String], refreshJwt: String, completion: @escaping @Sendable (Result<[DescopeTenant], Error>) -> Void) {
Expand Down
1 change: 1 addition & 0 deletions src/sdk/Routes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public protocol DescopeAuth: Sendable {
/// hasn't already been selected.
/// - tenantIds: Provide a non-empty array of tenant IDs and set `dct` to `false`
/// to request a specific list of tenants for the user.
/// - refreshJwt: the `refreshJwt` from an active ``DescopeSession``.
///
/// - Returns: A list of one or more ``DescopeTenant`` values.
func tenants(dct: Bool, tenantIds: [String], refreshJwt: String) async throws -> [DescopeTenant]
Expand Down
19 changes: 11 additions & 8 deletions src/session/Lifecycle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import Foundation
/// manages its ``DescopeSession`` while the application is running.
@MainActor
public protocol DescopeSessionLifecycle: AnyObject {
/// Set by the session manager whenever the current active session changes.
/// Holds the latest session value for the session manager.
var session: DescopeSession? { get set }

/// Called the session manager to conditionally refresh the active session.
func refreshSessionIfNeeded() async throws
/// Called by the session manager to conditionally refresh the active session.
func refreshSessionIfNeeded() async throws -> Bool
}

/// The default implementation of the ``DescopeSessionLifecycle`` protocol.
Expand Down Expand Up @@ -43,10 +43,11 @@ public class SessionLifecycle: DescopeSessionLifecycle {
}
}

public func refreshSessionIfNeeded() async throws {
guard let current = session, shouldRefresh(current) else { return }
public func refreshSessionIfNeeded() async throws -> Bool {
guard let current = session, shouldRefresh(current) else { return false }
let response = try await auth.refreshSession(refreshJwt: current.refreshJwt)
session?.updateTokens(with: response)
return true
}

// Conditional refresh
Expand All @@ -60,7 +61,7 @@ public class SessionLifecycle: DescopeSessionLifecycle {
private var timer: Timer?

private func resetTimer() {
if session != nil && stalenessCheckFrequency > 0 {
if stalenessCheckFrequency > 0, let refreshToken = session?.refreshToken, !refreshToken.isExpired {
startTimer()
} else {
stopTimer()
Expand All @@ -84,9 +85,11 @@ public class SessionLifecycle: DescopeSessionLifecycle {

private func periodicRefresh() async {
do {
try await refreshSessionIfNeeded()
_ = try await refreshSessionIfNeeded()
} catch DescopeError.networkError {
// allow retries on network errors
} catch {
// TODO check for refresh failure to not try again and again after expiry
stopTimer()
}
}
}
22 changes: 13 additions & 9 deletions src/session/Manager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ public class DescopeSessionManager {
/// - Note: When using a custom ``DescopeSessionManager`` object the exact behavior
/// here depends on the `storage` and `lifecycle` objects.
public func refreshSessionIfNeeded() async throws {
try await lifecycle.refreshSessionIfNeeded()
if let session {
storage.saveSession(session)
let refreshed = try await lifecycle.refreshSessionIfNeeded()
if refreshed {
saveSession()
}
}

Expand All @@ -152,9 +152,7 @@ public class DescopeSessionManager {
/// ``DescopeSessionStorage`` protocol.
public func updateTokens(with refreshResponse: RefreshResponse) {
lifecycle.session?.updateTokens(with: refreshResponse)
if let session {
storage.saveSession(session)
}
saveSession()
}

/// Updates the active session's user details.
Expand All @@ -170,8 +168,14 @@ public class DescopeSessionManager {
/// but this can be overridden with a custom ``DescopeSessionStorage`` object.
public func updateUser(with user: DescopeUser) {
lifecycle.session?.updateUser(with: user)
if let session {
storage.saveSession(session)
}
saveSession()
}

// Internal

/// Saves the latest session value to the storage.
private func saveSession() {
guard let session else { return }
storage.saveSession(session)
}
}

0 comments on commit ec9524f

Please sign in to comment.