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

Add SASL EXTERNAL authentication mechanism #93

Closed
wants to merge 2 commits into from

Conversation

AnMakc
Copy link

@AnMakc AnMakc commented Sep 1, 2016

Added support for external (e.g. X509 client cert) authentication mechanism.
Code is backward-compatible (old usages of Login will keep working).

To authenticate using SASL EXTERNAL mechanism, pass following object as Login to Connection constructor:
AMQP::Login(AMQP::LOGIN_EXTERNAL)

@@ -44,15 +59,20 @@ class Login
/**
* Default constructor
*/
Login() : _user("guest"), _password("guest") {}
Login() : _mechanism(LOGIN_PLAIN), _user("guest"), _password("guest") {}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use delegating constructor instead of copy-pasting

…ve LoginMechanisms definition into Login class.
@AnMakc
Copy link
Author

AnMakc commented Sep 1, 2016

Code refactored.

@trevorperrin
Copy link
Contributor

What is the status of this Pull Request? This is something I was going to add, but found there was already a request for it.

@EmielBruijntjes
Copy link
Member

The status is that I dislike the code, but I have not yet had time to dig into this. I dont know how this sasl is supposed to work. But I suspect that we need some sort of base class that is extended by a plain login system and by a sasl login system.

@troian
Copy link

troian commented Feb 19, 2019

@EmielBruijntjes this PR work with certificate authentication

@henris42
Copy link

henris42 commented Jan 3, 2021

Hi, this PR is also needed along with 224 for certificate auth to work. There is a bit missing still though - method to easily define when we use EXTERNAL mechanism. This is currently hidden in AMQP::Address. My current address.h test code assumes external auth at the moment when there is no username/pw in the URL. I'm sure there is nicer way to do that, but that would need bit more refactoring.

@EmielBruijntjes
Copy link
Member

I still don't like this code. The Login class is here to contain a username + password. If you don't need this, it makes more sense to not even construct it, doesn't it?

@henris42
Copy link

henris42 commented Jan 4, 2021

Neither do I. but we do need to give the login type PLAIN vs EXTERNAL when establsihing session. That is ugliness in the protocol itself. Its a stupid thing that makes no sense - but we still have it!
I'm all for to abstract this nicely.

@stertingen
Copy link

stertingen commented Jan 27, 2021

Are there currently ongoing developments on this topic? I'm willing to submit a PR introducing a Authentication base class with derived classes Login and External which provide the mechanism and the response field for the ConnectionStartOkFrame.

Edit: I was looking for other mechanisms, it seems PLAIN and EXTERNAL are the only relevant: https://www.rabbitmq.com/access-control.html#mechanisms

@EmielBruijntjes
Copy link
Member

That makes sense, I will check it when you send in that request

@plopp
Copy link

plopp commented Mar 31, 2021

@AnMakc I think #390 replaces this PR so it can be closed, agree?

@AnMakc
Copy link
Author

AnMakc commented Mar 31, 2021

@AnMakc I think #390 replaces this PR so it can be closed, agree?

Sure, sounds reasonable.

@AnMakc AnMakc closed this Mar 31, 2021
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.

8 participants