Skip to content

Added auth_mechanism configuration - #74

Open
lukas8219 wants to merge 4 commits into
mainfrom
external-mechanism
Open

lukas8219 wants to merge 4 commits into
mainfrom
external-mechanism

Conversation

@lukas8219

Copy link
Copy Markdown

What

Allow to specify the auth_mechanism in the connection

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for configuring the AMQP authentication mechanism used during connection negotiation, allowing callers (and AMQP_URL) to specify mechanisms like PLAIN or EXTERNAL.

Changes:

  • Add an auth_mechanism parameter to AMQP::Client construction/start flows and propagate it into the connection handshake.
  • Parse auth_mechanism from AMQP_URL query parameters and from URI-based client construction.
  • Add a spec suite covering defaulting, URI parsing, and unsupported mechanism handling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/amqp-client/connection.cr Propagates auth_mechanism into StartOk and selects the appropriate response payload.
src/amqp-client.cr Adds auth_mechanism to client configuration, env/URI parsing, and passes it into Connection.start.
spec/auth_mechanism_spec.cr Adds coverage for defaulting, explicit mechanism selection, URI parsing, and unsupported mechanism errors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/amqp-client.cr
Comment thread src/amqp-client/connection.cr
Comment thread src/amqp-client/connection.cr Outdated
- Validate auth_mechanism early in `connect` to fail fast without network I/O
- Use a dedicated `plain_response` variable for clarity
- Raise `AMQP::Client::Error` instead of `ArgumentError` for consistency

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread src/amqp-client.cr
Comment thread src/amqp-client/connection.cr
Comment thread src/amqp-client/connection.cr
Address Copilot review feedback: mechanisms passed via constructor,
setter, URI, or env are now upper-cased to their canonical form, so
values like "plain"/"external" are accepted instead of rejected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

2 participants