RSA4b #518

ricardopereira · 2016-10-17T19:18:19Z

No description provided.

ricardopereira · 2016-10-18T10:09:04Z

🚦 If the spec is well implemented then I will fix the client code.

mattheworiordan

Sorry, but you need to instance the libraries with a means to renew

mattheworiordan · 2016-10-18T10:54:01Z

Spec/Auth.swift

+                it("in REST, if the token creation failed or the subsequent request with the new token failed due to a token error, then the request should result in an error") {
+                    let options = AblyTests.clientOptions()
+                    options.key = AblyTests.commonAppSetup().key
+                    options.token = getTestToken()


I am not following why you provide a token. This spec states "When the client does have a means to renew the token automatically" yet you've provided an explicit token and thus no means to renew the token?

Fixed 5687309.

mattheworiordan · 2016-10-18T10:54:45Z

Spec/Auth.swift

+                it("in Realtime, if the token creation failed or the subsequent request with the new token failed due to a token error, then the connection should move to the FAILED state") {
+                    let options = AblyTests.clientOptions()
+                    options.key = AblyTests.commonAppSetup().key
+                    options.token = getTestToken(ttl: 0.5)


As above, providing a token explicitly is not a means to renew (authCallback or authUrl should be used ideally to demonstrate the point)

Fixed 5687309.

ricardopereira · 2016-10-18T18:31:36Z

@mattheworiordan Still failing. PTAL and if it's ok, then I fix the client.

tcard · 2016-10-20T16:34:34Z

Spec/Auth.swift

-                    let options = AblyTests.clientOptions()
-                    options.key = AblyTests.commonAppSetup().key
-                    options.token = getTestToken()
+                    let options = AblyTests.commonAppSetup()


This still won't have a means to renew the token or even use token auth, as it will use the key.

tcard · 2016-10-20T16:35:27Z

Spec/Auth.swift

                            }
-                            expect(error.code).to(equal(40141))
+                            print(errorInfo.code)


mattheworiordan

Sorry, seems you didn't copy & paste the spec correctly?

mattheworiordan · 2016-10-31T15:29:43Z

Spec/Auth.swift

+                }
+
+                // RSA4b
+                it("in Realtime, if the token creation failed or the subsequent request with the new token failed due to a token error, then the connection should move to the FAILED state") {


This description is wrong it's the wrong spec:

If the token creation failed or the subsequent request with the new token failed due to a token error, then the request should result in an error

It does not become failed, it only becomes failed if the connection fails with the provided token. If the token generation fails, it moves to disconnected and reports the error. Unfortunately this is entirely wrong.

ricardopereira · 2016-10-31T21:43:55Z

@mattheworiordan PTAL. If it's ok then I'll fix the client.

mattheworiordan · 2016-11-01T09:50:20Z

Thanks, LGTM, 👍

ricardopereira · 2016-11-02T10:15:05Z

Tests are passing now but I'm not sure about one thing.

ricardopereira · 2016-11-02T10:24:21Z

Source/ARTWebSocketTransport.m

+                    }
+                    else {
+                        // RSA4b
+                        [selfStrong.delegate realtimeTransportDisconnected:selfStrong withError:[[ARTRealtimeTransportError alloc] initWithError:error type:ARTRealtimeTransportErrorTypeAuth url:self.websocketURL]];


@mattheworiordan I'm not sure about this change I made.

Regarding one of your comments, I'm confused with RSA15c, it states that the Realtime should transition the connection state to FAILED which is incoherent with what you said.

If you're referring to #518 (diff) then that's a different spec

mattheworiordan · 2016-12-02T14:46:49Z

Spec/Auth.swift

+                }
+
+                // RSA4b
+                it("in Realtime, if the connection fails to a token error, then the connection should move to the FAILED state and reports the error") {


I think this should say "fails due to a terminal token error" to make it clear this is different to https://github.com/ably/ably-ios/pull/518/files#diff-b5d84fe9bce26902a7481c2ffec84fffR279

mattheworiordan

One small comment to change wording but otherwise approved

… move to the DISCONNECTED state and reports the error

@synchronized

* Start 0.9 version * TO3k7 (#513) * Start 0.9 version * Add fallbackHostsUseDefault option * Fallback: add initialiser accepting client options * TO3k7 * Add ARTFallback+Private * Default fallbackHosts as Array of Strings * fixup! TO3k7 * fixup! Add fallbackHostsUseDefault option * fixup! TO3k7 * Update RSC15a for 0.9 (#515) * Start 0.9 version * Update RSC15a: default fallback hosts * Default fallbackHosts as Array of Strings * RSA4a (#517) * Test suite: add TestProxyHTTPExecutor.simulateIncomingServerErrorOnNextRequest * RSA4a * Fix Realtime: indicate an error and not retry the request when the server responds with a token error * Update RSC15e (#514) * Remove specs RSA10c and RSA10d (#522) * RSA10l (#524) * Auth: deprecate `authorise` in favor of `authorize`. * RSA10l * Use `authorize` instead of `authorise` (close #496) * Remove `AuthOptions.force` (#527) * Remove AuthOptions.force * Fix: make a single attempt to reissue the token and resend the request * Fix RSA10a * Fix RSC9 * Remove `prepareAuthorisationHeader` access from test suite * Fix RSA10a * Update RSC15b for 0.9 (#516) * Update RSC15b * Fix: REST fallback should only apply when the default host is used * Update RSA10a for 0.9 (#520) * Update RSA10a * Fix: authorize should change auth method to Token for future requests * Update RSA10j for 0.9 (#521) * Update RSA10j * Fix: ttl when omitted should set the default value * Fix RTN15h * RTC8 (#526) * Add ARTAuthDetails * RTC8a * RTC8a1 (part 1) * RTC8a1 (part 2) * RTC8a1 (part 3) * RTC8a2 * RTC8a3 * RTC8b * RTC8b1 * RTC8c * Send AUTH protocol message on each authorize * Fix RTC8 * Test suite: `splitDone`, when a test fails, get the right location of the failure * RSA4b (#518) * RSA4b * Fix REST: should retry the request once if a token error occurs * Fix Realtime: if the token creation failed then the connection should move to the DISCONNECTED state and reports the error * RSA4c (#519) * RSA4c * Fix: 80019 and description of the underlying failure should be emitted * Fix: if connected and the client receives an ARTProtocolMessageAuth, then authorise * Fix: if authUrl or authCallback fails and is CONNECTED then the connection should remain CONNECTED - RSA4c3 * Add `artDispatchScheduled` and a way to cancel the scheduled block * Fix: authUrl/authCallback attempt times out after realtimeRequestTimeout * Test suite: reset networkConnectEvent after spec * Enhance: debug info * fixup! Fix: authUrl/authCallback attempt times out after realtimeRequestTimeout * fixup! Fix: if connected and the client receives an ARTProtocolMessageAuth, then authorise * Fix: every realtime auth attempt should check if deadline is reached * Rename createWithNSError to createFromNSError * Fix: remove Auth dependency from WebSocketTransport * Fix: should first authorize and then connect the transport * RTN22 (#537) * RTN22 * RTN22a * Fix: realtime transport can be nil * Fix: realtime should renew token by transitioning to CONNECTING * Update RTL3 for 0.9 (#544) * Update RTL3a * Update RTL3b and RTL3c * RTL3d * RTL3e * RTL14 (#550) * UPDATE event (#559) * UPDATE event (replace ERROR event) * Fix: Connection should emit an UPDATE event * Fix RTC8a1 * Fix specs and legacy tests * RTN4h * RTN4f * RTN24 * Update RTL2 for 0.9 (#543) * Add Realtime Channel Suspended state * Add ProtocolMessageActionToStr method * Add ChannelStateChange type * Update RTL2 * RTL2f: pending - functionality hasn't been deployed * Use ChannelStateChange on channel event emitter * Test suite: simulate client suspension with before suspension callback * Update tests using channel events * Fix RTL14 * Fix: Channel on suspended should transition to SUSPENDED state * Remove RTN18 * Fix: set Suspended on all channels when Connection moves to Suspended * RTL2g * Fix RTL12 * Fix RTL3d * Fix: channel should reattach when connection is Connected * Fix: should resume connection when the connection is Suspended * Fix RTN11 * Fix RTL3e * Fix RTC8a1 * Remove testSuspendingDetachesChannel * Fix RTL3d * Fix: channel is SUSPENDED then operation will result in an error * Update RTL13 for 0.9 (#549) * RTL13a * RTL13b * RTL13c * Add ClientOptions.channelRetryTimeout * RealtimeChannel reattach after timeout * Fix: if the channel receives a server initiated DETACHED message and if the channel is in the ATTACHED or SUSPENDED states, then an attempt to reattach the channel should be made immediately * Fix: move to Suspended if attach times out * Fix: if the channel receives a server initiated DETACHED message and the channel is Attaching * Remove test about #454 (replaced by RTL13) * Update RTL4f * Update RTL4 for 0.9 (#545) * RTL4i * RTL4h * Update RTL4e * Fix: attach request should be treated as though it has failed and the channel should transition to the SUSPENDED state * Fix: attach after Detaching * Fix: if it fails to detach then move back to ATTACHED * Fix RTL5f * Update RTL6c for 0.9 (#547) * Update RTL6c2 * Update RTL6c4 * RTL6c3 for Detached * Update RTL6c4 for channel Failed * Better code completion * Add ChannelStateChange.event property (#561) * Update RTN4 for 0.9 (#560) * Update RTN4e * Update RTN4f * Add ConnectionStateChange.event property * Update RTL5 for 0.9 (#546) * RTL5j * Update RTL5a * RTL5i * Fix: if channel is SUSPENDED then the detach request transitions immediately to DETACHED state * Fix: if channel is ATTACHING then do the detach operation after the completion of the attaching * Remove testSkipsFromAttachingToDetaching * Fix: presence sync can fail * Fix RTP11b * Fix RTP9e * RTP5: pending * Fix RTL6c3 * Fix: should check channel state when a queued message is processed * TR4i (#562) * ARTProtocolMessageFlag enum: use NS_OPTIONS to define a bitmask * TR4i * Fix: should indicate that the channel has been resumed or not (RESUMED flag) * Swift performance: speed up code completion (#569) * Speed up code completion - This problem is fixed on Xcode 8 but since we are still using Xcode 7 I decided to change that code. * Test suite: addMembersSequentiallyToChannel should return the realtime client * Test suite: set AsyncDefaults.Timeout with default value * Remove pending tests (#542) * Fix: RTP6c pending test * RSL1g4: remove pending * RTP2: remove pending * Fix RTC8 * Update RTP3 (#566) * Update RTP8d (#572) * RTP17 (#571) * PresenceMap: list of internal members * RTP17 * Add ARTPresenceActionToStr method * RTP17: pending * Update RTP2 for 0.9 (#563) * RTP2: remove pending * RTP2a * RTP2b * Test suite: set Nimble.AsyncDefaults.Timeout - increase the default timeout value from all async expectations * RTP2c * RTP2d * RTP2e * RTP2f * RTP2g * Test suite: ARTPresenceMessage convenience initializer * Test suite: NSDate custom operators - convenience for use of `dateByAddingTimeInterval` * Fix RTP2b1 * Fix RTP2b2 * PresenceMap: compare for newness * Fix RTP2 * Remove warnings * RTP18 (#567) * Test suite: ARTPresenceMessage convenience initializer * RTP18 * RTP18a * RTP18b * RTP18c * Update RSA9h for 0.9 (#574) * Auth: optional arguments * Update RSA9h * Update RSA8e for 0.9 (#573) * Update RSA8e * Auth: optional arguments * Comments * Update RSA10g for 0.9 (#575) * Auth: optional arguments * Update RSA10g * Auth: subsequent authorizations with stored values * Update RTP5 for 0.9 (#570) * Update RTP5 * Update RTP5a * Update RTP5b * RTP5c * Test suite: replaceAcksWithNacks - better code completion * RTP5f * PresenceMap: existing members before Sync * Fix: presence get members should not wait for sync if sync is not in progress * PresenceMap: reenter local member * Test suite: replacing acks with nacks even with Presence action * Fix: should queue messages before the attach operation * Test suite: ARTPresenceAction description * Better debug info * Fix: should continue incrementing msgSerial serially if the connection resumes successfully * Remove warnings * Test suite: timings * RTP19 (#568) * RTP19 * RTP19a * Fix: if channel resumed successfully then do not start sync * Enhance RTP5: presence get should not have Absent members * Fix race condition * Fix #583: update httpRequestTimeout and httpMaxRetryDuration * EventEmitter: use @synchronized because NSMutableArray are not thread safe * Thread safety (#586) * New EventEmitter (using NSNotificationCenter) - In most ways that matter NSNotificationCenter is thread safe. You can add/remove observers from any thread and you can post notifications from any thread. * Events for the EventEmitter * Fix: should cancel timers when connection times out * Fix: new state change can occur before receiving publishing acknowledgement * Test suite: async forced transitions * Test suite: ack order * Test suite: stop when there's no internet * Fix: instance objects released to soon * Performed a static analysis from Xcode * fixup! Test suite: ack order * Memory leak: call session invalidate to dispose of its strong reference to the delegate * fixup! Test suite: ack order * Fix RTN19a: guarantee of a new transport (check transport reference) * Fix: ACK or NACK has not yet been received for a message, the client should consider the delivery of those messages as failed * Enhance RTN14b: better timings * Fix: REST and Realtime, wait for last operation to release the object * fixup! Test suite: ack order * Fix: cancel timers when a connection gets closed * fixup! Test suite: ack order * Test suite: timings * fixup! Enhance RTN14b: better timings * Test suite: close connections * Fix: turn off immediately reachability when close occurs * Fix RTC1d: wait for host is not reachable error * fixup! Test suite: ack order * Travis update * Fix RTN19a

ricardopereira changed the base branch from master to 0-9-master October 17, 2016 19:18

ricardopereira force-pushed the rsa4b branch from d9cbb14 to fb3b2af Compare October 17, 2016 19:20

mattheworiordan requested changes Oct 18, 2016

View reviewed changes

tcard suggested changes Oct 20, 2016

View reviewed changes

mattheworiordan requested changes Oct 31, 2016

View reviewed changes

tcard approved these changes Nov 1, 2016

View reviewed changes

ricardopereira force-pushed the rsa4b branch 3 times, most recently from d80861f to fb1f5bb Compare November 2, 2016 09:18

ricardopereira commented Nov 2, 2016

View reviewed changes

ricardopereira force-pushed the 0-9-master branch from 0857337 to 5728e01 Compare November 30, 2016 12:03

ricardopereira force-pushed the rsa4b branch 2 times, most recently from d847d03 to ccba263 Compare December 1, 2016 14:45

ricardopereira added 2 commits December 1, 2016 14:47

RSA4b

50343a7

Fix REST: should retry the request once if a token error occurs

ef60187

ricardopereira force-pushed the rsa4b branch 2 times, most recently from ed273b4 to d252545 Compare December 2, 2016 10:24

mattheworiordan reviewed Dec 2, 2016

View reviewed changes

mattheworiordan approved these changes Dec 2, 2016

View reviewed changes

Fix Realtime: if the token creation failed then the connection should…

e125f6b

… move to the DISCONNECTED state and reports the error

ricardopereira force-pushed the rsa4b branch from d252545 to e125f6b Compare December 5, 2016 09:46

ricardopereira merged commit 89a0137 into 0-9-master Dec 5, 2016

ricardopereira deleted the rsa4b branch December 5, 2016 11:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RSA4b #518

RSA4b #518

ricardopereira commented Oct 17, 2016

ricardopereira commented Oct 18, 2016

mattheworiordan left a comment

mattheworiordan Oct 18, 2016

ricardopereira Oct 18, 2016

mattheworiordan Oct 18, 2016

ricardopereira Oct 18, 2016

ricardopereira commented Oct 18, 2016

tcard Oct 20, 2016

tcard Oct 20, 2016

mattheworiordan left a comment

mattheworiordan Oct 31, 2016

ricardopereira commented Oct 31, 2016

mattheworiordan commented Nov 1, 2016

ricardopereira commented Nov 2, 2016

ricardopereira Nov 2, 2016

mattheworiordan Nov 7, 2016

mattheworiordan Dec 2, 2016

ricardopereira Dec 5, 2016

mattheworiordan left a comment

RSA4b #518

RSA4b #518

Conversation

ricardopereira commented Oct 17, 2016

ricardopereira commented Oct 18, 2016

mattheworiordan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricardopereira commented Oct 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattheworiordan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricardopereira commented Oct 31, 2016

mattheworiordan commented Nov 1, 2016

ricardopereira commented Nov 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattheworiordan left a comment

Choose a reason for hiding this comment