protocol: change the format of handshake packets #54

aqrln · 2017-02-06T15:09:41Z

Introduce the concept of authentication strategies.
Implement login and anonymous strategies.
Make handshake packets include the strategy as an action field.
Update tests.
Update docs.

See more details about the implementation in the updated doc/protocol.md.

belochub · 2017-02-06T16:23:12Z

lib/connection.js

 };

 // Callback of authentication operation
 //   username - user login, if any


You can probably change order of arguments here as well, to correspond with function signature.

belochub · 2017-02-06T16:27:02Z

lib/simple-auth.js

-//   password - null (required by the auth callback contract)
+//   strategy - authentication strategy (only 'anonymous' is supported)
+//   credentials - authentication credentials
 //   callback - callback function that has signature (error, sessionId)


Callback signature here does not correspond to the callback you are calling on line 33.

belochub · 2017-02-06T16:32:50Z

lib/simple-auth.js

+  connection, application,
+  strategy, credentials, callback
+) => {
+  console.dir({ strategy, credentials });


Was it intentionally left here?

Leftover of fixing tests. Thanks for noticing it.

belochub · 2017-02-06T16:51:47Z

doc/protocol.md

+There are two supported strategies now:
+
+* `login` — authentication with login and password. The payload is an array of
+  two elements: user name and password, both represented as strings.


In this case it is usually more appropriate to write "username", not "user name".

aqrln · 2017-02-06T16:56:05Z

@belochub updated to resolve the issues and nits.

belochub

LGTM

* Introduce the concept of authentication strategies. * Implement `login` and `anonymous` strategies. * Make handshake packets include the strategy as an action field. * Update tests. * Update docs. Refs: #35 (comment)

aqrln · 2017-02-06T18:32:58Z

Rebased to resolve conflicts.

aqrln · 2017-02-06T18:55:14Z

@tshemsedinov awaiting your review.

tshemsedinov

@aqrln @belochub You can merge this changes but if you will do so, we will not use new JSTP for current projects. SO if there any opportunity to be backward compatible please do that. I'll hold that in mind and will try to change strategy to break compatibility this time.

aqrln · 2017-02-06T21:38:07Z

@tshemsedinov it's semver-major anyway, so it will go along with any other breaking changes. All non-breaking changes are still being landed on v0.6 too.

aqrln · 2017-02-06T21:42:05Z

@tshemsedinov and it is still possible to do a protocol-level compatibility fixup for v0.6 granted we have no users named 'login', 'anonymous' or 'session' :)
Whom we actually don't even if someone used such nicknames since the protocol-level authentication is not used in the current projects and all users are anonymous from the point of view of JSTP.

aqrln · 2017-02-06T22:22:25Z

@tshemsedinov @belochub it has just occurred to me that in fact the protocol-level compatibility will not be broken for our applications because of the reasons I told in the previous comment. Since only anonymous handshakes were used and authentication was performed using RPC methods, and anonymous handshakes syntax has not changed, we can update the server to JSTP 0.7 without updating the clients.

aqrln · 2017-02-06T22:23:27Z

@tshemsedinov @belochub so basically we don't even need backporting this PR to v0.6 and supporting v0.6 at all.

aqrln · 2017-02-06T22:31:41Z

If we introduce any new breaking changes that will not be feasible to work around, we'll release v0.7 in its current state, call new breaking changes v0.8 and continue landing non-breaking changes to v0.7.

* Introduce the concept of authentication strategies. * Implement `login` and `anonymous` strategies. * Make handshake packets include the strategy as an action field. * Update tests. * Update docs. Refs: #35 (comment) PR-URL: #54

aqrln · 2017-02-06T22:41:20Z

Landed in 9e55d3c.

* Introduce the concept of authentication strategies. * Implement `login` and `anonymous` strategies. * Make handshake packets include the strategy as an action field. * Update tests. * Update docs. Refs: #35 (comment) PR-URL: #54

