Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mortdiggiddy opened this issue Feb 21, 2024 · 11 comments
Closed

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

mortdiggiddy opened this issue Feb 21, 2024 · 11 comments

Comments

@mortdiggiddy
Copy link

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
Copy link
Collaborator

cressie176 commented Feb 21, 2024

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
Copy link
Collaborator

cressie176 commented Feb 22, 2024

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
Copy link
Author

mortdiggiddy commented Feb 23, 2024

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
Copy link
Collaborator

Any luck with the testing @mortdiggiddy?

@mortdiggiddy
Copy link
Author

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

@cressie176
Copy link
Collaborator

Any more updates @mortdiggiddy?

@cressie176
Copy link
Collaborator

@mortdiggiddy?

@mortdiggiddy
Copy link
Author

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
Copy link
Author

@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
Copy link
Collaborator

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants