Skip to content

Commit

Permalink
[CIS-1040] Expired token refresh backoff (#1313)
Browse files Browse the repository at this point in the history
  • Loading branch information
dmigach authored Jul 24, 2021
1 parent caf46fd commit ef7eaa7
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ _July 21, 2021_
- `urlRequest(forImage url:)` added to `ImageCDN` protocol, this can be used to inject custom HTTP headers into image loading requests [#1291](https://github.com/GetStream/stream-chat-swift/issues/1291)
- Functionality that allows [inviting](https://getstream.io/chat/docs/react/channel_invites/?language=swift) users to channels with subsequent acceptance or rejection on their part [#1276](https://github.com/GetStream/stream-chat-swift/pull/1276)
- `EventsController` which exposes event observing API [#1266](https://github.com/GetStream/stream-chat-swift/pull/1266)
- A back off mechanism for cases when a token expires and the new reobtained one is also expired, so there is no endless loop. [#1305](https://github.com/GetStream/stream-chat-swift/pull/1305)

### 🐞 Fixed
- Fix an issue where member role sent from backend was not recognized by the SDK [#1288](https://github.com/GetStream/stream-chat-swift/pull/1288)
Expand Down
57 changes: 43 additions & 14 deletions Sources/StreamChat/ChatClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ public class _ChatClient<ExtraData: ExtraDataTypes> {
/// The environment object containing all dependencies of this `Client` instance.
private let environment: Environment

/// Retry timing strategy for refreshing an expiried token
private var tokenExpirationRetryStrategy: WebSocketClientReconnectionStrategy

/// A timer that runs token refreshing job
private var tokenRetryTimer: TimerControl?

/// The default configuration of URLSession to be used for both the `APIClient` and `WebSocketClient`. It contains all
/// required header auth parameters to make a successful request.
private var urlSessionConfiguration: URLSessionConfiguration {
Expand Down Expand Up @@ -292,13 +298,15 @@ public class _ChatClient<ExtraData: ExtraDataTypes> {
tokenProvider: TokenProvider? = nil,
workerBuilders: [WorkerBuilder],
eventWorkerBuilders: [EventWorkerBuilder],
environment: Environment
environment: Environment,
tokenExpirationRetryStrategy: WebSocketClientReconnectionStrategy = DefaultReconnectionStrategy()
) {
self.config = config
self.tokenProvider = tokenProvider
self.environment = environment
self.workerBuilders = workerBuilders
self.eventWorkerBuilders = eventWorkerBuilders
self.tokenExpirationRetryStrategy = tokenExpirationRetryStrategy

currentUserId = fetchCurrentUserIdFromDatabase()
}
Expand Down Expand Up @@ -537,6 +545,8 @@ extension _ChatClient {
#endif
}
}

var timerType: Timer.Type = DefaultTimer.self
}
}

Expand Down Expand Up @@ -589,19 +599,7 @@ extension _ChatClient: ConnectionStateDelegate {
case let .disconnected(error: error):
if let error = error,
error.isTokenExpiredError {
if let tokenProvider = tokenProvider {
clientUpdater.reloadUserIfNeeded(
userConnectionProvider: .closure { _, completion in
tokenProvider() { result in
completion(result)
}
}
)
} else {
log.assertionFailure(
"In case if token expiration is enabled on backend you need to provide a way to reobtain it via `tokenProvider` on ChatClient"
)
}
refreshToken(error: error)
shouldNotifyConnectionIdWaiters = false
} else {
shouldNotifyConnectionIdWaiters = true
Expand All @@ -621,6 +619,37 @@ extension _ChatClient: ConnectionStateDelegate {
)
}

private func refreshToken(error: ClientError) {
guard let tokenProvider = tokenProvider else {
return log.assertionFailure(
"In case if token expiration is enabled on backend you need to provide a way to reobtain it via `tokenProvider` on ChatClient"
)
}

guard let reconnectionDelay = tokenExpirationRetryStrategy.reconnectionDelay(
forConnectionError: error
) else { return }
tokenRetryTimer = environment
.timerType
.schedule(
timeInterval: reconnectionDelay,
queue: .main
) { [clientUpdater] in
clientUpdater.reloadUserIfNeeded(
userConnectionProvider: .closure { _, completion in
tokenProvider() { result in
if case let .success(token) = result,
!token.isExpired {
self.tokenExpirationRetryStrategy.successfullyConnected()
}

completion(result)
}
}
)
}
}

/// Update connectionId and notify waiters if needed
/// - Parameters:
/// - connectionId: new connectionId (if present)
Expand Down
9 changes: 6 additions & 3 deletions Sources/StreamChat/ChatClient_Mock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ extension _ChatClient {
config: .init(apiKey: .init(.unique)),
workerBuilders: [],
eventWorkerBuilders: [],
environment: .mock
environment: .mock,
tokenExpirationRetryStrategy: DefaultReconnectionStrategy()
)
}

Expand Down Expand Up @@ -63,7 +64,8 @@ class ChatClientMock<ExtraData: ExtraDataTypes>: _ChatClient<ExtraData> {
tokenProvider: TokenProvider? = nil,
workerBuilders: [WorkerBuilder] = [],
eventWorkerBuilders: [EventWorkerBuilder] = [],
environment: Environment = .mock
environment: Environment = .mock,
tokenExpirationRetryStrategy: WebSocketClientReconnectionStrategy = DefaultReconnectionStrategy()
) {
init_config = config
init_tokenProvider = tokenProvider
Expand All @@ -76,7 +78,8 @@ class ChatClientMock<ExtraData: ExtraDataTypes>: _ChatClient<ExtraData> {
tokenProvider: tokenProvider,
workerBuilders: workerBuilders,
eventWorkerBuilders: eventWorkerBuilders,
environment: environment
environment: environment,
tokenExpirationRetryStrategy: tokenExpirationRetryStrategy
)
}

Expand Down
28 changes: 26 additions & 2 deletions Sources/StreamChat/ChatClient_Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import XCTest
class ChatClient_Tests: StressTestCase {
var userId: UserId!
private var testEnv: TestEnvironment<NoExtraData>!
private var time: VirtualTime!

// A helper providing ChatClientConfig with in-memory DB option
var inMemoryStorageConfig: ChatClientConfig {
Expand All @@ -31,6 +32,8 @@ class ChatClient_Tests: StressTestCase {
super.setUp()
userId = .unique
testEnv = .init()
time = VirtualTime()
VirtualTimeTimer.time = time
}

override func tearDown() {
Expand Down Expand Up @@ -392,7 +395,7 @@ class ChatClient_Tests: StressTestCase {
XCTAssertNil(providedConnectionId)
}

func test_client_webSocketIsDisconnected_becauseTokenExpired_callsReloadUserIfNeeded() throws {
func test_webSocketIsDisconnected_becauseTokenExpired_newTokenIsExpiredToo() throws {
// Create a new chat client
let client = ChatClient(
config: inMemoryStorageConfig,
Expand Down Expand Up @@ -426,9 +429,29 @@ class ChatClient_Tests: StressTestCase {
didUpdateConnectionState: .disconnected(error: error)
)

time.run(numberOfSeconds: 0.6)
// Was called one more time on receiving token expired error
AssertAsync.willBeEqual(testEnv.clientUpdater!.reloadUserIfNeeded_callsCount, 2)

// Token is expired again
testEnv.webSocketClient?
.connectionStateDelegate?
.webSocketClient(
testEnv.webSocketClient!,
didUpdateConnectionState: .disconnected(error: error)
)

// Does not call secondary token refresh right away
XCTAssertEqual(testEnv.clientUpdater!.reloadUserIfNeeded_callsCount, 2)

// Does not call secondary token refresh when not enough time has passed
time.run(numberOfSeconds: 0.1)
XCTAssertEqual(testEnv.clientUpdater!.reloadUserIfNeeded_callsCount, 2)

// Calls secondary token refresh when enough time has passed
time.run(numberOfSeconds: 3)
AssertAsync.willBeEqual(testEnv.clientUpdater!.reloadUserIfNeeded_callsCount, 3)

// We set connectionId to nil after token expiration disconnect
XCTAssertNil(client.connectionId)
}
Expand Down Expand Up @@ -1147,7 +1170,8 @@ private class TestEnvironment<ExtraData: ExtraDataTypes> {
backgroundTaskSchedulerBuilder: {
self.backgroundTaskScheduler = MockBackgroundTaskScheduler()
return self.backgroundTaskScheduler!
}
},
timerType: VirtualTimeTimer.self
)
}()
}
Expand Down
18 changes: 13 additions & 5 deletions Sources/StreamChat/Config/Token.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import Foundation
public struct Token: Decodable, Equatable, ExpressibleByStringLiteral {
public let rawValue: String
public let userId: UserId
public let expiration: Date?

public var isExpired: Bool {
expiration.map { $0 < Date() } ?? false
}

/// Created a new `Token` instance.
/// - Parameter value: The JWT string value. It must be in valid format and contain `user_id` in payload.
Expand All @@ -26,13 +31,16 @@ public struct Token: Decodable, Equatable, ExpressibleByStringLiteral {
guard let userId = rawValue.jwtPayload?["user_id"] as? String else {
throw ClientError.InvalidToken("Provided token does not contain `user_id`")
}

self.init(rawValue: rawValue, userId: userId)
let expiration = (rawValue.jwtPayload?["exp"] as? Int64).map {
Date(timeIntervalSince1970: TimeInterval($0))
}
self.init(rawValue: rawValue, userId: userId, expiration: expiration)
}

init(rawValue: String, userId: UserId) {
init(rawValue: String, userId: UserId, expiration: Date?) {
self.rawValue = rawValue
self.userId = userId
self.expiration = expiration
}

public init(from decoder: Decoder) throws {
Expand All @@ -49,7 +57,7 @@ public extension Token {
///
/// Is used by `anonymous` token provider.
static var anonymous: Self {
.init(rawValue: "", userId: .anonymous)
.init(rawValue: "", userId: .anonymous, expiration: .distantFuture)
}

/// The token which can be used during the development.
Expand All @@ -64,7 +72,7 @@ public extension Token {

let jwt = [header, payload, devSignature].joined(separator: ".")

return .init(rawValue: jwt, userId: userId)
return .init(rawValue: jwt, userId: userId, expiration: .distantFuture)
}
}

Expand Down
2 changes: 2 additions & 0 deletions Sources/StreamChatTestTools/ChatClientUpdater_Mock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class ChatClientUpdaterMock<ExtraData: ExtraDataTypes>: ChatClientUpdater<ExtraD
var reloadUserIfNeeded_callsCount = 0
@Atomic var reloadUserIfNeeded_completion: ((Error?) -> Void)?
@Atomic var reloadUserIfNeeded_callSuper: (() -> Void)?
@Atomic var reloadUserIfNeeded_userConnectionProvider: _UserConnectionProvider<ExtraData>?

@Atomic var connect_called = false
@Atomic var connect_completion: ((Error?) -> Void)?
Expand All @@ -41,6 +42,7 @@ class ChatClientUpdaterMock<ExtraData: ExtraDataTypes>: ChatClientUpdater<ExtraD
) {
reloadUserIfNeeded_called = true
reloadUserIfNeeded_completion = completion
reloadUserIfNeeded_userConnectionProvider = userConnectionProvider
reloadUserIfNeeded_callSuper = {
super.reloadUserIfNeeded(
userInfo: userInfo,
Expand Down
3 changes: 2 additions & 1 deletion Sources/StreamChatTestTools/ChatClient_Mock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ public extension _ChatClient {
internetConnection: $4
)
}
)
),
tokenExpirationRetryStrategy: DefaultReconnectionStrategy()
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/Shared/TemporaryData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ extension String {
extension Token {
/// Returns a new `Token` with the provided `user_id` but not in JWT format.
static func unique(userId: UserId = .unique) -> Self {
.init(rawValue: .unique, userId: userId)
.init(rawValue: .unique, userId: userId, expiration: nil)
}
}

Expand Down

0 comments on commit ef7eaa7

Please sign in to comment.