amqp-node / amqplib

AMQP 0-9-1 library and client for Node.JS
https://amqp-node.github.io/amqplib/
Other
3.67k stars 474 forks source link

Feature: Add support for AMQP 0.9.1 "update-secret" #755

Closed mortdiggiddy closed 5 months ago

mortdiggiddy commented 7 months ago

I have been trying (in vain) to create work around for refreshing the token based access to rabbitmq. I am using rabbitmq 3.12.

The update-secret on the connection was introduced a few years back: https://rabbitmq.com/amqp-0-9-1-reference.html#connection.update-secret

This allows you to dynamically update your password or OAUTH token without refreshing the connection and channels. This is extremely important when dealing with the rabbitmq_auth_backend_oauth2 plugin that was released, in conjunction with authenticating MQTT clients with the rabbitmq_mqtt and rabbitmq_web_mqtt plugins.

Multiple AMQP clients have implemented this:

I am hoping that a developer can add this feature to amqplib. This would aid us greatly because we depend on NestJS, which uses amqplib at the core.

Thank you

cressie176 commented 7 months ago

Hi @mortdiggiddy,

I'm not getting much time to work on amqplib features at the moment, but if you or anyone else wanted to give it a go, the best starting place would be to familiarise yourself with how amqplib generates the RabbitMQ operation definitions.

There's a script called generate-defs which is called from the makefile. This downloads an old version of the RabbitMQ code generation spec from before update-secret was added.

You could try updating the makefile to download the latest spec is here and see if the tests still pass, then work out how to add the updateSecret method to the callback model and channel model. The RabbitMQ spec says update-secret returns a confirmation, so it should expect a reply.

cressie176 commented 6 months ago

Hi @mortdiggiddy,

I've added an updateSecret method to both the callback and promise apis on this branch. Are you able to checkout the brandh and confirm everything works as you expect please?

await connection.updateSecret(Buffer.from('new secret'), 'some reason');

Because update-secret is a RabbitMQ extension you don't WireShark decodes both the operation and reply as Connection.Unknown

mortdiggiddy commented 6 months ago

I will be testing locally here in the next few days. Just adding this comment here for traceability.

The test will involve periodic OAUTH OpenID Connect token fetch from an IAM server, which dumps it into a running Rabbit MQ 3.13 Docker container instance equipped with the rabbitmq_auth_backend_oauth2 plugin.

Under normal conditions when this plugin is enabled and an RPC publish, send_to_queue, etc is invoked on AmqpConnectionManager from amqp-connection-manager (which delegates to amqplib) a 403 ACCESS_REFUSED error is thrown. This is one of the critical errors, which requires a closure of the client and all channels attached to it. A new client and channel must be created.

If this update is working, we can update the access token before expiration, and all is well.

It will be up to developers to integrate some type of automatic refresh mechanism that periodically checks for tokens approaching expiration, so that the update-secret method can be invoked before it expires with a new token. If operating entirely on the backend, this can be handled by REDIS using keyspace notifications, and using a SETEX to automatically expire a token when it reaches about 80% of its TTL (safety factor). Once the token is gone from REDIS, a notification can be sent to trigger the refresh.

cressie176 commented 6 months ago

Any luck with the testing @mortdiggiddy?

mortdiggiddy commented 6 months ago

So far so good, a few more test and I will update this.

cressie176 commented 5 months ago

Any more updates @mortdiggiddy?

cressie176 commented 5 months ago

@mortdiggiddy?

mortdiggiddy commented 5 months ago

Stephen,

Apologies for the delayed response. I have tested this in an isolated environment, and created a branch to update AmqpConnectionManager (https://github.com/jwalton/node-amqp-connection-manager),

This is working as expected. I have also tested the ability to send a JWT as the password using the rabbitmq-oauth2 plugin. This is working, the TTL shown in the debug logs of a running RabbitMQ 3.13-management container shows the updated expiration countdown.

I will now ask the code owner of AmqpConnectionManager to update his code as well inside this class: https://github.com/jwalton/node-amqp-connection-manager/blob/master/src/AmqpConnectionManager.ts

I will also be reaching out to the NestJS developers to allow for the same inside these classes:

https://github.com/nestjs/nest/blob/master/packages/microservices/client/client-rmq.ts https://github.com/nestjs/nest/blob/master/packages/microservices/server/server-rmq.ts

https://github.com/nestjs/nest/blob/master/packages/microservices/client/client-mqtt.ts https://github.com/nestjs/nest/blob/master/packages/microservices/server/server-mqtt.ts

mortdiggiddy commented 5 months ago

@cressie176 Will the 'generate defs' be updated to the new amqplib protocol as well? I am guessing this is how the ./defs class is generated. I forgot to ask how this will be updated once you merge the changes.

cressie176 commented 5 months ago

Yes, they'll be updated. Currently the process is to checkout from the tag to a clean directory, then test, build and publish. I want to change this so the defs are added to source control, and automate the release process. I might have a go at that next.

cressie176 commented 5 months ago

Published in v0.10.4. Thank you for the suggestion and all your help testing @mortdiggiddy