From 88bca43fb131e7035bfecfd7d6dda09add2ed0dd Mon Sep 17 00:00:00 2001 From: Marat Alekperov Date: Sun, 28 Dec 2025 16:53:24 +0000 Subject: [PATCH 1/3] Remove redundant offset check method `hasTimeOffsetWithValue` as well as checking `replacedOptions.queryTime` flag together with `hasTimeOffset`. Address #2147 by removing setting `self.options.queryTime = NO`. Fix test to take options for the second call to `authorize`, otherwise `queryTime` is false by default and the test doesn't make much sense. --- Source/ARTAuth.m | 9 +-------- Test/AblyTests/Tests/AuthTests.swift | 2 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/Source/ARTAuth.m b/Source/ARTAuth.m index ba771520c..232be1475 100644 --- a/Source/ARTAuth.m +++ b/Source/ARTAuth.m @@ -216,7 +216,6 @@ - (void)storeOptions:(ARTAuthOptions *)customOptions { self.options.authMethod = customOptions.authMethod; self.options.authParams = [customOptions.authParams copy]; self.options.useTokenAuth = customOptions.useTokenAuth; - self.options.queryTime = NO; } - (ARTTokenParams *)mergeParams:(ARTTokenParams *)customParams { @@ -701,7 +700,7 @@ - (void)createTokenRequest:(ARTTokenParams *)tokenParams options:(ARTAuthOptions return nil; } - if ([self hasTimeOffsetWithValue] && !replacedOptions.queryTime) { + if ([self hasTimeOffset]) { currentTokenParams.timestamp = [self currentDate]; callback([currentTokenParams sign:replacedOptions.key], nil); return nil; @@ -766,10 +765,6 @@ - (BOOL)hasTimeOffset { return _timeOffset != nil; } -- (BOOL)hasTimeOffsetWithValue { - return _timeOffset != nil && _timeOffset.doubleValue > 0; -} - - (void)discardTimeOffset { // This may run after dealloc has been called in _rest. I've seen this // happen when rest.auth is put in a variable, even if (apparently) that @@ -804,8 +799,6 @@ - (void)clearTimeOffset; { } - (void)fetchServerTimeWithCompletion:(void (^)(NSDate * _Nullable, ARTErrorInfo * _Nullable))completion { - // TODO: I don't know why elsewhere we use hasTimeOffsetWithValue; it's causing us to ignore the offset in the case where the stored time offset indicates that the local clock is ahead of the server clock. See https://github.com/ably/ably-cocoa/issues/2148 - if ([self hasTimeOffset]) { NSDate *serverTime = [self currentDate]; ARTLogDebug(self.logger, @"Server time fetch calculated time %@ from offset", serverTime); diff --git a/Test/AblyTests/Tests/AuthTests.swift b/Test/AblyTests/Tests/AuthTests.swift index ad53c2653..04dc4860f 100644 --- a/Test/AblyTests/Tests/AuthTests.swift +++ b/Test/AblyTests/Tests/AuthTests.swift @@ -3479,7 +3479,7 @@ class AuthTests: XCTestCase { rest.auth.internal.testSuite_forceTokenToExpire() waitUntil(timeout: testTimeout) { done in - rest.auth.authorize(nil, options: nil) { tokenDetails, error in + rest.auth.authorize(nil, options: authOptions) { tokenDetails, error in XCTAssertNil(error) guard tokenDetails != nil else { fail("TokenDetails is nil"); done(); return From eafd464912b8d51aabb74782948864b0e35478f6 Mon Sep 17 00:00:00 2001 From: Marat Alekperov Date: Sun, 28 Dec 2025 16:59:00 +0000 Subject: [PATCH 2/3] Add a version of the test that checks the case where the mockServerDate is behind the local clock. --- Test/AblyTests/Tests/AuthTests.swift | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Test/AblyTests/Tests/AuthTests.swift b/Test/AblyTests/Tests/AuthTests.swift index 04dc4860f..75f47d2a6 100644 --- a/Test/AblyTests/Tests/AuthTests.swift +++ b/Test/AblyTests/Tests/AuthTests.swift @@ -3438,13 +3438,13 @@ class AuthTests: XCTestCase { } // RSA10k - - func test__115__authorize__server_time_offset__should_obtain_server_time_once_and_persist_the_offset_from_the_local_clock() throws { + + func _test__115__authorize__server_time_offset__should_obtain_server_time_once_and_persist_the_offset_from_the_local_clock(_ offset: TimeInterval) throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) let rest = ARTRest(options: options) - let mockServerDate = Date().addingTimeInterval(120) + let mockServerDate = Date().addingTimeInterval(offset) rest.auth.internal.testSuite_returnValue(for: NSSelectorFromString("handleServerTime:"), with: mockServerDate) let currentDate = Date() @@ -3496,6 +3496,14 @@ class AuthTests: XCTestCase { } } + func test__115__authorize__server_time_offset__should_obtain_server_time_once_and_persist_the_offset_from_the_local_clock() throws { + try _test__115__authorize__server_time_offset__should_obtain_server_time_once_and_persist_the_offset_from_the_local_clock(120) + } + + func test__115b__authorize__server_time_offset__should_obtain_server_time_once_and_persist_the_offset_from_the_local_clock() throws { + try _test__115__authorize__server_time_offset__should_obtain_server_time_once_and_persist_the_offset_from_the_local_clock(-119) // looks like 120 is out of range in server side comparison + } + func test__116__authorize__server_time_offset__should_be_consistent_the_timestamp_request_with_the_server_time() throws { let test = Test() let options = try AblyTests.commonAppSetup(for: test) From 1d6bee0e789fb501c892a93528b7a9a3a068af99 Mon Sep 17 00:00:00 2001 From: Marat Alekperov Date: Sun, 4 Jan 2026 01:30:46 +0000 Subject: [PATCH 3/3] Remove checks that have no sense with new logic. --- Test/AblyTests/Tests/AuthTests.swift | 44 +++++----------------------- 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/Test/AblyTests/Tests/AuthTests.swift b/Test/AblyTests/Tests/AuthTests.swift index 75f47d2a6..eba487272 100644 --- a/Test/AblyTests/Tests/AuthTests.swift +++ b/Test/AblyTests/Tests/AuthTests.swift @@ -3539,6 +3539,8 @@ class AuthTests: XCTestCase { } } } + + // RSA10k func test__117__authorize__server_time_offset__should_be_possible_by_lib_Client_to_discard_the_cached_local_clock_offset() throws { let test = Test() @@ -3553,7 +3555,7 @@ class AuthTests: XCTestCase { defer { hook.remove() } waitUntil(timeout: testTimeout) { done in - rest.auth.authorize(nil, options: nil) { tokenDetails, error in + rest.auth.authorize(nil, options: options) { tokenDetails, error in XCTAssertNil(error) guard let tokenDetails = tokenDetails else { fail("TokenDetails is nil"); done(); return @@ -3578,13 +3580,15 @@ class AuthTests: XCTestCase { rest.auth.internal.testSuite_forceTokenToExpire() waitUntil(timeout: testTimeout) { done in - rest.auth.authorize(nil, options: nil) { tokenDetails, error in + rest.auth.authorize(nil, options: options) { tokenDetails, error in XCTAssertNil(error) guard tokenDetails != nil else { fail("TokenDetails is nil"); done(); return } - XCTAssertNil(rest.auth.internal.timeOffset) - XCTAssertEqual(serverTimeRequestCount, 1) + // RSA10k: "The client library itself MAY internally discard the cached local clock offset in situations in which it may have been invalidated, such as if there is a local change to the date, time, or timezone, of the client device", + // so I assume in this case the time should be requested again which makes sense. + XCTAssertNotNil(rest.auth.internal.timeOffset) + XCTAssertEqual(serverTimeRequestCount, 2) done() } } @@ -3619,38 +3623,6 @@ class AuthTests: XCTestCase { } } - func test__119__authorize__server_time_offset__should_request_server_time_when_queryTime_is_true_even_if_the_time_offset_is_assigned() throws { - let test = Test() - let options = try AblyTests.commonAppSetup(for: test) - let rest = ARTRest(options: options) - - var serverTimeRequestCount = 0 - let hook = rest.internal.testSuite_injectIntoMethod(after: #selector(rest.internal._time)) { - serverTimeRequestCount += 1 - } - defer { hook.remove() } - - let fakeOffset: TimeInterval = 60 // 1 minute - rest.auth.internal.setTimeOffset(fakeOffset) - - let authOptions = ARTAuthOptions() - authOptions.key = options.key - authOptions.queryTime = true - - waitUntil(timeout: testTimeout) { done in - rest.auth.authorize(nil, options: authOptions) { tokenDetails, error in - XCTAssertNil(error) - XCTAssertNotNil(tokenDetails) - XCTAssertEqual(serverTimeRequestCount, 1) - guard let timeOffset = rest.auth.internal.timeOffset?.doubleValue else { - fail("Server Time Offset is nil"); done(); return - } - XCTAssertNotEqual(timeOffset, fakeOffset) - done() - } - } - } - func test__120__authorize__server_time_offset__should_discard_the_time_offset_in_situations_in_which_it_may_have_been_invalidated() throws { let test = Test() let rest = ARTRest(options: try AblyTests.commonAppSetup(for: test))