This is a new and shiny first major release for `metarhia-jstp`. Changes include API refactoring and improvements, implementations of CLI, sessions, and application versions, native addon build optimizations, lots of bug fixes, test coverage increase, and other, less notable changes. This release also denotes the bump of the protocol version to v1.0. The only difference from the previous version of the protocol is that "old" heartbeat messages (`{}`) are now deprecated and `ping`/`pong` messages must be used for this purpose instead. Notable changes: * **src,build:** improve the native module subsystem *(Alexey Orlenko)* [#36](#36) **\[semver-minor\]** * **build:** compile in ISO C++11 mode *(Alexey Orlenko)* [#37](#37) **\[semver-minor\]** * **build:** improve error handling *(Alexey Orlenko)* [#40](#40) **\[semver-minor\]** * **lib:** refactor record-serialization.js *(Alexey Orlenko)* [#41](#41) **\[semver-minor\]** * **protocol:** change the format of handshake packets *(Alexey Orlenko)* [#54](#54) **\[semver-major\]** * **parser:** remove special case for '\0' literal *(Mykola Bilochub)* [#68](#68) **\[semver-major\]** * **client:** drop redundant callback argument *(Alexey Orlenko)* [#104](#104) **\[semver-major\]** * **client:** handle errors in connectAndInspect *(Alexey Orlenko)* [#105](#105) **\[semver-major\]** * **socket,ws:** use socket.destroy() properly *(Alexey Orlenko)* [#84](#84) **\[semver-major\]** * **cli:** add basic implementation *(Mykola Bilochub)* [#107](#107) **\[semver-minor\]** * **connection:** fix error handling in optional cbs *(Alexey Orlenko)* [#147](#147) **\[semver-major\]** * **lib:** change event signature *(Denys Otrishko)* [#187](#187) **\[semver-major\]** * **lib:** add address method to Server *(Denys Otrishko)* [#190](#190) **\[semver-minor\]** * **lib:** optimize connection events *(Denys Otrishko)* [#222](#222) **\[semver-major\]** * **lib:** refactor server and client API *(Denys Otrishko)* [#209](#209) **\[semver-major\]** * **lib,src:** rename term packet usages to message *(Denys Otrishko)* [#270](#270) **\[semver-major\]** * **lib:** emit events about connection messages *(Denys Otrishko)* [#252](#252) **\[semver-minor\]** * **connection:** make callback method private *(Alexey Orlenko)* [#306](#306) **\[semver-major\]** * **lib:** implement sessions *(Mykola Bilochub)* [#289](#289) **\[semver-major\]** * **connection:** use ping-pong instead of heartbeat *(Dmytro Nechai)* [#303](#303) **\[semver-major\]**

This is a new and shiny first major release for `metarhia-jstp`. Changes include API refactoring and improvements, implementations of CLI, sessions, and application versions, native addon build optimizations, lots of bug fixes, test coverage increase, and other, less notable changes. This release also denotes the bump of the protocol version to v1.0. The only difference from the previous version of the protocol is that "old" heartbeat messages (`{}`) are now deprecated and `ping`/`pong` messages must be used for this purpose instead. Notable changes: * **src,build:** improve the native module subsystem *(Alexey Orlenko)* [#36](#36) **\[semver-minor\]** * **build:** compile in ISO C++11 mode *(Alexey Orlenko)* [#37](#37) **\[semver-minor\]** * **build:** improve error handling *(Alexey Orlenko)* [#40](#40) **\[semver-minor\]** * **lib:** refactor record-serialization.js *(Alexey Orlenko)* [#41](#41) * **parser:** fix a possible memory leak *(Alexey Orlenko)* [#44](#44) **\[semver-minor\]** * **protocol:** change the format of handshake packets *(Alexey Orlenko)* [#54](#54) **\[semver-major\]** * **parser:** make parser single-pass *(Mykola Bilochub)* [#61](#61) * **parser:** remove special case for '\0' literal *(Mykola Bilochub)* [#68](#68) **\[semver-major\]** * **parser:** fix bug causing node to crash *(Mykola Bilochub)* [#75](#75) * **client:** drop redundant callback argument *(Alexey Orlenko)* [#104](#104) **\[semver-major\]** * **client:** handle errors in connectAndInspect *(Alexey Orlenko)* [#105](#105) **\[semver-major\]** * **socket,ws:** use socket.destroy() properly *(Alexey Orlenko)* [#84](#84) **\[semver-major\]** * **cli:** add basic implementation *(Mykola Bilochub)* [#107](#107) **\[semver-minor\]** * **connection:** fix error handling in optional cbs *(Alexey Orlenko)* [#147](#147) **\[semver-major\]** * **test:** add JSON5 specs test suite *(Alexey Orlenko)* [#158](#158) * **lib:** change event signature *(Denys Otrishko)* [#187](#187) **\[semver-major\]** * **lib:** add address method to Server *(Denys Otrishko)* [#190](#190) **\[semver-minor\]** * **parser:** implement NaN and Infinity parsing *(Mykola Bilochub)* [#201](#201) * **parser:** improve string parsing performance *(Mykola Bilochub)* [#220](#220) * **lib:** optimize connection events *(Denys Otrishko)* [#222](#222) **\[semver-major\]** * **lib:** refactor server and client API *(Denys Otrishko)* [#209](#209) **\[semver-major\]** * **lib,src:** rename term packet usages to message *(Denys Otrishko)* [#270](#270) **\[semver-major\]** * **lib:** emit events about connection messages *(Denys Otrishko)* [#252](#252) **\[semver-minor\]** * **lib:** implement API versioning *(Denys Otrishko)* [#231](#231) **\[semver-minor\]** * **lib:** allow to set event handlers in application *(Denys Otrishko)* [#286](#286) **\[semver-minor\]** * **lib:** allow to broadcast events from server *(Denys Otrishko)* [#287](#287) **\[semver-minor\]** * **connection:** make callback method private *(Alexey Orlenko)* [#306](#306) **\[semver-major\]** * **lib:** implement sessions *(Mykola Bilochub)* [#289](#289) **\[semver-major\]** * **connection:** use ping-pong instead of heartbeat *(Dmytro Nechai)* [#303](#303) **\[semver-major\]**

* Introduce the concept of authentication strategies. * Implement `login` and `anonymous` strategies. * Make handshake packets include the strategy as an action field. * Update tests. * Update docs. Refs: metarhia/jstp#35 (comment) PR-URL: metarhia/jstp#54

aqrln added connection feature-request protocol semver-major labels Feb 6, 2017

aqrln requested a review from tshemsedinov February 6, 2017 15:09

belochub reviewed Feb 6, 2017

View reviewed changes

aqrln mentioned this pull request Feb 6, 2017

doc: document protocol versioning policy #56

Closed

belochub reviewed Feb 6, 2017

View reviewed changes

aqrln force-pushed the new-handshake-format branch from b87f0c6 to 8e32661 Compare February 6, 2017 16:55

belochub approved these changes Feb 6, 2017

View reviewed changes

protocol: change the format of handshake packets

45e6696

* Introduce the concept of authentication strategies. * Implement `login` and `anonymous` strategies. * Make handshake packets include the strategy as an action field. * Update tests. * Update docs. Refs: #35 (comment)

aqrln force-pushed the new-handshake-format branch from 8e32661 to 45e6696 Compare February 6, 2017 18:32

tshemsedinov reviewed Feb 6, 2017

View reviewed changes

aqrln closed this Feb 6, 2017

aqrln deleted the new-handshake-format branch February 6, 2017 22:41

aqrln removed the feature-request label Mar 31, 2017

belochub mentioned this pull request Jan 22, 2018

v1.0.0 proposal #311

Merged

protocol: change the format of handshake packets #54

protocol: change the format of handshake packets #54

Uh oh!

Conversation

aqrln commented Feb 6, 2017

Uh oh!

belochub Feb 6, 2017

Choose a reason for hiding this comment

Uh oh!

belochub Feb 6, 2017

Choose a reason for hiding this comment

Uh oh!

belochub Feb 6, 2017

Choose a reason for hiding this comment

Uh oh!

aqrln Feb 6, 2017

Choose a reason for hiding this comment

Uh oh!

belochub Feb 6, 2017

Choose a reason for hiding this comment

Uh oh!

aqrln commented Feb 6, 2017

Uh oh!

belochub left a comment

Choose a reason for hiding this comment

Uh oh!

aqrln commented Feb 6, 2017

Uh oh!

aqrln commented Feb 6, 2017

Uh oh!

tshemsedinov left a comment

Choose a reason for hiding this comment

Uh oh!

aqrln commented Feb 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aqrln commented Feb 6, 2017

Uh oh!

aqrln commented Feb 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aqrln commented Feb 6, 2017

Uh oh!

aqrln commented Feb 6, 2017

Uh oh!

aqrln commented Feb 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aqrln commented Feb 6, 2017 •

edited

Loading

aqrln commented Feb 6, 2017 •

edited

Loading