ptejada / artillery-engine-socketio-v3

Socket.IO v3 engine for Artillery
Mozilla Public License 2.0
22 stars 15 forks source link

Bug fix for capturing json value & new options, beforeRequest and reconnect #3

Closed gruffT closed 3 years ago

gruffT commented 3 years ago

Hey Pablo

Thanks so much for creating this great plugin. I recently needed to simulate a login flow so I added a reconnect option with the ability to set the extraHeaders config field via a processor before triggering the new connection.

Hopefully this is helpful!

I have a separate test project here but I couldn't see an easy way to add testing to this library.

I hope this is useful and you would consider a merge.

Gareth

ptejada commented 3 years ago

The code looks good. Do note that in Socket.IO v3 the auth was added so you alternatively use this option instead headers.

gruffT commented 3 years ago

Thanks Pablo. Patching the options with extraHeaders from the context object felt a little hacky but I couldn't see a way around it. Good point on the auth option, do you think it would be also worth updating that on reconnect if set on the context?

Cheers

Gareth

On Sun, Apr 11, 2021 at 7:02 PM Pablo Tejada @.***> wrote:

The code looks good. Do note that in Socket.IO v3 the auth was added so you alternatively use this option instead headers.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ptejada/artillery-engine-socketio-v3/pull/3#issuecomment-817347195, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYIUMOZVTQZYYMXH53XW5TTIHPZVANCNFSM42WZYSFA .

ptejada commented 3 years ago

I understand what you are trying to do but is just looks like very odd scenario, to open a websocket connection to get a token then open a new connection to use that token on the same service.

Regardless of the changes needed to support your workflow and assuming you are using middlewares, you should have two middlewares: one to handle password based authentication and the other token based. If the token is provided then the password middleware should be skipped but if a password based request is detected then the token should be generated and pass on to the next middleware which should be the token base auth middleware.

The idea is that you should be able to add those two middleware to any namespace you want to secure.

gruffT commented 3 years ago

Thanks Pablo. Your comments make a lot of sense.

The changes in this PR are fine for my specific workflow right now. Thanks again for contributing this plug-in to the community. I hope it gets merged into the main Artillery project!

maitrungduc1410 commented 3 years ago

Is there any progress on this?

gruffT commented 3 years ago

The fork is approved for merge but not merged into master yet. I don't have permission to do that. Please let me know if there is anything i need to do to move this forward.