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

Update ExternalMechanism.java to be RFC compliant #48

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hariprasad-SAP
Copy link

RFC

SASL External mechanism is capable of transferring an authorization identity string. The client sends the initial response to the intial challenge by the server. It can be empty or non-empty.

Response is non-empty when the client is requesting to act as the identity represented by the (non-empty) string which is UTF-8 encoding of the requested authorization identity string. It is empty when the client is requesting to act as the identity the server associated with its authentication credentials

Why Apache qpid-jms doesn't support identity string?

The SASL External mechanism is configured in this class (ExternalMechanism.java) of Apache qpid-jms.

We can notice that the initial response is configured in Line 28. It is always set to EMPTY (empty byte array defined here) and cannot be configured to a custom string that we can use as identity string. Hence this library is NOT RFC compliant

If we have to pass the authorization identity string to the server then we must configure the initial response of that client.

The SASL External mechanism is configured in this class (ExternalMechanism.java) of Apache qpid-jms.

We can notice that the initial response is configured in Line 28. It is always set to EMPTY (empty byte array defined here) and cannot be configured to a custom string that we can use as identity string. Hence this library is NOT RFC compliant

If we have to pass the authorization identity string to the server then we must configure the initial response of that client.
@gemmellr
Copy link
Member

I wouldnt really say its 'not compliant' so much as it simply doenst support requesting to act as a particular non-server-associated identity, like various other clients (and servers) similarly dont support.

Which server is it you are looking to use that you actually require to pass a client-defined identity for EXTERNAL? I cant say I currently know of any that actually require it. Most I know of even ignore it, since its fairly typical for servers to want to govern the identity they consider the client to be, especially with EXTERNAL. To your question, such reasons are why the Qpid JMS client does not support this, it simply hasnt ever needed to, and there would actually be no point for various servers it has been common to use it with.

Its a notable change in behaviour to simply make it always do this now if the username field happens to be set, given it would have been ignored to this point. Perhaps an option would be needed to toggle the behaviour, though that is fairly ugly so I really dont want such a toggle..I guess I'd skip it for now and assess later.

It would be good if you can outline why you believe you need this currently. If then proceeding you would have to raise a JIRA for the change, squash your commits, update the commit to reference the JIRA (see prior commits), and you also havent included any tests which which would be required. Some of the indentation also looks wrong in the change itself and should be fixed.

@hariprasad-SAP
Copy link
Author

I have provided my comments on use case on the Vertx proton PR. And the same applies for qpid jms as well.

Vertx Proton PR comment

To add more, After the PR changes, identity string can be set by setting it as the username in JMSConnectionFactory.

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